[1.18] Fix command redirects by replacing Commands#fillUsableCommands (#8616)

Commands#fillUsableCommands does not handle redirect nodes properly. It
blindly pulls from the cached node map for a redirect node, which may
have no result because the target node of the redirect has not been
visited yet.

CommandHelper#mergeCommandNode is better in this regard, because it
visits the target node of the redirect instead of blindly pulling from
the node map.

Fixes #7551

src/main/java/net/minecraftforge/client/ClientCommandHandler.java

@@ -5,18 +5,13 @@
 
 package net.minecraftforge.client;
 
-import com.mojang.brigadier.Command;
 import com.mojang.brigadier.CommandDispatcher;
 import com.mojang.brigadier.ParseResults;
 import com.mojang.brigadier.StringReader;
 import com.mojang.brigadier.builder.ArgumentBuilder;
-import com.mojang.brigadier.builder.LiteralArgumentBuilder;
-import com.mojang.brigadier.builder.RequiredArgumentBuilder;
 import com.mojang.brigadier.exceptions.CommandSyntaxException;
 import com.mojang.brigadier.suggestion.SuggestionProvider;
-import com.mojang.brigadier.tree.ArgumentCommandNode;
 import com.mojang.brigadier.tree.CommandNode;
-import com.mojang.brigadier.tree.LiteralCommandNode;
 import com.mojang.brigadier.tree.RootCommandNode;
 import net.minecraft.ChatFormatting;
 import net.minecraft.Util;
@@ -36,12 +31,12 @@ import net.minecraft.network.chat.TranslatableComponent;
 import net.minecraftforge.client.event.ClientPlayerNetworkEvent;
 import net.minecraftforge.client.event.RegisterClientCommandsEvent;
 import net.minecraftforge.common.MinecraftForge;
+import net.minecraftforge.server.command.CommandHelper;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
 import java.util.IdentityHashMap;
 import java.util.Map;
