Fix MultiLiteralArguments - give nodes multiple parents instead of us…

…ing redirects

The Brigadier CommandDispatcher CommandNode structure does not need to be a tree. This means one node can have multiple parents. In this case, the node after a MultiLiteralArgument has parent node for each literal in the MultiLiteral.

This works around the issue documented in Mojang/brigadier#137, where arguments disappear from the CommandContext used when executing an argument when the command encounters a redirect. This way, we can remember which literal was chosen no matter which branch was taken, yet still converge on the same path.

commandapi-core/src/main/java/dev/jorel/commandapi/AbstractArgumentTree.java

@@ -125,12 +125,15 @@ public List<List<String>> getBranchesAsStrings() {
  /**
  * Builds the Brigadier {@link CommandNode} structure for this argument tree.
  *
- * @param previousNode  The {@link CommandNode} to add this argument tree onto.
+ * @param previousNodes A List of {@link CommandNode}s to add this argument onto.
  * @param previousArguments A List of CommandAPI arguments that came before this argument tree.
  * @param previousNonLiteralArgumentNames A List of Strings containing the node names that came before this argument.
  * @param <Source> The Brigadier Source object for running commands.
  */
- public <Source> void buildBrigadierNode(CommandNode<Source> previousNode, List<Argument> previousArguments, List<String> previousNonLiteralArgumentNames) {
+ public <Source> void buildBrigadierNode(
+ List<CommandNode<Source>> previousNodes,
+ List<Argument> previousArguments, List<String> previousNonLiteralArgumentNames
+ ) {
  // Check preconditions
  if (argument instanceof GreedyArgument && !arguments.isEmpty()) {
  // Argument is followed by at least some arguments
@@ -142,15 +145,15 @@ public <Source> void buildBrigadierNode(CommandNode<Source> previousNode, List<A
  }
 
  // Create node for this argument
- CommandNode<Source> rootNode = argument.addArgumentNodes(previousNode, previousArguments, previousNonLiteralArgumentNames, executor);
+ previousNodes = argument.addArgumentNodes(previousNodes, previousArguments, previousNonLiteralArgumentNames, executor);
 
  // Add our branches as children to the node
  for (AbstractArgumentTree<?, Argument, CommandSender> child : arguments) {
  // We need a new list for each branch of the tree
  List<Argument> newPreviousArguments = new ArrayList<>(previousArguments);
  List<String> newPreviousArgumentNames = new ArrayList<>(previousNonLiteralArgumentNames);
 
- child.buildBrigadierNode(rootNode, newPreviousArguments, newPreviousArgumentNames);
+ child.buildBrigadierNode(previousNodes, newPreviousArguments, newPreviousArgumentNames);
  }
  }
 

commandapi-core/src/main/java/dev/jorel/commandapi/AbstractCommandAPICommand.java

@@ -302,7 +302,7 @@ protected <Source> void createArgumentNodes(LiteralCommandNode<Source> rootNode)
 
  // Create arguments
  if (hasAnyArguments()) {
- CommandNode<Source> previousNode = rootNode;
+ List<CommandNode<Source>> previousNodes = List.of(rootNode);
  List<Argument> previousArguments = new ArrayList<>();
  List<String> previousArgumentNames = new ArrayList<>();
 
@@ -319,15 +319,15 @@ protected <Source> void createArgumentNodes(LiteralCommandNode<Source> rootNode)
  // Add required arguments
  for (int i = 0; i < requiredArguments.size(); i++) {
  Argument argument = requiredArguments.get(i);
- previousNode = argument.addArgumentNodes(previousNode, previousArguments, previousArgumentNames,
+ previousNodes = argument.addArgumentNodes(previousNodes, previousArguments, previousArgumentNames,
  // Only the last required argument is executable
  i == requiredArguments.size() - 1 ? executor : null);
  }
 
  // Add optional arguments
  for (Argument argument : optionalArguments) {
  // All optional arguments are executable
- previousNode = argument.addArgumentNodes(previousNode, previousArguments, previousArgumentNames, executor);
+ previousNodes = argument.addArgumentNodes(previousNodes, previousArguments, previousArgumentNames, executor);
  }
 
  // Check greedy argument constraint

commandapi-core/src/main/java/dev/jorel/commandapi/AbstractCommandTree.java

@@ -123,7 +123,7 @@ protected <Source> void createArgumentNodes(LiteralCommandNode<Source> rootNode)
  List<String> previousNonLiteralArgumentNames = new ArrayList<>();
  previousArguments.add(commandNames);
 
- argument.buildBrigadierNode(rootNode, previousArguments, previousNonLiteralArgumentNames);
+ argument.buildBrigadierNode(List.of(rootNode), previousArguments, previousNonLiteralArgumentNames);
  }
  }
 }

commandapi-core/src/main/java/dev/jorel/commandapi/arguments/AbstractArgument.java

@@ -360,20 +360,23 @@ public void appendToCommandPaths(List<List<String>> argumentStrings) {
  * <li>{@link #checkPreconditions(List, List)}</li>
  * <li>{@link #createArgumentBuilder(List, List)}</li>
  * <li>{@link #finishBuildingNode(ArgumentBuilder, List, CommandAPIExecutor)}</li>
- * <li>{@link #linkNode(CommandNode, CommandNode, List, List, CommandAPIExecutor)}</li>
+ * <li>{@link #linkNode(List, CommandNode, List, List, CommandAPIExecutor)}</li>
  * </ul>
  *
- * @param previousNode  The {@link CommandNode} to add this argument onto.
+ * @param previousNodes A List of {@link CommandNode}s to add this argument onto.
  * @param previousArguments A List of CommandAPI arguments that came before this argument.
  * @param previousNonLiteralArgumentNames A List of Strings containing the node names that came before this argument.
  * @param terminalExecutor The {@link CommandAPIExecutor} to apply at the end of the node structure.
  * This parameter can be null to indicate that this argument should not be
  * executable.
  * @param <Source> The Brigadier Source object for running commands.
- * @return The last node in the Brigadier node structure for this argument.
+ * @return The list of last nodes in the Brigadier node structure for this argument.
  */
- public <Source> CommandNode<Source> addArgumentNodes(CommandNode<Source> previousNode, List<Argument> previousArguments, List<String> previousNonLiteralArgumentNames,
- CommandAPIExecutor<CommandSender, AbstractCommandSender<? extends CommandSender>> terminalExecutor) {
+ public <Source> List<CommandNode<Source>> addArgumentNodes(
+ List<CommandNode<Source>> previousNodes,
+ List<Argument> previousArguments, List<String> previousNonLiteralArgumentNames,
+ CommandAPIExecutor<CommandSender, AbstractCommandSender<? extends CommandSender>> terminalExecutor
+ ) {
  CommandAPIHandler<Argument, CommandSender, Source> handler = CommandAPIHandler.getInstance();
 
  // Check preconditions
@@ -391,10 +394,10 @@ public <Source> CommandNode<Source> addArgumentNodes(CommandNode<Source> previou
  CommandNode<Source> rootNode = finishBuildingNode(rootBuilder, previousArguments, terminalExecutor);
 
  // Link node to previous
- previousNode = linkNode(previousNode, rootNode, previousArguments, previousNonLiteralArgumentNames, terminalExecutor);
+ previousNodes = linkNode(previousNodes, rootNode, previousArguments, previousNonLiteralArgumentNames, terminalExecutor);
 
- // Return last node
- return previousNode;
+ // Return last nodes
+ return previousNodes;
  }
 
  /**
@@ -464,34 +467,39 @@ public <Source> CommandNode<Source> finishBuildingNode(ArgumentBuilder<Source, ?
  /**
  * Links this argument into the Brigadier {@link CommandNode} structure.
  *
- * @param previousNode  The {@link CommandNode} to add this argument onto.
+ * @param previousNodes A List of {@link CommandNode}s to add this argument onto.
  * @param rootNode The {@link CommandNode} representing this argument.
  * @param previousArguments A List of CommandAPI arguments that came before this argument.
  * @param previousNonLiteralArgumentNames A List of Strings containing the node names that came before this argument.
  * @param terminalExecutor The {@link CommandAPIExecutor} to apply at the end of the node structure.
  * This parameter can be null to indicate that this argument should not be
  * executable.
  * @param <Source> The Brigadier Source object for running commands.
- * @return The last node in the Brigadier {@link CommandNode} structure representing this Argument. Note that this
- * is not necessarily the {@code rootNode} for this argument, since the Brigadier node structure may contain multiple
- * nodes. This might happen if {@link #combineWith(AbstractArgument[])} was called for this argument to merge it with
- * other arguments.
+ * @return The list of last nodes in the Brigadier {@link CommandNode} structure representing this Argument. Note that
+ * this is not necessarily the {@code rootNode} for this argument, since the Brigadier node structure may contain multiple
+ * nodes in a chain. This might happen if {@link #combineWith(AbstractArgument[])} was called for this argument to merge
+ * it with other arguments.
  */
- public <Source> CommandNode<Source> linkNode(CommandNode<Source> previousNode, CommandNode<Source> rootNode, List<Argument> previousArguments, List<String> previousNonLiteralArgumentNames,
- CommandAPIExecutor<CommandSender, AbstractCommandSender<? extends CommandSender>> terminalExecutor) {
- // Add rootNode to the previous
- previousNode.addChild(rootNode);
+ public <Source> List<CommandNode<Source>> linkNode(
+ List<CommandNode<Source>> previousNodes, CommandNode<Source> rootNode,
+ List<Argument> previousArguments, List<String> previousNonLiteralArgumentNames,
+ CommandAPIExecutor<CommandSender, AbstractCommandSender<? extends CommandSender>> terminalExecutor
+ ) {
+ // Add rootNode to the previous nodes
+ for(CommandNode<Source> previousNode : previousNodes) {
+ previousNode.addChild(rootNode);
+ }
 
  // Add combined arguments
- previousNode = rootNode;
+ previousNodes = List.of(rootNode);
  for (int i = 0; i < combinedArguments.size(); i++) {
  Argument subArgument = combinedArguments.get(i);
- previousNode = subArgument.addArgumentNodes(previousNode, previousArguments, previousNonLiteralArgumentNames,
+ previousNodes = subArgument.addArgumentNodes(previousNodes, previousArguments, previousNonLiteralArgumentNames,
  // Only apply the `terminalExecutor` to the last argument
  i == combinedArguments.size() - 1 ? terminalExecutor : null);
  }
 
- // Return last node
- return previousNode;
+ // Return last nodes
+ return previousNodes;
  }
 }

commandapi-core/src/main/java/dev/jorel/commandapi/arguments/MultiLiteral.java

@@ -112,44 +112,51 @@ default void appendToCommandPaths(List<List<String>> argumentStrings) {
  }
 
  /**
- * Overrides {@link AbstractArgument#linkNode(CommandNode, CommandNode, List, List, CommandAPIExecutor)}.
+ * Overrides {@link AbstractArgument#linkNode(List, CommandNode, List, List, CommandAPIExecutor)}.
  * <p>
  * Normally, Arguments are only represented by a single node, and so only need to link one node. However, a MultiLiteral
  * represents multiple literal node paths, which also need to be generated and inserted into the node structure.
  */
- default <Source> CommandNode<Source> linkNode(CommandNode<Source> previousNode, CommandNode<Source> rootNode, List<Argument> previousArguments, List<String> previousNonLiteralArgumentNames,
- CommandAPIExecutor<CommandSender, AbstractCommandSender<? extends CommandSender>> terminalExecutor) {
+ default <Source> List<CommandNode<Source>> linkNode(
+ List<CommandNode<Source>> previousNodes, CommandNode<Source> rootNode,
+ List<Argument> previousArguments, List<String> previousNonLiteralArgumentNames,
+ CommandAPIExecutor<CommandSender, AbstractCommandSender<? extends CommandSender>> terminalExecutor
+ ) {
+ List<CommandNode<Source>> newNodes = new ArrayList<>();
+ // Add root node to previous
+ for(CommandNode<Source> previousNode : previousNodes) {
+ previousNode.addChild(rootNode);
+ }
+ newNodes.add(rootNode);
+
  // Generate nodes for other literals
  Iterator<String> literals = Arrays.asList(getLiterals()).iterator();
  literals.next(); // Skip first literal; that was handled by `#createArgumentBuilder`
  while (literals.hasNext()) {
  // Create node
  MultiLiteralArgumentBuilder<Source> literalBuilder = MultiLiteralArgumentBuilder.multiLiteral(getNodeName(), literals.next());
 
- // Redirect to root node so all its arguments come after this
- literalBuilder.redirect(rootNode);
-
  // Finish building node
  CommandNode<Source> literalNode = finishBuildingNode(literalBuilder, previousArguments, terminalExecutor);
 
- // Link node to previous
- previousNode.addChild(literalNode);
+ // Add node to previous
+ for(CommandNode<Source> previousNode : previousNodes) {
+ previousNode.addChild(literalNode);
+ }
+ newNodes.add(literalNode);
  }
 
- // Add root node to previous
- previousNode.addChild(rootNode);
-
  // Add combined arguments
- previousNode = rootNode;
+ previousNodes = newNodes;
  List<Argument> combinedArguments = getCombinedArguments();
  for (int i = 0; i < combinedArguments.size(); i++) {
  Argument subArgument = combinedArguments.get(i);
- previousNode = subArgument.addArgumentNodes(previousNode, previousArguments, previousNonLiteralArgumentNames,
+ previousNodes = subArgument.addArgumentNodes(previousNodes, previousArguments, previousNonLiteralArgumentNames,
  // Only apply the `terminalExecutor` to the last argument
  i == combinedArguments.size() - 1 ? terminalExecutor : null);
  }
 
- // Return last node
- return previousNode;
+ // Return last nodes
+ return previousNodes;
  }
 }

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-core/src/main/java/dev/jorel/commandapi/arguments/MultiLiteralArgument.java

@@ -122,7 +122,7 @@ public void appendToCommandPaths(List<List<String>> argumentStrings) {
  }
 
  @Override
- public <Source> CommandNode<Source> linkNode(CommandNode<Source> previousNode, CommandNode<Source> rootNode, List<Argument<?>> previousArguments, List<String> previousNonLiteralArgumentNames, CommandAPIExecutor<CommandSender, AbstractCommandSender<? extends CommandSender>> terminalExecutor) {
- return MultiLiteral.super.linkNode(previousNode, rootNode, previousArguments, previousNonLiteralArgumentNames, terminalExecutor);
+ public <Source> List<CommandNode<Source>> linkNode(List<CommandNode<Source>> previousNodes, CommandNode<Source> rootNode, List<Argument<?>> previousArguments, List<String> previousNonLiteralArgumentNames, CommandAPIExecutor<CommandSender, AbstractCommandSender<? extends CommandSender>> terminalExecutor) {
+ return MultiLiteral.super.linkNode(previousNodes, rootNode, previousArguments, previousNonLiteralArgumentNames, terminalExecutor);
  }
 }