-
Notifications
You must be signed in to change notification settings - Fork 409
Description
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
parseNodescall (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:- Finds that
contextsis not empty and enters the loop - Reads the first (and only) context and finds that the child is not null
- Enters the
if(child != null) - Checks
if(child.hasNodes()), which will be false, as outlined above - it didn't match anything in the recursive call - This leads to the whole portion reassigning
nextto be skipped,commandFoundis not set and the outer while loop exists, ascontextsis set tonextwhich means tonull. - The last if realizes no command was found and throws an exception.
- Finds that
Closing remarks
I think that wall of text can be summed up with a few lines:
- Is redirecting to commands without any arguments deliberately forbidden?
- 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?
- Some stuff about why it happens and a way to likely make it work.
Thank you for reading up until here :)