diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index 3c6b63500..146f4cfb3 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -512,7 +512,7 @@ void Server::process_PlayerPos(RemotePlayer *player, PlayerSAO *playersao, if (playersao->checkMovementCheat()) { // Call callbacks m_script->on_cheat(playersao, "moved_too_fast"); - SendMovePlayer(pkt->getPeerId()); + SendMovePlayer(playersao); } } @@ -993,7 +993,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) return; } - playersao->getPlayer()->setWieldIndex(item_i); + player->setWieldIndex(item_i); // Get pointed to object (NULL if not POINTEDTYPE_OBJECT) ServerActiveObject *pointed_object = NULL; @@ -1161,7 +1161,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) // Get player's wielded item // See also: Game::handleDigging ItemStack selected_item, hand_item; - playersao->getPlayer()->getWieldedItem(&selected_item, &hand_item); + player->getWieldedItem(&selected_item, &hand_item); // Get diggability and expected digging time DigParams params = getDigParams(m_nodedef->get(n).groups, @@ -1253,7 +1253,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) // Do stuff if (m_script->item_OnSecondaryUse(selected_item, playersao, pointed)) { if (selected_item.has_value() && playersao->setWieldedItem(*selected_item)) - SendInventory(playersao, true); + SendInventory(player, true); } pointed_object->rightClick(playersao); @@ -1262,7 +1262,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) // Apply returned ItemStack if (selected_item.has_value() && playersao->setWieldedItem(*selected_item)) - SendInventory(playersao, true); + SendInventory(player, true); } if (pointed.type != POINTEDTHING_NODE) @@ -1296,7 +1296,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) if (m_script->item_OnUse(selected_item, playersao, pointed)) { // Apply returned ItemStack if (selected_item.has_value() && playersao->setWieldedItem(*selected_item)) - SendInventory(playersao, true); + SendInventory(player, true); } return; @@ -1315,7 +1315,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) if (m_script->item_OnSecondaryUse(selected_item, playersao, pointed)) { // Apply returned ItemStack if (selected_item.has_value() && playersao->setWieldedItem(*selected_item)) - SendInventory(playersao, true); + SendInventory(player, true); } return; diff --git a/src/remoteplayer.cpp b/src/remoteplayer.cpp index 20be7a8c8..9658dca06 100644 --- a/src/remoteplayer.cpp +++ b/src/remoteplayer.cpp @@ -69,6 +69,11 @@ RemotePlayer::RemotePlayer(const char *name, IItemDefManager *idef): m_star_params = SkyboxDefaults::getStarDefaults(); } +RemotePlayer::~RemotePlayer() +{ + if (m_sao) + m_sao->setPlayer(nullptr); +} RemotePlayerChatResult RemotePlayer::canSendChatMessage() { diff --git a/src/remoteplayer.h b/src/remoteplayer.h index 0ab33adfe..a38f31731 100644 --- a/src/remoteplayer.h +++ b/src/remoteplayer.h @@ -42,7 +42,7 @@ class RemotePlayer : public Player public: RemotePlayer(const char *name, IItemDefManager *idef); - virtual ~RemotePlayer() = default; + virtual ~RemotePlayer(); PlayerSAO *getPlayerSAO() { return m_sao; } void setPlayerSAO(PlayerSAO *sao) { m_sao = sao; } @@ -135,6 +135,7 @@ public: u16 protocol_version = 0; u16 formspec_version = 0; + /// returns PEER_ID_INEXISTENT when PlayerSAO is not ready session_t getPeerId() const { return m_peer_id; } void setPeerId(session_t peer_id) { m_peer_id = peer_id; } diff --git a/src/script/lua_api/l_object.cpp b/src/script/lua_api/l_object.cpp index 764cf87fe..c291e605a 100644 --- a/src/script/lua_api/l_object.cpp +++ b/src/script/lua_api/l_object.cpp @@ -326,7 +326,7 @@ int ObjectRef::l_set_wielded_item(lua_State *L) bool success = sao->setWieldedItem(item); if (success && sao->getType() == ACTIVEOBJECT_TYPE_PLAYER) { - getServer(L)->SendInventory((PlayerSAO *)sao, true); + getServer(L)->SendInventory(((PlayerSAO *)sao)->getPlayer(), true); } lua_pushboolean(L, success); return 1; diff --git a/src/server.cpp b/src/server.cpp index baacce3f6..1c9252581 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -343,7 +343,7 @@ Server::~Server() kick_msg = g_settings->get("kick_msg_shutdown"); } m_env->saveLoadedPlayers(true); - m_env->kickAllPlayers(SERVER_ACCESSDENIED_SHUTDOWN, + kickAllPlayers(SERVER_ACCESSDENIED_SHUTDOWN, kick_msg, reconnect); } @@ -590,7 +590,7 @@ void Server::step() std::string async_err = m_async_fatal_error.get(); if (!async_err.empty()) { if (!m_simple_singleplayer_mode) { - m_env->kickAllPlayers(SERVER_ACCESSDENIED_CRASH, + kickAllPlayers(SERVER_ACCESSDENIED_CRASH, g_settings->get("kick_msg_crash"), g_settings->getBool("ask_reconnect_on_crash")); } @@ -1105,7 +1105,7 @@ PlayerSAO* Server::StageTwoClientInit(session_t peer_id) /* Send complete position information */ - SendMovePlayer(peer_id); + SendMovePlayer(playersao); // Send privileges SendPlayerPrivileges(peer_id); @@ -1114,7 +1114,7 @@ PlayerSAO* Server::StageTwoClientInit(session_t peer_id) SendPlayerInventoryFormspec(peer_id); // Send inventory - SendInventory(playersao, false); + SendInventory(player, false); // Send HP SendPlayerHP(playersao, false); @@ -1458,10 +1458,8 @@ void Server::SendNodeDef(session_t peer_id, Non-static send methods */ -void Server::SendInventory(PlayerSAO *sao, bool incremental) +void Server::SendInventory(RemotePlayer *player, bool incremental) { - RemotePlayer *player = sao->getPlayer(); - // Do not send new format to old clients incremental &= player->protocol_version >= 38; @@ -1471,11 +1469,11 @@ void Server::SendInventory(PlayerSAO *sao, bool incremental) Serialize it */ - NetworkPacket pkt(TOCLIENT_INVENTORY, 0, sao->getPeerID()); + NetworkPacket pkt(TOCLIENT_INVENTORY, 0, player->getPeerId()); std::ostringstream os(std::ios::binary); - sao->getInventory()->serialize(os, incremental); - sao->getInventory()->setModified(false); + player->inventory.serialize(os, incremental); + player->inventory.setModified(false); player->setModified(true); const std::string &s = os.str(); @@ -1900,17 +1898,12 @@ void Server::SendPlayerBreath(PlayerSAO *sao) SendBreath(sao->getPeerID(), sao->getBreath()); } -void Server::SendMovePlayer(session_t peer_id) +void Server::SendMovePlayer(PlayerSAO *sao) { - RemotePlayer *player = m_env->getPlayer(peer_id); - assert(player); - PlayerSAO *sao = player->getPlayerSAO(); - assert(sao); - // Send attachment updates instantly to the client prior updating position sao->sendOutdatedData(); - NetworkPacket pkt(TOCLIENT_MOVE_PLAYER, sizeof(v3f) + sizeof(f32) * 2, peer_id); + NetworkPacket pkt(TOCLIENT_MOVE_PLAYER, sizeof(v3f) + sizeof(f32) * 2, sao->getPeerID()); pkt << sao->getBasePosition() << sao->getLookPitch() << sao->getRotation().Y; { @@ -2877,6 +2870,15 @@ void Server::DenyAccess(session_t peer_id, AccessDeniedCode reason, DisconnectPeer(peer_id); } +void Server::kickAllPlayers(AccessDeniedCode reason, + const std::string &str_reason, bool reconnect) +{ + std::vector clients = m_clients.getClientIDs(); + for (const session_t client_id : clients) { + DenyAccess(client_id, reason, str_reason, reconnect); + } +} + void Server::DisconnectPeer(session_t peer_id) { m_modchannel_mgr->leaveAllChannels(peer_id); @@ -3929,6 +3931,23 @@ PlayerSAO* Server::emergePlayer(const char *name, session_t peer_id, u16 proto_v return NULL; } + /* + Object construction sequence/hierarchy + -------------------------------------- + 1. RemoteClient (tightly connection-bound) + 2. RemotePlayer (controls, in-game appearance) + 3. PlayerSAO (movable object in-game) + PlayerSAO controls the peer_id assignment of RemotePlayer, + indicating whether the player is ready + + Destruction sequence + -------------------- + 1. PlayerSAO pending removal flag + 2. PlayerSAO save data before free + 3. RemotePlayer, then PlayerSAO freed + 4. RemoteClient freed + */ + if (!player) { player = new RemotePlayer(name, idef()); } diff --git a/src/server.h b/src/server.h index c57dde0d7..5a6b6b7cd 100644 --- a/src/server.h +++ b/src/server.h @@ -352,6 +352,8 @@ public: void DenySudoAccess(session_t peer_id); void DenyAccess(session_t peer_id, AccessDeniedCode reason, const std::string &custom_reason = "", bool reconnect = false); + void kickAllPlayers(AccessDeniedCode reason, + const std::string &str_reason, bool reconnect); void acceptAuth(session_t peer_id, bool forSudoMode); void DisconnectPeer(session_t peer_id); bool getClientConInfo(session_t peer_id, con::rtt_stat_type type, float *retval); @@ -363,8 +365,8 @@ public: void HandlePlayerHPChange(PlayerSAO *sao, const PlayerHPChangeReason &reason); void SendPlayerHP(PlayerSAO *sao, bool effect); void SendPlayerBreath(PlayerSAO *sao); - void SendInventory(PlayerSAO *playerSAO, bool incremental); - void SendMovePlayer(session_t peer_id); + void SendInventory(RemotePlayer *player, bool incremental); + void SendMovePlayer(PlayerSAO *sao); void SendMovePlayerRel(session_t peer_id, const v3f &added_pos); void SendPlayerSpeed(session_t peer_id, const v3f &added_vel); void SendPlayerFov(session_t peer_id); diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp index 688dcde4e..d72383382 100644 --- a/src/server/player_sao.cpp +++ b/src/server/player_sao.cpp @@ -29,10 +29,10 @@ PlayerSAO::PlayerSAO(ServerEnvironment *env_, RemotePlayer *player_, session_t p bool is_singleplayer): UnitSAO(env_, v3f(0,0,0)), m_player(player_), - m_peer_id(peer_id_), + m_peer_id_initial(peer_id_), m_is_singleplayer(is_singleplayer) { - SANITY_CHECK(m_peer_id != PEER_ID_INEXISTENT); + SANITY_CHECK(m_peer_id_initial != PEER_ID_INEXISTENT); m_prop.hp_max = PLAYER_MAX_HP_DEFAULT; m_prop.breath_max = PLAYER_MAX_BREATH_DEFAULT; @@ -88,7 +88,8 @@ void PlayerSAO::addedToEnvironment(u32 dtime_s) ServerActiveObject::addedToEnvironment(dtime_s); ServerActiveObject::setBasePosition(m_base_position); m_player->setPlayerSAO(this); - m_player->setPeerId(m_peer_id); + m_player->setPeerId(m_peer_id_initial); + m_peer_id_initial = PEER_ID_INEXISTENT; // don't try to use it again. m_last_good_position = m_base_position; } @@ -96,11 +97,13 @@ void PlayerSAO::addedToEnvironment(u32 dtime_s) void PlayerSAO::removingFromEnvironment() { ServerActiveObject::removingFromEnvironment(); - if (m_player->getPlayerSAO() == this) { - unlinkPlayerSessionAndSave(); - for (u32 attached_particle_spawner : m_attached_particle_spawners) { - m_env->deleteParticleSpawner(attached_particle_spawner, false); - } + + // If this fails, fix the ActiveObjectMgr code in ServerEnvironment + SANITY_CHECK(m_player->getPlayerSAO() == this); + + unlinkPlayerSessionAndSave(); + for (u32 attached_particle_spawner : m_attached_particle_spawners) { + m_env->deleteParticleSpawner(attached_particle_spawner, false); } } @@ -236,7 +239,7 @@ void PlayerSAO::step(float dtime, bool send_recommended) " is attached to nonexistent parent. This is a bug." << std::endl; clearParentAttachment(); setBasePosition(m_last_good_position); - m_env->getGameDef()->SendMovePlayer(m_peer_id); + m_env->getGameDef()->SendMovePlayer(this); } //dstream<<"PlayerSAO::step: dtime: "<getGameDef()->SendBlock(m_peer_id, blockpos); + m_env->getGameDef()->SendBlock(getPeerID(), blockpos); setBasePosition(pos); // Movement caused by this command is always valid m_last_good_position = getBasePosition(); m_move_pool.empty(); m_time_from_last_teleport = 0.0; - m_env->getGameDef()->SendMovePlayer(m_peer_id); + m_env->getGameDef()->SendMovePlayer(this); } void PlayerSAO::addPos(const v3f &added_pos) @@ -381,14 +384,14 @@ void PlayerSAO::addPos(const v3f &added_pos) // Send mapblock of target location v3f pos = getBasePosition() + added_pos; v3s16 blockpos = v3s16(pos.X / MAP_BLOCKSIZE, pos.Y / MAP_BLOCKSIZE, pos.Z / MAP_BLOCKSIZE); - m_env->getGameDef()->SendBlock(m_peer_id, blockpos); + m_env->getGameDef()->SendBlock(getPeerID(), blockpos); setBasePosition(pos); // Movement caused by this command is always valid m_last_good_position = getBasePosition(); m_move_pool.empty(); m_time_from_last_teleport = 0.0; - m_env->getGameDef()->SendMovePlayerRel(m_peer_id, added_pos); + m_env->getGameDef()->SendMovePlayerRel(getPeerID(), added_pos); } void PlayerSAO::moveTo(v3f pos, bool continuous) @@ -401,7 +404,7 @@ void PlayerSAO::moveTo(v3f pos, bool continuous) m_last_good_position = getBasePosition(); m_move_pool.empty(); m_time_from_last_teleport = 0.0; - m_env->getGameDef()->SendMovePlayer(m_peer_id); + m_env->getGameDef()->SendMovePlayer(this); } void PlayerSAO::setPlayerYaw(const float yaw) @@ -433,7 +436,7 @@ void PlayerSAO::setWantedRange(const s16 range) void PlayerSAO::setPlayerYawAndSend(const float yaw) { setPlayerYaw(yaw); - m_env->getGameDef()->SendMovePlayer(m_peer_id); + m_env->getGameDef()->SendMovePlayer(this); } void PlayerSAO::setLookPitch(const float pitch) @@ -447,7 +450,7 @@ void PlayerSAO::setLookPitch(const float pitch) void PlayerSAO::setLookPitchAndSend(const float pitch) { setLookPitch(pitch); - m_env->getGameDef()->SendMovePlayer(m_peer_id); + m_env->getGameDef()->SendMovePlayer(this); } u32 PlayerSAO::punch(v3f dir, @@ -578,16 +581,20 @@ bool PlayerSAO::setWieldedItem(const ItemStack &item) void PlayerSAO::disconnected() { - m_peer_id = PEER_ID_INEXISTENT; markForRemoval(); + m_player->setPeerId(PEER_ID_INEXISTENT); +} + +session_t PlayerSAO::getPeerID() const +{ + // Before adding `this` to the server env, m_player is still nullptr. + return m_player ? m_player->getPeerId() : PEER_ID_INEXISTENT; } void PlayerSAO::unlinkPlayerSessionAndSave() { assert(m_player->getPlayerSAO() == this); - m_player->setPeerId(PEER_ID_INEXISTENT); m_env->savePlayer(m_player); - m_player->setPlayerSAO(NULL); m_env->removePlayer(m_player); } diff --git a/src/server/player_sao.h b/src/server/player_sao.h index 854464508..b26304589 100644 --- a/src/server/player_sao.h +++ b/src/server/player_sao.h @@ -142,8 +142,9 @@ public: void disconnected(); + void setPlayer(RemotePlayer *player) { m_player = player; } RemotePlayer *getPlayer() { return m_player; } - session_t getPeerID() const { return m_peer_id; } + session_t getPeerID() const; // Cheat prevention @@ -193,7 +194,7 @@ private: std::string generateUpdatePhysicsOverrideCommand() const; RemotePlayer *m_player = nullptr; - session_t m_peer_id = 0; + session_t m_peer_id_initial = 0; ///< only used to initialize RemotePlayer // Cheat prevention LagPool m_dig_pool; diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp index e0cf99378..0d003e078 100644 --- a/src/serverenvironment.cpp +++ b/src/serverenvironment.cpp @@ -625,13 +625,6 @@ bool ServerEnvironment::removePlayerFromDatabase(const std::string &name) return m_player_database->removePlayer(name); } -void ServerEnvironment::kickAllPlayers(AccessDeniedCode reason, - const std::string &str_reason, bool reconnect) -{ - for (RemotePlayer *player : m_players) - m_server->DenyAccess(player->getPeerId(), reason, str_reason, reconnect); -} - void ServerEnvironment::saveLoadedPlayers(bool force) { for (RemotePlayer *player : m_players) { @@ -1643,9 +1636,8 @@ void ServerEnvironment::step(float dtime) if (player->getPeerId() == PEER_ID_INEXISTENT) continue; - PlayerSAO *sao = player->getPlayerSAO(); - if (sao && player->inventory.checkModified()) - m_server->SendInventory(sao, true); + if (player->inventory.checkModified()) + m_server->SendInventory(player, true); } // Send outdated detached inventories diff --git a/src/serverenvironment.h b/src/serverenvironment.h index ad6a3acc5..6238b2ae2 100644 --- a/src/serverenvironment.h +++ b/src/serverenvironment.h @@ -239,8 +239,6 @@ public: float getSendRecommendedInterval() { return m_recommended_send_interval; } - void kickAllPlayers(AccessDeniedCode reason, - const std::string &str_reason, bool reconnect); // Save players void saveLoadedPlayers(bool force = false); void savePlayer(RemotePlayer *player);