From 8bcd10b872bc88c6f474913d6efb8d53c50c5ae1 Mon Sep 17 00:00:00 2001 From: Loic Blot Date: Sat, 8 Oct 2016 10:38:04 +0200 Subject: [PATCH] Player/LocalPlayer/RemotePlayer inheritance cleanup (part 1 on X) * LocalPlayer take ownership of maxHudId as it's the only caller * RemotePlayer take ownership of day night ratio as it's the only user * Pass getPlayerControl as const reference to prevent object copy on each call (perf improvement in ObjectRef::l_get_player_control call) * getPlayerSAO is now only RemotePlayer call * get/setHotbarItemCount is now RemotePlayer owned * Server: Use RemotePlayer instead of Player object on concerned call to properly fix the object type * PlayerSAO now uses RemotePlayer instead of Player because it's only server side * ObjectRef::getplayer also returns RemotePlayer as it's linked with PlayerSAO --- src/content_sao.cpp | 7 ++-- src/content_sao.h | 14 ++----- src/game.cpp | 2 +- src/localplayer.h | 2 + src/network/clientpackethandler.cpp | 6 +-- src/network/serverpackethandler.cpp | 31 +++++++++++---- src/player.h | 62 +++++++++++------------------ src/script/lua_api/l_env.cpp | 4 +- src/script/lua_api/l_object.cpp | 16 ++++---- src/script/lua_api/l_object.h | 3 +- src/server.cpp | 52 +++++++++++------------- src/server.h | 13 +++--- 12 files changed, 103 insertions(+), 109 deletions(-) diff --git a/src/content_sao.cpp b/src/content_sao.cpp index 895c26044..2317cbdfe 100644 --- a/src/content_sao.cpp +++ b/src/content_sao.cpp @@ -751,7 +751,7 @@ bool LuaEntitySAO::collideWithObjects(){ // No prototype, PlayerSAO does not need to be deserialized -PlayerSAO::PlayerSAO(ServerEnvironment *env_, Player *player_, u16 peer_id_, +PlayerSAO::PlayerSAO(ServerEnvironment *env_, RemotePlayer *player_, u16 peer_id_, const std::set &privs, bool is_singleplayer): ServerActiveObject(env_, v3f(0,0,0)), m_player(player_), @@ -833,11 +833,10 @@ void PlayerSAO::addedToEnvironment(u32 dtime_s) void PlayerSAO::removingFromEnvironment() { ServerActiveObject::removingFromEnvironment(); - if(m_player->getPlayerSAO() == this) - { + if (m_player->getPlayerSAO() == this) { m_player->setPlayerSAO(NULL); m_player->peer_id = 0; - m_env->savePlayer((RemotePlayer*)m_player); + m_env->savePlayer(m_player); m_env->removePlayer(m_player); } } diff --git a/src/content_sao.h b/src/content_sao.h index ccae90b77..c97db4922 100644 --- a/src/content_sao.h +++ b/src/content_sao.h @@ -160,7 +160,7 @@ public: class PlayerSAO : public ServerActiveObject { public: - PlayerSAO(ServerEnvironment *env_, Player *player_, u16 peer_id_, + PlayerSAO(ServerEnvironment *env_, RemotePlayer *player_, u16 peer_id_, const std::set &privs, bool is_singleplayer); ~PlayerSAO(); ActiveObjectType getType() const @@ -231,14 +231,8 @@ public: void disconnected(); - Player* getPlayer() - { - return m_player; - } - u16 getPeerID() const - { - return m_peer_id; - } + RemotePlayer* getPlayer() { return m_player; } + u16 getPeerID() const { return m_peer_id; } // Cheat prevention @@ -291,7 +285,7 @@ public: private: std::string getPropertyPacket(); - Player *m_player; + RemotePlayer *m_player; u16 m_peer_id; Inventory *m_inventory; s16 m_damage; diff --git a/src/game.cpp b/src/game.cpp index 22d9ffef6..5bb08e27a 100644 --- a/src/game.cpp +++ b/src/game.cpp @@ -2851,7 +2851,7 @@ void Game::processItemSelection(u16 *new_playeritem) s32 wheel = input->getMouseWheel(); u16 max_item = MYMIN(PLAYER_INVENTORY_SIZE - 1, - player->hud_hotbar_itemcount - 1); + player->hud_hotbar_itemcount - 1); s32 dir = wheel; diff --git a/src/localplayer.h b/src/localplayer.h index 3ae0c4e51..8897adc5e 100644 --- a/src/localplayer.h +++ b/src/localplayer.h @@ -80,6 +80,8 @@ public: m_cao = toset; } + u32 maxHudId() const { return hud.size(); } + private: void accelerateHorizontal(const v3f &target_speed, const f32 max_increase); void accelerateVertical(const v3f &target_speed, const f32 max_increase); diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp index 6d42edd7d..35e350f20 100644 --- a/src/network/clientpackethandler.cpp +++ b/src/network/clientpackethandler.cpp @@ -1122,7 +1122,7 @@ void Client::handleCommand_HudSetParam(NetworkPacket* pkt) *pkt >> param >> value; - Player *player = m_env.getLocalPlayer(); + LocalPlayer *player = m_env.getLocalPlayer(); assert(player != NULL); if (param == HUD_PARAM_HOTBAR_ITEMCOUNT && value.size() == 4) { @@ -1131,10 +1131,10 @@ void Client::handleCommand_HudSetParam(NetworkPacket* pkt) player->hud_hotbar_itemcount = hotbar_itemcount; } else if (param == HUD_PARAM_HOTBAR_IMAGE) { - ((LocalPlayer *) player)->hotbar_image = value; + player->hotbar_image = value; } else if (param == HUD_PARAM_HOTBAR_SELECTED_IMAGE) { - ((LocalPlayer *) player)->hotbar_selected_image = value; + player->hotbar_selected_image = value; } } diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index f9061cc4f..3fba7f720 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -800,7 +800,8 @@ void Server::handleCommand_PlayerPos(NetworkPacket* pkt) pitch = modulo360f(pitch); yaw = modulo360f(yaw); - Player *player = m_env->getPlayer(pkt->getPeerId()); + RemotePlayer *player = + dynamic_cast(m_env->getPlayer(pkt->getPeerId())); if (player == NULL) { errorstream << "Server::ProcessData(): Canceling: " "No player for peer_id=" << pkt->getPeerId() @@ -879,7 +880,9 @@ void Server::handleCommand_DeletedBlocks(NetworkPacket* pkt) void Server::handleCommand_InventoryAction(NetworkPacket* pkt) { - Player *player = m_env->getPlayer(pkt->getPeerId()); + RemotePlayer *player = + dynamic_cast(m_env->getPlayer(pkt->getPeerId())); + if (player == NULL) { errorstream << "Server::ProcessData(): Canceling: " "No player for peer_id=" << pkt->getPeerId() @@ -1078,7 +1081,9 @@ void Server::handleCommand_Damage(NetworkPacket* pkt) *pkt >> damage; - Player *player = m_env->getPlayer(pkt->getPeerId()); + RemotePlayer *player = + dynamic_cast(m_env->getPlayer(pkt->getPeerId())); + if (player == NULL) { errorstream << "Server::ProcessData(): Canceling: " "No player for peer_id=" << pkt->getPeerId() @@ -1112,7 +1117,9 @@ void Server::handleCommand_Breath(NetworkPacket* pkt) *pkt >> breath; - Player *player = m_env->getPlayer(pkt->getPeerId()); + RemotePlayer *player = + dynamic_cast(m_env->getPlayer(pkt->getPeerId())); + if (player == NULL) { errorstream << "Server::ProcessData(): Canceling: " "No player for peer_id=" << pkt->getPeerId() @@ -1224,7 +1231,9 @@ void Server::handleCommand_PlayerItem(NetworkPacket* pkt) if (pkt->getSize() < 2) return; - Player *player = m_env->getPlayer(pkt->getPeerId()); + RemotePlayer *player = + dynamic_cast(m_env->getPlayer(pkt->getPeerId())); + if (player == NULL) { errorstream << "Server::ProcessData(): Canceling: " "No player for peer_id=" << pkt->getPeerId() @@ -1299,7 +1308,9 @@ void Server::handleCommand_Interact(NetworkPacket* pkt) verbosestream << "TOSERVER_INTERACT: action=" << (int)action << ", item=" << item_i << ", pointed=" << pointed.dump() << std::endl; - Player *player = m_env->getPlayer(pkt->getPeerId()); + RemotePlayer *player = + dynamic_cast(m_env->getPlayer(pkt->getPeerId())); + if (player == NULL) { errorstream << "Server::ProcessData(): Canceling: " "No player for peer_id=" << pkt->getPeerId() @@ -1719,7 +1730,9 @@ void Server::handleCommand_NodeMetaFields(NetworkPacket* pkt) fields[fieldname] = pkt->readLongString(); } - Player *player = m_env->getPlayer(pkt->getPeerId()); + RemotePlayer *player = + dynamic_cast(m_env->getPlayer(pkt->getPeerId())); + if (player == NULL) { errorstream << "Server::ProcessData(): Canceling: " "No player for peer_id=" << pkt->getPeerId() @@ -1769,7 +1782,9 @@ void Server::handleCommand_InventoryFields(NetworkPacket* pkt) fields[fieldname] = pkt->readLongString(); } - Player *player = m_env->getPlayer(pkt->getPeerId()); + RemotePlayer *player = + dynamic_cast(m_env->getPlayer(pkt->getPeerId())); + if (player == NULL) { errorstream << "Server::ProcessData(): Canceling: " "No player for peer_id=" << pkt->getPeerId() diff --git a/src/player.h b/src/player.h index f38effa9d..fbd88fc71 100644 --- a/src/player.h +++ b/src/player.h @@ -233,16 +233,6 @@ public: return size; } - void setHotbarItemcount(s32 hotbar_itemcount) - { - hud_hotbar_itemcount = hotbar_itemcount; - } - - s32 getHotbarItemcount() - { - return hud_hotbar_itemcount; - } - void setHotbarImage(const std::string &name) { hud_hotbar_image = name; @@ -278,18 +268,6 @@ public: *params = m_sky_params; } - void overrideDayNightRatio(bool do_override, float ratio) - { - m_day_night_ratio_do_override = do_override; - m_day_night_ratio = ratio; - } - - void getDayNightRatio(bool *do_override, float *ratio) - { - *do_override = m_day_night_ratio_do_override; - *ratio = m_day_night_ratio; - } - void setLocalAnimations(v2s32 frames[4], float frame_speed) { for (int i = 0; i < 4; i++) @@ -309,11 +287,6 @@ public: return false; } - virtual PlayerSAO *getPlayerSAO() - { - return NULL; - } - virtual void setPlayerSAO(PlayerSAO *sao) { FATAL_ERROR("FIXME"); @@ -335,7 +308,7 @@ public: void setModified(const bool x) { m_dirty = x; - if (x == false) + if (!x) inventory.setModified(x); } @@ -391,10 +364,7 @@ public: std::string inventory_formspec; PlayerControl control; - PlayerControl getPlayerControl() - { - return control; - } + const PlayerControl& getPlayerControl() { return control; } u32 keyPressed; @@ -403,9 +373,6 @@ public: u32 addHud(HudElement* hud); HudElement* removeHud(u32 id); void clearHud(); - u32 maxHudId() { - return hud.size(); - } u32 hud_flags; s32 hud_hotbar_itemcount; @@ -429,9 +396,6 @@ protected: std::string m_sky_type; video::SColor m_sky_bgcolor; std::vector m_sky_params; - - bool m_day_night_ratio_do_override; - float m_day_night_ratio; private: // Protect some critical areas // hud for example can be modified by EmergeThread @@ -463,6 +427,25 @@ public: const RemotePlayerChatResult canSendChatMessage(); + void setHotbarItemcount(s32 hotbar_itemcount) + { + hud_hotbar_itemcount = hotbar_itemcount; + } + + s32 getHotbarItemcount() const { return hud_hotbar_itemcount; } + + void overrideDayNightRatio(bool do_override, float ratio) + { + m_day_night_ratio_do_override = do_override; + m_day_night_ratio = ratio; + } + + void getDayNightRatio(bool *do_override, float *ratio) + { + *do_override = m_day_night_ratio_do_override; + *ratio = m_day_night_ratio; + } + private: PlayerSAO *m_sao; @@ -473,6 +456,9 @@ private: u32 m_last_chat_message_sent; float m_chat_message_allowance; u16 m_message_rate_overhead; + + bool m_day_night_ratio_do_override; + float m_day_night_ratio; }; #endif diff --git a/src/script/lua_api/l_env.cpp b/src/script/lua_api/l_env.cpp index ebdea09e4..f6ea23e95 100644 --- a/src/script/lua_api/l_env.cpp +++ b/src/script/lua_api/l_env.cpp @@ -494,8 +494,8 @@ int ModApiEnvMod::l_get_player_by_name(lua_State *L) // Do it const char *name = luaL_checkstring(L, 1); - Player *player = env->getPlayer(name); - if(player == NULL){ + RemotePlayer *player = dynamic_cast(env->getPlayer(name)); + if (player == NULL){ lua_pushnil(L); return 1; } diff --git a/src/script/lua_api/l_object.cpp b/src/script/lua_api/l_object.cpp index befae4253..4e1a1c159 100644 --- a/src/script/lua_api/l_object.cpp +++ b/src/script/lua_api/l_object.cpp @@ -107,7 +107,7 @@ PlayerSAO* ObjectRef::getplayersao(ObjectRef *ref) return (PlayerSAO*)obj; } -Player* ObjectRef::getplayer(ObjectRef *ref) +RemotePlayer* ObjectRef::getplayer(ObjectRef *ref) { PlayerSAO *playersao = getplayersao(ref); if (playersao == NULL) @@ -1212,8 +1212,8 @@ int ObjectRef::l_get_player_control(lua_State *L) lua_pushlstring(L, "", 0); return 1; } - // Do it - PlayerControl control = player->getPlayerControl(); + + const PlayerControl &control = player->getPlayerControl(); lua_newtable(L); lua_pushboolean(L, control.up); lua_setfield(L, -2, "up"); @@ -1467,7 +1467,7 @@ int ObjectRef::l_hud_set_flags(lua_State *L) { NO_MAP_LOCK_REQUIRED; ObjectRef *ref = checkobject(L, 1); - Player *player = getplayer(ref); + RemotePlayer *player = getplayer(ref); if (player == NULL) return 0; @@ -1519,7 +1519,7 @@ int ObjectRef::l_hud_set_hotbar_itemcount(lua_State *L) { NO_MAP_LOCK_REQUIRED; ObjectRef *ref = checkobject(L, 1); - Player *player = getplayer(ref); + RemotePlayer *player = getplayer(ref); if (player == NULL) return 0; @@ -1537,7 +1537,7 @@ int ObjectRef::l_hud_get_hotbar_itemcount(lua_State *L) { NO_MAP_LOCK_REQUIRED; ObjectRef *ref = checkobject(L, 1); - Player *player = getplayer(ref); + RemotePlayer *player = getplayer(ref); if (player == NULL) return 0; @@ -1677,7 +1677,7 @@ int ObjectRef::l_override_day_night_ratio(lua_State *L) { NO_MAP_LOCK_REQUIRED; ObjectRef *ref = checkobject(L, 1); - Player *player = getplayer(ref); + RemotePlayer *player = getplayer(ref); if (player == NULL) return 0; @@ -1700,7 +1700,7 @@ int ObjectRef::l_get_day_night_ratio(lua_State *L) { NO_MAP_LOCK_REQUIRED; ObjectRef *ref = checkobject(L, 1); - Player *player = getplayer(ref); + RemotePlayer *player = getplayer(ref); if (player == NULL) return 0; diff --git a/src/script/lua_api/l_object.h b/src/script/lua_api/l_object.h index a37d29535..a7ac9dff9 100644 --- a/src/script/lua_api/l_object.h +++ b/src/script/lua_api/l_object.h @@ -27,6 +27,7 @@ class ServerActiveObject; class LuaEntitySAO; class PlayerSAO; class Player; +class RemotePlayer; /* ObjectRef @@ -47,7 +48,7 @@ private: static PlayerSAO* getplayersao(ObjectRef *ref); - static Player* getplayer(ObjectRef *ref); + static RemotePlayer* getplayer(ObjectRef *ref); // Exported functions diff --git a/src/server.cpp b/src/server.cpp index 2dd070b1a..11ffba343 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -1256,7 +1256,7 @@ Inventory* Server::getInventory(const InventoryLocation &loc) break; case InventoryLocation::PLAYER: { - Player *player = m_env->getPlayer(loc.name.c_str()); + RemotePlayer *player = dynamic_cast(m_env->getPlayer(loc.name.c_str())); if(!player) return NULL; PlayerSAO *playersao = player->getPlayerSAO(); @@ -1296,9 +1296,12 @@ void Server::setInventoryModified(const InventoryLocation &loc, bool playerSend) if (!playerSend) return; - Player *player = m_env->getPlayer(loc.name.c_str()); - if(!player) + RemotePlayer *player = + dynamic_cast(m_env->getPlayer(loc.name.c_str())); + + if (!player) return; + PlayerSAO *playersao = player->getPlayerSAO(); if(!playersao) return; @@ -2637,19 +2640,16 @@ void Server::DeleteClient(u16 peer_id, ClientDeletionReason reason) ++i; } - Player *player = m_env->getPlayer(peer_id); + RemotePlayer *player = dynamic_cast(m_env->getPlayer(peer_id)); /* Run scripts and remove from environment */ - { - if(player != NULL) - { - PlayerSAO *playersao = player->getPlayerSAO(); - assert(playersao); + if(player != NULL) { + PlayerSAO *playersao = player->getPlayerSAO(); + assert(playersao); - m_script->on_leaveplayer(playersao, reason == CDR_TIMEOUT); + m_script->on_leaveplayer(playersao, reason == CDR_TIMEOUT); - playersao->disconnected(); - } + playersao->disconnected(); } /* @@ -2691,7 +2691,7 @@ void Server::DeleteClient(u16 peer_id, ClientDeletionReason reason) SendChatMessage(PEER_ID_INEXISTENT,message); } -void Server::UpdateCrafting(Player* player) +void Server::UpdateCrafting(RemotePlayer* player) { DSTACK(FUNCTION_NAME); @@ -2701,7 +2701,8 @@ void Server::UpdateCrafting(Player* player) loc.setPlayer(player->getName()); std::vector output_replacements; getCraftingResult(&player->inventory, preview, output_replacements, false, this); - m_env->getScriptIface()->item_CraftPredict(preview, player->getPlayerSAO(), (&player->inventory)->getList("craft"), loc); + m_env->getScriptIface()->item_CraftPredict(preview, player->getPlayerSAO(), + (&player->inventory)->getList("craft"), loc); // Put the new preview in InventoryList *plist = player->inventory.getList("craftpreview"); @@ -2851,8 +2852,8 @@ std::string Server::getPlayerName(u16 peer_id) PlayerSAO* Server::getPlayerSAO(u16 peer_id) { - Player *player = m_env->getPlayer(peer_id); - if(player == NULL) + RemotePlayer *player = dynamic_cast(m_env->getPlayer(peer_id)); + if (player == NULL) return NULL; return player->getPlayerSAO(); } @@ -2917,8 +2918,9 @@ void Server::reportPrivsModified(const std::string &name) reportPrivsModified(player->getName()); } } else { - Player *player = m_env->getPlayer(name.c_str()); - if(!player) + RemotePlayer *player = + dynamic_cast(m_env->getPlayer(name.c_str())); + if (!player) return; SendPlayerPrivileges(player->peer_id); PlayerSAO *sao = player->getPlayerSAO(); @@ -3025,7 +3027,7 @@ bool Server::hudChange(Player *player, u32 id, HudElementStat stat, void *data) return true; } -bool Server::hudSetFlags(Player *player, u32 flags, u32 mask) +bool Server::hudSetFlags(RemotePlayer *player, u32 flags, u32 mask) { if (!player) return false; @@ -3043,10 +3045,11 @@ bool Server::hudSetFlags(Player *player, u32 flags, u32 mask) return true; } -bool Server::hudSetHotbarItemcount(Player *player, s32 hotbar_itemcount) +bool Server::hudSetHotbarItemcount(RemotePlayer *player, s32 hotbar_itemcount) { if (!player) return false; + if (hotbar_itemcount <= 0 || hotbar_itemcount > HUD_HOTBAR_ITEMCOUNT_MAX) return false; @@ -3057,13 +3060,6 @@ bool Server::hudSetHotbarItemcount(Player *player, s32 hotbar_itemcount) return true; } -s32 Server::hudGetHotbarItemcount(Player *player) -{ - if (!player) - return 0; - return player->getHotbarItemcount(); -} - void Server::hudSetHotbarImage(Player *player, std::string name) { if (!player) @@ -3130,7 +3126,7 @@ bool Server::setSky(Player *player, const video::SColor &bgcolor, return true; } -bool Server::overrideDayNightRatio(Player *player, bool do_override, +bool Server::overrideDayNightRatio(RemotePlayer *player, bool do_override, float ratio) { if (!player) diff --git a/src/server.h b/src/server.h index 34427a71a..331a91a52 100644 --- a/src/server.h +++ b/src/server.h @@ -34,6 +34,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "environment.h" #include "chat_interface.h" #include "clientiface.h" +#include "player.h" #include "network/networkpacket.h" #include #include @@ -48,7 +49,6 @@ class IWritableCraftDefManager; class BanManager; class EventManager; class Inventory; -class Player; class PlayerSAO; class IRollbackManager; struct RollbackAction; @@ -336,9 +336,10 @@ public: u32 hudAdd(Player *player, HudElement *element); bool hudRemove(Player *player, u32 id); bool hudChange(Player *player, u32 id, HudElementStat stat, void *value); - bool hudSetFlags(Player *player, u32 flags, u32 mask); - bool hudSetHotbarItemcount(Player *player, s32 hotbar_itemcount); - s32 hudGetHotbarItemcount(Player *player); + bool hudSetFlags(RemotePlayer *player, u32 flags, u32 mask); + bool hudSetHotbarItemcount(RemotePlayer *player, s32 hotbar_itemcount); + s32 hudGetHotbarItemcount(RemotePlayer *player) const + { return player->getHotbarItemcount(); } void hudSetHotbarImage(Player *player, std::string name); std::string hudGetHotbarImage(Player *player); void hudSetHotbarSelectedImage(Player *player, std::string name); @@ -353,7 +354,7 @@ public: bool setSky(Player *player, const video::SColor &bgcolor, const std::string &type, const std::vector ¶ms); - bool overrideDayNightRatio(Player *player, bool do_override, float brightness); + bool overrideDayNightRatio(RemotePlayer *player, bool do_override, float brightness); /* con::PeerHandler implementation. */ void peerAdded(con::Peer *peer); @@ -475,7 +476,7 @@ private: void DiePlayer(u16 peer_id); void RespawnPlayer(u16 peer_id); void DeleteClient(u16 peer_id, ClientDeletionReason reason); - void UpdateCrafting(Player *player); + void UpdateCrafting(RemotePlayer *player); void handleChatInterfaceEvent(ChatEvent *evt); -- 2.25.1