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

@@ -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<std::string> &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);
 	}
 }

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<std::string> &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;

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;
 

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);

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;
 	}
 }
 

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<RemotePlayer *>(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<RemotePlayer *>(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<RemotePlayer *>(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<RemotePlayer *>(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<RemotePlayer *>(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<RemotePlayer *>(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<RemotePlayer *>(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<RemotePlayer *>(m_env->getPlayer(pkt->getPeerId()));
+
 	if (player == NULL) {
 		errorstream << "Server::ProcessData(): Canceling: "
 				"No player for peer_id=" << pkt->getPeerId()

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<std::string> 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

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<RemotePlayer *>(env->getPlayer(name));
+	if (player == NULL){
 		lua_pushnil(L);
 		return 1;
 	}

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;
 

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
 

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<RemotePlayer *>(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<RemotePlayer *>(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<RemotePlayer *>(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<ItemStack> 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<RemotePlayer *>(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<RemotePlayer *>(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)

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 <string>
 #include <list>
@@ -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<std::string> &params);
 
-	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);