Merge pull request #94984 from Faless/mp/fix_safer_cache_cleanup

[MP] Partially revert cache cleanup, track paths as fallback
Sunfly-Studios · Jul 31, 2024 · 019cf2b · 019cf2b
2 parents 8460a72 + 90d5d26
commit 019cf2b
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 10 deletions.
diff --git a/modules/multiplayer/scene_cache_interface.cpp b/modules/multiplayer/scene_cache_interface.cpp
@@ -54,11 +54,14 @@ void SceneCacheInterface::_remove_node_cache(ObjectID p_oid) {
 	if (nc->cache_id) {
 		assigned_ids.erase(nc->cache_id);
 	}
+#if 0
+	// TODO: Find a way to cleanup recv_nodes without breaking visibility and RPCs interactions.
 	for (KeyValue<int, int> &E : nc->recv_ids) {
 		PeerInfo *pinfo = peers_info.getptr(E.key);
 		ERR_CONTINUE(!pinfo);
 		pinfo->recv_nodes.erase(E.value);
 	}
+#endif
 	for (KeyValue<int, bool> &E : nc->confirmed_peers) {
 		PeerInfo *pinfo = peers_info.getptr(E.key);
 		ERR_CONTINUE(!pinfo);
@@ -73,9 +76,12 @@ void SceneCacheInterface::on_peer_change(int p_id, bool p_connected) {
 	} else {
 		PeerInfo *pinfo = peers_info.getptr(p_id);
 		ERR_FAIL_NULL(pinfo); // Bug.
-		for (KeyValue<int, ObjectID> E : pinfo->recv_nodes) {
-			NodeCache *nc = nodes_cache.getptr(E.value);
-			ERR_CONTINUE(!nc);
+		for (KeyValue<int, RecvNode> E : pinfo->recv_nodes) {
+			NodeCache *nc = nodes_cache.getptr(E.value.oid);
+			if (!nc) {
+				// Node might have already been deleted locally.
+				continue;
+			}
 			nc->recv_ids.erase(p_id);
 		}
 		for (const ObjectID &oid : pinfo->sent_nodes) {
@@ -115,7 +121,7 @@ void SceneCacheInterface::process_simplify_path(int p_from, const uint8_t *p_pac
 		ERR_PRINT("The rpc node checksum failed. Make sure to have the same methods on both nodes. Node path: " + path);
 	}
 
-	peers_info[p_from].recv_nodes.insert(id, node->get_instance_id());
+	peers_info[p_from].recv_nodes.insert(id, RecvNode(node->get_instance_id(), path));
 	NodeCache &cache = _track(node);
 	cache.recv_ids.insert(p_from, id);
 
@@ -269,14 +275,21 @@ bool SceneCacheInterface::send_object_cache(Object *p_obj, int p_peer_id, int &r
 }
 
 Object *SceneCacheInterface::get_cached_object(int p_from, uint32_t p_cache_id) {
-	Node *root_node = SceneTree::get_singleton()->get_root()->get_node(multiplayer->get_root_path());
-	ERR_FAIL_NULL_V(root_node, nullptr);
 	PeerInfo *pinfo = peers_info.getptr(p_from);
 	ERR_FAIL_NULL_V(pinfo, nullptr);
 
-	const ObjectID *oid = pinfo->recv_nodes.getptr(p_cache_id);
-	ERR_FAIL_NULL_V_MSG(oid, nullptr, vformat("ID %d not found in cache of peer %d.", p_cache_id, p_from));
-	Node *node = Object::cast_to<Node>(ObjectDB::get_instance(*oid));
+	RecvNode *recv_node = pinfo->recv_nodes.getptr(p_cache_id);
+	ERR_FAIL_NULL_V_MSG(recv_node, nullptr, vformat("ID %d not found in cache of peer %d.", p_cache_id, p_from));
+	Node *node = Object::cast_to<Node>(ObjectDB::get_instance(recv_node->oid));
+	if (!node) {
+		// Fallback to path lookup.
+		Node *root_node = SceneTree::get_singleton()->get_root()->get_node(multiplayer->get_root_path());
+		ERR_FAIL_NULL_V(root_node, nullptr);
+		node = root_node->get_node(recv_node->path);
+		if (node) {
+			recv_node->oid = node->get_instance_id();
+		}
+	}
 	ERR_FAIL_NULL_V_MSG(node, nullptr, vformat("Failed to get cached node from peer %d with cache ID %d.", p_from, p_cache_id));
 	return node;
 }

diff --git a/modules/multiplayer/scene_cache_interface.h b/modules/multiplayer/scene_cache_interface.h
@@ -49,8 +49,18 @@ class SceneCacheInterface : public RefCounted {
 		HashMap<int, bool> confirmed_peers; // peer id, confirmed
 	};
 
+	struct RecvNode {
+		ObjectID oid;
+		NodePath path;
+
+		RecvNode(const ObjectID &p_oid, const NodePath &p_path) {
+			oid = p_oid;
+			path = p_path;
+		}
+	};
+
 	struct PeerInfo {
-		HashMap<int, ObjectID> recv_nodes; // remote cache id, ObjectID
+		HashMap<int, RecvNode> recv_nodes; // remote cache id, (ObjectID, NodePath)
 		HashSet<ObjectID> sent_nodes;
 	};