From be10c0893e2e6d1852a027abccc4cd3cdc85a140 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Lo=C3=AFc=20Blot?= Date: Sat, 30 Sep 2017 12:00:05 +0200 Subject: [PATCH] Make Player::peer_id server-side only and add getters and setters (#6478) * Make Player::peer_id server-side only and add getters and setters Player::peer_id has no sense client side, move it to server, make it private and add setter and getter Also add some PEER_ID_INEXISTENT instead of harcoded 0 --- src/client.cpp | 21 ++--------- src/content_sao.cpp | 4 +-- src/player.h | 2 -- src/remoteplayer.h | 6 ++++ src/script/lua_api/l_object.cpp | 2 +- src/script/lua_api/l_server.cpp | 28 ++++++++------- src/server.cpp | 62 +++++++++++++++++---------------- src/serverenvironment.cpp | 12 +++---- 8 files changed, 64 insertions(+), 73 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 13ee8bb76..2538f52aa 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -1190,7 +1190,7 @@ void Client::sendReady() void Client::sendPlayerPos() { LocalPlayer *myplayer = m_env.getLocalPlayer(); - if(myplayer == NULL) + if (!myplayer) return; ClientMap &map = m_env.getClientMap(); @@ -1216,16 +1216,6 @@ void Client::sendPlayerPos() myplayer->last_camera_fov = camera_fov; myplayer->last_wanted_range = wanted_range; - //infostream << "Sending Player Position information" << std::endl; - - session_t our_peer_id = m_con->GetPeerID(); - - // Set peer id if not set already - if(myplayer->peer_id == PEER_ID_INEXISTENT) - myplayer->peer_id = our_peer_id; - - assert(myplayer->peer_id == our_peer_id); - NetworkPacket pkt(TOSERVER_PLAYERPOS, 12 + 12 + 4 + 4 + 4 + 1 + 1); writePlayerPos(myplayer, &map, &pkt); @@ -1236,16 +1226,9 @@ void Client::sendPlayerPos() void Client::sendPlayerItem(u16 item) { LocalPlayer *myplayer = m_env.getLocalPlayer(); - if(myplayer == NULL) + if (!myplayer) return; - session_t our_peer_id = m_con->GetPeerID(); - - // Set peer id if not set already - if(myplayer->peer_id == PEER_ID_INEXISTENT) - myplayer->peer_id = our_peer_id; - assert(myplayer->peer_id == our_peer_id); - NetworkPacket pkt(TOSERVER_PLAYERITEM, 2); pkt << item; diff --git a/src/content_sao.cpp b/src/content_sao.cpp index 1d120f0a1..819c63ac3 100644 --- a/src/content_sao.cpp +++ b/src/content_sao.cpp @@ -848,7 +848,7 @@ void PlayerSAO::addedToEnvironment(u32 dtime_s) ServerActiveObject::addedToEnvironment(dtime_s); ServerActiveObject::setBasePosition(m_base_position); m_player->setPlayerSAO(this); - m_player->peer_id = m_peer_id; + m_player->setPeerId(m_peer_id); m_last_good_position = m_base_position; } @@ -1359,7 +1359,7 @@ void PlayerSAO::disconnected() void PlayerSAO::unlinkPlayerSessionAndSave() { assert(m_player->getPlayerSAO() == this); - m_player->peer_id = 0; + m_player->setPeerId(PEER_ID_INEXISTENT); m_env->savePlayer(m_player); m_player->setPlayerSAO(NULL); m_env->removePlayer(m_player); diff --git a/src/player.h b/src/player.h index aaafd2ae1..e343f55a5 100644 --- a/src/player.h +++ b/src/player.h @@ -147,8 +147,6 @@ public: v2s32 local_animations[4]; float local_animation_speed; - session_t peer_id = PEER_ID_INEXISTENT; - std::string inventory_formspec; PlayerControl control; diff --git a/src/remoteplayer.h b/src/remoteplayer.h index 84978f343..59870f14b 100644 --- a/src/remoteplayer.h +++ b/src/remoteplayer.h @@ -135,6 +135,10 @@ public: u16 protocol_version = 0; + session_t getPeerId() const { return m_peer_id; } + + void setPeerId(session_t peer_id) { m_peer_id = peer_id; } + private: /* serialize() writes a bunch of text that can contain @@ -166,4 +170,6 @@ private: bool m_sky_clouds; CloudParams m_cloud_params; + + session_t m_peer_id = PEER_ID_INEXISTENT; }; diff --git a/src/script/lua_api/l_object.cpp b/src/script/lua_api/l_object.cpp index 4e4901db5..bc150d70f 100644 --- a/src/script/lua_api/l_object.cpp +++ b/src/script/lua_api/l_object.cpp @@ -1023,7 +1023,7 @@ int ObjectRef::l_is_player_connected(lua_State *L) NO_MAP_LOCK_REQUIRED; ObjectRef *ref = checkobject(L, 1); RemotePlayer *player = getplayer(ref); - lua_pushboolean(L, (player != NULL && player->peer_id != 0)); + lua_pushboolean(L, (player != NULL && player->getPeerId() != PEER_ID_INEXISTENT)); return 1; } diff --git a/src/script/lua_api/l_server.cpp b/src/script/lua_api/l_server.cpp index d99f46cd1..d663503e5 100644 --- a/src/script/lua_api/l_server.cpp +++ b/src/script/lua_api/l_server.cpp @@ -124,7 +124,7 @@ int ModApiServer::l_get_player_ip(lua_State *L) } try { - Address addr = getServer(L)->getPeerAddress(player->peer_id); + Address addr = getServer(L)->getPeerAddress(player->getPeerId()); std::string ip_str = addr.serializeString(); lua_pushstring(L, ip_str.c_str()); return 1; @@ -150,7 +150,7 @@ int ModApiServer::l_get_player_information(lua_State *L) Address addr; try { - addr = getServer(L)->getPeerAddress(player->peer_id); + addr = getServer(L)->getPeerAddress(player->getPeerId()); } catch(const con::PeerNotFoundException &) { dstream << FUNCTION_NAME << ": peer was not found" << std::endl; lua_pushnil(L); // error @@ -171,16 +171,18 @@ int ModApiServer::l_get_player_information(lua_State *L) return 1; \ } - ERET(getServer(L)->getClientConInfo(player->peer_id,con::MIN_RTT,&min_rtt)) - ERET(getServer(L)->getClientConInfo(player->peer_id,con::MAX_RTT,&max_rtt)) - ERET(getServer(L)->getClientConInfo(player->peer_id,con::AVG_RTT,&avg_rtt)) - ERET(getServer(L)->getClientConInfo(player->peer_id,con::MIN_JITTER,&min_jitter)) - ERET(getServer(L)->getClientConInfo(player->peer_id,con::MAX_JITTER,&max_jitter)) - ERET(getServer(L)->getClientConInfo(player->peer_id,con::AVG_JITTER,&avg_jitter)) + ERET(getServer(L)->getClientConInfo(player->getPeerId(), con::MIN_RTT, &min_rtt)) + ERET(getServer(L)->getClientConInfo(player->getPeerId(), con::MAX_RTT, &max_rtt)) + ERET(getServer(L)->getClientConInfo(player->getPeerId(), con::AVG_RTT, &avg_rtt)) + ERET(getServer(L)->getClientConInfo(player->getPeerId(), con::MIN_JITTER, + &min_jitter)) + ERET(getServer(L)->getClientConInfo(player->getPeerId(), con::MAX_JITTER, + &max_jitter)) + ERET(getServer(L)->getClientConInfo(player->getPeerId(), con::AVG_JITTER, + &avg_jitter)) - ERET(getServer(L)->getClientInfo(player->peer_id, - &state, &uptime, &ser_vers, &prot_vers, - &major, &minor, &patch, &vers_string)) + ERET(getServer(L)->getClientInfo(player->getPeerId(), &state, &uptime, &ser_vers, + &prot_vers, &major, &minor, &patch, &vers_string)) lua_newtable(L); int table = lua_gettop(L); @@ -291,7 +293,7 @@ int ModApiServer::l_ban_player(lua_State *L) try { Address addr = getServer(L)->getPeerAddress( - dynamic_cast(getEnv(L))->getPlayer(name)->peer_id); + dynamic_cast(getEnv(L))->getPlayer(name)->getPeerId()); std::string ip_str = addr.serializeString(); getServer(L)->setIpBanned(ip_str, name); } catch(const con::PeerNotFoundException &) { @@ -323,7 +325,7 @@ int ModApiServer::l_kick_player(lua_State *L) lua_pushboolean(L, false); // No such player return 1; } - getServer(L)->DenyAccess_Legacy(player->peer_id, utf8_to_wide(message)); + getServer(L)->DenyAccess_Legacy(player->getPeerId(), utf8_to_wide(message)); lua_pushboolean(L, true); return 1; } diff --git a/src/server.cpp b/src/server.cpp index bd90afb32..b7627f6cd 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -1015,7 +1015,7 @@ PlayerSAO* Server::StageTwoClientInit(session_t peer_id) // If failed, cancel if (!playersao || !player) { - if (player && player->peer_id != 0) { + if (player && player->getPeerId() != PEER_ID_INEXISTENT) { actionstream << "Server: Failed to emerge player \"" << playername << "\" (player allocated to an another client)" << std::endl; DenyAccess_Legacy(peer_id, L"Another client is connected with this " @@ -1057,7 +1057,7 @@ PlayerSAO* Server::StageTwoClientInit(session_t peer_id) // Send information about server to player in chat SendChatMessage(peer_id, ChatMessage(CHATMESSAGE_TYPE_SYSTEM, getStatusString())); } - Address addr = getPeerAddress(player->peer_id); + Address addr = getPeerAddress(player->getPeerId()); std::string ip_str = addr.serializeString(); actionstream<getName() <<" [" << ip_str << "] joins game. " << std::endl; /* @@ -1872,7 +1872,7 @@ void Server::SendPlayerPrivileges(session_t peer_id) { RemotePlayer *player = m_env->getPlayer(peer_id); assert(player); - if(player->peer_id == PEER_ID_INEXISTENT) + if(player->getPeerId() == PEER_ID_INEXISTENT) return; std::set privs; @@ -1892,7 +1892,7 @@ void Server::SendPlayerInventoryFormspec(session_t peer_id) { RemotePlayer *player = m_env->getPlayer(peer_id); assert(player); - if(player->peer_id == PEER_ID_INEXISTENT) + if(player->getPeerId() == PEER_ID_INEXISTENT) return; NetworkPacket pkt(TOCLIENT_INVENTORY_FORMSPEC, 0, peer_id); @@ -1948,12 +1948,12 @@ s32 Server::playSound(const SimpleSoundSpec &spec, <<"\" not found"<peer_id == PEER_ID_INEXISTENT){ + if (player->getPeerId() == PEER_ID_INEXISTENT) { infostream<<"Server::playSound: Player \""<peer_id); + dst_clients.push_back(player->getPeerId()); } else { std::vector clients = m_clients.getClientIDs(); @@ -2501,7 +2501,7 @@ void Server::sendDetachedInventory(const std::string &name, session_t peer_id) return m_clients.sendToAll(&pkt); RemotePlayer *p = m_env->getPlayer(check.c_str()); if (p) - m_clients.send(p->peer_id, 0, &pkt, true); + m_clients.send(p->getPeerId(), 0, &pkt, true); } else { if (check.empty() || getPlayerName(peer_id) == check) Send(&pkt); @@ -2765,7 +2765,7 @@ std::wstring Server::handleChat(const std::string &name, const std::wstring &wna return ws.str(); } case RPLAYER_CHATRESULT_KICK: - DenyAccess_Legacy(player->peer_id, + DenyAccess_Legacy(player->getPeerId(), L"You have been kicked due to message flooding."); return L""; case RPLAYER_CHATRESULT_OK: @@ -2818,7 +2818,9 @@ std::wstring Server::handleChat(const std::string &name, const std::wstring &wna if they are using protocol version >= 29 */ - session_t peer_id_to_avoid_sending = (player ? player->peer_id : PEER_ID_INEXISTENT); + session_t peer_id_to_avoid_sending = + (player ? player->getPeerId() : PEER_ID_INEXISTENT); + if (player && player->protocol_version >= 29) peer_id_to_avoid_sending = PEER_ID_INEXISTENT; @@ -2935,7 +2937,7 @@ void Server::reportPrivsModified(const std::string &name) RemotePlayer *player = m_env->getPlayer(name.c_str()); if (!player) return; - SendPlayerPrivileges(player->peer_id); + SendPlayerPrivileges(player->getPeerId()); PlayerSAO *sao = player->getPlayerSAO(); if(!sao) return; @@ -2950,7 +2952,7 @@ void Server::reportInventoryFormspecModified(const std::string &name) RemotePlayer *player = m_env->getPlayer(name.c_str()); if (!player) return; - SendPlayerInventoryFormspec(player->peer_id); + SendPlayerInventoryFormspec(player->getPeerId()); } void Server::setIpBanned(const std::string &ip, const std::string &name) @@ -2983,10 +2985,10 @@ void Server::notifyPlayer(const char *name, const std::wstring &msg) return; } - if (player->peer_id == PEER_ID_INEXISTENT) + if (player->getPeerId() == PEER_ID_INEXISTENT) return; - SendChatMessage(player->peer_id, ChatMessage(msg)); + SendChatMessage(player->getPeerId(), ChatMessage(msg)); } bool Server::showFormspec(const char *playername, const std::string &formspec, @@ -3000,7 +3002,7 @@ bool Server::showFormspec(const char *playername, const std::string &formspec, if (!player) return false; - SendShowFormspecMessage(player->peer_id, formspec, formname); + SendShowFormspecMessage(player->getPeerId(), formspec, formname); return true; } @@ -3011,7 +3013,7 @@ u32 Server::hudAdd(RemotePlayer *player, HudElement *form) u32 id = player->addHud(form); - SendHUDAdd(player->peer_id, id, form); + SendHUDAdd(player->getPeerId(), id, form); return id; } @@ -3027,7 +3029,7 @@ bool Server::hudRemove(RemotePlayer *player, u32 id) { delete todel; - SendHUDRemove(player->peer_id, id); + SendHUDRemove(player->getPeerId(), id); return true; } @@ -3036,7 +3038,7 @@ bool Server::hudChange(RemotePlayer *player, u32 id, HudElementStat stat, void * if (!player) return false; - SendHUDChange(player->peer_id, id, stat, data); + SendHUDChange(player->getPeerId(), id, stat, data); return true; } @@ -3045,7 +3047,7 @@ bool Server::hudSetFlags(RemotePlayer *player, u32 flags, u32 mask) if (!player) return false; - SendHUDSetFlags(player->peer_id, flags, mask); + SendHUDSetFlags(player->getPeerId(), flags, mask); player->hud_flags &= ~mask; player->hud_flags |= flags; @@ -3069,7 +3071,7 @@ bool Server::hudSetHotbarItemcount(RemotePlayer *player, s32 hotbar_itemcount) player->setHotbarItemcount(hotbar_itemcount); std::ostringstream os(std::ios::binary); writeS32(os, hotbar_itemcount); - SendHUDSetParam(player->peer_id, HUD_PARAM_HOTBAR_ITEMCOUNT, os.str()); + SendHUDSetParam(player->getPeerId(), HUD_PARAM_HOTBAR_ITEMCOUNT, os.str()); return true; } @@ -3084,7 +3086,7 @@ void Server::hudSetHotbarImage(RemotePlayer *player, std::string name) return; player->setHotbarImage(name); - SendHUDSetParam(player->peer_id, HUD_PARAM_HOTBAR_IMAGE, name); + SendHUDSetParam(player->getPeerId(), HUD_PARAM_HOTBAR_IMAGE, name); } std::string Server::hudGetHotbarImage(RemotePlayer *player) @@ -3100,7 +3102,7 @@ void Server::hudSetHotbarSelectedImage(RemotePlayer *player, std::string name) return; player->setHotbarSelectedImage(name); - SendHUDSetParam(player->peer_id, HUD_PARAM_HOTBAR_SELECTED_IMAGE, name); + SendHUDSetParam(player->getPeerId(), HUD_PARAM_HOTBAR_SELECTED_IMAGE, name); } const std::string& Server::hudGetHotbarSelectedImage(RemotePlayer *player) const @@ -3120,7 +3122,7 @@ bool Server::setLocalPlayerAnimations(RemotePlayer *player, return false; player->setLocalAnimations(animation_frames, frame_speed); - SendLocalPlayerAnimations(player->peer_id, animation_frames, frame_speed); + SendLocalPlayerAnimations(player->getPeerId(), animation_frames, frame_speed); return true; } @@ -3131,7 +3133,7 @@ bool Server::setPlayerEyeOffset(RemotePlayer *player, v3f first, v3f third) player->eye_offset_first = first; player->eye_offset_third = third; - SendEyeOffset(player->peer_id, first, third); + SendEyeOffset(player->getPeerId(), first, third); return true; } @@ -3143,7 +3145,7 @@ bool Server::setSky(RemotePlayer *player, const video::SColor &bgcolor, return false; player->setSky(bgcolor, type, params, clouds); - SendSetSky(player->peer_id, bgcolor, type, params, clouds); + SendSetSky(player->getPeerId(), bgcolor, type, params, clouds); return true; } @@ -3157,7 +3159,7 @@ bool Server::setClouds(RemotePlayer *player, float density, if (!player) return false; - SendCloudParams(player->peer_id, density, + SendCloudParams(player->getPeerId(), density, color_bright, color_ambient, height, thickness, speed); return true; @@ -3170,7 +3172,7 @@ bool Server::overrideDayNightRatio(RemotePlayer *player, bool do_override, return false; player->overrideDayNightRatio(do_override, ratio); - SendOverrideDayNightRatio(player->peer_id, do_override, ratio); + SendOverrideDayNightRatio(player->getPeerId(), do_override, ratio); return true; } @@ -3196,7 +3198,7 @@ void Server::spawnParticle(const std::string &playername, v3f pos, RemotePlayer *player = m_env->getPlayer(playername.c_str()); if (!player) return; - peer_id = player->peer_id; + peer_id = player->getPeerId(); proto_ver = player->protocol_version; } @@ -3223,7 +3225,7 @@ u32 Server::addParticleSpawner(u16 amount, float spawntime, RemotePlayer *player = m_env->getPlayer(playername.c_str()); if (!player) return -1; - peer_id = player->peer_id; + peer_id = player->getPeerId(); proto_ver = player->protocol_version; } @@ -3255,7 +3257,7 @@ void Server::deleteParticleSpawner(const std::string &playername, u32 id) RemotePlayer *player = m_env->getPlayer(playername.c_str()); if (!player) return; - peer_id = player->peer_id; + peer_id = player->getPeerId(); } m_env->deleteParticleSpawner(id); @@ -3489,7 +3491,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->peer_id != 0) { + if (player && player->getPeerId() != PEER_ID_INEXISTENT) { infostream<<"emergePlayer(): Player already connected"<peer_id == peer_id) + if (player->getPeerId() == peer_id) return player; } return NULL; @@ -431,8 +431,8 @@ void ServerEnvironment::addPlayer(RemotePlayer *player) Exception: there can be multiple players with peer_id=0 */ // If peer id is non-zero, it has to be unique. - if (player->peer_id != 0) - FATAL_ERROR_IF(getPlayer(player->peer_id) != NULL, "Peer id not unique"); + if (player->getPeerId() != PEER_ID_INEXISTENT) + FATAL_ERROR_IF(getPlayer(player->getPeerId()) != NULL, "Peer id not unique"); // Name has to be unique. FATAL_ERROR_IF(getPlayer(player->getName()) != NULL, "Player name not unique"); // Add. @@ -487,7 +487,7 @@ void ServerEnvironment::kickAllPlayers(AccessDeniedCode reason, const std::string &str_reason, bool reconnect) { for (RemotePlayer *player : m_players) { - m_server->DenyAccessVerCompliant(player->peer_id, + m_server->DenyAccessVerCompliant(player->getPeerId(), player->protocol_version, reason, str_reason, reconnect); } } @@ -1124,7 +1124,7 @@ void ServerEnvironment::step(float dtime) ScopeProfiler sp(g_profiler, "SEnv: handle players avg", SPT_AVG); for (RemotePlayer *player : m_players) { // Ignore disconnected players - if (player->peer_id == 0) + if (player->getPeerId() == PEER_ID_INEXISTENT) continue; // Move @@ -1143,7 +1143,7 @@ void ServerEnvironment::step(float dtime) std::vector players_blockpos; for (RemotePlayer *player: m_players) { // Ignore disconnected players - if (player->peer_id == 0) + if (player->getPeerId() == PEER_ID_INEXISTENT) continue; PlayerSAO *playersao = player->getPlayerSAO(); -- 2.25.1