Redirected command needs to have arguments

[I-Al-Istannen]

Problem

I wanted to make an alias for a command, so I registered a node that just redirected to my target, which worked nicely if the target command had arguments.

MWE

CommandDispatcher<Object> subject = new CommandDispatcher<>();
Object source = new Object();

LiteralCommandNode<Object> node = subject.register(
    literal("Hey")
        .executes((it) -> {
            System.out.println("Hey!");
            return 0;
        })
);
subject.register(
    literal("redirected")
        .redirect(node)
);
System.out.println(subject.execute("redirected", source));

Expected result

Hey!
0

Actual result

com.mojang.brigadier.exceptions.CommandSyntaxException: Unknown command at position 10: redirected<--[HERE]

	at com.mojang.brigadier.exceptions.SimpleCommandExceptionType.createWithContext(SimpleCommandExceptionType.java:21)
	at com.mojang.brigadier.CommandDispatcher.execute(CommandDispatcher.java:283)
	at com.mojang.brigadier.CommandDispatcher.execute(CommandDispatcher.java:176)
	at com.mojang.brigadier.CommandDispatcher.execute(CommandDispatcher.java:141)

Small analysis

It looks like this might be by design, if so documenting it would be nice (or a RTFM link :)

If am not mistaken the reason reason for this is as follows:

context.withCommand(child.getCommand());

// (1)
if (reader.canRead(child.getRedirect() == null ? 2 : 1)) {
    reader.skip();
    if (child.getRedirect() != null) {
        final CommandContextBuilder<S> childContext = new CommandContextBuilder<>(this, source, child.getRedirect(), reader.getCursor());

        // (2)
        final ParseResults<S> parse = parseNodes(child.getRedirect(), reader, childContext);
        context.withChild(parse.getContext());
        return new ParseResults<>(context, parse.getReader(), parse.getExceptions());
    } else {
        final ParseResults<S> parse = parseNodes(child, reader, context);
        if (potentials == null) {
            potentials = new ArrayList<>(1);
        }
        potentials.add(parse);
    }

Line (1) prevents the command from being parsed correctly, as the input ends after the literal of the redirected command. This is also the line that makes me think it might be deliberate.

You'd need to change that 1 to 0 in the first if and special case the case when the reader can not read anymore. Additionally you need to take care that the child context is correctly set, so that redirect modifiers work.

Further remarks

Line (1) could also hint at somebody just wanting to require an argument separator behind a redirected command and allow it to have no arguments, but that has a few flaws:

• It doesn't work if there just is no space at the end, which is likely the norm
• It doesn't work. It will recursively try to parse the rest and find nothing (as the reader is now at the end thanks to the skip call), which means that the recursive parseNodes call (2) fails. This leads to the current context having the wrong node, and the child context having no nodes.
  This context is then passed on to execute, which does the following:
  1. Finds that contexts is not empty and enters the loop
  2. Reads the first (and only) context and finds that the child is not null
  3. Enters the if(child != null)
  4. Checks if(child.hasNodes()), which will be false, as outlined above - it didn't match anything in the recursive call
  5. This leads to the whole portion reassigning next to be skipped, commandFound is not set and the outer while loop exists, as contexts is set to next which means to null.
  6. The last if realizes no command was found and throws an exception.

Closing remarks

I think that wall of text can be summed up with a few lines:

1. Is redirecting to commands without any arguments deliberately forbidden?
2. Why does the if check if the command is followed by only one character if it has a modifier, when that does not seem to change the outcome?
3. Some stuff about why it happens and a way to likely make it work.

Thank you for reading up until here :)

[Gamebuster19901]

I had the same issue. I was able to get around it by doing the following:

public static void register(CommandDispatcher<MessageContext> dispatcher) {
	LiteralArgumentBuilder<MessageContext> builder = Commands.literal("!online").executes((command) -> {
		return sendResponse(command.getSource());
	});
	
	dispatcher.register(builder);
	dispatcher.register(Commands.literal("!o").executes(builder.getCommand()));
}

https://github.com/Gamebuster19901/ExciteBot/compare/e83499560e289a4fab6b965546225359ba562285..1656342766c2566a26ffcd5adb8b0cbea26a847c

[hugmanrique]

See https://github.com/VelocityPowered/Velocity/blob/8abc9c80a69158ebae0121fda78b55c865c0abad/proxy/src/main/java/com/velocitypowered/proxy/util/BrigadierUtils.java#L38 for a brute-force solution. Gamebuster's solution is incomplete and only works for nodes with no children. Note that this is not a proper fix.

[5HT2]

Still no official solution on this? I'd love it if I could just register multiple literals for LiteralArgumentBuilder

[xpple]

Could this be fixed?

[polvallverdu]

still happening. bump

[xknat]

Any ETA? Will this ever be picked up?
We used this workaround: https://github.com/PaperMC/Velocity/blob/8abc9c80a69158ebae0121fda78b55c865c0abad/proxy/src/main/java/com/velocitypowered/proxy/util/BrigadierUtils.java#L38

[xNoobyyy]

would be nice to have that fixed or have someone comment if this is intentional behaviour

[sakurawald]

Maybe try this solution: #137