Converted command entity selector flattening fails when an argument contains a space

[Bafy78]

CommandAPI version

9.4.1

Minecraft version

1.20.2

Are you shading the CommandAPI?

No

What I did

• I wrote this in the configuration :

  - LuckPerms:
  - luckperms (user) <user>[api:players] <args>[api:greedy_string]

What actually happened

The command is converted, as the @... is suggested when using /execute run luckperms user But if I actually use a vanilla target selector I get a luckperms error message saying @p is not a valid username/uuid

(in the screenshot the first output is when using /execute run luckperms user Bafy78 parent info, and the second one is when replacing Bafy78 by @p

What should have happened

@p should have been converted into the name of the nearest player

Server logs and CommandAPI config

No response

Other

LuckPerms uses the Commodore library for registering commands. It’s hard to tell on my phone :P, but when combined with CommandAPI conversion there’s definitely weird stuff that could happen (link). Might be possible for the CommandAPI to do something to work with Commodore commands.

[willkroboth]

I don't know if this is mentioned anywhere in Luckperm's documentation, but I discovered that at the literal bottom of the config file, they have a resolve-command-selectors option. If you set this to true, then Luckperms will resolve symbols like @p.

The client weirdly shows red text, but it does work. You also can't use /execute run luckperms. However, both of these things are kinda fixed if you also use the CommandAPI to convert the luckperms command (I had to add LuckPerms to the CommandAPI's skip-sender-proxy to get the command to execute without error).

This is just a short term solution though. I think there is still a bug where the CommandAPI is not resolving the selector itself for some reason when resolve-command-selectors is false.

[Bafy78]

However, both of these things are kinda fixed if you also use the CommandAPI to convert the luckperms command (I had to add LuckPerms to the CommandAPI's skip-sender-proxy to get the command to execute without error).

What is your config file exactly? I tried multiple things but only get errors when trying to run the luckperms command with /execute run

[willkroboth]

My CommandAPI config looks like this:

plugins-to-convert:
  - LuckPerms:
      - luckperms (user) <user>[api:players] <args>[api:greedy_string]

skip-sender-proxy:
  - LuckPerms

[willkroboth]

Ah, I figured out why this is happening. It doesn't have anything to do with LuckPerms or Commodore; fully a CommandAPI bug. Basically, the logic for resolving the entity selectors assumed that each argument would not contain a space, which was not the case when you input info parent into a GreedyStringArgument. I've made a basic fix for the problem that is specifically happening here (zipped jar: CommandAPI-9.5.0-SNAPSHOT_26_May2024(04-34-06PM).zip), though I've realized that there are other problems with the current logic that should also be addressed.

---

For reference if anyone else wants to solve the underlying issue, here's what's going on. The problem here is line 249:

https://github.com/JorelAli/CommandAPI/blob/43d31e0d0c09556989f23efe33314ade2b9e985b/commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIHandler.java#L247-L257

When the command minecraft:luckperms user @p info parent is executed with one player on the server, the relevant variables have the following values:

Argument[] args = {
    LiteralArgument("user"), 
    EntitySelectorArgument.ManyPlayers("players"),
    GreedyStringArgument("args")
}

String[] result = {"user", "@p", "info", "parent"}

List<List<String>> product = List.of(List.of(null, "playerName", null))

The result array comes from splitting the command line by spaces. The code above wants to modify this array to expand all entity selectors. The product list tells it that args[1] expands to "playerName", while args[0] and args[2] don't need to be changed (null value).

However, since the greedy argument's value info parent contained a space, the result array actually contains 4 values, which doesn't match with the expected 3 arguments. Hence, result.length == strings.size() is false, and the result array is never modified before executor.execute(convertedExecutionInfo) is called. This means the results array still contains {"user", "@p", "info", "parent"}, when it should have had {"user", "playerName", "info", "parent"}. When the converted result is then passed to LuckPerms, it sees the command luckperms user @p info parent when it should have seen luckperms user playerName info parent. If LuckPerms' resolve-command-selectors option is false, it will complain that @p is not a player.

---

In the jar I linked, I solved this issue by applying this change:

for (List<String> strings : product) {
-   // We assume result.length == strings.size
-   if (result.length == strings.size()) {
-       for (int i = 0; i < result.length; i++) {
+       for (int i = 0; i < strings.size(); i++) {
            if (strings.get(i) != null) {
                result[i] = strings.get(i);
            }
        }
-   }
    resultValue += executor.execute(convertedExecutionInfo);
}

This works fine in this case since the argument with spaces is at the end, so the strings list still aligns with the result array.

{null,   "playerName", null}
 V       V             V
{"user", "@p",         "info", "parent"}

= {"user", "playerName", "info", "parent"}

However, arguments like the text or location argument can also contain spaces, and may not be located at the end of the command, which could cause the insertion to misalign.

/command <location> <player>
/command 10 10 10 @p

{null, "playerName"}
 V     V
{"10", "10", "10", "@p"}

= {"10", "playerName", "10", "@p"}

So, a more robust system is needed to take cases like this into account.

---

On another note, I haven't tested this, but I believe I have partially solved this issue on the dev/command-build-rewrite branch. It uses the CommandArguments#rawArgs array to solve the alignment issue.

/command <location> <player>
/command 10 10 10 @p

{null,       "playerName"}
 V           V
{"10 10 10", "@p"}

= {"10 10 10", "playerName"}

However, I believe the current implementation there is still flawed, as Command#execute expects all arguments to be split by the spaces. However, this should be easy enough to add as another processing step.

{"10 10 10", "playerName"} -> {"10", "10", "10", "playerName"}