Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interesting issue with single argument children alongside multi argument children #38

Closed
ZeroMemes opened this issue Oct 22, 2018 · 2 comments

Comments

@ZeroMemes
Copy link

Consider the following command tree:

dispatcher.register(literal("goal")
    .then(argument("y", integer())
        .executes(c -> {
            setGoal(new GoalYLevel(
                c.getArgument("y", Integer.class)
            ));
            return 0;
        })
    )
    .then(argument("x", integer())
        .then(argument("z", integer())
            .executes(c -> {
                setGoal(new GoalXZ(
                    c.getArgument("x", Integer.class),
                    c.getArgument("z", Integer.class)
                ));
                return 0;
            })
        )
    )
    .then(argument("x", integer())
        .then(argument("y", integer())
            .then(argument("z", integer())
                .executes(c -> {
                    setGoal(new GoalBlock(
                        c.getArgument("x", Integer.class),
                        c.getArgument("y", Integer.class),
                        c.getArgument("z", Integer.class)
                    ));
                    return 0;
                })
            )
        )
    )
);

This should have 3 possible usages

goal <y>
goal <x> <z>
goal <x> <y> <z>

However, when I try to execute the command with a single integer argument (via goal 1) an exception is thrown!

com.mojang.brigadier.exceptions.CommandSyntaxException: Unknown command at position 6: goal 1<--[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:178)
  at com.mojang.brigadier.CommandDispatcher.execute(CommandDispatcher.java:143)
  ...

Both the 2-arg and 3-arg usages work as intended:
Chat Output

@NeunEinser
Copy link

NeunEinser commented Oct 22, 2018

Interesting.
the CommandDispatcher when parsing looks at all child nodes and tries to parse the first node and add it to potential commands. When the node could not parse, it also collects the exceptions in a list. However, in this case we have 3 integer arguments as child of the literal. Of course, all 3 children will parse without any exceptions.
Later on, the parsing method will try to find the best child. When doing so, it prefers children that parsed the complete command string and then, children that did collect exceptions.
In this case, all 3 children will be evaluated to be suitable.

This means that your example is, as far the command parser is conserned, just as ambiguous as the following would be:

dispatcher.register(literal("goal")
	.then(argument("x", integer())
		.executes(c -> {
			System.out.println(c.getArgument("x", Integer.class));
			return 0;
		})
	)
	.then(argument("y", integer())
		.executes(c -> {
			System.out.println(c.getArgument("y", Integer.class));
			return 0;
		})
	)
	.then(argument("z", integer())
		.executes(c -> {
			System.out.println(c.getArgument("z", Integer.class));
			return 0;
		})
	)
);

Maybe the parse-method should already collect an exception, when the command is not executable and the reader doesn't have anything to read anymore, rather than leaving that for the execute-method.

@ZeroMemes
Copy link
Author

Accidental duplicate due to GitHub downtime. See #39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants