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

Fix Multiplayer Spawner freeing node after client disconnected Issue #92359

Merged

Conversation

DanielSnd
Copy link
Contributor

Fixes an error that happens when freeing a node that was synced using a MultiplayerSpawner after a client disconnects. Closes #92358

When a client disconnects on on_peer_change it looks like the intention of the code was to remove the peer ID from the list of received ids of that node with nc->recv_ids.erase(p_id); but what is actually in the code today is nc->recv_ids.erase(E.key); which in this case is trying to erase the remote cache id of the node from the list of received ids of the node, which doesn't really make sense since that is a list of peer ids that received that node.

I'm assuming it was a mistake, this one line fix fixes it.

The error happens because since the id wasn't being removed from the list of recv_ids when the node was eventually freed it tried to get the peer_info for that id and failed since the peer at this point no longer exists, showing the error.

@akien-mga akien-mga requested a review from a team May 25, 2024 19:32
@akien-mga akien-mga added this to the 4.3 milestone May 25, 2024
@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label May 25, 2024
Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely spotted! Great work! 🥇

@akien-mga akien-mga merged commit bf2deac into godotengine:master May 28, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release topic:multiplayer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when deleting a "MultiplayerSpawner" tracked node after a client has left.
4 participants