Incorrect Argument Required

[ImNotStable]

CommandAPI version

9.20

Minecraft version

1.19

Are you shading the CommandAPI?

Yes

What I did

CommandAPI all works correctly, this is the code for the command I'm having an issue with.

  private static final CommandTree command = new CommandTree("request")
    .then(new LiteralArgument("toggle")
      .executesPlayer(RequestCommand::toggleRequests))
    .then(new MultiLiteralArgument("answer", "accept", "deny")
      .withRequirement(sender -> {
        if (!(sender instanceof Player player)) return false;
        return requests.containsKey(player.getUniqueId());
      })
      .then(new PlayerArgument("target")
        .executesPlayer((RequestCommand::answerRequest))))
    .then(new OfflinePlayerArgument("target")
      .replaceSuggestions(ArgumentSuggestions.strings(Misc::getOfflinePlayerSuggestion))
      .then(new DoubleArgument("amount", Number.getMinimumValue())
        .executesPlayer(RequestCommand::request)));

What actually happened

What should have happened

Not what is portrayed in the screenshots above

Server logs and CommandAPI config

Other

No response

[DerEchtePilz]

I split up your command so it's easier to see what's going on and ended with this:

new CommandTree("request")
    .then(new LiteralArgument("toggle")
        .executesPlayer(info -> {
            info.sender().sendMessage(Component.text().content("Toggled"));
        })
    )
    .then(new MultiLiteralArgument("answer", "accept", "deny")
        .withRequirement(sender -> {
            if (!(sender instanceof Player player)) return false;
            return true;
        })
        .then(new PlayerArgument("target")
            .executesPlayer(info -> {
                info.sender().sendMessage(Component.text().content("You chose option " + info.args().get("answer") + " with player name " + ((Player) info.args().get("target")).getName()));
            })
        )
    )
    .then(new OfflinePlayerArgument("target")
        .replaceSuggestions(ArgumentSuggestions.strings(strings -> new String[0]))
        .then(new DoubleArgument("amount", Double.MIN_VALUE)
            .executesPlayer(info -> {
                info.sender().sendMessage(Component.text().content("You chose " + ((OfflinePlayer) info.args().get("target")).getName() + " with value " + info.args().get("amount")));
            })
        )
    )
    .register();

From this, and from your logs it isn't apparent what would cause your issue so I also tested these commands ingame and they worked without issues.
It needs to be mentioned though that I tested your code on 1.20.2 because for some reason I couldn't get a project setup for 1.19 to initialize so it might still be a 1.19 issue (but to be honest, it would surprise me!).
Have you tested, or could you test, your code in 1.20.2?

[willkroboth]

Oooh, actually, I think this is a very subtle vanilla bug.

So, usually, literal nodes should take priority over argument nodes. In this case, that means that while accept is a valid player name, the command path following the literal accept is used by default. That's why when you send request accept 1.0 to the server it gets annoyed. The CommandNode wants to use the literal accept, but when it evaluates the requirement for that node, requests.containsKey(player.getUniqueId()) is currently false, so it can't find a valid node to run and sends back the Incorrect argument message.

However, requirements are always evaluated server-side. When the server sends the Commands to the client, it evaluates whether the player is allowed to use the accept literal command path. Since requests.containsKey(player.getUniqueId()) is currently false, the player can not use the request accept subcommand, so the server just doesn't send it. That means the client thinks that request <target> ... is the only option. So, when trying to figure out what to do with request accept, it doesn't realize that there is supposed to be a literal accept and treats it as an offline player's name. accept is a valid player name, so it moves on and asks you for the DoubleArgument amount.

---

So, the problem here happens because the client does not know about the literal argument, while the server always tries to use the literal accept branch. I'd say this is either a bug with how Brigadier defaults to literals whose requirements aren't satisfied or a bug with how the vanilla server sends literals to the player.

I think it makes the most sense for Mojang to fix this. Unfortunately, I'm not sure if this bug affects vanilla gameplay so they might not be willing to fix it. They might not have any problems with this if they don't happen to have a command with the specific structure needed to reveal this bug, similar to the resolution of https://bugs.mojang.com/browse/MC-261429.

I also don't know if Spigot would fix this. I don't think they officially support custom Brigadier commands, so their stance might be the same as Mojang's. Paper might have a good reason to fix this once PaperMC/Paper#8235 is merged, so I might bring this up with them if I can't find a good reason to bring this directly to Mojang.

