Don't auto-create any brig redirects (#11954)

Author: Machine-Maker

paper-api/src/main/java/io/papermc/paper/command/brigadier/CommandRegistrationFlag.java

@@ -10,5 +10,10 @@
  */
 @ApiStatus.Internal
 public enum CommandRegistrationFlag {
+
+    /**
+     * @deprecated This is the default behavior now.
+     */
+    @Deprecated(since = "1.21.4")
     FLATTEN_ALIASES
 }

paper-api/src/main/java/io/papermc/paper/command/brigadier/Commands.java

@@ -113,6 +113,7 @@ static <T> RequiredArgumentBuilder<CommandSourceStack, T> argument(final String
      * <p>Commands have certain overriding behavior:
      * <ul>
      *   <li>Aliases will not override already existing commands (excluding namespaced ones)</li>
+     *   <li>Aliases are <b>not</b> Brigadier redirects, they just copy the command to a different label</li>
      *   <li>The main command/namespaced label will override already existing commands</li>
      * </ul>
      *
@@ -129,6 +130,7 @@ static <T> RequiredArgumentBuilder<CommandSourceStack, T> argument(final String
      * <p>Commands have certain overriding behavior:
      * <ul>
      *   <li>Aliases will not override already existing commands (excluding namespaced ones)</li>
+     *   <li>Aliases are <b>not</b> Brigadier redirects, they just copy the command to a different label</li>
      *   <li>The main command/namespaced label will override already existing commands</li>
      * </ul>
      *
@@ -146,6 +148,7 @@ static <T> RequiredArgumentBuilder<CommandSourceStack, T> argument(final String
      * <p>Commands have certain overriding behavior:
      * <ul>
      *   <li>Aliases will not override already existing commands (excluding namespaced ones)</li>
+     *   <li>Aliases are <b>not</b> Brigadier redirects, they just copy the command to a different label</li>
      *   <li>The main command/namespaced label will override already existing commands</li>
      * </ul>
      *
@@ -163,6 +166,7 @@ static <T> RequiredArgumentBuilder<CommandSourceStack, T> argument(final String
      * <p>Commands have certain overriding behavior:
      * <ul>
      *   <li>Aliases will not override already existing commands (excluding namespaced ones)</li>
+     *   <li>Aliases are <b>not</b> Brigadier redirects, they just copy the command to a different label</li>
      *   <li>The main command/namespaced label will override already existing commands</li>
      * </ul>
      *
@@ -179,6 +183,7 @@ static <T> RequiredArgumentBuilder<CommandSourceStack, T> argument(final String
      * <p>Commands have certain overriding behavior:
      * <ul>
      *   <li>Aliases will not override already existing commands (excluding namespaced ones)</li>
+     *   <li>Aliases are <b>not</b> Brigadier redirects, they just copy the command to a different label</li>
      *   <li>The main command/namespaced label will override already existing commands</li>
      * </ul>
      *

paper-server/patches/sources/com/mojang/brigadier/tree/CommandNode.java.patch

@@ -1,6 +1,6 @@
 --- a/com/mojang/brigadier/tree/CommandNode.java
 +++ b/com/mojang/brigadier/tree/CommandNode.java
-@@ -27,11 +_,21 @@
+@@ -27,11 +_,22 @@
      private final Map<String, CommandNode<S>> children = new LinkedHashMap<>();
      private final Map<String, LiteralCommandNode<S>> literals = new LinkedHashMap<>();
      private final Map<String, ArgumentCommandNode<S, ?>> arguments = new LinkedHashMap<>();
@@ -13,6 +13,7 @@
 +    public CommandNode<net.minecraft.commands.CommandSourceStack> clientNode; // Paper - Brigadier API
 +    public CommandNode<io.papermc.paper.command.brigadier.CommandSourceStack> unwrappedCached = null; // Paper - Brigadier Command API
 +    public CommandNode<io.papermc.paper.command.brigadier.CommandSourceStack> wrappedCached = null; // Paper - Brigadier Command API
++    public io.papermc.paper.command.brigadier.PluginCommandMeta pluginCommandMeta; // Paper - Brigadier Command API
 +    // CraftBukkit start
 +    public void removeCommand(String name) {
 +        this.children.remove(name);

paper-server/patches/sources/net/minecraft/commands/Commands.java.patch

@@ -1,6 +1,6 @@
 --- a/net/minecraft/commands/Commands.java
 +++ b/net/minecraft/commands/Commands.java
-@@ -251,6 +_,30 @@
+@@ -251,6 +_,24 @@
              PublishCommand.register(this.dispatcher);
          }
 
@@ -14,17 +14,11 @@
 +        // Paper start - Brigadier Command API
 +        // Create legacy minecraft namespace commands
 +        for (final CommandNode<CommandSourceStack> node : new java.util.ArrayList<>(this.dispatcher.getRoot().getChildren())) {
-+            // The brigadier dispatcher is not able to resolve nested redirects.
-+            // E.g. registering the alias minecraft:tp cannot redirect to tp, as tp itself redirects to teleport.
-+            // Instead, target the first none redirecting node.
-+            CommandNode<CommandSourceStack> flattenedAliasTarget = node;
-+            while (flattenedAliasTarget.getRedirect() != null) flattenedAliasTarget = flattenedAliasTarget.getRedirect();
-+
-+            this.dispatcher.register(
-+                com.mojang.brigadier.builder.LiteralArgumentBuilder.<CommandSourceStack>literal("minecraft:" + node.getName())
-+                    .executes(flattenedAliasTarget.getCommand())
-+                    .requires(flattenedAliasTarget.getRequirement())
-+                    .redirect(flattenedAliasTarget)
++            this.dispatcher.getRoot().addChild(
++                io.papermc.paper.command.brigadier.PaperBrigadier.copyLiteral(
++                    "minecraft:" + node.getName(),
++                    (com.mojang.brigadier.tree.LiteralCommandNode<CommandSourceStack>) node
++                )
 +            );
 +        }
 +        // Paper end - Brigadier Command API
@@ -150,11 +144,10 @@
              }
 
              return null;
-@@ -360,25 +_,130 @@
+@@ -360,26 +_,85 @@
      }
 
      public void sendCommands(ServerPlayer player) {
--        Map<CommandNode<CommandSourceStack>, CommandNode<SharedSuggestionProvider>> map = Maps.newHashMap();
 +        // Paper start - Send empty commands if tab completion is disabled
 +        if (org.spigotmc.SpigotConfig.tabComplete < 0) {
 +            player.connection.send(new ClientboundCommandsPacket(new RootCommandNode<>()));
@@ -182,7 +175,7 @@
 +
 +    private void sendAsync(ServerPlayer player, java.util.Collection<CommandNode<CommandSourceStack>> dispatcherRootChildren) {
 +        // Paper end - Perf: Async command map building
-+        Map<CommandNode<CommandSourceStack>, CommandNode<SharedSuggestionProvider>> map = Maps.newIdentityHashMap(); // Use identity to prevent aliasing issues
+         Map<CommandNode<CommandSourceStack>, CommandNode<SharedSuggestionProvider>> map = Maps.newHashMap();
          RootCommandNode<SharedSuggestionProvider> rootCommandNode = new RootCommandNode<>();
          map.put(this.dispatcher.getRoot(), rootCommandNode);
 -        this.fillUsableCommands(this.dispatcher.getRoot(), rootCommandNode, player.createCommandSourceStack(), map);
@@ -224,7 +217,6 @@
          Map<CommandNode<CommandSourceStack>, CommandNode<SharedSuggestionProvider>> commandNodeToSuggestionNode
      ) {
 -        for (CommandNode<CommandSourceStack> commandNode : rootCommandSource.getChildren()) {
-+        commandNodeToSuggestionNode.keySet().removeIf((node) -> !org.spigotmc.SpigotConfig.sendNamespaced && node.getName().contains(":")); // Paper - Remove namedspaced from result nodes to prevent redirect trimming ~ see comment below
 +        for (CommandNode<CommandSourceStack> commandNode : children) { // Paper - Perf: Async command map building; pass copy of children
 +            // Paper start - Brigadier API
 +            if (commandNode.clientNode != null) {
@@ -234,58 +226,16 @@
 +            if (!org.spigotmc.SpigotConfig.sendNamespaced && commandNode.getName().contains(":")) continue; // Spigot
              if (commandNode.canUse(source)) {
                  ArgumentBuilder<SharedSuggestionProvider, ?> argumentBuilder = (ArgumentBuilder) commandNode.createBuilder();
-+                // Paper start
-+                /*
-+                Because of how commands can be yeeted right left and center due to bad bukkit practices
-+                we need to be able to ensure that ALL commands are registered (even redirects).
-+
-+                What this will do is IF the redirect seems to be "dead" it will create a builder and essentially populate (flatten)
-+                all the children from the dead redirect to the node.
-+
-+                So, if minecraft:msg redirects to msg but the original msg node has been overriden minecraft:msg will now act as msg and will explicilty inherit its children.
-+
-+                The only way to fix this is to either:
-+                - Send EVERYTHING flattened, don't use redirects
-+                - Don't allow command nodes to be deleted
-+                - Do this :)
-+                 */
-+
-+                // Is there an invalid command redirect?
-+                if (argumentBuilder.getRedirect() != null && commandNodeToSuggestionNode.get(argumentBuilder.getRedirect()) == null) {
-+                    // Create the argument builder with the same values as the specified node, but with a different literal and populated children
-+
-+                    CommandNode<SharedSuggestionProvider> redirect = argumentBuilder.getRedirect();
-+                    // Diff copied from LiteralCommand#createBuilder
-+                    final com.mojang.brigadier.builder.LiteralArgumentBuilder<SharedSuggestionProvider> builder = com.mojang.brigadier.builder.LiteralArgumentBuilder.literal(commandNode.getName());
-+                    builder.requires(redirect.getRequirement());
-+                    // builder.forward(redirect.getRedirect(), redirect.getRedirectModifier(), redirect.isFork()); We don't want to migrate the forward, since it's invalid.
-+                    if (redirect.getCommand() != null) {
-+                        builder.executes(redirect.getCommand());
-+                    }
-+                    // Diff copied from LiteralCommand#createBuilder
-+                    for (CommandNode<SharedSuggestionProvider> child : redirect.getChildren()) {
-+                        builder.then(child);
-+                    }
-+
-+                    argumentBuilder = builder;
-+                }
-+                // Paper end
                  argumentBuilder.requires(suggestions -> true);
-                 if (argumentBuilder.getCommand() != null) {
+-                if (argumentBuilder.getCommand() != null) {
 -                    argumentBuilder.executes(commandContext -> 0);
-+                    // Paper start - fix suggestions due to falsely equal nodes
-+                    // Always create a new instance
-+                    //noinspection Convert2Lambda
-+                    argumentBuilder.executes(new com.mojang.brigadier.Command<>() {
-+                        @Override
-+                        public int run(com.mojang.brigadier.context.CommandContext<SharedSuggestionProvider> commandContext) {
-+                            return 0;
-+                        }
-+                    });
-+                    // Paper end - fix suggestions due to falsely equal nodes
-                 }
+-                }
++                // Paper - don't replace Command instance on suggestion node
++                // we want the exact command instance to be used for equality checks
++                // when assigning serialization ids to each command node
 
                  if (argumentBuilder instanceof RequiredArgumentBuilder) {
+                     RequiredArgumentBuilder<SharedSuggestionProvider, ?> requiredArgumentBuilder = (RequiredArgumentBuilder<SharedSuggestionProvider, ?>)argumentBuilder;
 @@ -396,7 +_,7 @@
                  commandNodeToSuggestionNode.put(commandNode, commandNode1);
                  rootSuggestion.addChild(commandNode1);

paper-server/src/main/java/io/papermc/paper/command/brigadier/ApiMirrorRootNode.java

@@ -124,7 +124,7 @@ public static void validatePrimitiveType(ArgumentType<?> type) {
                 }
 
                 converted = this.unwrapArgumentWrapper(pureArgumentNode, customArgumentType, customArgumentType.getNativeType(), suggestionProvider);
-            } else if (pureArgumentType instanceof final VanillaArgumentProviderImpl.NativeWrapperArgumentType<?,?> nativeWrapperArgumentType) {
+            } else if (pureArgumentType instanceof final VanillaArgumentProviderImpl.NativeWrapperArgumentType<?, ?> nativeWrapperArgumentType) {
                 converted = this.unwrapArgumentWrapper(pureArgumentNode, nativeWrapperArgumentType, nativeWrapperArgumentType, null); // "null" for suggestion provider so it uses the argument type's suggestion provider
 
             /*
@@ -140,6 +140,8 @@ public static void validatePrimitiveType(ArgumentType<?> type) {
                 // Unknown argument type was passed
                 throw new IllegalArgumentException("Custom unknown argument type was passed, should be wrapped inside an CustomArgumentType.");
             }
+        } else if (pureNode == this) {
+            return (CommandNode) this.getDispatcher().getRoot();
         } else {
             throw new IllegalArgumentException("Unknown command node passed. Don't know how to unwrap this.");
         }

paper-server/src/main/java/io/papermc/paper/command/brigadier/PaperBrigadier.java

@@ -1,24 +1,19 @@
 package io.papermc.paper.command.brigadier;
 
 import com.mojang.brigadier.CommandDispatcher;
+import com.mojang.brigadier.builder.LiteralArgumentBuilder;
 import com.mojang.brigadier.tree.CommandNode;
 import com.mojang.brigadier.tree.LiteralCommandNode;
-import io.papermc.paper.command.brigadier.bukkit.BukkitBrigForwardingMap;
 import io.papermc.paper.command.brigadier.bukkit.BukkitCommandNode;
+import java.util.Map;
 import net.minecraft.commands.CommandSource;
 import net.minecraft.commands.Commands;
 import net.minecraft.network.chat.CommonComponents;
-import net.minecraft.server.MinecraftServer;
 import net.minecraft.world.phys.Vec2;
 import net.minecraft.world.phys.Vec3;
-import org.bukkit.Bukkit;
-import org.bukkit.Server;
 import org.bukkit.command.Command;
-import org.bukkit.command.CommandMap;
 import org.bukkit.craftbukkit.command.VanillaCommandWrapper;
 
-import java.util.Map;
-
 public final class PaperBrigadier {
 
     @SuppressWarnings("DataFlowIssue")
@@ -40,7 +35,8 @@ public static Command wrapNode(CommandNode node) {
             throw new IllegalArgumentException("Unsure how to wrap a " + node);
         }
 
-        if (!(node instanceof PluginCommandNode pluginCommandNode)) {
+        final PluginCommandMeta meta;
+        if ((meta = node.pluginCommandMeta) == null) {
             return new VanillaCommandWrapper(null, node);
         }
         CommandNode<CommandSourceStack> argumentCommandNode = node;
@@ -49,8 +45,8 @@ public static Command wrapNode(CommandNode node) {
         }
 
         Map<CommandNode<CommandSourceStack>, String> map = PaperCommands.INSTANCE.getDispatcherInternal().getSmartUsage(argumentCommandNode, DUMMY);
-        String usage = map.isEmpty() ? pluginCommandNode.getUsageText() :  pluginCommandNode.getUsageText() + " " + String.join("\n" + pluginCommandNode.getUsageText() + " ", map.values());
-        return new PluginVanillaCommandWrapper(pluginCommandNode.getName(), pluginCommandNode.getDescription(), usage, pluginCommandNode.getAliases(), node, pluginCommandNode.getPlugin());
+        String usage = map.isEmpty() ? node.getUsageText() :  node.getUsageText() + " " + String.join("\n" + node.getUsageText() + " ", map.values());
+        return new PluginVanillaCommandWrapper(node.getName(), meta.description(), usage, meta.aliases(), node, meta.plugin());
     }
 
     /*
@@ -70,4 +66,19 @@ public static void moveBukkitCommands(Commands before, Commands after) {
             }
         }
     }
+
+    public static <S> LiteralCommandNode<S> copyLiteral(final String newLiteral, final LiteralCommandNode<S> source) {
+        // logic copied from LiteralCommandNode#createBuilder
+        final LiteralArgumentBuilder<S> copyBuilder = LiteralArgumentBuilder.<S>literal(newLiteral)
+            .requires(source.getRequirement())
+            .forward(source.getRedirect(), source.getRedirectModifier(), source.isFork());
+        if (source.getCommand() != null) {
+            copyBuilder.executes(source.getCommand());
+        }
+
+        for (final CommandNode<S> child : source.getChildren()) {
+            copyBuilder.then(child);
+        }
+        return copyBuilder.build();
+    }
 }