godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.13k stars 21.19k forks source link

SceneReplicationConfig adding/removing sync properties through code does not update the targeted sync properties #65725

Closed Migzus closed 1 year ago

Migzus commented 2 years ago

Godot version

4.0.beta6

System information

Windows 10

Issue description

As one would expect, the new MultiplayerSynchronizer would sync whatever properties you give it; as long as certain conditions are met. The focus of this issue is not related to, if the conditions are correct or not. But when you add/remove new sync properties through code; it simply does not sync them.

The odd thing here is that it syncs perfectly fine, when adding these sync properties from the Editor. But when keeping the list empty, and adding them through code. They are not synced anymore. One would immediately assume that the node paths are incorrect. However, even when writing ".:position" directly (where we assume that the root_node is a type of Node2D), it still does not work.

This also happens in reverse. If you already have linked properties in the editor, and try to remove them from code. They still sync... Even when they do not show up in the array, when printing the result from get_properties() in SceneReplicationConfig.

I have also tried to multiplayer.object_configuration_remove(), updated the properties through code and then multiplayer.object_configuration_add() the MultiplayerSynchronizer; in order to see if that updated the properties.

Some things of note: (but most likely does not have an influence on this potential issue) The test project assumes one of the clients playing, is the host/server The test project has a dedicated puppet scene and a separate player scene. (puppet is always an external client, and the player is always local)

If there is something I have missed (like a function call or something else) in order to make it work or update the syncing, tell me and this issue will close.

Steps to reproduce

Establish a simple server connection. Add the local player and client nodes accordingly upon a connection, let them each have a MultiplayerSynchronizer node. (Leave the property list empty) Set the MultiplayerSynchronizer nodes' authority respectively, according to which client who owns it. Then link the properties you would like to sync (position, rotation, etc.) through code MultiplayerSynchronizer.replication_config.add_property("NodePath")

Optionally, you may add controls, so you can see the effect.

Minimal reproduction project

Control the local player with arrow keys. You may also rotate it with "ui_cancel" The red stick figure with the godot icon is always the local player, and the blue ones are always other clients. This means that when moving locally, you move the red figure. But for other clients, you appear as the blue figure.

Simple Server Connection.zip

samcorcoran commented 2 years ago

I have also had this issue in beta4: any properties added from code are not sync'd.

I tried a bunch of different ways, and this is one example: @export var sync_test = 1 var variable_path = "MultiplayerSynchronizer:sync_test" self.replication_config.add_property(variable_path) This script was on a MultiplayerSynchronizer node itself. Another server-side script repeatedly incremented sync_test, and I would confirm that on my client the increments did not show.

I did a little investigating, and here are some things I noticed:

If the property path was already added via the editor, then adding it again via code resulted in triggering an error in SceneReplicationConfig.cpp on line 113: ERR_FAIL_COND(properties.find(p_path)); Which implies my variable path is correctly formated etc, as it is "matching" in a find() the property path that was added via the editor.

In a test where the property path has not been added via the editor, I called self.replication_config.get_properties() before and after I add a property from code. The returned NodePaths showed the property I tried to add was successfully included in replication config, but it still didn't sync my variable. This implies the adding is executing fine, and the problem is with how that scene replication config is being used.

I began to wonder if it was only an issue for replication configs that are set up after the rest of the game state was initialised. So I moved my MultiplayerSynchronizer with its script containing the variable into a separate scene. I used the editor to add the property path, and I instantiated the MultiplayerSynchronizer at runtime, long after the program started up. In this test the synchronisation (set via the editor ui) worked correctly. This implies the problem isn't something that occurs at SceneTree initialisation (but could still be a problem with MultiplayerSynchronizer initialisation when it enters the tree). Note, adding the property from code to a new MultiplayerSynchronizer node before adding it to the SceneTree also did not work correctly.

That reached the end of my investigation. If someone wanted to look further then some useful places might be:

  1. Are NodePaths added from code being considered invalid some time after SceneReplicationConfig::add_property (even if the given path is identical enough to trigger an error if already present)?
  2. Is the MultiplayerSynchronizer reading and making use of the editor-created SceneReplicationConfig, so any subsequent changes (i.e. those in code) don't matter?
  3. Are the editor and API referencing the same SceneReplicationConfig? In code, the API call get_properties() returns any editor-added, so is there somehow a copy taken early on so my code is operating on an object that has all the editor-added properties but does absolutely nothing when updating it with more.
samcorcoran commented 2 years ago

One more data point I missed: I tested whether adding properties from code before any network connection was attempted/established, to see whether the handshake between the two peers was freezing any existing changes (those set in the editor) and those made later via code. This did not affect the issue, and the variable was not sync'd regardless of whether it was added before or after the connection was established.

pleyland commented 1 year ago

This class is still completely non-functional

zeplar-exe commented 1 year ago

For anyone else who comes across this, here's a limited but working implementation (in C#) that allows this use case.