Skip to content

Commit

Permalink
Environment cleanup
Browse files Browse the repository at this point in the history 
* Move client list to ServerEnvironment and use RemotePlayer members instead of Player
* ClientEnvironment only use setLocalPlayer to specify the current player
* Remove ClientEnvironment dead code on player list (in fact other players are CAO not Player objects)
* Drop LocalPlayer::getPlayer(xxx) functions which aren't used.
* Improve a little bit performance by using const ref list for ClientEnvironment::getPlayerNames() & Client::getConnectedPlayerNames()
* Drop isLocal() function from (Local)Player which is not needed anymore because of previous changes

This change permits to cleanup shared client list which is very old code.
ClientEnvironment doesn't use player list anymore, it only contains the local player, as addPlayer is only called from Client constructor client side.
Clients are only CAO on client side, this cleanup permit to remove confusion about player list.
  • Loading branch information
@nerzhul
nerzhul committed Oct 9, 2016
1 parent b3fc133 commit 70f104b
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 162 deletions.
14 changes: 5 additions & 9 deletions src/client.cpp
View file
Expand Up @@ -257,7 +257,7 @@ Client::Client(
m_localdb(NULL)
{
// Add local player
m_env.addPlayer(new LocalPlayer(this, playername));
m_env.setLocalPlayer(new LocalPlayer(this, playername));

m_mapper = new Mapper(device, this);
m_cache_save_interval = g_settings->getU16("server_map_save_interval");
Expand Down Expand Up @@ -1418,8 +1418,9 @@ Inventory* Client::getInventory(const InventoryLocation &loc)
break;
case InventoryLocation::PLAYER:
{
LocalPlayer *player = m_env.getPlayer(loc.name.c_str());
if(!player)
// Check if we are working with local player inventory
LocalPlayer *player = m_env.getLocalPlayer();
if (!player || strcmp(player->getName(), loc.name.c_str()) != 0)
return NULL;
return &player->inventory;
}
Expand Down Expand Up @@ -1500,11 +1501,6 @@ ClientActiveObject * Client::getSelectedActiveObject(
return NULL;
}

std::list<std::string> Client::getConnectedPlayerNames()
{
return m_env.getPlayerNames();
}

float Client::getAnimationTime()
{
return m_animation_time;
Expand Down Expand Up @@ -1664,7 +1660,7 @@ void Client::addUpdateMeshTaskForNode(v3s16 nodepos, bool ack_to_server, bool ur
ClientEvent Client::getClientEvent()
{
ClientEvent event;
if(m_client_event_queue.size() == 0) {
if (m_client_event_queue.empty()) {
event.type = CE_NONE;
}
else {
Expand Down
5 changes: 4 additions & 1 deletion src/client.h
View file
Expand Up @@ -452,7 +452,10 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
core::line3d<f32> shootline_on_map
);

std::list<std::string> getConnectedPlayerNames();
const std::list<std::string> &getConnectedPlayerNames()
{
return m_env.getPlayerNames();
}

float getAnimationTime();

Expand Down
8 changes: 4 additions & 4 deletions src/content_cao.cpp
View file
Expand Up @@ -653,10 +653,10 @@ void GenericCAO::initialize(const std::string &data)
pos_translator.init(m_position);
updateNodePos();

if(m_is_player)
{
LocalPlayer *player = m_env->getPlayer(m_name.c_str());
if (player && player->isLocal()) {
if (m_is_player) {
// Check if it's the current player
LocalPlayer *player = m_env->getLocalPlayer();
if (player && strcmp(player->getName(), m_name.c_str()) == 0) {
m_is_local_player = true;
m_is_visible = false;
LocalPlayer* localplayer = player;
Expand Down
165 changes: 59 additions & 106 deletions src/environment.cpp
View file
Expand Up @@ -69,62 +69,6 @@ Environment::Environment():

Environment::~Environment()
{
// Deallocate players
for(std::vector<Player*>::iterator i = m_players.begin();
i != m_players.end(); ++i) {
delete (*i);
}
}

void Environment::addPlayer(Player *player)
{
DSTACK(FUNCTION_NAME);
/*
Check that peer_ids are unique.
Also check that names are unique.
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");
// Name has to be unique.
FATAL_ERROR_IF(getPlayer(player->getName()) != NULL, "Player name not unique");
// Add.
m_players.push_back(player);
}

void Environment::removePlayer(Player* player)
{
for (std::vector<Player*>::iterator it = m_players.begin();
it != m_players.end(); ++it) {
if ((*it) == player) {
delete *it;
m_players.erase(it);
return;
}
}
}

Player *Environment::getPlayer(u16 peer_id)
{
for (std::vector<Player*>::iterator i = m_players.begin();
i != m_players.end(); ++i) {
Player *player = *i;
if (player->peer_id == peer_id)
return player;
}
return NULL;
}

Player *Environment::getPlayer(const char *name)
{
for (std::vector<Player*>::iterator i = m_players.begin();
i != m_players.end(); ++i) {
Player *player = *i;
if(strcmp(player->getName(), name) == 0)
return player;
}
return NULL;
}

u32 Environment::getDayNightRatio()
Expand Down Expand Up @@ -545,10 +489,16 @@ ServerEnvironment::~ServerEnvironment()
m_map->drop();

// Delete ActiveBlockModifiers
for(std::vector<ABMWithState>::iterator
for (std::vector<ABMWithState>::iterator
i = m_abms.begin(); i != m_abms.end(); ++i){
delete i->abm;
}

// Deallocate players
for (std::vector<RemotePlayer *>::iterator i = m_players.begin();
i != m_players.end(); ++i) {
delete (*i);
}
}

Map & ServerEnvironment::getMap()
Expand All @@ -563,12 +513,53 @@ ServerMap & ServerEnvironment::getServerMap()

RemotePlayer *ServerEnvironment::getPlayer(const u16 peer_id)
{
return dynamic_cast<RemotePlayer *>(Environment::getPlayer(peer_id));
for (std::vector<RemotePlayer *>::iterator i = m_players.begin();
i != m_players.end(); ++i) {
RemotePlayer *player = *i;
if (player->peer_id == peer_id)
return player;
}
return NULL;
}

RemotePlayer *ServerEnvironment::getPlayer(const char* name)
{
return dynamic_cast<RemotePlayer *>(Environment::getPlayer(name));
for (std::vector<RemotePlayer *>::iterator i = m_players.begin();
i != m_players.end(); ++i) {
RemotePlayer *player = *i;
if (strcmp(player->getName(), name) == 0)
return player;
}
return NULL;
}

void ServerEnvironment::addPlayer(RemotePlayer *player)
{
DSTACK(FUNCTION_NAME);
/*
Check that peer_ids are unique.
Also check that names are unique.
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");
// Name has to be unique.
FATAL_ERROR_IF(getPlayer(player->getName()) != NULL, "Player name not unique");
// Add.
m_players.push_back(player);
}

void ServerEnvironment::removePlayer(RemotePlayer *player)
{
for (std::vector<RemotePlayer *>::iterator it = m_players.begin();
it != m_players.end(); ++it) {
if ((*it) == player) {
delete *it;
m_players.erase(it);
return;
}
}
}

bool ServerEnvironment::line_of_sight(v3f pos1, v3f pos2, float stepsize, v3s16 *p)
Expand Down Expand Up @@ -601,7 +592,7 @@ bool ServerEnvironment::line_of_sight(v3f pos1, v3f pos2, float stepsize, v3s16
void ServerEnvironment::kickAllPlayers(AccessDeniedCode reason,
const std::string &str_reason, bool reconnect)
{
for (std::vector<Player*>::iterator it = m_players.begin();
for (std::vector<RemotePlayer *>::iterator it = m_players.begin();
it != m_players.end(); ++it) {
RemotePlayer *player = dynamic_cast<RemotePlayer *>(*it);
((Server*)m_gamedef)->DenyAccessVerCompliant(player->peer_id,
Expand All @@ -614,7 +605,7 @@ void ServerEnvironment::saveLoadedPlayers()
std::string players_path = m_path_world + DIR_DELIM "players";
fs::CreateDir(players_path);

for (std::vector<Player*>::iterator it = m_players.begin();
for (std::vector<RemotePlayer *>::iterator it = m_players.begin();
it != m_players.end();
++it) {
RemotePlayer *player = static_cast<RemotePlayer*>(*it);
Expand Down Expand Up @@ -1253,7 +1244,7 @@ void ServerEnvironment::step(float dtime)
*/
{
ScopeProfiler sp(g_profiler, "SEnv: handle players avg", SPT_AVG);
for (std::vector<Player*>::iterator i = m_players.begin();
for (std::vector<RemotePlayer *>::iterator i = m_players.begin();
i != m_players.end(); ++i) {
RemotePlayer *player = dynamic_cast<RemotePlayer *>(*i);
assert(player);
Expand All @@ -1276,7 +1267,7 @@ void ServerEnvironment::step(float dtime)
Get player block positions
*/
std::vector<v3s16> players_blockpos;
for (std::vector<Player*>::iterator i = m_players.begin();
for (std::vector<RemotePlayer *>::iterator i = m_players.begin();
i != m_players.end(); ++i) {
RemotePlayer *player = dynamic_cast<RemotePlayer *>(*i);
assert(player);
Expand Down Expand Up @@ -2255,6 +2246,7 @@ ClientEnvironment::ClientEnvironment(ClientMap *map, scene::ISceneManager *smgr,
ITextureSource *texturesource, IGameDef *gamedef,
IrrlichtDevice *irr):
m_map(map),
m_local_player(NULL),
m_smgr(smgr),
m_texturesource(texturesource),
m_gamedef(gamedef),
Expand Down Expand Up @@ -2291,37 +2283,16 @@ ClientMap & ClientEnvironment::getClientMap()
return *m_map;
}

LocalPlayer *ClientEnvironment::getPlayer(const u16 peer_id)
{
return dynamic_cast<LocalPlayer *>(Environment::getPlayer(peer_id));
}

LocalPlayer *ClientEnvironment::getPlayer(const char* name)
{
return dynamic_cast<LocalPlayer *>(Environment::getPlayer(name));
}

void ClientEnvironment::addPlayer(LocalPlayer *player)
void ClientEnvironment::setLocalPlayer(LocalPlayer *player)
{
DSTACK(FUNCTION_NAME);
/*
It is a failure if already is a local player
*/
FATAL_ERROR_IF(getLocalPlayer() != NULL,
"Player is local but there is already a local player");

Environment::addPlayer(player);
}
FATAL_ERROR_IF(m_local_player != NULL,
"Local player already allocated");

LocalPlayer *ClientEnvironment::getLocalPlayer()
{
for(std::vector<Player*>::iterator i = m_players.begin();
i != m_players.end(); ++i) {
Player *player = *i;
if (player->isLocal())
return (LocalPlayer*)player;
}
return NULL;
m_local_player = player;
}

void ClientEnvironment::step(float dtime)
Expand Down Expand Up @@ -2558,24 +2529,6 @@ void ClientEnvironment::step(float dtime)
}
}

/*
Stuff that can be done in an arbitarily large dtime
*/
for (std::vector<Player*>::iterator i = m_players.begin();
i != m_players.end(); ++i) {
Player *player = *i;
assert(player);

/*
Handle non-local players
*/
if (!player->isLocal()) {
// Move
player->move(dtime, this, 100*BS);

}
}

// Update lighting on local player (used for wield item)
u32 day_night_ratio = getDayNightRatio();
{
Expand Down
32 changes: 11 additions & 21 deletions src/environment.h
View file
Expand Up @@ -72,9 +72,6 @@ class Environment

virtual Map & getMap() = 0;

virtual void addPlayer(Player *player);
void removePlayer(Player *player);

u32 getDayNightRatio();

// 0-23999
Expand All @@ -94,12 +91,6 @@ class Environment
u32 m_added_objects;

protected:
Player *getPlayer(u16 peer_id);
Player *getPlayer(const char *name);

// peer_ids in here should be unique, except that there may be many 0s
std::vector<Player*> m_players;

GenericAtomic<float> m_time_of_day_speed;

/*
Expand Down Expand Up @@ -325,6 +316,8 @@ class ServerEnvironment : public Environment
void saveLoadedPlayers();
void savePlayer(RemotePlayer *player);
RemotePlayer *loadPlayer(const std::string &playername);
void addPlayer(RemotePlayer *player);
void removePlayer(RemotePlayer *player);

/*
Save and load time of day and game timer
Expand Down Expand Up @@ -520,6 +513,9 @@ class ServerEnvironment : public Environment
// Can raise to high values like 15s with eg. map generation mods.
float m_max_lag_estimate;

// peer_ids in here should be unique, except that there may be many 0s
std::vector<RemotePlayer*> m_players;

// Particles
IntervalLimiter m_particle_management_interval;
UNORDERED_MAP<u32, float> m_particle_spawners;
Expand Down Expand Up @@ -579,8 +575,8 @@ class ClientEnvironment : public Environment

void step(f32 dtime);

virtual void addPlayer(LocalPlayer *player);
LocalPlayer * getLocalPlayer();
virtual void setLocalPlayer(LocalPlayer *player);
LocalPlayer *getLocalPlayer() { return m_local_player; }

/*
ClientSimpleObjects
Expand Down Expand Up @@ -630,21 +626,15 @@ class ClientEnvironment : public Environment

u16 attachement_parent_ids[USHRT_MAX + 1];

std::list<std::string> getPlayerNames()
{ return m_player_names; }
void addPlayerName(std::string name)
{ m_player_names.push_back(name); }
void removePlayerName(std::string name)
{ m_player_names.remove(name); }
const std::list<std::string> &getPlayerNames() { return m_player_names; }
void addPlayerName(const std::string &name) { m_player_names.push_back(name); }
void removePlayerName(const std::string &name) { m_player_names.remove(name); }
void updateCameraOffset(v3s16 camera_offset)
{ m_camera_offset = camera_offset; }
v3s16 getCameraOffset() const { return m_camera_offset; }

LocalPlayer *getPlayer(const u16 peer_id);
LocalPlayer *getPlayer(const char* name);

private:
ClientMap *m_map;
LocalPlayer *m_local_player;
scene::ISceneManager *m_smgr;
ITextureSource *m_texturesource;
IGameDef *m_gamedef;
Expand Down

0 comments on commit 70f104b

Please sign in to comment.