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

Defer sponge command registration #184

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

RacoonDog
Copy link
Contributor

The Problem

As it currently stands, trying to register sponge commands through CommandRegistrar#EVENT from either a ClientModInitializer or a DedicatedServerModInitializer results in non-determinism as whether they are successfully registered depends on the order the initializers were invoked by fabric loader (because CommandRegistrar#EVENT is also invoked from these entrypoints).

The Solution

I added a CommandRegistrationCallback#EVENT event in the command api v1 library that is invoked before the vanilla command manager is filled with mod commands during the ServerLifecycleEvents#SERVER_STARTING callback. I then wrapped the sponge command registration event invocations in the ImplInit classes so that their registration is now deferred to the ServerLifecycleEvents#SERVER_STARTING callback rather than the mod initializers.

This pull request ensures a strict load order between sponge commands and vanilla commands without breaking backwards compatibility.

@thecatcore
Copy link
Member

Interesting, we never realized this issue as test mod commands are registered through a ModInitialiser instead. What would be the advantage of registering them through a ClientModInitialiser or a DedicatedServerModInitialiser in this case?

@RacoonDog
Copy link
Contributor Author

Interesting, we never realized this issue as test mod commands are registered through a ModInitialiser instead. What would be the advantage of registering them through a ClientModInitialiser or a DedicatedServerModInitialiser in this case?

In my case, I ran into this issue while I was developing https://github.com/LegacyHypixel/LegacyBetterConfig

@thecatcore
Copy link
Member

Alright, after looking more into it, the test mods somehow don't use the command registering event. Which isn't the intended way of registering commands. Can you fix the test mods as well in this pr?

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.

2 participants