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

Unexpected argument collision in single-argument branches #39

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

Unexpected argument collision in single-argument branches #39

ZeroMemes opened this issue Oct 22, 2018 · 3 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

I have found that the only solution to this is to rename the argument in the single argument branch from y.
I believe this issue relates to the comment made by Dinnerbone Here.

@ZeroMemes ZeroMemes changed the title Unexpected argument collision in single-argument trees Unexpected argument collision in single-argument branches Oct 22, 2018
@Marcono1234
Copy link

It appears that one of the x nodes is choosen (see also @NeunEinser's comment). This could possibly be solved (unless I missed something) by adding a check to the sorting to prefer nodes which do not have children in case the reader reached the end.

@NeunEinser
Copy link

NeunEinser commented Oct 23, 2018

Yes, changing the sorting would be the easy fix and was my first thought as well (Not nodes without children, though, but nodes that have an command assigned)

However, I still think it would be a bit neater if the parser already threw an exception when the reader reached the end and the parsed command is not executable.

There are two reasons why I'd prefer that soution
(1) In my understanding it just makes more sense if execute only fails when the parser collected an exception or there is a runtime reason. Might be a bit more efficient, too.
(2) It should be very easily possible to provide a better error message in the parsing method (like "Missing argument. Usage: [getSmartUsage]"). I really dislike the current "Unkown command"

@GiantNuker
Copy link

GiantNuker commented Feb 7, 2020

There are 2 xes on the same level. Try this:

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("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;
                })
            )
        )
    )
);

@ZeroMemes

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

4 participants