Skip to content

Commit

Permalink
Server class code cleanups (minetest#9769)
Browse files Browse the repository at this point in the history
* Server::overrideDayNightRatio doesn't require to return bool
There is no sense to sending null player, the caller should send a valid object

* Server::init: make private & cleanup
This function is always called before start() and loads some variables which can be loaded in constructor directly.
Make it private and call it directly in start

* Split Server inventory responsibility to a dedicated object

This splits permit to found various historical issues:
* duplicate lookups on player connection
* sending inventory to non related player when a player connects
* non friendly lookups on detached inventories ownership

This reduce the detached inventory complexity and also increased the
lookup performance in a quite interesting way for servers with thousands
of inventories.
  • Loading branch information
nerzhul committed May 7, 2020
1 parent 650168c commit 454dbf8
Show file tree
Hide file tree
Showing 12 changed files with 312 additions and 207 deletions.
1 change: 0 additions & 1 deletion src/client/game.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1302,7 +1302,6 @@ bool Game::createSingleplayerServer(const std::string &map_dir,
}

server = new Server(map_dir, gamespec, simple_singleplayer_mode, bind_addr, false);
server->init();
server->start();

return true;
Expand Down
2 changes: 0 additions & 2 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,6 @@ static bool run_dedicated_server(const GameParams &game_params, const Settings &
// Create server
Server server(game_params.world_path, game_params.game_spec,
false, bind_addr, true, &iface);
server.init();

g_term_console.setup(&iface, &kill, admin_nick);

Expand Down Expand Up @@ -922,7 +921,6 @@ static bool run_dedicated_server(const GameParams &game_params, const Settings &
// Create server
Server server(game_params.world_path, game_params.game_spec, false,
bind_addr, true);
server.init();
server.start();

// Run server
Expand Down
11 changes: 6 additions & 5 deletions src/network/serverpackethandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
#include "network/networkprotocol.h"
#include "network/serveropcodes.h"
#include "server/player_sao.h"
#include "server/serverinventorymgr.h"
#include "util/auth.h"
#include "util/base64.h"
#include "util/pointedthing.h"
Expand Down Expand Up @@ -620,9 +621,9 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt)
ma->from_inv.applyCurrentPlayer(player->getName());
ma->to_inv.applyCurrentPlayer(player->getName());

setInventoryModified(ma->from_inv);
m_inventory_mgr->setInventoryModified(ma->from_inv);
if (ma->from_inv != ma->to_inv)
setInventoryModified(ma->to_inv);
m_inventory_mgr->setInventoryModified(ma->to_inv);

bool from_inv_is_current_player =
(ma->from_inv.type == InventoryLocation::PLAYER) &&
Expand Down Expand Up @@ -687,7 +688,7 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt)

da->from_inv.applyCurrentPlayer(player->getName());

setInventoryModified(da->from_inv);
m_inventory_mgr->setInventoryModified(da->from_inv);

/*
Disable dropping items out of craftpreview
Expand Down Expand Up @@ -723,7 +724,7 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt)

ca->craft_inv.applyCurrentPlayer(player->getName());

setInventoryModified(ca->craft_inv);
m_inventory_mgr->setInventoryModified(ca->craft_inv);

//bool craft_inv_is_current_player =
// (ca->craft_inv.type == InventoryLocation::PLAYER) &&
Expand All @@ -739,7 +740,7 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt)
}

// Do the action
a->apply(this, playersao, this);
a->apply(m_inventory_mgr.get(), playersao, this);
// Eat the action
delete a;
}
Expand Down
5 changes: 5 additions & 0 deletions src/script/lua_api/l_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ Server *ModApiBase::getServer(lua_State *L)
return getScriptApiBase(L)->getServer();
}

ServerInventoryManager *ModApiBase::getServerInventoryMgr(lua_State *L)
{
return getScriptApiBase(L)->getServer()->getInventoryMgr();
}

#ifndef SERVER
Client *ModApiBase::getClient(lua_State *L)
{
Expand Down
2 changes: 2 additions & 0 deletions src/script/lua_api/l_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@ class GUIEngine;
class ScriptApiBase;
class Server;
class Environment;
class ServerInventoryManager;

class ModApiBase : protected LuaHelper {

public:
static ScriptApiBase* getScriptApiBase(lua_State *L);
static Server* getServer(lua_State *L);
static ServerInventoryManager *getServerInventoryMgr(lua_State *L);
#ifndef SERVER
static Client* getClient(lua_State *L);
static GUIEngine* getGuiEngine(lua_State *L);
Expand Down
13 changes: 7 additions & 6 deletions src/script/lua_api/l_inventory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
#include "common/c_converter.h"
#include "common/c_content.h"
#include "server.h"
#include "server/serverinventorymgr.h"
#include "remoteplayer.h"

/*
Expand All @@ -38,7 +39,7 @@ InvRef* InvRef::checkobject(lua_State *L, int narg)

Inventory* InvRef::getinv(lua_State *L, InvRef *ref)
{
return getServer(L)->getInventory(ref->m_loc);
return getServerInventoryMgr(L)->getInventory(ref->m_loc);
}

InventoryList* InvRef::getlist(lua_State *L, InvRef *ref,
Expand All @@ -54,7 +55,7 @@ InventoryList* InvRef::getlist(lua_State *L, InvRef *ref,
void InvRef::reportInventoryChange(lua_State *L, InvRef *ref)
{
// Inform other things that the inventory has changed
getServer(L)->setInventoryModified(ref->m_loc);
getServerInventoryMgr(L)->setInventoryModified(ref->m_loc);
}

// Exported functions
Expand Down Expand Up @@ -497,7 +498,7 @@ int ModApiInventory::l_get_inventory(lua_State *L)
v3s16 pos = check_v3s16(L, -1);
loc.setNodeMeta(pos);

if (getServer(L)->getInventory(loc) != NULL)
if (getServerInventoryMgr(L)->getInventory(loc) != NULL)
InvRef::create(L, loc);
else
lua_pushnil(L);
Expand All @@ -515,7 +516,7 @@ int ModApiInventory::l_get_inventory(lua_State *L)
lua_pop(L, 1);
}

if (getServer(L)->getInventory(loc) != NULL)
if (getServerInventoryMgr(L)->getInventory(loc) != NULL)
InvRef::create(L, loc);
else
lua_pushnil(L);
Expand All @@ -530,7 +531,7 @@ int ModApiInventory::l_create_detached_inventory_raw(lua_State *L)
NO_MAP_LOCK_REQUIRED;
const char *name = luaL_checkstring(L, 1);
std::string player = readParam<std::string>(L, 2, "");
if (getServer(L)->createDetachedInventory(name, player) != NULL) {
if (getServerInventoryMgr(L)->createDetachedInventory(name, getServer(L)->idef(), player) != NULL) {
InventoryLocation loc;
loc.setDetached(name);
InvRef::create(L, loc);
Expand All @@ -545,7 +546,7 @@ int ModApiInventory::l_remove_detached_inventory_raw(lua_State *L)
{
NO_MAP_LOCK_REQUIRED;
const std::string &name = luaL_checkstring(L, 1);
lua_pushboolean(L, getServer(L)->removeDetachedInventory(name));
lua_pushboolean(L, getServerInventoryMgr(L)->removeDetachedInventory(name));
return 1;
}

Expand Down
7 changes: 3 additions & 4 deletions src/script/lua_api/l_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
#include "scripting_server.h"
#include "server/luaentity_sao.h"
#include "server/player_sao.h"
#include "server/serverinventorymgr.h"

/*
ObjectRef
Expand Down Expand Up @@ -289,7 +290,7 @@ int ObjectRef::l_get_inventory(lua_State *L)
if (co == NULL) return 0;
// Do it
InventoryLocation loc = co->getInventoryLocation();
if (getServer(L)->getInventory(loc) != NULL)
if (getServerInventoryMgr(L)->getInventory(loc) != NULL)
InvRef::create(L, loc);
else
lua_pushnil(L); // An object may have no inventory (nil)
Expand Down Expand Up @@ -2172,9 +2173,7 @@ int ObjectRef::l_override_day_night_ratio(lua_State *L)
ratio = readParam<float>(L, 2);
}

if (!getServer(L)->overrideDayNightRatio(player, do_override, ratio))
return 0;

getServer(L)->overrideDayNightRatio(player, do_override, ratio);
lua_pushboolean(L, true);
return 1;
}
Expand Down
Loading

0 comments on commit 454dbf8

Please sign in to comment.