---

In the meantime, you could modify your command structure to avoid the bug. Instead of /request <target> ... potentially conflicting with /request [accept/deny] ..., you could change the send request subcommand to /request send <target> <amount>. That way, it is not ambiguous wether accept is supposed to be a player name or a literal node.

[ImNotStable]

Still happens on 1.20.2, the command still works it's just that it says it won't work before executing.
If this changes anything I'm using 1.18.2 version of the paper api

[DerEchtePilz]

Oooh, actually, I think this is a very subtle vanilla bug.

So, usually, literal nodes should take priority over argument nodes. In this case, that means that while accept is a valid player name, the command path following the literal accept is used by default. That's why when you send request accept 1.0 to the server it gets annoyed. The CommandNode wants to use the literal accept, but when it evaluates the requirement for that node, requests.containsKey(player.getUniqueId()) is currently false, so it can't find a valid node to run and sends back the Incorrect argument message.

However, requirements are always evaluated server-side. When the server sends the Commands to the client, it evaluates whether the player is allowed to use the accept literal command path. Since requests.containsKey(player.getUniqueId()) is currently false, the player can not use the request accept subcommand, so the server just doesn't send it. That means the client thinks that request <target> ... is the only option. So, when trying to figure out what to do with request accept, it doesn't realize that there is supposed to be a literal accept and treats it as an offline player's name. accept is a valid player name, so it moves on and asks you for the DoubleArgument amount.

So, the problem here happens because the client does not know about the literal argument, while the server always tries to use the literal accept branch. I'd say this is either a bug with how Brigadier defaults to literals whose requirements aren't satisfied or a bug with how the vanilla server sends literals to the player.

I think it makes the most sense for Mojang to fix this. Unfortunately, I'm not sure if this bug affects vanilla gameplay so they might not be willing to fix it. They might not have any problems with this if they don't happen to have a command with the specific structure needed to reveal this bug, similar to the resolution of https://bugs.mojang.com/browse/MC-261429.

I also don't know if Spigot would fix this. I don't think they officially support custom Brigadier commands, so their stance might be the same as Mojang's. Paper might have a good reason to fix this once PaperMC/Paper#8235 is merged, so I might bring this up with them if I can't find a good reason to bring this directly to Mojang.

In the meantime, you could modify your command structure to avoid the bug. Instead of /request <target> ... potentially conflicting with /request [accept/deny] ..., you could change the send request subcommand to /request send <target> <amount>. That way, it is not ambiguous wether accept is supposed to be a player name or a literal node.

Wait, if this is a requirement issue, it should be enough to just resend the commands after the player has been added to that map, right?

[willkroboth]

Yeah, if the player meets the requirements to run the /request [accept/deny] ... branch, it should all work as expected. The client and server will agree on which commands are available. When typing /request accept..., both will agree that accept refers to the literal argument and not the offline player in the same location.

---

I think the problem here is that when the requirement is not met, the client and server handle the command differently. I think the server shows the intended behavior, where literal arguments are preferred by default. Typing /request accept 1 should be invalid because the player does not meet the requirements necessary to use the literal accept argument. However, the client is told that /request <target> ... is the only command path available. So, when displaying /request accept 1, the client treats accept as a player name and says that the player can run that command.

So, the vanilla bug happens when there is a literal node with an unmet requirement and an argument node in the same place. When the server receives a command using the literal, it will send back the Incorrect argument message because the player does not meet the requirements necessary to use the literal node.

However, the client will show that the command is valid because it thinks the input is referring to the argument node.

So, I think this is a client issue that only Mojang or a client+server side mod can fix. Unfortunately, I don't think Mojang is going to fix this. There doesn't seem to be a vanilla command that has the specific structure necessary to reveal this issue. I am still looking, but even if they did fix it, they probably wouldn't backport the fix to 1.19 or any other version.

---

Right now, I think the only way to fix the command when the requirement is not met is to change its structure to avoid the ambiguous case. The main conflict is that accept could be a player name, so the client thinks it is fine when the server disagrees. To fix this example, @ImNotStable could change the 'send request' subcommand to /request send <target> <amount>. That way the <target> argument never conflicts with the deny/accept literal.

[willkroboth]

This seems to be a bug with Brigadier, as reported by Mojang/brigadier#87. That means it will probably never be fixed. The best way to approach this is probably just to avoid cases where a literal and argument may be ambiguous.

[ImNotStable]

sad