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

Avoid packets getting sent to disconnected players #14444

Merged
merged 2 commits into from Mar 10, 2024

Conversation

SmallJoker
Copy link
Member

Many functions expect RemotePlayer to have a valid peer ID, this however is not the case immediately after disconnecting where the object is still alive and pending for removal.

ServerEnvironment::getPlayer(const char *, bool) now only returns players that are connected unless forced to.

Fixes #14443

To do

This PR is Ready for Review.^

How to test

  1. Test script from Possible SIGABRT when showing formspecs after async without safety checks #14443 (host server, connect with a 2nd client)
  2. ????
  3. No error

Many functions expect RemotePlayer to have a valid peer ID,
this however is not the case immediately after disconnecting
where the object is still alive and pending for removal.

ServerEnvironment::getPlayer(const char *, bool) now only
returns players that are connected unless forced to.
@SmallJoker SmallJoker added @ Script API Bugfix 🐛 PRs that fix a bug labels Mar 8, 2024
Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

looks sensible

src/serverenvironment.cpp Outdated Show resolved Hide resolved
Co-authored-by: sfan5 <sfan5@live.de>
Copy link
Member

@Ekdohibs Ekdohibs left a comment

Choose a reason for hiding this comment

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

Looks good; I also checked that you considered all instances of getPlayer and this seems to be the case.

@sfan5 sfan5 merged commit 32f68f3 into minetest:master Mar 10, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible SIGABRT when showing formspecs after async without safety checks
3 participants