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

@@ -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);

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<v3s16> 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;
 	}
 

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,

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

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
@@ -639,6 +640,7 @@ enum ToClientCommand
 	 	u16 message length
 	 	std::string message
 	*/
+
 	TOCLIENT_MODCHANNEL_SIGNAL = 0x58,
 	/*
 		u8 signal id
@@ -646,6 +648,11 @@ enum ToClientCommand
 	 	std::string channel name
 	*/
 
+	TOCLIENT_NODEMETA_CHANGED = 0x59,
+	/*
+		serialized and compressed node metadata
+	*/
+
 	TOCLIENT_SRP_BYTES_S_B = 0x60,
 	/*
 		Belonging to AUTH_MECHANISM_SRP.

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,

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);
+		if (ma->from_inv != ma->to_inv) {
 			setInventoryModified(ma->to_inv, false);
+		}
 
 		bool from_inv_is_current_player =
 			(ma->from_inv.type == InventoryLocation::PLAYER) &&

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;
+		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<v3s16> NodeMetadataList::getAllKeys()
 {
 	std::vector<v3s16> keys;
 
-	std::map<v3s16, NodeMetadata *>::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<v3s16> NodeMetadataList::getAllKeys()
 
 NodeMetadata *NodeMetadataList::get(v3s16 p)
 {
-	std::map<v3s16, NodeMetadata *>::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,6 +226,7 @@ void NodeMetadataList::remove(v3s16 p)
 {
 	NodeMetadata *olddata = get(p);
 	if (olddata) {
+		if (m_is_metadata_owner)
 			delete olddata;
 		m_data.erase(p);
 	}
@@ -225,8 +240,9 @@ void NodeMetadataList::set(v3s16 p, NodeMetadata *d)
 
 void NodeMetadataList::clear()
 {
-	std::map<v3s16, NodeMetadata*>::iterator it;
-	for (it = m_data.begin(); it != m_data.end(); ++it) {
+	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<v3s16, NodeMetadata*>::const_iterator it;
+	NodeMetadataMap::const_iterator it;
 	for (it = m_data.begin(); it != m_data.end(); ++it) {
 		if (!it->second->empty())
 			n++;

src/nodemetadata.h

@@ -70,13 +70,21 @@ private:
 	List of metadata of all the nodes of a block
 */
 
+typedef std::map<v3s16, NodeMetadata *> 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<v3s16> 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<v3s16, NodeMetadata *> m_data;
+	bool m_is_metadata_owner;
+	NodeMetadataMap m_data;
 };

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()

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
 }

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)
 	{

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;
 }
 

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;
 

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<NodeMetadata*>(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

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);

src/server.cpp

@@ -861,6 +861,8 @@ void Server::AsyncRunStep(bool initial_step)
 		// We'll log the amount of each
 		Profiler prof;
 
+		std::list<v3s16> 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;
+			case MEET_BLOCK_NODE_METADATA_CHANGED: {
+				verbosestream << "Server: MEET_BLOCK_NODE_METADATA_CHANGED" << std::endl;
 				prof.add("MEET_BLOCK_NODE_METADATA_CHANGED", 1);
-						m_clients.markBlockposAsNotSent(event->p);
+				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<u16> *far_player
 	m_clients.unlock();
 }
 
+void Server::sendMetadataChanged(const std::list<v3s16> &meta_updates, float far_d_nodes)
+{
+	float maxd = far_d_nodes * BS;
+	NodeMetadataList meta_updates_list(false);
+	std::vector<session_t> 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)
 {

src/server.h

@@ -424,6 +424,9 @@ private:
 			std::unordered_set<u16> *far_players = nullptr,
 			float far_d_nodes = 100, bool remove_metadata = true);
 
+	void sendMetadataChanged(const std::list<v3s16> &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);