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

feat: Run post-initialization method at the first server tick #1465

Open
wants to merge 1 commit into
base: nightly
Choose a base branch
from

Conversation

GiantTreeLP
Copy link
Member

Motivation

Certain features of CloudNet react to services updating and thus changing their state to "Running". In the current implementation, this update happens at the time the bridge module/plugin/extension is enabled. This timing is not ideal as a lot of time-consuming tasks, like generating or loading the worlds/dimensions and other modules/plugins/extensions may happen after that. This causes the other components, like the signs module or the NPCs module, to report those services as being already online and ready to join by players, whilst the opposite is actually the case.

Modification

All currently supported server software platform integrations are augmented, so that PlatformBridgeManagement#postInit is called in the first game tick. This tick is only called, once the server is actually initialized, loaded and running. This also means that the server is ready to accept players.

Result

Amongst other things, signs now show the server for joining only, once the server is actually ready to accept players.

Testing

This change has been successfully tested with servers running Paper, Fabric and Limbo.

### Motivation

Certain features of CloudNet react to services updating and thus changing their state to "Running". In the current implementation, this update happens at the time the bridge module/plugin/extension is enabled.
This timing is not ideal as a lot of time-consuming tasks, like generating or loading the worlds/dimensions and other modules/plugins/extensions may happen after that.
This causes the other components, like the signs module or the NPCs module, to report those services as being already online and ready to join by players, whilst the opposite is actually the case.

### Modification

All currently supported server software platform integrations are augmented, so that `PlatformBridgeManagement#postInit` is called in the first game tick. This tick is only called, once the server is actually initialized, loaded and running. This also means that the server is ready to accept players.

### Result

Amongst other things, signs now show the server for joining only, once the server is actually ready to accept players.
@GiantTreeLP GiantTreeLP force-pushed the feat/post-initialize-in-first-tick branch from 86d7171 to 28f16a6 Compare July 26, 2024 21:07
@GiantTreeLP
Copy link
Member Author

The forced push fixes an issue where the method reference used as a Runnable is not removed from the list of tickable tasks in Fabric.

@0utplay
Copy link
Member

0utplay commented Aug 12, 2024

It is guaranteed that no player can join before the first server tick happens right? Because otherwise this might cause issues with fallbacks and some of our login checks

@GiantTreeLP
Copy link
Member Author

For vanilla, ie. Bukkit, Spigot and Paper, I could not absolutely determine that, due to the obfuscated nature of the vanilla Java Edition server.

As I have understood from MinecraftServer.java and DedicatedServer.java, ticking scheduled tasks happens before ticking network connections. I don't know, however, if the login sequence happens on a tick or rather in parallel; for that I did not have access to enough decompiled source code.

I will check with other server software.

@DasBabyPixel
Copy link
Contributor

DasBabyPixel commented Aug 16, 2024

Or set a flag that denies all logins before the logic happens, which in turn also removes the flag. Would work on all platforms, no issues

EDIT: LuckPerms does something very similar

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

Successfully merging this pull request may close these issues.

3 participants