Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultiplayerSynchronizer does not sync properties on scene root #62027

Closed
jgillich opened this issue Jun 14, 2022 · 7 comments
Closed

MultiplayerSynchronizer does not sync properties on scene root #62027

jgillich opened this issue Jun 14, 2022 · 7 comments

Comments

@jgillich
Copy link
Contributor

jgillich commented Jun 14, 2022

Godot version

136f84f

System information

Fedora Silverblue 36, Ryzen 2400G

Issue description

MultiplayerSynchronizer does not sync properties if they are located on the root node of a scene. Instead they remain at their initial values. I have tested this with CharacterBody2D and CharacterBody3D nodes, not sure if it would be the same with a simple Node at the scene root.

Steps to reproduce

Get the reproduction project: https://github.com/jgillich/gotodot-mp-repro (run with at least version 0df0a07, not alpha9)

  1. Run 3 instances:
godot --server
godot --client
godot --client
  1. In one of the client windows, use A/D or Left/Right to move your character
  2. Observe how the position in the other client window does not update
  3. Repeat the steps above but add --use-child to the run arguments
  4. Observe how the position updates in all clients when a character is moved

There is potentially another bug, if you uncomment the create_player line in world.gd so the server is also a player, and then try to join with a client, you get a bunch of errors. In the bomber example the server is also a player, but I haven't managed to figure out what the difference is.

Minimal reproduction project

https://github.com/jgillich/gotodot-mp-repro

@jgillich
Copy link
Contributor Author

Did some more investigation, if we change root_path it to literally anything other than the default .., like ../Camera2D, the example starts working. What exactly is the purpose of root_path?

@Faless
Copy link
Collaborator

Faless commented Jun 27, 2022

What exactly is the purpose of root_path?

It's the starting path to use for referencing properties.

@danhealy
Copy link

danhealy commented Jul 1, 2022

It also works if you make a standard node the parent of CharacterBody2D, i.e. if .. is not the root of the scene

@Faless
Copy link
Collaborator

Faless commented Jul 20, 2022

This is fixed after #62961 with the following small change to the gdscript code (adding await get_tree().process_frame):

diff --git a/world.gd b/world.gd
index 32e01b3..f481f3d 100644
--- a/world.gd
+++ b/world.gd
@@ -16,7 +16,8 @@ func _enter_tree():
 
                print('server listening on localhost 12100')
                multiplayer.set_multiplayer_peer(peer)
-               #create_player(multiplayer.get_unique_id())
+               await get_tree().process_frame
+               create_player(multiplayer.get_unique_id())
        else:
                peer.create_client("localhost", 12100)
                multiplayer.set_multiplayer_peer(peer)

Spawning on a child during the _ready() phase of the parent has known issues, and we should probably document that while try to make an internal fix.

I'll leave this issue open for now since it has nice keywords for search, but I should probably open a dedicated issue soon.

@jgillich
Copy link
Contributor Author

One more thing I've noticed while testing is that MPSynchronizer bypasses property setters. Is this expected or a bug?

@Faless
Copy link
Collaborator

Faless commented Jul 22, 2022

Is this expected or a bug?

That's likely a bug, it's supposed to call setter/getters.
I think I have an idea why it's happening, we should track that in a separate issue.
Thanks for reporting it :)

@jgillich
Copy link
Contributor Author

@Faless I made #63308

Haven't bothered to make a reproduction project since you already have an idea, lmk if you'd like one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants