Incendo / cloud

Command framework & dispatcher for the JVM
https://cloud.incendo.org
MIT License
433 stars 54 forks source link

Literal argument aliases are ignored by cloud-brigadier #760

Closed metabrixkt closed 2 months ago

metabrixkt commented 2 months ago

Steps to reproduce

Here's an example command:

manager.command(
    manager
        .commandBuilder("reputation", Description.of("Change player's reputation"))
        .permission("example-plugin.command.reputation.good")
        .senderType(Player.class)
        .required("player", offlinePlayerParser())
        .literal("good", "potato", "tomato")
        .handler(new AsyncHandler<>(ctx -> handleAddRemove(ctx.sender(), ctx.get("player"), ReputationAction.UPVOTE)))
);

Expected behavior

Expected command tree:

literal "reputation"
    offline player "player"
        literal "good"
        literal "potato"
        literal "tomato"

Observed behavior

However, the resulting Brigadier command tree looks like this:

literal "reputation"
    offline player "player"
        literal "good"

Other

I might be wrong, but I think this happens because o.i.c.brigadier.node.LiteralBrigadierNodeFactory#constructCommandNode(CommandNode<C>, BrigadierPermissionChecker<C>, com.mojang.brigadier.Command<S>) creates a literal node for the component name only, ignoring its aliases:

final ArgumentBuilder<S, ?> argumentBuilder;
if (root.component().type() == CommandComponent.ComponentType.LITERAL) {
    argumentBuilder = this.createLiteralArgumentBuilder(root.component(), root, permissionChecker);
} else {
    argumentBuilder = this.createVariableArgumentBuilder(root.component(), root, permissionChecker);
}
this.updateExecutes(argumentBuilder, root, executor);

And then in LiteralBrigadierNodeFactory#createLiteralArgumentBuilder(CommandComponent<C>, CommandNode<C>, BrigadierPermissionChecker<C>):

private @NonNull ArgumentBuilder<S, ?> createLiteralArgumentBuilder(
        final @NonNull CommandComponent<C> component,
        final @NonNull CommandNode<C> root,
        final @NonNull BrigadierPermissionChecker<C> permissionChecker
) {
    return LiteralArgumentBuilder.<S>literal(component.name()) // <--- aliases not used
            .requires(this.requirement(root, permissionChecker));
}

One possible fix would be making constructCommandNode(...) return a list of ArgumentBuilders which would work well with Brigadier node's then(...) method

But I might be wrong about why this happens, so ¯\_(ツ)_/¯

metabrixkt commented 2 months ago

oops, wrong repo