Merge pull request #3380 from Multiverse/fix/issuer-aware-argument

Better error message for teleport command when no players match selectors

Author: benwoo1110

src/main/java/org/mvplugins/multiverse/core/command/MVCommandContexts.java

@@ -278,7 +278,12 @@ private IssuerAwareContextBuilder<Player[]> playerArrayContextBuilder() {
                 })
                 .issuerOnlyFailMessage((context) -> Message.of("This command can only be used by a player."))
                 .issuerAwareInputFailMessage((context, input) -> Message.of("Invalid player: " + input + ". Either specify an online player or use this command as a player."))
-                .inputOnlyFailMessage((context, input) -> Message.of("Player " + input + " not found."));
+                .inputOnlyFailMessage((context, input) -> {
+                    if (PlayerFinder.isSelector(input)) {
+                        return Message.of("No player(s) matched selector: " + input + ".");
+                    }
+                    return Message.of("Player(s) " + input + " not found.");
+                });
     }
 
     private PlayerLocation parsePlayerLocation(BukkitCommandExecutionContext context) {

src/main/java/org/mvplugins/multiverse/core/command/context/issueraware/IssuerAwareContextBuilder.java

@@ -123,48 +123,8 @@ public IssuerAwareContextBuilder<T> inputOnlyFailMessage(BiFunction<BukkitComman
      */
     @ApiStatus.AvailableSince("5.1")
     public IssuerAwareContextResolver<T, BukkitCommandExecutionContext> generateContext() {
-        Objects.requireNonNull(fromPlayer);
-        Objects.requireNonNull(fromInput);
-        Objects.requireNonNull(issuerOnlyFailMessage);
-        Objects.requireNonNull(issuerAwarePlayerFailMessage);
-        Objects.requireNonNull(issuerAwareInputFailMessage);
-        Objects.requireNonNull(inputOnlyFailMessage);
-
-        return context -> {
-            BukkitCommandIssuer issuer = context.getIssuer();
-            String resolve = context.getFlagValue("resolve", "");
-
-            if (resolve.equals("issuerOnly")) {
-                if (issuer.isPlayer()) {
-                    T result = fromPlayer.apply(context, issuer.getPlayer());
-                    if (result != null) {
-                        return result;
-                    }
-                }
-                throw new InvalidCommandArgument(issuerOnlyFailMessage.apply(context).formatted(issuer));
-            }
-
-            String input = context.getFirstArg();
-            T result = fromInput.apply(context, input);
-            if (result != null) {
-                context.popFirstArg();
-                return result;
-            }
-
-            if (resolve.equals("issuerAware")) {
-                if (issuer.isPlayer()) {
-                    Player player = issuer.getPlayer();
-                    result = fromPlayer.apply(context, player);
-                    if (result != null) {
-                        return result;
-                    }
-                    throw new InvalidCommandArgument(issuerAwarePlayerFailMessage.apply(context, player).formatted(issuer));
-                }
-                throw new InvalidCommandArgument(issuerAwareInputFailMessage.apply(context, input).formatted(issuer));
-            }
-
-            throw new InvalidCommandArgument(inputOnlyFailMessage.apply(context, input).formatted(issuer));
-        };
+        validateRequiredVariables();
+        return context -> resolveValue(context, GenericIssuerAwareValue::new).value;
     }
 
     /**
@@ -178,49 +138,66 @@ public IssuerAwareContextResolver<T, BukkitCommandExecutionContext> generateCont
      */
     @ApiStatus.AvailableSince("5.1")
     public <I extends IssuerAwareValue> IssuerAwareContextResolver<I, BukkitCommandExecutionContext> generateContext(BiFunction<Boolean, T, I> createValue) {
-        // todo: This is a copy and paste from above
+        validateRequiredVariables();
+        return context -> resolveValue(context, createValue);
+    }
 
+    private void validateRequiredVariables() {
         Objects.requireNonNull(fromPlayer);
         Objects.requireNonNull(fromInput);
         Objects.requireNonNull(issuerOnlyFailMessage);
         Objects.requireNonNull(issuerAwarePlayerFailMessage);
         Objects.requireNonNull(issuerAwareInputFailMessage);
         Objects.requireNonNull(inputOnlyFailMessage);
+    }
 
-        return context -> {
-            BukkitCommandIssuer issuer = context.getIssuer();
-            String resolve = context.getFlagValue("resolve", "");
+    private <I extends IssuerAwareValue> I resolveValue(BukkitCommandExecutionContext context, BiFunction<Boolean, T, I> createValue) {
+        BukkitCommandIssuer issuer = context.getIssuer();
+        String resolve = context.getFlagValue("resolve", "");
 
-            if (resolve.equals("issuerOnly")) {
-                if (issuer.isPlayer()) {
-                    T result = fromPlayer.apply(context, issuer.getPlayer());
-                    if (result != null) {
-                        return createValue.apply(true, result);
-                    }
+        if (resolve.equals("issuerOnly")) {
+            if (issuer.isPlayer()) {
+                T result = fromPlayer.apply(context, issuer.getPlayer());
+                if (result != null) {
+                    return createValue.apply(true, result);
                 }
-                throw new InvalidCommandArgument(issuerOnlyFailMessage.apply(context).formatted(issuer));
             }
-
-            String input = context.getFirstArg();
-            T result = fromInput.apply(context, input);
-            if (result != null) {
-                context.popFirstArg();
-                return createValue.apply(false, result);
-            }
-
-            if (resolve.equals("issuerAware")) {
-                if (issuer.isPlayer()) {
-                    Player player = issuer.getPlayer();
-                    result = fromPlayer.apply(context, player);
-                    if (result != null) {
-                        return createValue.apply(true, result);
-                    }
-                    throw new InvalidCommandArgument(issuerAwarePlayerFailMessage.apply(context, player).formatted(issuer));
+            throw new InvalidCommandArgument(issuerOnlyFailMessage.apply(context).formatted(issuer));
+        }
+
+        String input = context.getFirstArg();
+        T result = fromInput.apply(context, input);
+        if (result != null) {
+            context.popFirstArg();
+            return createValue.apply(false, result);
+        }
+
+        int maxArgForAware = context.getFlagValue("maxArgForAware", Integer.MAX_VALUE);
+        long argLengthWithoutFlags = context.getArgs().stream()
+                .takeWhile(value -> !value.startsWith("--") && !value.isEmpty())
+                .count();
+
+        if (resolve.equals("issuerAware") && argLengthWithoutFlags <= maxArgForAware) {
+            if (issuer.isPlayer()) {
+                Player player = issuer.getPlayer();
+                result = fromPlayer.apply(context, player);
+                if (result != null) {
+                    return createValue.apply(true, result);
                 }
-                throw new InvalidCommandArgument(issuerAwareInputFailMessage.apply(context, input).formatted(issuer));
+                throw new InvalidCommandArgument(issuerAwarePlayerFailMessage.apply(context, player).formatted(issuer));
             }
+            throw new InvalidCommandArgument(issuerAwareInputFailMessage.apply(context, input).formatted(issuer));
+        }
+
+        throw new InvalidCommandArgument(inputOnlyFailMessage.apply(context, input).formatted(issuer));
+    }
+
+    private static class GenericIssuerAwareValue<T> extends IssuerAwareValue {
+        private final T value;
 
-            throw new InvalidCommandArgument(inputOnlyFailMessage.apply(context, input).formatted(issuer));
-        };
+        public GenericIssuerAwareValue(boolean byIssuer, T value) {
+            super(byIssuer);
+            this.value = value;
+        }
     }
 }

src/main/java/org/mvplugins/multiverse/core/commands/TeleportCommand.java

@@ -64,7 +64,7 @@ final class TeleportCommand extends CoreCommand {
     void onTeleportCommand(
             MVCommandIssuer issuer,
 
-            @co.aikar.commands.annotation.Flags("resolve=issuerAware")
+            @co.aikar.commands.annotation.Flags("resolve=issuerAware,maxArgForAware=1")
             @Syntax("[player]")
             @Description("{@@mv-core.teleport.player.description}")
             PlayerArrayValue playersValue,

src/main/java/org/mvplugins/multiverse/core/utils/PlayerFinder.java

@@ -3,14 +3,14 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.UUID;
-import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import com.dumptruckman.minecraft.util.Logging;
 import com.google.common.base.Strings;
 import org.bukkit.Bukkit;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
+import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -112,7 +112,16 @@ public final class PlayerFinder {
         return playerResults;
     }
 
-    private static boolean isSelector(@NotNull String playerIdentifier) {
+    /**
+     * Check if the player identifier is a selector.
+     *
+     * @param playerIdentifier  An identifier of name, UUID or selector.
+     * @return True if the identifier is a selector, else false.
+     *
+     * @since 5.4
+     */
+    @ApiStatus.AvailableSince("5.4")
+    public static boolean isSelector(@NotNull String playerIdentifier) {
         return VANILLA_SELECTORS.stream().anyMatch(playerIdentifier::startsWith);
     }