[Internal] DetachServer isn't called for extensions. Is it really needed?
altrisi opened this issue ยท 4 comments
SettingsManager
s have a detachServer
method that resets their rules to default (with a null
source) and sets their server
to null
. That method is only called for Carpet's SettingsManager in CarpetServer (at onServerClosed
). That seems that it may be bad, because that may mean that rules won't be reset when changing a world.
However, the first thing (after setting the server) that the manager does when attachServer
is called (this one is correctly called for extensions ) is to reset the rules to default with an actual ServerCommandSource
. That means that, no matter if server is detached or not, rules will be reset the same way, and probably the most effective method is the one in attachServer
, since rules that may require a source to change (e.g. one of skyblock extension's rules) will be called correctly.
Therefore: Is detachServer
really needed? If so, should it be called for extensions too? Should it keep the reset functionality?
The only check to see if the server is null (condition that only happens when server is detached or before attaching) is in notifyPlayersCommandsChanged
.
May it be to clear pointers to the MinecraftServer
? (not sure if that would be useful for anything or how that works, just thought about gc)
loosing ties with the server is most appropriate way to do, other than just veering off static classes and replace them with instances - might be less convenient to use though. I would honestly call that on the extensions in case they have things to clean as well. Find it most appopriate myself.
I didn't fully understand that last comment (can't see relations with instances). Basically the thing is that detachServer
is only called for Carpet's manager, but not for extensions. However that doesn't really affect extensions since the server will be exchanged anyway in attachServer
, and rules will be reset (which happens twice for Carpet's since it resets when detaching without a source and when attaching with a source).
And the question was about whether to keep it as is, keeping it and calling it in extensions too, keep it without resetting rules on it (aka only setting the server to null
in detachServer
), or just don't keep it.