From 3d66622772e66154b7624957a27f9be54c4c7c28 Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Tue, 4 Dec 2018 20:37:48 +0100 Subject: [PATCH] Send only changed node metadata to clients instead of whole mapblock (#5268) Includes newer style changes and fixes by est31 Improve the block position de-serialization Add type NodeMetadataMap --- src/client/client.h | 1 + src/map.h | 5 +- src/network/clientopcodes.cpp | 2 +- src/network/clientpackethandler.cpp | 27 ++++++++ src/network/networkprotocol.h | 11 +++- src/network/serveropcodes.cpp | 6 +- src/network/serverpackethandler.cpp | 4 +- src/nodemetadata.cpp | 62 +++++++++++------- src/nodemetadata.h | 27 +++++++- src/rollback_interface.cpp | 9 +-- src/script/lua_api/l_itemstackmeta.cpp | 2 +- src/script/lua_api/l_itemstackmeta.h | 2 +- src/script/lua_api/l_metadata.cpp | 6 +- src/script/lua_api/l_metadata.h | 2 +- src/script/lua_api/l_nodemeta.cpp | 14 ++--- src/script/lua_api/l_nodemeta.h | 2 +- src/server.cpp | 87 ++++++++++++++++++++++---- src/server.h | 3 + 18 files changed, 202 insertions(+), 70 deletions(-) diff --git a/src/client/client.h b/src/client/client.h index 68a832d8e..ef700e477 100644 --- a/src/client/client.h +++ b/src/client/client.h @@ -185,6 +185,7 @@ public: void handleCommand_AccessDenied(NetworkPacket* pkt); void handleCommand_RemoveNode(NetworkPacket* pkt); void handleCommand_AddNode(NetworkPacket* pkt); + void handleCommand_NodemetaChanged(NetworkPacket *pkt); void handleCommand_BlockData(NetworkPacket* pkt); void handleCommand_Inventory(NetworkPacket* pkt); void handleCommand_TimeOfDay(NetworkPacket* pkt); diff --git a/src/map.h b/src/map.h index 712a0a51a..7ef34b279 100644 --- a/src/map.h +++ b/src/map.h @@ -63,8 +63,7 @@ enum MapEditEventType{ MEET_REMOVENODE, // Node swapped (changed without metadata change) MEET_SWAPNODE, - // Node metadata of block changed (not knowing which node exactly) - // p stores block coordinate + // Node metadata changed MEET_BLOCK_NODE_METADATA_CHANGED, // Anything else (modified_blocks are set unsent) MEET_OTHER @@ -76,6 +75,7 @@ struct MapEditEvent v3s16 p; MapNode n = CONTENT_AIR; std::set modified_blocks; + bool is_private_change = false; MapEditEvent() = default; @@ -86,6 +86,7 @@ struct MapEditEvent event->p = p; event->n = n; event->modified_blocks = modified_blocks; + event->is_private_change = is_private_change; return event; } diff --git a/src/network/clientopcodes.cpp b/src/network/clientopcodes.cpp index 923a92d32..7f3ab50ed 100644 --- a/src/network/clientopcodes.cpp +++ b/src/network/clientopcodes.cpp @@ -113,7 +113,7 @@ const ToClientCommandHandler toClientCommandTable[TOCLIENT_NUM_MSG_TYPES] = { "TOCLIENT_UPDATE_PLAYER_LIST", TOCLIENT_STATE_CONNECTED, &Client::handleCommand_UpdatePlayerList }, // 0x56 { "TOCLIENT_MODCHANNEL_MSG", TOCLIENT_STATE_CONNECTED, &Client::handleCommand_ModChannelMsg }, // 0x57 { "TOCLIENT_MODCHANNEL_SIGNAL", TOCLIENT_STATE_CONNECTED, &Client::handleCommand_ModChannelSignal }, // 0x58 - null_command_handler, + { "TOCLIENT_NODEMETA_CHANGED", TOCLIENT_STATE_CONNECTED, &Client::handleCommand_NodemetaChanged }, // 0x59 null_command_handler, null_command_handler, null_command_handler, diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp index 1be7d4eeb..0dca4aff4 100644 --- a/src/network/clientpackethandler.cpp +++ b/src/network/clientpackethandler.cpp @@ -243,6 +243,33 @@ void Client::handleCommand_AddNode(NetworkPacket* pkt) addNode(p, n, remove_metadata); } + +void Client::handleCommand_NodemetaChanged(NetworkPacket *pkt) +{ + if (pkt->getSize() < 1) + return; + + std::istringstream is(pkt->readLongString(), std::ios::binary); + std::stringstream sstr; + decompressZlib(is, sstr); + + NodeMetadataList meta_updates_list(false); + meta_updates_list.deSerialize(sstr, m_itemdef, true); + + Map &map = m_env.getMap(); + for (NodeMetadataMap::const_iterator i = meta_updates_list.begin(); + i != meta_updates_list.end(); ++i) { + v3s16 pos = i->first; + + if (map.isValidPosition(pos) && + map.setNodeMetadata(pos, i->second)) + continue; // Prevent from deleting metadata + + // Meta couldn't be set, unused metadata + delete i->second; + } +} + void Client::handleCommand_BlockData(NetworkPacket* pkt) { // Ignore too small packet diff --git a/src/network/networkprotocol.h b/src/network/networkprotocol.h index 855afc638..12a91e647 100644 --- a/src/network/networkprotocol.h +++ b/src/network/networkprotocol.h @@ -190,6 +190,7 @@ with this program; if not, write to the Free Software Foundation, Inc., Add TOCLIENT_FORMSPEC_PREPEND PROTOCOL VERSION 37: Redo detached inventory sending + Add TOCLIENT_NODEMETA_CHANGED */ #define LATEST_PROTOCOL_VERSION 37 @@ -638,13 +639,19 @@ enum ToClientCommand std::string channel name u16 message length std::string message - */ + */ + TOCLIENT_MODCHANNEL_SIGNAL = 0x58, /* u8 signal id u16 channel name length std::string channel name - */ + */ + + TOCLIENT_NODEMETA_CHANGED = 0x59, + /* + serialized and compressed node metadata + */ TOCLIENT_SRP_BYTES_S_B = 0x60, /* diff --git a/src/network/serveropcodes.cpp b/src/network/serveropcodes.cpp index e4e313611..013b549c6 100644 --- a/src/network/serveropcodes.cpp +++ b/src/network/serveropcodes.cpp @@ -200,9 +200,9 @@ const ClientCommandFactory clientCommandFactoryTable[TOCLIENT_NUM_MSG_TYPES] = { "TOCLIENT_CLOUD_PARAMS", 0, true }, // 0x54 { "TOCLIENT_FADE_SOUND", 0, true }, // 0x55 { "TOCLIENT_UPDATE_PLAYER_LIST", 0, true }, // 0x56 - { "TOCLIENT_MODCHANNEL_MSG", 0, true}, // 0x57 - { "TOCLIENT_MODCHANNEL_SIGNAL", 0, true}, // 0x58 - null_command_factory, + { "TOCLIENT_MODCHANNEL_MSG", 0, true }, // 0x57 + { "TOCLIENT_MODCHANNEL_SIGNAL", 0, true }, // 0x58 + { "TOCLIENT_NODEMETA_CHANGED", 0, true }, // 0x59 null_command_factory, null_command_factory, null_command_factory, diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index 8299ad002..329b38765 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -609,7 +609,9 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt) ma->to_inv.applyCurrentPlayer(player->getName()); setInventoryModified(ma->from_inv, false); - setInventoryModified(ma->to_inv, false); + if (ma->from_inv != ma->to_inv) { + setInventoryModified(ma->to_inv, false); + } bool from_inv_is_current_player = (ma->from_inv.type == InventoryLocation::PLAYER) && diff --git a/src/nodemetadata.cpp b/src/nodemetadata.cpp index 708a08099..b84ffc8cb 100644 --- a/src/nodemetadata.cpp +++ b/src/nodemetadata.cpp @@ -112,7 +112,8 @@ int NodeMetadata::countNonPrivate() const NodeMetadataList */ -void NodeMetadataList::serialize(std::ostream &os, u8 blockver, bool disk) const +void NodeMetadataList::serialize(std::ostream &os, u8 blockver, bool disk, + bool absolute_pos) const { /* Version 0 is a placeholder for "nothing to see here; go away." @@ -128,20 +129,29 @@ void NodeMetadataList::serialize(std::ostream &os, u8 blockver, bool disk) const writeU8(os, version); writeU16(os, count); - for (const auto &it : m_data) { - v3s16 p = it.first; - NodeMetadata *data = it.second; + for (NodeMetadataMap::const_iterator + i = m_data.begin(); + i != m_data.end(); ++i) { + v3s16 p = i->first; + NodeMetadata *data = i->second; if (data->empty()) continue; - u16 p16 = p.Z * MAP_BLOCKSIZE * MAP_BLOCKSIZE + p.Y * MAP_BLOCKSIZE + p.X; - writeU16(os, p16); - + if (absolute_pos) { + writeS16(os, p.X); + writeS16(os, p.Y); + writeS16(os, p.Z); + } else { + // Serialize positions within a mapblock + u16 p16 = (p.Z * MAP_BLOCKSIZE + p.Y) * MAP_BLOCKSIZE + p.X; + writeU16(os, p16); + } data->serialize(os, version, disk); } } -void NodeMetadataList::deSerialize(std::istream &is, IItemDefManager *item_def_mgr) +void NodeMetadataList::deSerialize(std::istream &is, + IItemDefManager *item_def_mgr, bool absolute_pos) { clear(); @@ -162,15 +172,19 @@ void NodeMetadataList::deSerialize(std::istream &is, IItemDefManager *item_def_m u16 count = readU16(is); for (u16 i = 0; i < count; i++) { - u16 p16 = readU16(is); - v3s16 p; - p.Z = p16 / MAP_BLOCKSIZE / MAP_BLOCKSIZE; - p16 &= MAP_BLOCKSIZE * MAP_BLOCKSIZE - 1; - p.Y = p16 / MAP_BLOCKSIZE; - p16 &= MAP_BLOCKSIZE - 1; - p.X = p16; - + if (absolute_pos) { + p.X = readS16(is); + p.Y = readS16(is); + p.Z = readS16(is); + } else { + u16 p16 = readU16(is); + p.X = p16 & (MAP_BLOCKSIZE - 1); + p16 /= MAP_BLOCKSIZE; + p.Y = p16 & (MAP_BLOCKSIZE - 1); + p16 /= MAP_BLOCKSIZE; + p.Z = p16; + } if (m_data.find(p) != m_data.end()) { warningstream << "NodeMetadataList::deSerialize(): " << "already set data at position " << PP(p) @@ -193,7 +207,7 @@ std::vector NodeMetadataList::getAllKeys() { std::vector keys; - std::map::const_iterator it; + NodeMetadataMap::const_iterator it; for (it = m_data.begin(); it != m_data.end(); ++it) keys.push_back(it->first); @@ -202,7 +216,7 @@ std::vector NodeMetadataList::getAllKeys() NodeMetadata *NodeMetadataList::get(v3s16 p) { - std::map::const_iterator n = m_data.find(p); + NodeMetadataMap::const_iterator n = m_data.find(p); if (n == m_data.end()) return NULL; return n->second; @@ -212,7 +226,8 @@ void NodeMetadataList::remove(v3s16 p) { NodeMetadata *olddata = get(p); if (olddata) { - delete olddata; + if (m_is_metadata_owner) + delete olddata; m_data.erase(p); } } @@ -225,9 +240,10 @@ void NodeMetadataList::set(v3s16 p, NodeMetadata *d) void NodeMetadataList::clear() { - std::map::iterator it; - for (it = m_data.begin(); it != m_data.end(); ++it) { - delete it->second; + if (m_is_metadata_owner) { + NodeMetadataMap::const_iterator it; + for (it = m_data.begin(); it != m_data.end(); ++it) + delete it->second; } m_data.clear(); } @@ -235,7 +251,7 @@ void NodeMetadataList::clear() int NodeMetadataList::countNonEmpty() const { int n = 0; - std::map::const_iterator it; + NodeMetadataMap::const_iterator it; for (it = m_data.begin(); it != m_data.end(); ++it) { if (!it->second->empty()) n++; diff --git a/src/nodemetadata.h b/src/nodemetadata.h index 40f6dec65..c028caf88 100644 --- a/src/nodemetadata.h +++ b/src/nodemetadata.h @@ -70,13 +70,21 @@ private: List of metadata of all the nodes of a block */ +typedef std::map NodeMetadataMap; + class NodeMetadataList { public: + NodeMetadataList(bool is_metadata_owner = true) : + m_is_metadata_owner(is_metadata_owner) + {} + ~NodeMetadataList(); - void serialize(std::ostream &os, u8 blockver, bool disk=true) const; - void deSerialize(std::istream &is, IItemDefManager *item_def_mgr); + void serialize(std::ostream &os, u8 blockver, bool disk = true, + bool absolute_pos = false) const; + void deSerialize(std::istream &is, IItemDefManager *item_def_mgr, + bool absolute_pos = false); // Add all keys in this list to the vector keys std::vector getAllKeys(); @@ -89,8 +97,21 @@ public: // Deletes all void clear(); + size_t size() const { return m_data.size(); } + + NodeMetadataMap::const_iterator begin() + { + return m_data.begin(); + } + + NodeMetadataMap::const_iterator end() + { + return m_data.end(); + } + private: int countNonEmpty() const; - std::map m_data; + bool m_is_metadata_owner; + NodeMetadataMap m_data; }; diff --git a/src/rollback_interface.cpp b/src/rollback_interface.cpp index fef5389c1..3ac15544c 100644 --- a/src/rollback_interface.cpp +++ b/src/rollback_interface.cpp @@ -168,17 +168,10 @@ bool RollbackAction::applyRevert(Map *map, InventoryManager *imgr, IGameDef *gam meta->deSerialize(is, 1); // FIXME: version bump?? } // Inform other things that the meta data has changed - v3s16 blockpos = getContainerPos(p, MAP_BLOCKSIZE); MapEditEvent event; event.type = MEET_BLOCK_NODE_METADATA_CHANGED; - event.p = blockpos; + event.p = p; map->dispatchEvent(&event); - // Set the block to be saved - MapBlock *block = map->getBlockNoCreateNoEx(blockpos); - if (block) { - block->raiseModified(MOD_STATE_WRITE_NEEDED, - MOD_REASON_REPORT_META_CHANGE); - } } catch (InvalidPositionException &e) { infostream << "RollbackAction::applyRevert(): " << "InvalidPositionException: " << e.what() diff --git a/src/script/lua_api/l_itemstackmeta.cpp b/src/script/lua_api/l_itemstackmeta.cpp index 07ab86499..d1ba1bda4 100644 --- a/src/script/lua_api/l_itemstackmeta.cpp +++ b/src/script/lua_api/l_itemstackmeta.cpp @@ -46,7 +46,7 @@ void ItemStackMetaRef::clearMeta() istack->metadata.clear(); } -void ItemStackMetaRef::reportMetadataChange() +void ItemStackMetaRef::reportMetadataChange(const std::string *name) { // TODO } diff --git a/src/script/lua_api/l_itemstackmeta.h b/src/script/lua_api/l_itemstackmeta.h index 2f4c37fd9..c3198be4f 100644 --- a/src/script/lua_api/l_itemstackmeta.h +++ b/src/script/lua_api/l_itemstackmeta.h @@ -40,7 +40,7 @@ private: virtual void clearMeta(); - virtual void reportMetadataChange(); + virtual void reportMetadataChange(const std::string *name = nullptr); void setToolCapabilities(const ToolCapabilities &caps) { diff --git a/src/script/lua_api/l_metadata.cpp b/src/script/lua_api/l_metadata.cpp index 4f64cc8a6..21002e6a7 100644 --- a/src/script/lua_api/l_metadata.cpp +++ b/src/script/lua_api/l_metadata.cpp @@ -122,7 +122,7 @@ int MetaDataRef::l_set_string(lua_State *L) return 0; meta->setString(name, str); - ref->reportMetadataChange(); + ref->reportMetadataChange(&name); return 0; } @@ -160,7 +160,7 @@ int MetaDataRef::l_set_int(lua_State *L) return 0; meta->setString(name, str); - ref->reportMetadataChange(); + ref->reportMetadataChange(&name); return 0; } @@ -198,7 +198,7 @@ int MetaDataRef::l_set_float(lua_State *L) return 0; meta->setString(name, str); - ref->reportMetadataChange(); + ref->reportMetadataChange(&name); return 0; } diff --git a/src/script/lua_api/l_metadata.h b/src/script/lua_api/l_metadata.h index cee38ed45..e655eb684 100644 --- a/src/script/lua_api/l_metadata.h +++ b/src/script/lua_api/l_metadata.h @@ -37,7 +37,7 @@ public: protected: static MetaDataRef *checkobject(lua_State *L, int narg); - virtual void reportMetadataChange() {} + virtual void reportMetadataChange(const std::string *name = nullptr) {} virtual Metadata *getmeta(bool auto_create) = 0; virtual void clearMeta() = 0; diff --git a/src/script/lua_api/l_nodemeta.cpp b/src/script/lua_api/l_nodemeta.cpp index 33b158ae0..22fc61782 100644 --- a/src/script/lua_api/l_nodemeta.cpp +++ b/src/script/lua_api/l_nodemeta.cpp @@ -58,21 +58,17 @@ void NodeMetaRef::clearMeta() m_env->getMap().removeNodeMetadata(m_p); } -void NodeMetaRef::reportMetadataChange() +void NodeMetaRef::reportMetadataChange(const std::string *name) { // NOTE: This same code is in rollback_interface.cpp // Inform other things that the metadata has changed - v3s16 blockpos = getNodeBlockPos(m_p); + NodeMetadata *meta = dynamic_cast(m_meta); + MapEditEvent event; event.type = MEET_BLOCK_NODE_METADATA_CHANGED; - event.p = blockpos; + event.p = m_p; + event.is_private_change = name && meta && meta->isPrivate(*name); m_env->getMap().dispatchEvent(&event); - // Set the block to be saved - MapBlock *block = m_env->getMap().getBlockNoCreateNoEx(blockpos); - if (block) { - block->raiseModified(MOD_STATE_WRITE_NEEDED, - MOD_REASON_REPORT_META_CHANGE); - } } // Exported functions diff --git a/src/script/lua_api/l_nodemeta.h b/src/script/lua_api/l_nodemeta.h index b0b4a9623..fdc1766ed 100644 --- a/src/script/lua_api/l_nodemeta.h +++ b/src/script/lua_api/l_nodemeta.h @@ -60,7 +60,7 @@ private: virtual Metadata* getmeta(bool auto_create); virtual void clearMeta(); - virtual void reportMetadataChange(); + virtual void reportMetadataChange(const std::string *name = nullptr); virtual void handleToTable(lua_State *L, Metadata *_meta); virtual bool handleFromTable(lua_State *L, int table, Metadata *_meta); diff --git a/src/server.cpp b/src/server.cpp index f9550c54f..f1fbe8668 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -861,6 +861,8 @@ void Server::AsyncRunStep(bool initial_step) // We'll log the amount of each Profiler prof; + std::list node_meta_updates; + while (!m_unsent_map_edit_queue.empty()) { MapEditEvent* event = m_unsent_map_edit_queue.front(); m_unsent_map_edit_queue.pop(); @@ -883,11 +885,22 @@ void Server::AsyncRunStep(bool initial_step) sendRemoveNode(event->p, &far_players, disable_single_change_sending ? 5 : 30); break; - case MEET_BLOCK_NODE_METADATA_CHANGED: - infostream << "Server: MEET_BLOCK_NODE_METADATA_CHANGED" << std::endl; - prof.add("MEET_BLOCK_NODE_METADATA_CHANGED", 1); - m_clients.markBlockposAsNotSent(event->p); + case MEET_BLOCK_NODE_METADATA_CHANGED: { + verbosestream << "Server: MEET_BLOCK_NODE_METADATA_CHANGED" << std::endl; + prof.add("MEET_BLOCK_NODE_METADATA_CHANGED", 1); + if (!event->is_private_change) { + // Don't send the change yet. Collect them to eliminate dupes. + node_meta_updates.remove(event->p); + node_meta_updates.push_back(event->p); + } + + if (MapBlock *block = m_env->getMap().getBlockNoCreateNoEx( + getNodeBlockPos(event->p))) { + block->raiseModified(MOD_STATE_WRITE_NEEDED, + MOD_REASON_REPORT_META_CHANGE); + } break; + } case MEET_OTHER: infostream << "Server: MEET_OTHER" << std::endl; prof.add("MEET_OTHER", 1); @@ -931,6 +944,9 @@ void Server::AsyncRunStep(bool initial_step) prof.print(verbosestream); } + // Send all metadata updates + if (node_meta_updates.size()) + sendMetadataChanged(node_meta_updates); } /* @@ -1224,6 +1240,7 @@ Inventory* Server::getInventory(const InventoryLocation &loc) } return NULL; } + void Server::setInventoryModified(const InventoryLocation &loc, bool playerSend) { switch(loc.type){ @@ -1248,13 +1265,10 @@ void Server::setInventoryModified(const InventoryLocation &loc, bool playerSend) break; case InventoryLocation::NODEMETA: { - v3s16 blockpos = getNodeBlockPos(loc.p); - - MapBlock *block = m_env->getMap().getBlockNoCreateNoEx(blockpos); - if (block) - block->raiseModified(MOD_STATE_WRITE_NEEDED); - - m_clients.markBlockposAsNotSent(blockpos); + MapEditEvent event; + event.type = MEET_BLOCK_NODE_METADATA_CHANGED; + event.p = loc.p; + m_env->getMap().dispatchEvent(&event); } break; case InventoryLocation::DETACHED: @@ -2159,6 +2173,57 @@ void Server::sendAddNode(v3s16 p, MapNode n, std::unordered_set *far_player m_clients.unlock(); } +void Server::sendMetadataChanged(const std::list &meta_updates, float far_d_nodes) +{ + float maxd = far_d_nodes * BS; + NodeMetadataList meta_updates_list(false); + std::vector clients = m_clients.getClientIDs(); + + m_clients.lock(); + + for (session_t i : clients) { + RemoteClient *client = m_clients.lockedGetClientNoEx(i); + if (!client) + continue; + + ServerActiveObject *player = m_env->getActiveObject(i); + v3f player_pos = player ? player->getBasePosition() : v3f(); + + for (const v3s16 &pos : meta_updates) { + NodeMetadata *meta = m_env->getMap().getNodeMetadata(pos); + + if (!meta) + continue; + + v3s16 block_pos = getNodeBlockPos(pos); + if (!client->isBlockSent(block_pos) || (player && + player_pos.getDistanceFrom(intToFloat(pos, BS)) > maxd)) { + client->SetBlockNotSent(block_pos); + continue; + } + + // Add the change to send list + meta_updates_list.set(pos, meta); + } + if (meta_updates_list.size() == 0) + continue; + + // Send the meta changes + std::ostringstream os(std::ios::binary); + meta_updates_list.serialize(os, client->net_proto_version, false, true); + std::ostringstream oss(std::ios::binary); + compressZlib(os.str(), oss); + + NetworkPacket pkt(TOCLIENT_NODEMETA_CHANGED, 0); + pkt.putLongString(oss.str()); + m_clients.send(i, 0, &pkt, true); + + meta_updates_list.clear(); + } + + m_clients.unlock(); +} + void Server::SendBlockNoLock(session_t peer_id, MapBlock *block, u8 ver, u16 net_proto_version) { diff --git a/src/server.h b/src/server.h index 9643fe456..0a3e48072 100644 --- a/src/server.h +++ b/src/server.h @@ -424,6 +424,9 @@ private: std::unordered_set *far_players = nullptr, float far_d_nodes = 100, bool remove_metadata = true); + void sendMetadataChanged(const std::list &meta_updates, + float far_d_nodes = 100); + // Environment and Connection must be locked when called void SendBlockNoLock(session_t peer_id, MapBlock *block, u8 ver, u16 net_proto_version);