CommandAPI icon indicating copy to clipboard operation
CommandAPI copied to clipboard

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

Open JorelAli opened this issue 2 years ago • 2 comments

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 https://github.com/JorelAli/CommandAPI/blob/2efd3a3ffdee797ab8b2a4b12784c9e5741d197a/commandapi-nms/commandapi-1.19.3/src/main/java/dev/jorel/commandapi/nms/NMS_1_19_3_R2.java#L395-L398

with MINECRAFT_SERVER.getCommands():

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

Extra details

No response

JorelAli avatar Jan 17 '23 13:01 JorelAli

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?

willkroboth avatar Jan 17 '23 14:01 willkroboth

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!)

JorelAli avatar Jan 22 '23 01:01 JorelAli