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

The CommandAPI should use MinecraftServer#getCommands instead of MinecraftServer.vanillaCommandDispatcher #406

Open
JorelAli opened this issue Jan 17, 2023 · 2 comments · May be fixed by #517
Labels
enhancement New feature or request

Comments

@JorelAli
Copy link
Owner

Description

As per this comment:

People using nms

Please move away from MinecraftServer.vanillaCommandDispatcher, instead, use MinecraftServer#getCommands().

This change supports the internal dispatcher being reloaded. We will need to figure out some api in order to properly support re-registering commands under some kind of lifecycle api.

Expected code

Replace the use of MINECRAFT_SERVER.vanillaCommandDispatcher in NMS implementations of NMS#getBrigadierDispatcher

@Override
public final com.mojang.brigadier.CommandDispatcher<CommandSourceStack> getBrigadierDispatcher() {
return MINECRAFT_SERVER.vanillaCommandDispatcher.getDispatcher();
}

with MINECRAFT_SERVER.getCommands():

@Override
public final com.mojang.brigadier.CommandDispatcher<CommandSourceStack> getBrigadierDispatcher() {
    return MINECRAFT_SERVER.getCommands().getDispatcher();
}

Extra details

No response

@JorelAli JorelAli added the enhancement New feature or request label Jan 17, 2023
@willkroboth
Copy link
Collaborator

I notice that comment is from a Paper PR. If this change was applied, would the CommandAPI still work on Spigot? If not, I suppose we would just need to migrate calls to getBrigadierDispatcher over to PaperImplementations.

Also, does this change apply to Paper right now, or only after that PR is merged?

@JorelAli
Copy link
Owner Author

I notice that comment is from a Paper PR. If this change was applied, would the CommandAPI still work on Spigot? If not, I suppose we would just need to migrate calls to getBrigadierDispatcher over to PaperImplementations.

Also, does this change apply to Paper right now, or only after that PR is merged?

Yes, this will still work on Spigot. The MinecraftServer#getCommands method exists in Spigot and has always existed since 1.13. There isn't any particular reason why the CommandAPI should not be using this method, other than the fact it hasn't been tested and testing will be required to ensure it's a viable solution (i.e. make sure everything works as it should!)

@DerEchtePilz DerEchtePilz linked a pull request Apr 22, 2024 that will close this issue
28 tasks
@DerEchtePilz DerEchtePilz linked a pull request Apr 22, 2024 that will close this issue
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Could have
Development

Successfully merging a pull request may close this issue.

2 participants