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

Is "SceneReplicationInterface::on_sync_receive: Ignoring sync data" an error? #86146

Closed
isaaccp opened this issue Dec 14, 2023 · 4 comments · Fixed by #87186
Closed

Is "SceneReplicationInterface::on_sync_receive: Ignoring sync data" an error? #86146

isaaccp opened this issue Dec 14, 2023 · 4 comments · Fixed by #87186

Comments

@isaaccp
Copy link
Contributor

isaaccp commented Dec 14, 2023

Tested versions

4.2 and HEAD @ aa5b6ed

System information

Windows 10

Issue description

I have two MultiplayerSpawner that spawn scenes ("Skeleton" and "Knife" respectively).

The multiplayer authority for all the spawners and spawned scenes is the server.

I call queue_free() in the server in order to remove spawned scenes.

Everything seems to work well gameplay-wise, but the debug log gets one of those:
"Ignoring sync data from non-authority or for missing node."
sometimes on the client from here:

ERR_CONTINUE_MSG(true, "Ignoring sync data from non-authority or for missing node.");

I have added significant logging in the Godot source and I can see that when this happens:

  • the notification for exit_tree is received by MultiplayerSynchronizer in the client, as a result
  • _stop() is called on the MultiplayerSynchronizer in the client, which sets root_path_cache to ObjectID() (0).
  • then ~1ms later the sync message arrives, root_path_cache.is_valid() is false as _stop() has set it to zero, as a result so sync.get_root_node() returns null and the message above is printed.

Here is a sample screenshot with my debug output:
image

I am not sure if I am doing something wrong or if it's just expected that somehow the "client" gets syncs shortly after being de-spawned and those cause this error.

If it's the latter, can we make this not appear in the debug log?

Steps to reproduce

Create a MultiplayerSpawner, spawn children including a MultiplayerSynchronizer from the server, remove them by calling queue_free() in the server

Minimal reproduction project (MRP)

I don't have a minimum reproduction project as of now. If the behavior described above is not expected, I can try to reproduce in a smaller form, but it may be hard given that it requires multiplayer.

@PatientLightning
Copy link

This is also happening to me, and I was wondering the same thing.

I have added significant logging in the Godot source

You're a champ, I was just starting to wish the error was more specific so I could determine if it was a real problem or not.

I think my understanding now is that the server is always sending sync packets from the node until it is de-spawned, then the client is receives the command to de-spawn before the the last sync packet is received and processed. Then when that last sync packet is processed, it throws an error because the node was de-spawned.

On one hand, It does seem reasonable that receiving sync data from a non-authority or a missing node is wrong, and therefore an error.
But in this case, we intended for the node to not exist, so seeing red in the log is alarming.

In the long term: I would suggest two things;
First, demote this error to a warning because I expect we will see it every time we de-spawn a node over the network.
Second, include more information like a node path and the peer id of the sender so we can comfortably ignore it or troubleshoot in case there really is a real problem, just like your modification does
Something like;
on_sync_receive: Ignoring sync data from non-authority or for missing node. Source peer id '1', NodePath '/root/level/ammobox'. This may happen if the node was recently freed

In the short term: I wrote a hacky little thing to avoid the error and keep my log clean

func q_free():
	$MultiplayerSynchronizer.queue_free()
	await get_tree().process_frame
	await get_tree().process_frame
	queue_free()

Basically, it frees the synchronizer then waits, then frees the scene. It's probably not foolproof, but at least it should help reduce red on localhost testing for now.

@Faless
Copy link
Collaborator

Faless commented Jan 14, 2024

The fact that this case is reported as an error is indeed a bug, I've opened #87186 which should fix the issue.

@isaaccp
Copy link
Contributor Author

isaaccp commented Jan 18, 2024

@Faless it would be great if the error message could be updated to print something like what @PatientLightning suggested too, e.g.:
on_sync_receive: Ignoring sync data from non-authority or for missing node. Source peer id '1', NodePath '/root/level/ammobox'.

@Pshy0
Copy link

Pshy0 commented Mar 8, 2024

I am on v4.3.dev4.official [df78c06] and this is still happening.

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