-import java.util.function.Function;
 
 public class ClientCommandHandler
 {
@@ -66,7 +61,7 @@ public class ClientCommandHandler
 
     /*
      * For internal use
-     * 
+     *
      * Merges command dispatcher use for suggestions to the command dispatcher used for client commands so they can be sent to the server, and vice versa so client commands appear
      * with server commands in suggestions
      */
@@ -86,11 +81,11 @@ public class ClientCommandHandler
 
         // Copies the server side commands into a temporary server side commands root node to be used later without the client commands
         RootCommandNode<SharedSuggestionProvider> serverCommandsCopy = new RootCommandNode<>();
-        mergeCommandNode(newServerCommands.getRoot(), serverCommandsCopy, new IdentityHashMap<>(),
+        CommandHelper.mergeCommandNode(newServerCommands.getRoot(), serverCommandsCopy, new IdentityHashMap<>(),
                 Minecraft.getInstance().getConnection().getSuggestionsProvider(), (context) -> 0, (suggestions) -> null);
 
         // Copies the client side commands into the server side commands to be used for suggestions
-        mergeCommandNode(commands.getRoot(), newServerCommands.getRoot(), new IdentityHashMap<>(), getSource(), (context) -> 0, (suggestions) -> {
+        CommandHelper.mergeCommandNode(commands.getRoot(), newServerCommands.getRoot(), new IdentityHashMap<>(), getSource(), (context) -> 0, (suggestions) -> {
             SuggestionProvider<SharedSuggestionProvider> suggestionProvider = SuggestionProviders
                     .safelySwap((SuggestionProvider<SharedSuggestionProvider>) (SuggestionProvider<?>) suggestions);
             if (suggestionProvider == SuggestionProviders.ASK_SERVER)
@@ -111,7 +106,7 @@ public class ClientCommandHandler
         });
 
         // Copies the server side commands into the client side commands so that they can be sent to the server as a chat message
-        mergeCommandNode(serverCommandsCopy, commands.getRoot(), new IdentityHashMap<>(), Minecraft.getInstance().getConnection().getSuggestionsProvider(),
+        CommandHelper.mergeCommandNode(serverCommandsCopy, commands.getRoot(), new IdentityHashMap<>(), Minecraft.getInstance().getConnection().getSuggestionsProvider(),
                 (context) -> {
                     Minecraft.getInstance().player.chat((context.getInput().startsWith("/") ? "" : "/") + context.getInput());
                     return 0;
@@ -138,9 +133,9 @@ public class ClientCommandHandler
     }
 
     /**
-     * 
+     *
      * Creates a deep copy of the sourceNode while keeping the redirects referring to the old command tree
-     * 
+     *
      * @param sourceNode
      *            the original
      * @param resultNode
@@ -163,106 +158,12 @@ public class ClientCommandHandler
         }
     }
 
-    /**
-     * 
-     * Deep copies the children of a command node and stores a link between the source and the copy
-     * 
-     * @param sourceNode
-     *            the original command node
-     * @param resultNode
-     *            the result command node
-     * @param sourceToResult
-     *            a map storing the original command node as the key and the result command node as the value
-     * @param canUse
-     *            used to check if the player can use the command
-     * @param execute
-     *            the command to execute in place of the old command
-     * @param sourceToResultSuggestion
-     *            a function to convert from the {@link SuggestionProvider} with the original source stack to the {@link SuggestionProvider} with the result source stack
-     */
-    private static <S, T> void mergeCommandNode(CommandNode<S> sourceNode, CommandNode<T> resultNode, Map<CommandNode<S>, CommandNode<T>> sourceToResult,
-            S canUse, Command<T> execute, Function<SuggestionProvider<S>, SuggestionProvider<T>> sourceToResultSuggestion)
-    {
-        sourceToResult.put(sourceNode, resultNode);
-        for (CommandNode<S> sourceChild : sourceNode.getChildren())
-        {
-            if (sourceChild.canUse(canUse)) 
-            {
-                resultNode.addChild(toResult(sourceChild, sourceToResult, canUse, execute, sourceToResultSuggestion));
-            }
-        }
-    }
-
-    /**
-     * 
-     * Creates a deep copy of a command node with a different source stack
-     * 
-     * @param sourceNode
-     *            the original command node
-     * @param sourceToResult
-     *            a map storing the original command node as the key and the result command node as the value
-     * @param canUse
-     *            used to check if the player can use the command
-     * @param execute
-     *            the command to execute in place of the old command
-     * @param sourceToResultSuggestion
-     *            a function to convert from the {@link SuggestionProvider} with the original source stack to the {@link SuggestionProvider} with the result source stack
-     * @return the deep copied command node with the new source stack
-     */
-    private static <S, T> CommandNode<T> toResult(CommandNode<S> sourceNode, Map<CommandNode<S>, CommandNode<T>> sourceToResult, S canUse, Command<T> execute,
-            Function<SuggestionProvider<S>, SuggestionProvider<T>> sourceToResultSuggestion)
-    {
-        if (sourceToResult.containsKey(sourceNode))
-            return sourceToResult.get(sourceNode);
-
-        ArgumentBuilder<T, ?> resultBuilder;
-        if (sourceNode instanceof ArgumentCommandNode<?, ?>)
-        {
-            ArgumentCommandNode<S, ?> sourceArgument = (ArgumentCommandNode<S, ?>) sourceNode;
-            RequiredArgumentBuilder<T, ?> resultArgumentBuilder = RequiredArgumentBuilder.argument(sourceArgument.getName(), sourceArgument.getType());
-            if (sourceArgument.getCustomSuggestions() != null)
-            {
-                resultArgumentBuilder.suggests(sourceToResultSuggestion.apply(sourceArgument.getCustomSuggestions()));
-            }
-            resultBuilder = resultArgumentBuilder;
-        }
-        else if (sourceNode instanceof LiteralCommandNode<?>)
-        {
-            LiteralCommandNode<S> sourceLiteral = (LiteralCommandNode<S>) sourceNode;
-            resultBuilder = LiteralArgumentBuilder.literal(sourceLiteral.getLiteral());
-        }
-        else if (sourceNode instanceof RootCommandNode<?>)
-        {
-            CommandNode<T> resultNode = new RootCommandNode<>();
-            mergeCommandNode(sourceNode, resultNode, sourceToResult, canUse, execute, sourceToResultSuggestion);
-            return resultNode;
-        }
-        else
-        {
-            throw new IllegalStateException("Node type " + sourceNode + " is not a standard node type");
-        }
-
-        if (sourceNode.getCommand() != null)
-        {
-            resultBuilder.executes(execute);
-        }
-
-        if (sourceNode.getRedirect() != null)
-        {
-            resultBuilder.redirect(toResult(sourceNode.getRedirect(), sourceToResult, canUse, execute, sourceToResultSuggestion));
-        }
-        
-        CommandNode<T> resultNode = resultBuilder.build();
-        mergeCommandNode(sourceNode, resultNode, sourceToResult, canUse, execute, sourceToResultSuggestion);
-        return resultNode;
-    }
-
     /**
      * Always try to execute the cached parsing of client message as a command. Requires that the execute field of the commands to be set to send to server so that they aren't
      * treated as client command's that do nothing.
-     * 
+     *
      * {@link net.minecraft.commands.Commands#performCommand(CommandSourceStack, String)} for reference
-     * 
+     *
      * @param sendMessage
      *            the chat message
      * @return false leaves the message to be sent to the server, true means it should be caught before {@link LocalPlayer#chat(String)}

src/main/java/net/minecraftforge/server/command/CommandHelper.java

@@ -0,0 +1,111 @@
+/*
+ * Minecraft Forge - Forge Development LLC
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.server.command;
+
+import com.mojang.brigadier.Command;
+import com.mojang.brigadier.builder.ArgumentBuilder;
+import com.mojang.brigadier.builder.LiteralArgumentBuilder;
+import com.mojang.brigadier.builder.RequiredArgumentBuilder;
+import com.mojang.brigadier.suggestion.SuggestionProvider;
+import com.mojang.brigadier.tree.ArgumentCommandNode;
+import com.mojang.brigadier.tree.CommandNode;
+import com.mojang.brigadier.tree.LiteralCommandNode;
+import com.mojang.brigadier.tree.RootCommandNode;
+
+import java.util.Map;
+import java.util.function.Function;
+
+/**
+ * Internal utility class for various command-related operations.
+ *
+ * <strong>For internal Forge use only.</strong>
+ *
+ * @hidden
+ */
+public final class CommandHelper
+{
+    private CommandHelper()
+    {
+    }
+
+    /**
+     * Deep copies the children of a command node and stores a link between the source and the copy
+     *
+     * @param sourceNode               the original command node
+     * @param resultNode               the result command node
+     * @param sourceToResult           a map storing the original command node as the key and the result command node as the value
+     * @param canUse                   used to check if the player can use the command
+     * @param execute                  the command to execute in place of the old command
+     * @param sourceToResultSuggestion a function to convert from the {@link SuggestionProvider} with the original source stack to the {@link SuggestionProvider} with the result source stack
+     */
+    public static <S, T> void mergeCommandNode(CommandNode<S> sourceNode, CommandNode<T> resultNode, Map<CommandNode<S>, CommandNode<T>> sourceToResult,
+                                               S canUse, Command<T> execute, Function<SuggestionProvider<S>, SuggestionProvider<T>> sourceToResultSuggestion)
+    {
+        sourceToResult.put(sourceNode, resultNode);
+        for (CommandNode<S> sourceChild : sourceNode.getChildren())
+        {
+            if (sourceChild.canUse(canUse))
+            {
+                resultNode.addChild(toResult(sourceChild, sourceToResult, canUse, execute, sourceToResultSuggestion));
+            }
+        }
+    }
+
+    /**
+     * Creates a deep copy of a command node with a different source stack
+     *
+     * @param sourceNode               the original command node
+     * @param sourceToResult           a map storing the original command node as the key and the result command node as the value
+     * @param canUse                   used to check if the player can use the command
+     * @param execute                  the command to execute in place of the old command
+     * @param sourceToResultSuggestion a function to convert from the {@link SuggestionProvider} with the original source stack to the {@link SuggestionProvider} with the result source stack
+     * @return the deep copied command node with the new source stack
+     */
+    private static <S, T> CommandNode<T> toResult(CommandNode<S> sourceNode, Map<CommandNode<S>, CommandNode<T>> sourceToResult, S canUse, Command<T> execute,
+                                                  Function<SuggestionProvider<S>, SuggestionProvider<T>> sourceToResultSuggestion)
+    {
+        if (sourceToResult.containsKey(sourceNode))
+            return sourceToResult.get(sourceNode);
+
+        ArgumentBuilder<T, ?> resultBuilder;
+        if (sourceNode instanceof ArgumentCommandNode<?, ?>)
+        {
+            ArgumentCommandNode<S, ?> sourceArgument = (ArgumentCommandNode<S, ?>) sourceNode;
+            RequiredArgumentBuilder<T, ?> resultArgumentBuilder = RequiredArgumentBuilder.argument(sourceArgument.getName(), sourceArgument.getType());
+            if (sourceArgument.getCustomSuggestions() != null)
+            {
+                resultArgumentBuilder.suggests(sourceToResultSuggestion.apply(sourceArgument.getCustomSuggestions()));
+            }
+            resultBuilder = resultArgumentBuilder;
+        } else if (sourceNode instanceof LiteralCommandNode<?>)
+        {
+            LiteralCommandNode<S> sourceLiteral = (LiteralCommandNode<S>) sourceNode;
+            resultBuilder = LiteralArgumentBuilder.literal(sourceLiteral.getLiteral());
+        } else if (sourceNode instanceof RootCommandNode<?>)
+        {
+            CommandNode<T> resultNode = new RootCommandNode<>();
+            mergeCommandNode(sourceNode, resultNode, sourceToResult, canUse, execute, sourceToResultSuggestion);
+            return resultNode;
+        } else
+        {
+            throw new IllegalStateException("Node type " + sourceNode + " is not a standard node type");
+        }
+
+        if (sourceNode.getCommand() != null)
+        {
+            resultBuilder.executes(execute);
+        }
+
+        if (sourceNode.getRedirect() != null)
+        {
+            resultBuilder.redirect(toResult(sourceNode.getRedirect(), sourceToResult, canUse, execute, sourceToResultSuggestion));
+        }
+
+        CommandNode<T> resultNode = resultBuilder.build();
+        mergeCommandNode(sourceNode, resultNode, sourceToResult, canUse, execute, sourceToResultSuggestion);
+        return resultNode;
+    }
+}