godotengine/godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[MP] Gracefully handle cache confirmation of deleted nodes

Browse Source 
It's possible that after sending a cached node reference (e.g. RPC or
static MultiplayerSynchronizer) the reference node is removed from tree
before the remote peer(s) can confirm the referenced path.

To better detect that case, and avoid spamming errors when it happens,
this commit modifies the multiplayer API caching protocol, to send the
received ID instead of the Node path when sending the confirmation
packet.

**This is a breaking change** because it makes the runtime multiplayer
protocol incompatible with previous versions of Godot.
This commit is contained in:
Fabio Alessandrelli 2024-03-29 22:35:03 +01:00
parent 29b3d9e9e5
commit 4b973f451e
2 changed files with 23 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
modules/multiplayer/scene_cache_interface.cpp
View File

@ -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;
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 ObjectID *oid = assigned_ids.getptr(id);
if (oid == nullptr) {
return; // May be trying to confirm a node that was removed.
}
Node *node = root_node->get_node(path);
ERR_FAIL_NULL(node);
if (valid_rpc_checksum == false) {
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());
}
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;
}

3
modules/multiplayer/scene_cache_interface.h
View File

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