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

Improve handling of offline players during events #1370

Open
PseudoKnight opened this issue Oct 17, 2023 · 1 comment
Open

Improve handling of offline players during events #1370

PseudoKnight opened this issue Oct 17, 2023 · 1 comment

Comments

@PseudoKnight
Copy link
Contributor

PseudoKnight commented Oct 17, 2023

Some events pass a player object for players that are missing from the player list during the event. This causes a CREPlayerOfflineException when getting these players from Static.GetPlayer() or Static.GetCommandSender(), as well as made it tricky to pass the player to the bind environment. This was originally solved by hijacking the player injection system used for capture_runas(), but it wasn't designed for this.

Now, after some changes, the player is added to the environment from MCPlayerEvent instead, but injection is still used for getting players by name (and as a fallback for environment adding). But this happens in AbstractEvent, and this is undesirable as it mixes MC and MethodScript stuff. This will need to be changed some day in the future, but extensions would need to be considered.

Two events use injection still: player_login, where the player is not yet added to the player list at all; and player_spawn where the player is missing from the byName player map but not the byUUID player map (byUUID determines whether the player is "online" or not), which means they're technically online but Static.GetPlayer() can't find them without injection. I consider the player_spawn situation a bug, so it might be better to move the workaround for this to the bukkit abstraction layer.

As for player_login, the player is technically offline, so I'm not sure what the expectation should be here. Due to this, currently you can run functions from player context but not by name, except functions that take a CommandSender instead. (e.g. has_permission(<playerName>, <permissionNode>)) We could change Static.GetPlayer() to take an environment object for situations like this (is player_login the only one?), but that'll affect a lot of functions and some will malfunction in unknown ways for truly offline players. I haven't done a comprehensive test.

@PseudoKnight
Copy link
Contributor Author

I replaced player injection in the player_spawn event with a new workaround that also addresses an additional issue with world changes during a quit event: b39b8f0. Using this in the abstraction implementation for Spigot is a more appropriate location to workaround a bug in Spigot. Unfortunately this doesn't just affect getting players by name, but also all_players(), so we might want to write a workaround for that as well.

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

No branches or pull requests

1 participant