[Suggestion] Argument class combining name and type

[Marcono1234]

Currently when creating an argument, the name and argument type are not joined in any way. This can lead to errors where the command uses the wrong type when accessing the value.

Therefore it might be good to have a class which combines:

• The argument name
• The argument type

Then the same instance of this class can be used when creating the tree and inside the commands. Additionally the methods getting the value from the context would not have to be static anymore, but could be generic, like T getValue(CommandContext<?>). This would make #18 easier as well since the getTypeNameOrDefault method could be non-static and implemented by the base class only.

Example

CommandDispatcher<CommandSourceStack> dispatcher = new CommandDispatcher<>();

Argument<Integer> barArgument = new Argument<>(barArgument, integer());

dispatcher.register(
    literal("foo")
        .then(
            argument(barArgument)
                .executes(c -> {
                    System.out.println("Bar is " + barArgument.getValue(c));
                    return 1;
                })
        )
        .executes(c -> {
            System.out.println("Called foo with no arguments");
            return 1;
        })
);

[tterrag1098]

This is pretty much exactly what I was trying out on my fork.

Pros:

• Strong type bounds for argument object mappers
• "Impossible" to accidentally use the wrong type
• No more need for static methods on each argument type class

Cons:

• Breaks the "single chain of method calls" pattern by requiring a field definition
• Adds a bit more generic noise since a non-inferred type is required

Overall I think it's a good change but I'd be interested to see what others think.

[kashike]

I'd personally prefer to be doing context.getArgument(MY_ARGUMENT) instead of MY_ARGUMENT.getValue(context).

[tterrag1098]

That's what I add: https://github.com/tterrag1098/brigadier/blob/0247f59a2ef4a9680b85d069e71df4430973774a/src/main/java/com/mojang/brigadier/context/CommandContext.java#L85

[Marcono1234]

@tterrag1098, Argument should probably not extend ArgumentType. In my opinion this is only confusing since when used as such, the name is basically lost / ignored.

And using an enum for BoolArgumentType might be convenient when writing the class, but could be confusing for the user because they expect multiple values and not just a single INSTANCE. What do you others think about that?

[tterrag1098]

It's a common pattern: https://javarevisited.blogspot.com/2012/07/why-enum-singleton-are-better-in-java.html

[kashike]

:+1: for Argument not extending ArgumentType.