Improve message output for tp command with proper fail reason

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

@@ -1,6 +1,9 @@
 package org.mvplugins.multiverse.core.commands;
 
+import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import co.aikar.commands.annotation.CommandAlias;
 import co.aikar.commands.annotation.CommandCompletion;
@@ -23,7 +26,11 @@
 import org.mvplugins.multiverse.core.destination.DestinationInstance;
 import org.mvplugins.multiverse.core.permissions.CorePermissionsChecker;
 import org.mvplugins.multiverse.core.teleportation.AsyncSafetyTeleporter;
+import org.mvplugins.multiverse.core.teleportation.TeleportFailureReason;
 import org.mvplugins.multiverse.core.utils.MVCorei18n;
+import org.mvplugins.multiverse.core.utils.message.Message;
+
+import static org.mvplugins.multiverse.core.utils.message.MessageReplacement.replace;
 
 @Service
 @CommandAlias("mv")
@@ -70,30 +77,78 @@ void onTeleportCommand(
             String[] flags) {
         ParsedCommandFlags parsedFlags = parseFlags(flags);
 
-        // TODO: Add warning if teleporting too many players at once.
-        String playerName = players.length == 1
-                ? issuer.getPlayer() == players[0] ? "you" : players[0].getName()
-                : players.length + " players";
+        if (players.length == 1) {
+            teleportSinglePlayer(issuer, players[0], destination, parsedFlags);
+        } else {
+            teleportMultiplePlayers(issuer, players, destination, parsedFlags);
+        }
+    }
 
-        // TODO: Multi player permission checking
-        if (!permissionsChecker.checkTeleportPermissions(issuer.getIssuer(), players[0], destination)) {
-            issuer.sendMessage("You do not have teleport permissions");
+    private void teleportSinglePlayer(MVCommandIssuer issuer, Player player, DestinationInstance<?, ?> destination, ParsedCommandFlags parsedFlags) {
+        if (!permissionsChecker.checkTeleportPermissions(issuer.getIssuer(), player, destination)) {
+            issuer.sendMessage(player == issuer.getPlayer()
+                    ? "You do not have permission to teleport yourself!"
+                    : "You do not have permission to teleport other players!");
             return;
         }
 
+        safetyTeleporter.to(destination)
+                .by(issuer)
+                .checkSafety(!parsedFlags.hasFlag(UNSAFE_FLAG) && destination.checkTeleportSafety())
+                .teleport(player)
+                .onSuccess(() -> issuer.sendInfo(MVCorei18n.TELEPORT_SUCCESS,
+                        replace("{player}").with(getYouOrName(issuer, player)),
+                        replace("{destination}").with(destination.toString())))
+                .onFailure(failure -> issuer.sendError(MVCorei18n.TELEPORT_FAILED,
+                        replace("{player}").with(getYouOrName(issuer, player)),
+                        replace("{destination}").with(destination.toString()),
+                        replace("{reason}").with(failure.getFailureMessage())));
+    }
+
+    private String getYouOrName(MVCommandIssuer issuer, Player player) {
+        return player == issuer.getPlayer() ? "you" : player.getName();
+    }
+
+    private void teleportMultiplePlayers(MVCommandIssuer issuer, Player[] players, DestinationInstance<?, ?> destination, ParsedCommandFlags parsedFlags) {
+        var selfPlayer = Arrays.stream(players).filter(p -> p == issuer.getPlayer()).findFirst();
+        var otherPlayer = Arrays.stream(players).filter(p -> p != issuer.getPlayer()).findFirst();
+        if (selfPlayer.isPresent() && !permissionsChecker.checkTeleportPermissions(issuer.getIssuer(), selfPlayer.get(), destination)) {
+            issuer.sendMessage("You do not have permission to teleport yourself!");
+            return;
+        }
+        if (otherPlayer.isPresent() && !permissionsChecker.checkTeleportPermissions(issuer.getIssuer(), otherPlayer.get(), destination)) {
+            issuer.sendMessage("You do not have permission to teleport other players!");
+            return;
+        }
         safetyTeleporter.to(destination)
                 .by(issuer)
                 .checkSafety(!parsedFlags.hasFlag(UNSAFE_FLAG) && destination.checkTeleportSafety())
                 .teleport(List.of(players))
                 .thenAccept(attempts -> {
-                    //todo: Check for attempt results
-                    Logging.fine("Async teleport completed: %s", attempts);
-                    issuer.sendInfo(MVCorei18n.TELEPORT_SUCCESS,
-                            "{player}", playerName, "{destination}", destination.toString());
-                })
-                .exceptionally(throwable -> {
-                    Logging.severe("Error while teleporting %s to %s: %s",
-                            playerName, destination, throwable.getMessage());
+                    int successCount = 0;
+                    Map<TeleportFailureReason, Integer> failures = new HashMap<>();
+                    for (var attempt : attempts) {
+                        if (attempt.isSuccess()) {
+                            successCount++;
+                        } else {
+                            failures.compute(attempt.getFailureReason(), (reason, count) -> count == null ? 1 : count + 1);
+                        }
+                    }
+                    if (successCount > 0) {
+                        Logging.finer("Teleported %s players to %s", successCount, destination);
+                        issuer.sendInfo(MVCorei18n.TELEPORT_SUCCESS,
+                                replace("{player}").with(successCount + " players"),
+                                replace("{destination}").with(destination.toString()));
+                    }
+                    if (!failures.isEmpty()) {
+                        for (var entry : failures.entrySet()) {
+                            Logging.finer("Failed to teleport %s players to %s: %s", entry.getValue(), destination, entry.getKey());
+                            issuer.sendError(MVCorei18n.TELEPORT_FAILED,
+                                    replace("{player}").with(entry.getValue() + " players"),
+                                    replace("{destination}").with(destination.toString()),
+                                    replace("{reason}").with(Message.of(entry.getKey(), "")));
+                        }
+                    }
                 });
     }
 }

