Merge pull request #90027 from Faless/mp/cache_no_spam

[MP] Gracefully handle cache confirmation of deleted nodes

2 files changed, 22 insertions, 19 deletions

diff --git a/modules/multiplayer/scene_cache_interface.cpp b/modules/multiplayer/scene_cache_interface.cpp
index 33b05d4cc2..c08ccbe4cc 100644
--- a/modules/multiplayer/scene_cache_interface.cpp
+++ b/modules/multiplayer/scene_cache_interface.cpp
@@ -52,6 +52,9 @@ void SceneCacheInterface::_remove_node_cache(ObjectID p_oid) {
 	if (!nc) {
 		return;
 	}
+	if (nc->cache_id) {
+		assigned_ids.erase(nc->cache_id);
+	}
 	for (KeyValue<int, int> &E : nc->recv_ids) {
 		PeerInfo *pinfo = peers_info.getptr(E.key);
 		ERR_CONTINUE(!pinfo);
@@ -117,16 +120,12 @@ void SceneCacheInterface::process_simplify_path(int p_from, const uint8_t *p_pac
 	NodeCache &cache = _track(node);
 	cache.recv_ids.insert(p_from, id);
 
-	// Encode path to send ack.
-	CharString pname = String(path).utf8();
-	int len = encode_cstring(pname.get_data(), nullptr);
-
+	// Send ack.
 	Vector<uint8_t> packet;
-
-	packet.resize(1 + 1 + len);
+	packet.resize(1 + 1 + 4);
 	packet.write[0] = SceneMultiplayer::NETWORK_COMMAND_CONFIRM_PATH;
 	packet.write[1] = valid_rpc_checksum;
-	encode_cstring(pname.get_data(), &packet.write[2]);
+	encode_uint32(id, &packet.write[2]);
 
 	Ref<MultiplayerPeer> multiplayer_peer = multiplayer->get_multiplayer_peer();
 	ERR_FAIL_COND(multiplayer_peer.is_null());
@@ -137,26 +136,26 @@ void SceneCacheInterface::process_simplify_path(int p_from, const uint8_t *p_pac
 }
 
 void SceneCacheInterface::process_confirm_path(int p_from, const uint8_t *p_packet, int p_packet_len) {
-	ERR_FAIL_COND_MSG(p_packet_len < 3, "Invalid packet received. Size too small.");
+	ERR_FAIL_COND_MSG(p_packet_len != 6, "Invalid packet received. Size too small.");
 	Node *root_node = SceneTree::get_singleton()->get_root()->get_node(multiplayer->get_root_path());
 	ERR_FAIL_NULL(root_node);
 
 	const bool valid_rpc_checksum = p_packet[1];
+	int id = decode_uint32(&p_packet[2]);
 
-	String paths;
-	paths.parse_utf8((const char *)&p_packet[2], p_packet_len - 2);
-
-	const NodePath path = paths;
+	const ObjectID *oid = assigned_ids.getptr(id);
+	if (oid == nullptr) {
+		return; // May be trying to confirm a node that was removed.
+	}
 
 	if (valid_rpc_checksum == false) {
-		ERR_PRINT("The rpc node checksum failed. Make sure to have the same methods on both nodes. Node path: " + path);
+		const Node *node = Object::cast_to<Node>(ObjectDB::get_instance(*oid));
+		ERR_FAIL_NULL(node); // Bug.
+		ERR_PRINT("The rpc node checksum failed. Make sure to have the same methods on both nodes. Node path: " + node->get_path());
 	}
 
-	Node *node = root_node->get_node(path);
-	ERR_FAIL_NULL(node);
-
-	NodeCache *cache = nodes_cache.getptr(node->get_instance_id());
-	ERR_FAIL_NULL_MSG(cache, "Invalid packet received. Tries to confirm a node which was not requested.");
+	NodeCache *cache = nodes_cache.getptr(*oid);
+	ERR_FAIL_NULL(cache); // Bug.
 
 	bool *confirmed = cache->confirmed_peers.getptr(p_from);
 	ERR_FAIL_NULL_MSG(confirmed, "Invalid packet received. Tries to confirm a node which was not requested.");
@@ -216,6 +215,7 @@ int SceneCacheInterface::make_object_cache(Object *p_obj) {
 	NodeCache &cache = _track(node);
 	if (cache.cache_id == 0) {
 		cache.cache_id = last_send_cache_id++;
+		assigned_ids[cache.cache_id] = p_obj->get_instance_id();
 	}
 	return cache.cache_id;
 }
@@ -227,6 +227,7 @@ bool SceneCacheInterface::send_object_cache(Object *p_obj, int p_peer_id, int &r
 	NodeCache &cache = _track(node);
 	if (cache.cache_id == 0) {
 		cache.cache_id = last_send_cache_id++;
+		assigned_ids[cache.cache_id] = p_obj->get_instance_id();
 	}
 	r_id = cache.cache_id;
 
@@ -289,5 +290,6 @@ void SceneCacheInterface::clear() {
 	}
 	peers_info.clear();
 	nodes_cache.clear();
+	assigned_ids.clear();
 	last_send_cache_id = 1;
 }
diff --git a/modules/multiplayer/scene_cache_interface.h b/modules/multiplayer/scene_cache_interface.h
index ab4a20c078..73d6bde6ef 100644
--- a/modules/multiplayer/scene_cache_interface.h
+++ b/modules/multiplayer/scene_cache_interface.h
@@ -44,7 +44,7 @@ private:
 
 	//path sent caches
 	struct NodeCache {
-		int cache_id;
+		int cache_id = 0;
 		HashMap<int, int> recv_ids; // peer id, remote cache id
 		HashMap<int, bool> confirmed_peers; // peer id, confirmed
 	};
@@ -55,6 +55,7 @@ private:
 	};
 
 	HashMap<ObjectID, NodeCache> nodes_cache;
+	HashMap<int, ObjectID> assigned_ids;
 	HashMap<int, PeerInfo> peers_info;
 	int last_send_cache_id = 1;