Peer Disconnect Reconnect Replication Issue

[pennyloafers]

Tested versions

• Reproducible 4.2.1, 4.3-dev4

System information

Ubuntu 22.04 - Godot 4.2.1

Issue description

If a MultiplayerSynchronizer's SceneReplicationConfig is modified via script and client disconnects and reconnects. Previous multiplayer unique id replication is still cached and throughs errors. Newly connected Clients are no longer replicated.

If replication_config is not touched with script there is no issue.

Steps to reproduce

Peer Connects
Create "player" with MultiplayerSynchronizer

• add new property to replication_config
  Disconnect player
  Reconnect player

Minimal reproduction project (MRP)

client_reconnect_issue.zip

Project has interface to reproduce issue.
Read short instructions watch output.

Example output

Server started putting api on node /root/main/Server
Client started putting api on node /root/main/Client
Client : peer connected 1
Server : peer connected 2076042518
Client: _exit_tree()
Server : peer disconnected 2076042518
2076042518: removing peer
Client started putting api on node /root/main/Client
Client : peer connected 1
Server : peer connected 601296107

E 0:00:09:0138   multiplayer_api.gd:42 @ _on_peer_connected(): Node '/root/main/Server/2076042518:dynamic_sync' not found.
  <C++ Error>    Condition "!node || !node->has_node(p_path)" is true. Returning: nullptr
  <C++ Source>   modules/multiplayer/multiplayer_synchronizer.cpp:41 @ _get_prop_target()
  <Stack Trace>  multiplayer_api.gd:42 @ _on_peer_connected()

E 0:00:09:0138   multiplayer_api.gd:42 @ _on_peer_connected(): Parameter "obj" is null.
  <C++ Source>   modules/multiplayer/multiplayer_synchronizer.cpp:164 @ get_state()
  <Stack Trace>  multiplayer_api.gd:42 @ _on_peer_connected()

E 0:00:09:0138   multiplayer_api.gd:42 @ _on_peer_connected(): Unable to retrieve spawn state.
  <C++ Error>    Condition "err != OK" is true. Returning: err
  <C++ Source>   modules/multiplayer/scene_replication_interface.cpp:516 @ _make_spawn_packet()
  <Stack Trace>  multiplayer_api.gd:42 @ _on_peer_connected()

E 0:00:09:0138   multiplayer_api.gd:42 @ _on_peer_connected(): Condition "!p_buffer || p_size < 1" is true. Returning: ERR_INVALID_PARAMETER
  <C++ Source>   modules/multiplayer/scene_replication_interface.cpp:462 @ _send_raw()
  <Stack Trace>  multiplayer_api.gd:42 @ _on_peer_connected()


 endless repeat of errors...

E 0:00:11:0154   _get_prop_target: Node '/root/main/Server/2076042518:dynamic_sync' not found.
  <C++ Error>    Condition "!node || !node->has_node(p_path)" is true. Returning: nullptr
  <C++ Source>   modules/multiplayer/multiplayer_synchronizer.cpp:41 @ _get_prop_target()
E 0:00:11:0154   _watch_changes: Node not found for property '/root/main/Server/2076042518:dynamic_sync'.
  <C++ Error>    Condition "!obj" is true. Continuing.
  <C++ Source>   modules/multiplayer/multiplayer_synchronizer.cpp:389 @ _watch_changes()

[pennyloafers]

I've made some progress
When adding a property to the config here are some recommendations:

• should use relative NodePaths and not absolute, or relative, Strings.
• should create new SceneReplicationConfig (SRC) if using dynamic properties.
  -- it is unknown if a fresh new SRC should be used every time the config is updated
  --- or is it a consequence of the SRC resource that is part of the .tscn part of the issue?

If I use relative NodePaths over Strings and create a new config the issue goes away. There is one consequence in doing this, the old editor SRC is overwritten. Which makes sense since I'm using a new SRC in its place.

[pennyloafers]

Using those recommendations above, minus making a new config and using the scene generated one. I think the issue is resolved.

I have made version 2
client_reconnect_issue_v2.zip

the last thing that keeps happening is every time I add a property to the SRC it throws an error.

E 0:00:08:0680   syncer.gd:20 @ _ready(): Condition "properties.find(p_path)" is true.
  <C++ Source>   modules/multiplayer/scene_replication_config.cpp:115 @ add_property()
  <Stack Trace>  syncer.gd:20 @ _ready()

Happens once on first connect, twice on second connect

[pennyloafers]

I did a little more debugging. It seems like the replication_config is cached somewhere for the MultiplayerSynchronizer even if its deleted from the scene tree.

I got it down to one error relating to "Condition 'properties.find(p_path)' is true." if I only allowed the server side to add the new parameter when the client first connects. This makes sense if the only the server creates it, I assume it gets replicated in some fashion to the client.

Then, if the client disconnects and the syncer is deleted, upon returning when the server creates a new syncer, the added property already exists in the replication config, throwing the error message.

This then explains the original issue relating to the old player node path still existing when using an absolute path, after a new client reconnects. The old property still exists with the old absolute path!

(I tried switching the order of client server on who added the new property and it seemed the error only happened on whoever was the second actor to add the property. It turns out the client can add the property and determine what is replicated for itself?! 😮) (on second thought, this could be a consequence of two SceneMultiplayer setup on the same tree, my other project had the original issue, but on separate processes, but after updating to a relative path that issue went away and a different issue cropped in that could be unrelated.)

[patentsil]

I have also noticed that the MultiplayerSynchronizer is being cached, and even after it's deleted, errors continue to appear related to a MultiplayerSynchronizer that no longer exists. Additionally, when the MultiplayerSynchronizer was still in place but I removed one of its fields, I kept encountering the same error, likely because it remains cached somewhere. I should be able to debug this issue.

E 0:00:16:0955   MultiplayerSynchronizer::_watch_changes: Node not found for property 'Camera3D/Flashlight:visible'.
  <C++ Error>    Condition "!obj" is true. Continuing.
  <C++ Source>   modules\multiplayer\multiplayer_synchronizer.cpp:390 @ MultiplayerSynchronizer::_watch_changes()

My version that I compiled: v4.4.dev.custom_build [e4e024a]