src/main/java/org/mvplugins/multiverse/core/teleportation/TeleportFailureReason.java

@@ -1,13 +1,28 @@
 package org.mvplugins.multiverse.core.teleportation;
 
+import co.aikar.locales.MessageKey;
+import co.aikar.locales.MessageKeyProvider;
+import org.mvplugins.multiverse.core.utils.MVCorei18n;
 import org.mvplugins.multiverse.core.utils.result.FailureReason;
 
 public enum TeleportFailureReason implements FailureReason {
-    NULL_DESTINATION,
-    NULL_LOCATION,
-    UNSAFE_LOCATION,
-    TELEPORT_FAILED,
-    TELEPORT_FAILED_EXCEPTION,
-    PLAYER_OFFLINE,
-    EVENT_CANCELLED,
+    NULL_DESTINATION(MVCorei18n.TELEPORTFAILUREREASON_NULL_DESTINATION),
+    NULL_LOCATION(MVCorei18n.TELEPORTFAILUREREASON_NULL_LOCATION),
+    UNSAFE_LOCATION(MVCorei18n.TELEPORTFAILUREREASON_UNSAFE_LOCATION),
+    TELEPORT_FAILED(MVCorei18n.TELEPORTFAILUREREASON_TELEPORT_FAILED),
+    TELEPORT_FAILED_EXCEPTION(MVCorei18n.TELEPORTFAILUREREASON_TELEPORT_FAILED_EXCEPTION),
+    EVENT_CANCELLED(MVCorei18n.TELEPORTFAILUREREASON_EVENT_CANCELLED),
+    ;
+
+    private final MessageKeyProvider messageKey;
+
+    TeleportFailureReason(MessageKeyProvider message) {
+        this.messageKey = message;
+    }
+
+
+    @Override
+    public MessageKey getMessageKey() {
+        return messageKey.getMessageKey();
+    }
 }

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

@@ -107,6 +107,7 @@ public enum MVCorei18n implements MessageKeyProvider {
 
     // teleport command
     TELEPORT_SUCCESS,
+    TELEPORT_FAILED,
 
     // unload command
     UNLOAD_UNLOADING,
@@ -139,6 +140,14 @@ public enum MVCorei18n implements MessageKeyProvider {
     ENTRYCHECK_EXCEEDPLAYERLIMIT,
     ENTRYCHECK_NOWORLDACCESS,
 
+    // teleport failure reasons
+    TELEPORTFAILUREREASON_NULL_DESTINATION,
+    TELEPORTFAILUREREASON_NULL_LOCATION,
+    TELEPORTFAILUREREASON_UNSAFE_LOCATION,
+    TELEPORTFAILUREREASON_TELEPORT_FAILED,
+    TELEPORTFAILUREREASON_TELEPORT_FAILED_EXCEPTION,
+    TELEPORTFAILUREREASON_EVENT_CANCELLED,
+
     // world manager result
     CLONEWORLD_INVALIDWORLDNAME,
     CLONEWORLD_WORLDEXISTFOLDER,

src/main/resources/multiverse-core_en.properties

@@ -145,6 +145,7 @@ mv-core.teleport.description=Allows you to teleport to a location on your server
 mv-core.teleport.player.description=Target player to teleport.
 mv-core.teleport.destination.description=Location, can be a world name.
 mv-core.teleport.success=Teleported {player} to {destination}.
+mv-core.teleport.failed=Failed to teleport {player} to {destination}. {reason}
 
 # /mv unload
 mv-core.unload.description=Unloads a world from Multiverse. This does NOT remove the world folder. This does NOT remove it from the config file.
@@ -180,6 +181,14 @@ mv-core.entrycheck.cannotpayentryfee=you do not have the ability to pay entry fe
 mv-core.entrycheck.exceedplayerlimit=the world has reached its player limit.
 mv-core.entrycheck.noworldaccess=you do not have permissions to access the world.
 
+# teleport failure reason
+mv-core.teleportfailurereason.null.destination=Destination is invalid!
+mv-core.teleportfailurereason.null.location=Location is invalid!
+mv-core.teleportfailurereason.unsafe.location=Location is unsafe! Use `--unsafe` to override.
+mv-core.teleportfailurereason.teleport.failed=Something rejected the teleport.
+mv-core.teleportfailurereason.teleport.failed.exception=An error occurred during the teleport. See console for more details.
+mv-core.teleportfailurereason.event.cancelled=The teleport was cancelled another plugin.
+
 # world manager result
 mv-core.cloneworld.invalidworldname=World '{world}' contains invalid characters!
 mv-core.cloneworld.worldexistfolder=World '{world}' exists in server folders! You need to delete it first before cloning.

src/test/java/org/mvplugins/multiverse/core/commands/TeleportCommandTest.kt

@@ -30,6 +30,14 @@ class TeleportCommandTest : AbstractCommandTest() {
         assertLocation(server.getWorld("otherworld")?.spawnLocation, server.getPlayer("Player2")?.location)
     }
 
+    @Test
+    fun `Teleport multiple players to invalid world`() {
+        assertTrue(Bukkit.dispatchCommand(console, "mv tp Player1,Player2 invalidworld"))
+        Thread.sleep(100) // wait for the player to teleport asynchronously
+        assertLocation(server.getWorld("world")?.spawnLocation, server.getPlayer("Player1")?.location)
+        assertLocation(server.getWorld("world")?.spawnLocation, server.getPlayer("Player2")?.location)
+    }
+
     @Test
     fun `Player no permission to teleport`() {
         player.performCommand("mv tp otherworld")