Cancel of PlayerLoginEvent doing strange things

pmmp / PocketMine-MP

A server software for Minecraft: Bedrock Edition in PHP

https://pmmp.io

GNU Lesser General Public License v3.0

3.27k stars 1.54k forks source link

Cancel of PlayerLoginEvent doing strange things #3360

Closed yoraze closed 4 years ago

yoraze commented 4 years ago

Issue description

Expected result: https://github.com/pmmp/PocketMine-MP/blob/093a7c239ee61c72314d281ef2592f63146ee726/src/event/player/PlayerLoginEvent.php#L33
Actual result: Player does not destroyed by GC and has the infinity Generating world on screen

Screenshot_20200315_170203_com mojang minecraftpe

Steps to reproduce the issue

Create plugin where canceling PlayerLoginEvent.
Start and join to the server.
Now you can see what can be seen in the screenshot above.

OS and versions

PocketMine-MP: 4.0.0 (093a7c239)
PHP: 7.3.14
Server OS: Linux and Windows
Game version: PE/Win10 (both have the same bug)

Plugins

I wrote example plugin where canceling PlayerLoginEvent

Crashdump, backtrace or other files

No crashdumps, no backtraces.

TwistedAsylumMC commented 4 years ago

Minecraft has had this issue for a while, if a client gets disconnected during the login process, there's a chance it will hang on that screen indefinitely. This is not a PocketMine bug so this issue is pointless.

yoraze commented 4 years ago

I think you do not quite understand me correctly - I have already come to the conclusion that this is a client-bug, but what's the point of leaving PlayerLoginEvent cancelable if canceling it does such things with the client? I think something needs to be done with this.

Also, during the inspection of the logs, I noticed that the player does not destroyed by the GC after the cancelling of this event.

TwistedAsylumMC commented 4 years ago

There's no point removing a working feature that stopped working due to a client bug that will possibly be fixed in the future.

And about the GC, I believe the player object is not fully created at that stage so there is no need for the GC to destroy the player.

dktapps commented 4 years ago

If we changed the API every time Mojang created a bug, the API would be extremely volatile. This is supposed to work, and someday it'll get fixed.

ethaniccc commented 4 years ago

Instead I cancelled PlayerJoinEvent. It kicks the player with the reason, but gives back an error saying that it was still getting the permissions of the player or something.

Edit: It doesn't crash the server