From 8b2039b073697338fb48c9835c601259759eebe0 Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Wed, 29 Aug 2018 19:32:43 +0200 Subject: [PATCH 1/4] Add core.remove_detached_inventory Breaks backwards compatibility for good --- builtin/game/detached_inventory.lua | 4 ++ doc/lua_api.txt | 2 + src/network/clientpackethandler.cpp | 29 +++++++---- src/script/lua_api/l_inventory.cpp | 10 ++++ src/script/lua_api/l_inventory.h | 2 + src/server.cpp | 76 +++++++++++++++++++++-------- src/server.h | 4 +- 7 files changed, 96 insertions(+), 31 deletions(-) diff --git a/builtin/game/detached_inventory.lua b/builtin/game/detached_inventory.lua index 420e89ff28dd..2e27168a1daf 100644 --- a/builtin/game/detached_inventory.lua +++ b/builtin/game/detached_inventory.lua @@ -18,3 +18,7 @@ function core.create_detached_inventory(name, callbacks, player_name) return core.create_detached_inventory_raw(name, player_name) end +function core.remove_detached_inventory(name) + core.detached_inventories[name] = nil + return core.remove_detached_inventory_raw(name) +end diff --git a/doc/lua_api.txt b/doc/lua_api.txt index 80d14bff5a71..a3aa2a7bce4e 100644 --- a/doc/lua_api.txt +++ b/doc/lua_api.txt @@ -4139,6 +4139,8 @@ Inventory Note that this parameter is mostly just a workaround and will be removed in future releases. * Creates a detached inventory. If it already exists, it is cleared. +* `minetest.remove_detached_inventory(name)` + * Returns a `boolean` indicating whether the removal succeeded. * `minetest.do_item_eat(hp_change, replace_with_item, itemstack, user, pointed_thing)`: returns left over ItemStack. * See `minetest.item_eat` and `minetest.register_on_item_eat` diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp index f8be613177fb..b7a68bc9653d 100644 --- a/src/network/clientpackethandler.cpp +++ b/src/network/clientpackethandler.cpp @@ -843,21 +843,32 @@ void Client::handleCommand_InventoryFormSpec(NetworkPacket* pkt) void Client::handleCommand_DetachedInventory(NetworkPacket* pkt) { - std::string datastring(pkt->getString(0), pkt->getSize()); - std::istringstream is(datastring, std::ios_base::binary); - - std::string name = deSerializeString(is); + std::string name; + bool keep_inv = true; + *pkt >> name >> keep_inv; infostream << "Client: Detached inventory update: \"" << name - << "\"" << std::endl; + << "\", mode=" << (keep_inv ? "update" : "remove") << std::endl; - Inventory *inv = NULL; - if (m_detached_inventories.count(name) > 0) - inv = m_detached_inventories[name]; - else { + const auto &inv_it = m_detached_inventories.find(name); + if (!keep_inv) { + if (inv_it != m_detached_inventories.end()) { + delete inv_it->second; + m_detached_inventories.erase(inv_it); + } + return; + } + Inventory *inv = nullptr; + if (inv_it == m_detached_inventories.end()) { inv = new Inventory(m_itemdef); m_detached_inventories[name] = inv; + } else { + inv = inv_it->second; } + + std::string contents; + *pkt >> contents; + std::istringstream is(contents, std::ios::binary); inv->deSerialize(is); } diff --git a/src/script/lua_api/l_inventory.cpp b/src/script/lua_api/l_inventory.cpp index 04fa3a196bc3..6e7afa4a4e67 100644 --- a/src/script/lua_api/l_inventory.cpp +++ b/src/script/lua_api/l_inventory.cpp @@ -536,8 +536,18 @@ int ModApiInventory::l_create_detached_inventory_raw(lua_State *L) return 1; } +// remove_detached_inventory_raw(name) +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)); + return 1; +} + void ModApiInventory::Initialize(lua_State *L, int top) { API_FCT(create_detached_inventory_raw); + API_FCT(remove_detached_inventory_raw); API_FCT(get_inventory); } diff --git a/src/script/lua_api/l_inventory.h b/src/script/lua_api/l_inventory.h index 2b7910ac35fa..94f670c9d51d 100644 --- a/src/script/lua_api/l_inventory.h +++ b/src/script/lua_api/l_inventory.h @@ -120,6 +120,8 @@ class ModApiInventory : public ModApiBase { private: static int l_create_detached_inventory_raw(lua_State *L); + static int l_remove_detached_inventory_raw(lua_State *L); + static int l_get_inventory(lua_State *L); public: diff --git a/src/server.cpp b/src/server.cpp index 37aad4bceccf..7d47fdc1f25c 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -2475,33 +2475,42 @@ void Server::sendRequestedMedia(session_t peer_id, void Server::sendDetachedInventory(const std::string &name, session_t peer_id) { - if(m_detached_inventories.count(name) == 0) { - errorstream<serialize(os); + if (player_it == m_detached_inventories_player.end() || + player_it->second.empty()) { + // ok. no restriction + } else { + RemotePlayer *p = m_env->getPlayer(player_it->second.c_str()); + if (!p) + return; // Player is offline - // Make data buffer - std::string s = os.str(); + if (peer_id != PEER_ID_INEXISTENT && peer_id != p->getPeerId()) + return; // Send to nobody + + peer_id = p->getPeerId(); + } NetworkPacket pkt(TOCLIENT_DETACHED_INVENTORY, 0, peer_id); - pkt.putRawString(s.c_str(), s.size()); + pkt << name; - const std::string &check = m_detached_inventories_player[name]; - if (peer_id == PEER_ID_INEXISTENT) { - if (check.empty()) - return m_clients.sendToAll(&pkt); - RemotePlayer *p = m_env->getPlayer(check.c_str()); - if (p) - m_clients.send(p->getPeerId(), 0, &pkt, true); + if (inv_it == m_detached_inventories.end()) { + pkt << false; // Remove inventory } else { - if (check.empty() || getPlayerName(peer_id) == check) - Send(&pkt); + pkt << true; // Add/update inventory + + std::ostringstream os(std::ios_base::binary); + inv_it->second->serialize(os); + pkt << os.str(); } + + // Make data buffer + + if (peer_id == PEER_ID_INEXISTENT) + m_clients.sendToAll(&pkt); + else + Send(&pkt); } void Server::sendDetachedInventories(session_t peer_id) @@ -2662,9 +2671,10 @@ void Server::DeleteClient(session_t peer_id, ClientDeletionReason reason) playersao->clearParentAttachment(); // inform connected clients + const std::string &player_name = player->getName(); NetworkPacket notice(TOCLIENT_UPDATE_PLAYER_LIST, 0, PEER_ID_INEXISTENT); // (u16) 1 + std::string represents a vector serialization representation - notice << (u8) PLAYER_LIST_REMOVE << (u16) 1 << std::string(playersao->getPlayer()->getName()); + notice << (u8) PLAYER_LIST_REMOVE << (u16) 1 << player_name; m_clients.sendToAll(¬ice); // run scripts m_script->on_leaveplayer(playersao, reason == CDR_TIMEOUT); @@ -3262,6 +3272,30 @@ Inventory* Server::createDetachedInventory(const std::string &name, const std::s return inv; } +bool Server::removeDetachedInventory(const std::string &name) +{ + const auto &inv_it = m_detached_inventories.find(name); + if (inv_it == m_detached_inventories.end()) + return false; + + delete inv_it->second; + m_detached_inventories.erase(inv_it); + + const auto &player_it = m_detached_inventories_player.find(name); + if (player_it != m_detached_inventories_player.end()) { + RemotePlayer *player = m_env->getPlayer(player_it->second.c_str()); + + if (player && player->getPeerId() != PEER_ID_INEXISTENT) + sendDetachedInventory(name, player->getPeerId()); + + m_detached_inventories_player.erase(player_it); + } else { + // Notify all players about the change + sendDetachedInventory(name, PEER_ID_INEXISTENT); + } + return true; +} + // actions: time-reversed list // Return value: success/failure bool Server::rollbackRevertActions(const std::list &actions, diff --git a/src/server.h b/src/server.h index 751eaa5f2dce..18de37053532 100644 --- a/src/server.h +++ b/src/server.h @@ -248,7 +248,9 @@ class Server : public con::PeerHandler, public MapEventReceiver, void deleteParticleSpawner(const std::string &playername, u32 id); // Creates or resets inventory - Inventory* createDetachedInventory(const std::string &name, const std::string &player=""); + Inventory *createDetachedInventory(const std::string &name, + const std::string &player = ""); + bool removeDetachedInventory(const std::string &name); // Envlock and conlock should be locked when using scriptapi ServerScripting *getScriptIface(){ return m_script; } From c980f4c24f4be19f3667f2f753be63381bb43c14 Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Wed, 29 Aug 2018 19:34:12 +0200 Subject: [PATCH 2/4] Bump proto version --- src/network/networkprotocol.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/network/networkprotocol.h b/src/network/networkprotocol.h index 113b11177252..f7c56450a394 100644 --- a/src/network/networkprotocol.h +++ b/src/network/networkprotocol.h @@ -188,19 +188,21 @@ with this program; if not, write to the Free Software Foundation, Inc., Nodebox version 5 Add disconnected nodeboxes Add TOCLIENT_FORMSPEC_PREPEND + PROTOCOL VERSION 37: + Redo detached inventory sending */ -#define LATEST_PROTOCOL_VERSION 36 +#define LATEST_PROTOCOL_VERSION 37 #define LATEST_PROTOCOL_VERSION_STRING TOSTRING(LATEST_PROTOCOL_VERSION) // Server's supported network protocol range -#define SERVER_PROTOCOL_VERSION_MIN 36 +#define SERVER_PROTOCOL_VERSION_MIN 37 #define SERVER_PROTOCOL_VERSION_MAX LATEST_PROTOCOL_VERSION // Client's supported network protocol range // The minimal version depends on whether // send_pre_v25_init is enabled or not -#define CLIENT_PROTOCOL_VERSION_MIN 36 +#define CLIENT_PROTOCOL_VERSION_MIN 37 #define CLIENT_PROTOCOL_VERSION_MAX LATEST_PROTOCOL_VERSION // Constant that differentiates the protocol from random data and other protocols From d4459fc5356e1753ad9e514f7cc1889fc612def6 Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Wed, 29 Aug 2018 19:38:39 +0200 Subject: [PATCH 3/4] Fix comment --- src/server.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/server.cpp b/src/server.cpp index 7d47fdc1f25c..4707cf8dc28a 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -2498,15 +2498,14 @@ void Server::sendDetachedInventory(const std::string &name, session_t peer_id) if (inv_it == m_detached_inventories.end()) { pkt << false; // Remove inventory } else { - pkt << true; // Add/update inventory + pkt << true; // Update inventory + // Serialization & NetworkPacket isn't a love story std::ostringstream os(std::ios_base::binary); inv_it->second->serialize(os); pkt << os.str(); } - // Make data buffer - if (peer_id == PEER_ID_INEXISTENT) m_clients.sendToAll(&pkt); else From aae5a92e3aa95ea8b1035ca90fca59c6638806d8 Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Mon, 8 Oct 2018 19:13:29 +0200 Subject: [PATCH 4/4] Improve comments --- src/server.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server.cpp b/src/server.cpp index 4707cf8dc28a..66efbcfaf2d3 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -2480,14 +2480,14 @@ void Server::sendDetachedInventory(const std::string &name, session_t peer_id) if (player_it == m_detached_inventories_player.end() || player_it->second.empty()) { - // ok. no restriction + // OK. Send to everyone } else { RemotePlayer *p = m_env->getPlayer(player_it->second.c_str()); if (!p) return; // Player is offline if (peer_id != PEER_ID_INEXISTENT && peer_id != p->getPeerId()) - return; // Send to nobody + return; // Caller requested send to a different player, so don't send. peer_id = p->getPeerId(); }