lucko / commodore

Utility for using Minecraft's 1.13 'brigadier' library in Bukkit plugins.
MIT License
176 stars 15 forks source link

Remove Bukkit’s data from dispatcher before adding our custom CommandNode #12

Closed marcbal closed 4 years ago

marcbal commented 4 years ago

For classic Bukkit’s commands to work properly, the server puts in the Brigadier’s command dispatcher a CommandNode that look like this :

LiteralArgumentBuilder.literal(nameOfTheCommand)
    .then(RequiredArgumentBuilder.argument("args", StringArgumentType.greedyString()))

CraftBukkit source: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/command/BukkitCommandWrapper.java#32

The problem is that calling the method dispatcher.getRoot().addChild(node); merge the added CommandNode (ours) to the existing one (Bukkit’s) (see the source here https://github.com/Mojang/brigadier/blob/559d8f39727e98d374b0726ad88aed9832beee4e/src/main/java/com/mojang/brigadier/tree/CommandNode.java#L69 ). As a result, the <args> argument from Bukkit conflict with the custom arguments. In my case, only LitteralArguments worked as first argument, but using a RequiredArgument as a first argument of my command was not working.

The solution I submit to solve this problem is to remove the Bukkit’s data from the commandDispatcher before adding our custom CommandNode.

Tests

Without the patch (what is displayed in the chat is the structure of the command, using a home made command 🙂 ): image

With the patch: image

The code for my command:

LiteralArgumentBuilder.literal("sign")
    .then(RequiredArgumentBuilder.argument("x", IntegerArgumentType.integer())
        .then(RequiredArgumentBuilder.argument("y", IntegerArgumentType.integer())
            .then(RequiredArgumentBuilder.argument("z", IntegerArgumentType.integer())
                .then(LiteralArgumentBuilder.literal("edit")
                    .then(RequiredArgumentBuilder.argument("ligne", IntegerArgumentType.integer(1, 4))
                        .then(RequiredArgumentBuilder.argument("texte", StringArgumentType.greedyString()))
                    )
                )
                .then(LiteralArgumentBuilder.literal("color")
                    .then(RequiredArgumentBuilder.argument("colorant", StringArgumentType.word()))
                )
            )
        )
    );
lucko commented 4 years ago

Ah very interesting.

Thank you for your (very well explained) contribution! :)

lucko commented 4 years ago

Did some quick testing and this seems to work perfectly! I've released commodore version 1.8 (it's propagating to Maven Central now) to include this fix. :)

Thanks again!