More chat fixes (#1557)

* Fix positional translation arguments not being handled

* Fix locale fallback

* Fix command completion

* Remove the expensive call to `containsKey`

* Unify adventure versions

* Fix some more formatting issues due to parity

* Fix and update tests

* Update adventure

* Add Javadoc for getCommandNames

* Formatting

Co-authored-by: Camotoy <20743703+DoctorMacc@users.noreply.github.com>

bootstrap/bungeecord/src/main/java/org/geysermc/platform/bungeecord/command/GeyserBungeeCommandExecutor.java

@@ -67,7 +67,7 @@ public class GeyserBungeeCommandExecutor extends Command implements TabExecutor
     @Override
     public Iterable<String> onTabComplete(CommandSender sender, String[] args) {
         if (args.length == 1) {
-            return Arrays.asList("?", "help", "reload", "shutdown", "stop");
+            return connector.getCommandManager().getCommandNames();
         }
         return new ArrayList<>();
     }

bootstrap/spigot/src/main/java/org/geysermc/platform/spigot/command/GeyserSpigotCommandExecutor.java

@@ -67,7 +67,7 @@ public class GeyserSpigotCommandExecutor implements TabExecutor {
     @Override
     public List<String> onTabComplete(CommandSender sender, Command command, String label, String[] args) {
         if (args.length == 1) {
-            return Arrays.asList("?", "help", "reload", "shutdown", "stop");
+            return connector.getCommandManager().getCommandNames();
         }
         return new ArrayList<>();
     }

bootstrap/sponge/src/main/java/org/geysermc/platform/sponge/command/GeyserSpongeCommandExecutor.java

@@ -70,7 +70,7 @@ public class GeyserSpongeCommandExecutor implements CommandCallable {
     @Override
     public List<String> getSuggestions(CommandSource source, String arguments, @Nullable Location<World> targetPosition) throws CommandException {
         if (arguments.split(" ").length == 1) {
-            return Arrays.asList("?", "help", "reload", "shutdown", "stop");
+            return connector.getCommandManager().getCommandNames();
         }
         return new ArrayList<>();
     }

bootstrap/velocity/src/main/java/org/geysermc/platform/velocity/command/GeyserVelocityCommandExecutor.java

@@ -25,8 +25,8 @@
 
 package org.geysermc.platform.velocity.command;
 
-import com.velocitypowered.api.command.Command;
 import com.velocitypowered.api.command.CommandSource;
+import com.velocitypowered.api.command.SimpleCommand;
 import lombok.AllArgsConstructor;
 import org.geysermc.connector.GeyserConnector;
 import org.geysermc.connector.command.CommandSender;
@@ -34,29 +34,39 @@ import org.geysermc.connector.command.GeyserCommand;
 import org.geysermc.connector.common.ChatColor;
 import org.geysermc.connector.utils.LanguageUtils;
 
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.List;
 
 @AllArgsConstructor
-public class GeyserVelocityCommandExecutor implements Command {
+public class GeyserVelocityCommandExecutor implements SimpleCommand {
 
     private final GeyserConnector connector;
 
     @Override
-    public void execute(CommandSource source, String[] args) {
+    public void execute(Invocation invocation) {
-        if (args.length > 0) {
+        if (invocation.arguments().length > 0) {
-            if (getCommand(args[0]) != null) {
+            if (getCommand(invocation.arguments()[0]) != null) {
-                if (!source.hasPermission(getCommand(args[0]).getPermission())) {
+                if (!invocation.source().hasPermission(getCommand(invocation.arguments()[0]).getPermission())) {
-                    CommandSender sender = new VelocityCommandSender(source);
+                    CommandSender sender = new VelocityCommandSender(invocation.source());
                     sender.sendMessage(ChatColor.RED + LanguageUtils.getPlayerLocaleString("geyser.bootstrap.command.permission_fail", sender.getLocale()));
                     return;
                 }
-                getCommand(args[0]).execute(new VelocityCommandSender(source), args.length > 1 ? Arrays.copyOfRange(args, 1, args.length) : new String[0]);
+                getCommand(invocation.arguments()[0]).execute(new VelocityCommandSender(invocation.source()), invocation.arguments().length > 1 ? Arrays.copyOfRange(invocation.arguments(), 1, invocation.arguments().length) : new String[0]);
             }
         } else {
-            getCommand("help").execute(new VelocityCommandSender(source), new String[0]);
+            getCommand("help").execute(new VelocityCommandSender(invocation.source()), new String[0]);
         }
     }
 
+    @Override
+    public List<String> suggest(Invocation invocation) {
+        if (invocation.arguments().length == 0) {
+            return connector.getCommandManager().getCommandNames();
+        }
+        return new ArrayList<>();
+    }
+
     private GeyserCommand getCommand(String label) {
         return connector.getCommandManager().getCommands().get(label);
     }

connector/pom.xml

@@ -136,21 +136,21 @@
             <version>2.1.3</version>
         </dependency>
         <dependency>
-            <groupId>net.kyori</groupId>
+            <groupId>com.github.kyoripowered.adventure</groupId>
             <artifactId>adventure-api</artifactId>
-            <version>4.1.1</version>
+            <version>7acd956</version>
             <scope>compile</scope>
         </dependency>
         <dependency>
             <groupId>com.github.kyoripowered.adventure</groupId>
             <artifactId>adventure-text-serializer-gson</artifactId>
-            <version>4d8a67d798</version>
+            <version>7acd956</version>
             <scope>compile</scope>
         </dependency>
         <dependency>
             <groupId>com.github.kyoripowered.adventure</groupId>
             <artifactId>adventure-text-serializer-legacy</artifactId>
-            <version>0599048</version>
+            <version>7acd956</version>
             <scope>compile</scope>
         </dependency>
         <dependency>

connector/src/main/java/org/geysermc/connector/command/CommandManager.java

@@ -31,9 +31,7 @@ import org.geysermc.connector.GeyserConnector;
 import org.geysermc.connector.command.defaults.*;
 import org.geysermc.connector.utils.LanguageUtils;
 
-import java.util.Collections;
+import java.util.*;
-import java.util.HashMap;
-import java.util.Map;
 
 public abstract class CommandManager {
 
@@ -92,6 +90,13 @@ public abstract class CommandManager {
         cmd.execute(sender, args);
     }
 
+    /**
+     * @return a list of all subcommands under {@code /geyser}.
+     */
+    public List<String> getCommandNames() {
+        return Arrays.asList(connector.getCommandManager().getCommands().keySet().toArray(new String[0]));
+    }
+
     /**
      * Returns the description of the given command
      *

connector/src/main/java/org/geysermc/connector/network/translators/chat/MessageTranslator.java

@@ -84,7 +84,17 @@ public class MessageTranslator {
         Locale localeCode = Locale.forLanguageTag(locale.replace('_', '-'));
         component = RENDERER.render(component, localeCode);
 
-        return LegacyComponentSerializer.legacySection().serialize(component);
+        String legacy = LegacyComponentSerializer.legacySection().serialize(component);
+
+        // Strip strikethrough and underline as they are not supported on bedrock
+        legacy = legacy.replaceAll("\u00a7[mn]", "");
+
+        // Make color codes reset formatting like Java
+        // See https://minecraft.gamepedia.com/Formatting_codes#Usage
+        legacy = legacy.replaceAll("\u00a7([0-9a-f])", "\u00a7r\u00a7$1");
+        legacy = legacy.replaceAll("\u00a7r\u00a7r", "\u00a7r");
+
+        return legacy;
     }
 
     public static String convertMessage(String message) {
@@ -106,7 +116,7 @@ public class MessageTranslator {
             String convertedMessage = convertMessage(convertToJavaMessage(message), locale);
 
             // We have to do this since Adventure strips the starting reset character
-            if (message.startsWith(getColor(ChatColor.RESET))) {
+            if (message.startsWith(getColor(ChatColor.RESET)) && !convertedMessage.startsWith(getColor(ChatColor.RESET))) {
                 convertedMessage = getColor(ChatColor.RESET) + convertedMessage;
             }
 

connector/src/main/java/org/geysermc/connector/network/translators/chat/MinecraftTranslationRegistry.java

@@ -61,6 +61,16 @@ public class MinecraftTranslationRegistry implements TranslationRegistry {
         }
         m.appendTail(sb);
 
+        // Replace the `%x$s` with numbered inserts `{x}`
+        p = Pattern.compile("%([0-9]+)\\$s");
+        m = p.matcher(sb.toString());
+        sb = new StringBuffer();
+        while (m.find()) {
+            i = Integer.parseInt(m.group(1)) - 1;
+            m.appendReplacement(sb, "{" + i + "}");
+        }
+        m.appendTail(sb);
+
         return new MessageFormat(sb.toString(), locale);
     }
 

connector/src/main/java/org/geysermc/connector/utils/LanguageUtils.java

@@ -105,7 +105,11 @@ public class LanguageUtils {
         locale = formatLocale(locale);
 
         Properties properties = LOCALE_MAPPINGS.get(locale);
-        String formatString = properties.getProperty(key);
+        String formatString = null;
+
+        if (properties != null) {
+            formatString = properties.getProperty(key);
+        }
 
         // Try and get the key from the default locale
         if (formatString == null) {

connector/src/test/java/org/geysermc/connector/network/translators/chat/MessageTranslatorTest.java

@@ -39,22 +39,31 @@ public class MessageTranslatorTest {
     @Before
     public void setUp() throws Exception {
         messages.put("{\"text\":\"\",\"extra\":[{\"text\":\"DoctorMad9952 joined the game\",\"color\":\"yellow\"}]}",
-                "§eDoctorMad9952 joined the game");
+                "§r§eDoctorMad9952 joined the game");
 
         messages.put("{\"text\":\"\",\"extra\":[\"Plugins (3): \",{\"text\":\"WorldEdit\",\"color\":\"green\"},{\"text\":\", \",\"color\":\"white\"},{\"text\":\"ViaVersion\",\"color\":\"green\"},{\"text\":\", \",\"color\":\"white\"},{\"text\":\"Geyser-Spigot\",\"color\":\"green\"}]}",
-                "Plugins (3): §aWorldEdit§f, §aViaVersion§f, §aGeyser-Spigot");
+                "Plugins (3): §r§aWorldEdit§r§f, §r§aViaVersion§r§f, §r§aGeyser-Spigot");
 
         // RGB downgrade test
         messages.put("{\"extra\":[{\"text\":\"          \"},{\"color\":\"gold\",\"text\":\"The \"},{\"color\":\"#E14248\",\"obfuscated\":true,\"text\":\"||\"},{\"color\":\"#3AA9FF\",\"bold\":true,\"text\":\"CubeCraft\"},{\"color\":\"#E14248\",\"obfuscated\":true,\"text\":\"||\"},{\"color\":\"gold\",\"text\":\" Network \"},{\"color\":\"green\",\"text\":\"[1.8/1.9+]\\n         \"},{\"color\":\"#f5e342\",\"text\":\"✦ \"},{\"color\":\"#b042f5\",\"bold\":true,\"text\":\"N\"},{\"color\":\"#c142f5\",\"bold\":true,\"text\":\"E\"},{\"color\":\"#d342f5\",\"bold\":true,\"text\":\"W\"},{\"color\":\"#e442f5\",\"bold\":true,\"text\":\":\"},{\"color\":\"#f542f5\",\"bold\":true,\"text\":\" \"},{\"color\":\"#bcf542\",\"bold\":true,\"text\":\"A\"},{\"color\":\"#acee3f\",\"bold\":true,\"text\":\"M\"},{\"color\":\"#9ce73c\",\"bold\":true,\"text\":\"O\"},{\"color\":\"#8ce039\",\"bold\":true,\"text\":\"N\"},{\"color\":\"#7cd936\",\"bold\":true,\"text\":\"G\"},{\"color\":\"#6cd233\",\"bold\":true,\"text\":\" \"},{\"color\":\"#5ccb30\",\"bold\":true,\"text\":\"S\"},{\"color\":\"#4cc42d\",\"bold\":true,\"text\":\"L\"},{\"color\":\"#3cbd2a\",\"bold\":true,\"text\":\"I\"},{\"color\":\"#2cb627\",\"bold\":true,\"text\":\"M\"},{\"color\":\"#1caf24\",\"bold\":true,\"text\":\"E\"},{\"color\":\"#0ca821\",\"bold\":true,\"text\":\"S\"},{\"color\":\"#f5e342\",\"text\":\" \"},{\"color\":\"#6d7c87\",\"text\":\"(kinda sus) \"},{\"color\":\"#f5e342\",\"text\":\"✦\"}],\"text\":\"\"}",
-                "          §6The §c§k||§r§3§lCubeCraft§r§c§k||§r§6 Network §a[1.8/1.9+]\n" +
+                "          §r§6The §r§c§k||§r§3§lCubeCraft§r§c§k||§r§6 Network §r§a[1.8/1.9+]\n" +
-                        "         §e✦ §d§lN§r§d§lE§r§d§lW§r§d§l:§r§d§l §r§e§lA§r§e§lM§r§a§lO§r§a§lN§r§a§lG§r§a§l §r§a§lS§r§a§lL§r§2§lI§r§2§lM§r§2§lE§r§2§lS§r§e §8(kinda sus) §e✦");
+                        "         §r§e✦ §r§d§lN§r§d§lE§r§d§lW§r§d§l:§r§d§l §r§e§lA§r§e§lM§r§a§lO§r§a§lN§r§a§lG§r§a§l §r§a§lS§r§a§lL§r§2§lI§r§2§lM§r§2§lE§r§2§lS§r§e §r§8(kinda sus) §r§e✦");
+
+        // Color code format resetting
+        messages.put("{\"text\":\"\",\"extra\":[{\"text\":\"\",\"extra\":[{\"text\":\"[\",\"color\":\"gray\"},{\"text\":\"H\",\"color\":\"yellow\"},{\"text\":\"]\",\"color\":\"gray\"},{\"text\":\" \",\"color\":\"white\"},{\"text\":\"GUEST\",\"color\":\"#b7b7b7\",\"bold\":true}]},{\"text\":\"\",\"extra\":[{\"text\":\" \",\"bold\":true},{\"text\":\"»\",\"color\":\"blue\"},{\"text\":\" \",\"color\":\"gray\"}]},{\"text\":\"\",\"extra\":[{\"text\":\"rtm516\",\"color\":\"white\"},{\"text\":\": \",\"color\":\"gray\"},{\"text\":\"\",\"color\":\"white\"}]},{\"text\":\"\",\"extra\":[{\"text\":\"This is an amazing bedrock test message\",\"color\":\"white\"}]}]}\n",
+                "§r§7[§r§eH§r§7]§r§f §r§7§lGUEST§r§l §r§9»§r§7 §r§frtm516§r§7: §r§fThis is an amazing bedrock test message");
+
+        // Test translation and positional arguments
+        // Disabled due to not having an GeyserConnector instance, hence it fails
+        //messages.put("{\"translate\":\"death.attack.player\",\"with\":[{\"text\":\"rtm516\",\"insertion\":\"rtm516\"},{\"text\":\"*invincible_rt\",\"insertion\":\"*invincible_rt\"}]}",
+        //        "rtm516 was slain by *invincible_rt");
     }
 
     @Test
     public void convertMessage() {
         for (Map.Entry<String, String> entry : messages.entrySet()) {
             String bedrockMessage = MessageTranslator.convertMessage(entry.getKey(), "en_US");
-            Assert.assertEquals("Translation of messages is incorrect", bedrockMessage, entry.getValue());
+            Assert.assertEquals("Translation of messages is incorrect", entry.getValue(), bedrockMessage);
         }
     }
 
@@ -63,5 +72,6 @@ public class MessageTranslatorTest {
         Assert.assertEquals("All newline message is not handled properly", "\n\n\n\n", MessageTranslator.convertMessageLenient("\n\n\n\n"));
         Assert.assertEquals("Empty message is not handled properly", "", MessageTranslator.convertMessageLenient(""));
         Assert.assertEquals("Reset before message is not handled properly", "§r§eGame Selector", MessageTranslator.convertMessageLenient("§r§eGame Selector"));
+        Assert.assertEquals("Unimplemented formatting chars not stripped", "Bold Underline", MessageTranslator.convertMessageLenient("§m§nBold Underline"));
     }
 }