Redirect with brigadier API does not work as expected

[xaanndeer]

Expected behavior

When redirecting aliases to a command using LiteralArgumentBuilder.redirect(LiteralCommandNode) the alias should execute said command.

Observed/Actual behavior

Aliases get sent to the client, however upon executing (without namespace) an unknown or incomplete command exception gets thrown. When using an alias without a namespace arguments don't get recognized and are marked as incorrect as well.

Steps/models to reproduce

Create a command using the brigadier API and add an alias that redirects to this command.

        LiteralArgumentBuilder<CommandSourceStack> builder = Commands.literal("teleport");
        builder.requires(commandSourceStack -> commandSourceStack.getSender().hasPermission("example.command.teleport"))
                .then(Commands.argument("target", ArgumentTypes.player())
                .executes(source -> {
                    Player player = source.getArgument("target", PlayerSelectorArgumentResolver.class).resolve(source.getSource()).getFirst();
                    source.getSource().getSender().sendMessage("Teleporting to %s".formatted(player.getName()));
                    return 1;
                }));
        LiteralCommandNode<CommandSourceStack> literalCommandNode = builder.build();
        commands.register(javaPlugin.getPluginMeta(), literalCommandNode, "Teleport command", Collections.emptySet());
        commands.register(javaPlugin.getPluginMeta(), Commands.literal("tp").redirect(literalCommandNode).build(), "Teleport command", Collections.emptySet());

Try to run this command by using the alias without namespace: Arguments are marked as incorrect and Unknown or incomplete command

Plugin and Datapack List

Paper version

30.05 01:35:37 [Server] INFO This server is running Paper version 1.20.6-121-master@a31dc90 (2024-05-29T21:07:31Z) (Implementing API version 1.20.6-R0.1-SNAPSHOT)
30.05 01:35:37 [Server] INFO You are 3 version(s) behind
30.05 01:35:37 [Server] INFO Download the new version at: https://papermc.io/downloads/paper
30.05 01:35:37 [Server] INFO Previous version: 1.20.6-115-9d6f2cc (MC: 1.20.6)

Other

No response

[Machine-Maker]

Well, first thing is that the structure of this command is probably not what you thought it was. You aren't marking the "argument" as executable. You need to call .executes() on the argument like this:

LiteralArgumentBuilder<CommandSourceStack> builder = Commands.literal("teleport");
builder
    .requires(commandSourceStack -> commandSourceStack.getSender().hasPermission("example.command.teleport"))
    .then(Commands.argument("target", ArgumentTypes.player())
        .executes(source -> {
            Player player = source.getArgument("target", PlayerSelectorArgumentResolver.class).resolve(source.getSource()).getFirst();
            source.getSource().getSender().sendMessage("Teleporting to %s".formatted(player.getName()));
            return 1;
        })
    );

Then at least doing /teleport @p does successfully teleport the player to itself.

[Machine-Maker]

As for the problem that still exists if you fix ^, I guess I'd recommend just avoiding brigadier's "redirect" system if possible. There are inconsistencies within the library itself which can lead to all sorts of unexpected behavior. For example, the actually dispatcher logic does not handle double redirects at all (a redirect to a redirect to an executable). For root literals, I would just use the "aliases" collection parameter in the various register methods.

[xaanndeer]

Yea sorry, it's pretty late and I quickly put together this example with a foggy brain. However, even with a correct structure the issue still persists. I was told to open up an issue on the Discord server as this is not intented behavior. Using the aliases collection in the register method does indeed work correctly, but I wanted to override the vanilla alias so I had to use redirect as using the register method will not override these.

EDIT: I updated the post with correct code and the correct observed behavior. Sorry for my negligence!

[lynxplay]

Imo, while the brig redirect system seems to be a bit of a mess, this is still solvable by us.
Some may not be.

A) A node that does not have children cannot be redirected to from a single node.
The CommandDispatcher only resolves redirects if the reader can read post the initial literal node (e.g. arguments are provided).
Annoying.
B) When flattening an alias redirect (either because child.len == 0 or the internal flag was passed) the logic does not take into
account that the root of the alias itself might be a redirect.

So we'd need to fix brigadiers redirect BS by flattening them all (both the directly registered redirect and the then registered alias) to directly redirect or be a flattened copy of the redirect target in case of 0 children.