Paper
Paper copied to clipboard
PaperMC
Save players before plugin disabling on server shutdown
For reference: https://discord.com/channels/289587909051416579/555462289851940864/1147468559887384586
If this behaviour is intended, feel free to close this PR, but as written in that message, I think /restart and /stop should follow the same order of operations when disabling plugins and saving players.
I also wanted to say that this allows the PlayerKickEvent to work with the /stop command because players are kicked before plugins get disabled and therefore the PlayerKickEvent is called.
This is 100% not something that can be done mid patch release, this is more a major change in behavior in which needs to be done on a major release with proper testing to ensure that we don't break anything;
This is how this logic has worked for a decade, and while you can maybe argue that it's weird, it is a side-effect of various dozen other choices in the codebase over a decade which has left us here
So you say this could be added but this is more likely something for a new major MC version? Or did you mean something else with "major release"?
Yea, this logic would be desirable in the long run imo, it just a) would best fit something like, 1.21 release b) might be best to wait for papers hardfork or be proposed to spigot.
Given that the quit/onDisable stuff has worked like this for years and most logic that relies on this logic deals with rather crucial parts of plugins (e.g. data saving) changing this mid version is rather terrible for plugin developers to update. Additionally, given that this breaks spigot compatibility, imo it is a rough PR even during a major release, as spigot plugins would just silently fail if they rely on spigots behaviour.
b) might be best to wait for papers hardfork or be proposed to spigot.
#6911 suggests to add a config option. Would that, assuming this is getting merged at some point, be a feasable option until a hardfork? Or is this something that better waits until the hardfork?
a config option was proposed for /stop to boot everybody, but there is no expectation that it would change event behavior, the server has stopped ticking and many things relied upon that in order to fire events, etc;
If we're going to change the behavior around this, it should just be changed without a config option, rather than having such a dangerous setting sitting around with massive side-effects
I guess that would be an option. But probably only if it's disabled per default, logically.
On the other side if you enable it and have spigot plugins that rely on the old system it breaks again. So maybe an unsupported option until Hardfork?
