gnembon / fabric-carpet

Fabric Carpet
MIT License
1.73k stars 275 forks source link

`__on_player_disconnects` does not run when on an integrated server. #1256

Open James103 opened 2 years ago

James103 commented 2 years ago

As of Carpet mod 1.4.56 and Minecraft 1.18.1, when you save and exit the game, the integrated server stops, but it does not fire the __on_player_disconnects event for you. I believe this used to work in previous versions (including 1.16.x).

To reproduce:

  1. Create or join a singleplayer world.
  2. /script load event_test
  3. Save and exit.
  4. Notice that in the logs, __on_start and __on_close were run, but not __on_player_disconnects (__on_player_connects does run as well, but it needs stay_loaded to be true for a Scarpet app)
  5. Start a dedicated server.
  6. /script load event_test on the dedicated server.
  7. Start the client and join the local server.
  8. Disconnect from the server.
  9. Notice the resulting output in the logs.

For example, an offline progress system for a server needs to store when the player was last online (i.e. when the player disconnected from the server). Optimizing this fully requires the use of __on_player_disconnects, but that is currently blocked by this bug.

Ghoulboy78 commented 2 years ago

Well technically on singleplayer __on_player_disconnects and __on_close are the same thing... However I do agree that this doesn't work with, say, bots, and also the title says integrated servers, and while I think those are only singleplayer, I could be mistaken.

Ghoulboy78 commented 2 years ago

Well it appears that __on_player_disconnects is handled by ServerPlayNetworkHandler_coreMixin, and name makes me suspect that it's only ever triggered in the context of a server.

Ghoulboy78 commented 2 years ago

I've looked into it more, and it should be triggered in the ClientLevel#disconnect() function, but there you can't get a ServerPlayer variable required to trigger the function, you only get a LocalPlayer type variable (which makes sense), so without a big refactor I don't think it would work.