Avoid packets getting sent to disconnected players (#14444)

Many functions expect RemotePlayer to have a valid peer ID,
this however is not the case immediately after disconnecting
where the object is still alive and pending for removal.

ServerEnvironment::getPlayer(const char *, bool) now only
returns players that are connected unless forced to.

src/network/serverpackethandler.cpp

@@ -179,7 +179,7 @@ void Server::handleCommand_Init(NetworkPacket* pkt)
 		return;
 	}
 
-	RemotePlayer *player = m_env->getPlayer(playername);
+	RemotePlayer *player = m_env->getPlayer(playername, true);
 
 	// If player is already connected, cancel
 	if (player && player->getPeerId() != PEER_ID_INEXISTENT) {

src/script/lua_api/l_env.cpp

@@ -695,8 +695,6 @@ int ModApiEnv::l_get_connected_players(lua_State *L)
 	lua_createtable(L, env->getPlayerCount(), 0);
 	u32 i = 0;
 	for (RemotePlayer *player : env->getPlayers()) {
-		if (player->getPeerId() == PEER_ID_INEXISTENT)
-			continue;
 		PlayerSAO *sao = player->getPlayerSAO();
 		if (sao && !sao->isGone()) {
 			getScriptApiBase(L)->objectrefGetOrCreate(L, sao);
@@ -714,7 +712,7 @@ int ModApiEnv::l_get_player_by_name(lua_State *L)
 	// Do it
 	const char *name = luaL_checkstring(L, 1);
 	RemotePlayer *player = env->getPlayer(name);
-	if (!player || player->getPeerId() == PEER_ID_INEXISTENT)
+	if (!player)
 		return 0;
 	PlayerSAO *sao = player->getPlayerSAO();
 	if (!sao || sao->isGone())

src/server.cpp

@@ -1107,7 +1107,7 @@ PlayerSAO* Server::StageTwoClientInit(session_t peer_id)
 		}
 	}
 
-	RemotePlayer *player = m_env->getPlayer(playername.c_str());
+	RemotePlayer *player = m_env->getPlayer(playername.c_str(), true);
 
 	// If failed, cancel
 	if (!playersao || !player) {
@@ -1948,9 +1948,13 @@ void Server::SendMovePlayerRel(session_t peer_id, const v3f &added_pos)
 
 void Server::SendPlayerFov(session_t peer_id)
 {
+	RemotePlayer *player = m_env->getPlayer(peer_id);
+	if (!player)
+		return;
+
 	NetworkPacket pkt(TOCLIENT_FOV, 4 + 1 + 4, peer_id);
 
-	PlayerFovSpec fov_spec = m_env->getPlayer(peer_id)->getFov();
+	PlayerFovSpec fov_spec = player->getFov();
 	pkt << fov_spec.fov << fov_spec.is_multiplier << fov_spec.transition_time;
 
 	Send(&pkt);
@@ -1978,8 +1982,7 @@ void Server::SendEyeOffset(session_t peer_id, v3f first, v3f third, v3f third_fr
 void Server::SendPlayerPrivileges(session_t peer_id)
 {
 	RemotePlayer *player = m_env->getPlayer(peer_id);
-	assert(player);
-	if(player->getPeerId() == PEER_ID_INEXISTENT)
+	if (!player)
 		return;
 
 	std::set<std::string> privs;
@@ -1998,8 +2001,7 @@ void Server::SendPlayerPrivileges(session_t peer_id)
 void Server::SendPlayerInventoryFormspec(session_t peer_id)
 {
 	RemotePlayer *player = m_env->getPlayer(peer_id);
-	assert(player);
-	if (player->getPeerId() == PEER_ID_INEXISTENT)
+	if (!player)
 		return;
 
 	NetworkPacket pkt(TOCLIENT_INVENTORY_FORMSPEC, 0, peer_id);
@@ -2011,8 +2013,7 @@ void Server::SendPlayerInventoryFormspec(session_t peer_id)
 void Server::SendPlayerFormspecPrepend(session_t peer_id)
 {
 	RemotePlayer *player = m_env->getPlayer(peer_id);
-	assert(player);
-	if (player->getPeerId() == PEER_ID_INEXISTENT)
+	if (!player)
 		return;
 
 	NetworkPacket pkt(TOCLIENT_FORMSPEC_PREPEND, 0, peer_id);
@@ -2190,11 +2191,6 @@ s32 Server::playSound(ServerPlayingSound &params, bool ephemeral)
 					<<"\" not found"<<std::endl;
 			return -1;
 		}
-		if (player->getPeerId() == PEER_ID_INEXISTENT) {
-			infostream<<"Server::playSound: Player \""<<params.to_player
-					<<"\" not connected"<<std::endl;
-			return -1;
-		}
 		dst_clients.push_back(player->getPeerId());
 	} else {
 		std::vector<session_t> clients = m_clients.getClientIDs();
@@ -2817,8 +2813,7 @@ void Server::SendMinimapModes(session_t peer_id,
 		std::vector<MinimapMode> &modes, size_t wanted_mode)
 {
 	RemotePlayer *player = m_env->getPlayer(peer_id);
-	assert(player);
-	if (player->getPeerId() == PEER_ID_INEXISTENT)
+	if (!player)
 		return;
 
 	NetworkPacket pkt(TOCLIENT_MINIMAP_MODES, 0, peer_id);
@@ -3363,11 +3358,7 @@ void Server::notifyPlayer(const char *name, const std::wstring &msg)
 	}
 
 	RemotePlayer *player = m_env->getPlayer(name);
-	if (!player) {
-		return;
-	}
-
-	if (player->getPeerId() == PEER_ID_INEXISTENT)
+	if (!player)
 		return;
 
 	SendChatMessage(player->getPeerId(), ChatMessage(msg));
@@ -4039,7 +4030,7 @@ PlayerSAO* Server::emergePlayer(const char *name, session_t peer_id, u16 proto_v
 	RemotePlayer *player = m_env->getPlayer(name);
 
 	// If player is already connected, cancel
-	if (player && player->getPeerId() != PEER_ID_INEXISTENT) {
+	if (player) {
 		infostream<<"emergePlayer(): Player already connected"<<std::endl;
 		return NULL;
 	}

src/server/serverinventorymgr.cpp

@@ -124,7 +124,7 @@ Inventory *ServerInventoryManager::createDetachedInventory(
 		RemotePlayer *p = m_env->getPlayer(name.c_str());
 
 		// if player is connected, send him the inventory
-		if (p && p->getPeerId() != PEER_ID_INEXISTENT) {
+		if (p) {
 			m_env->getGameDef()->sendDetachedInventory(
 					inv, name, p->getPeerId());
 		}
@@ -152,7 +152,7 @@ bool ServerInventoryManager::removeDetachedInventory(const std::string &name)
 		if (m_env) {
 			RemotePlayer *player = m_env->getPlayer(owner.c_str());
 
-			if (player && player->getPeerId() != PEER_ID_INEXISTENT)
+			if (player)
 				m_env->getGameDef()->sendDetachedInventory(
 						nullptr, name, player->getPeerId());
 		}

src/serverenvironment.cpp

@@ -584,13 +584,17 @@ RemotePlayer *ServerEnvironment::getPlayer(const session_t peer_id)
 	return NULL;
 }
 
-RemotePlayer *ServerEnvironment::getPlayer(const char* name)
+RemotePlayer *ServerEnvironment::getPlayer(const char* name, bool match_invalid_peer)
 {
 	for (RemotePlayer *player : m_players) {
-		if (strcmp(player->getName(), name) == 0)
+		if (strcmp(player->getName(), name) != 0)
+			continue;
+
+		if (match_invalid_peer || player->getPeerId() != PEER_ID_INEXISTENT)
 			return player;
+		break;
 	}
-	return NULL;
+	return nullptr;
 }
 
 void ServerEnvironment::addPlayer(RemotePlayer *player)

src/serverenvironment.h

@@ -388,7 +388,7 @@ public:
 		bool static_exists, v3s16 static_block=v3s16(0,0,0));
 
 	RemotePlayer *getPlayer(const session_t peer_id);
-	RemotePlayer *getPlayer(const char* name);
+	RemotePlayer *getPlayer(const char* name, bool match_invalid_peer = false);
 	const std::vector<RemotePlayer *> getPlayers() const { return m_players; }
 	u32 getPlayerCount() const { return m_players.size(); }