Skip to content

Commit

Permalink
Rewrite RegisteredCommand
Browse files Browse the repository at this point in the history 
- Reordered constructor arguments to more closely match the order in `ExecutableCommand`
  - This branch is now definitely backward incompatible (though not majorly)
- Replaced `List<String> argsAsStr` with `Node rootNode`
  - Added `RegisteredCommand.Node` class
    - Has an `argsAsStr` method as a replacement for the parameter of `RegisteredCommand`
    - Simplified representation of the Brigadier's `CommandNode` structure for the command
  - Removed `AbstractArgument#getHelpString` and `RegisteredCommand#arguments` (Added by #537)
    - The `RegisteredCommand.Node` structure is used to generate usage
    - `Literal` and `MultiLiteral` have separate logic for creating thier `RegisteredCommand.Node, wherein they define a different help string
    - TODO: #537 didn't have any tests, so I'm only guessing this new implementation works the same. In general, add more tests for usage generation.
- Removed `ExecutableCommand#getArgumentsAsStrings` and `AbstractArgument#appendToCommandPaths`
  - Added `AbstractArgument.NodeInformation` to help pass around enough information to generate Brigadier nodes and `RegisteredCommand.Node`s in one `AbstractArgument` tree traversal.
  - Modified the signatures of a few methods to facilitate this, including overriding methods
  - TODO: This broke `Previewable` arguments, so I'll have to tweak those
- Changed `CommandAPIHandler#registeredCommands` from an `ArrayList` to a `LinkedHashMap`
  - Instead of creating one `RegisteredCommand` object for each command path, `RegisteredCommand`s are merged when they share thier command name and namespace
  - One call to `ExecutableCommand#register` creates one `RegisteredCommand` object (if it has a namespace, an unnamespaced copy is created too)
  - `CommandAPIPlatform#postCommandRegistration` was reverted to original signature
  - NOTE: `CommandAPI#getRegisteredCommands` still returns a `List<RegisteredCommand>`
- Updated `CommandAPICommandRegisteredCommandTests` and `CommandTreeRegisteredCommandTests`
  - Added `RegisteredCommandTestBase` to handle common utility methods
  • Loading branch information
@willkroboth
willkroboth committed May 14, 2024
1 parent 597ca3c commit c8fe05a
Show file tree
Hide file tree
Showing 23 changed files with 1,202 additions and 678 deletions.
60 changes: 21 additions & 39 deletions commandapi-core/src/main/java/dev/jorel/commandapi/AbstractArgumentTree.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.mojang.brigadier.tree.CommandNode;
import dev.jorel.commandapi.arguments.AbstractArgument;
import dev.jorel.commandapi.arguments.AbstractArgument.NodeInformation;
import dev.jorel.commandapi.exceptions.MissingCommandExecutorException;

import java.util.ArrayList;
Expand Down Expand Up @@ -89,47 +90,16 @@ public void setArguments(List<AbstractArgumentTree<?, Argument, CommandSender>>
//////////////////
// Registration //
//////////////////

/**
* @return A list of paths that represent the possible branches of this argument tree as Strings, starting with the
* base argument held by this tree.
*/
public List<List<String>> getBranchesAsStrings() {
List<List<String>> argumentStrings = new ArrayList<>();

// Create paths for the argument at this node
List<List<String>> baseArgumentPaths = new ArrayList<>();
baseArgumentPaths.add(new ArrayList<>());
argument.appendToCommandPaths(baseArgumentPaths);

// If this node is executable, add it as a valid path
if (this.executor.hasAnyExecutors()) argumentStrings.addAll(baseArgumentPaths);

// Add paths for the branches
for (AbstractArgumentTree<?, Argument, CommandSender> child : arguments) {
for (List<String> subArgs : child.getBranchesAsStrings()) {
for (List<String> basePath : baseArgumentPaths) {
List<String> mergedPaths = new ArrayList<>();
mergedPaths.addAll(basePath);
mergedPaths.addAll(subArgs);
argumentStrings.add(mergedPaths);
}
}
}

return argumentStrings;
}

/**
* Builds the Brigadier {@link CommandNode} structure for this argument tree.
*
* @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 previousArgumentNames A List of Strings containing the node names that came before this argument.
* @param <Source> The Brigadier Source object for running commands.
* @param previousNodeInformation The {@link NodeInformation} of the argument this argument is being added to.
* @param previousArguments A List of CommandAPI arguments that came before this argument tree.
* @param previousArgumentNames 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(
List<CommandNode<Source>> previousNodes,
NodeInformation<Source> previousNodeInformation,
List<Argument> previousArguments, List<String> previousArgumentNames
) {
CommandAPIHandler<Argument, CommandSender, Source> handler = CommandAPIHandler.getInstance();
Expand All @@ -141,16 +111,28 @@ public <Source> void buildBrigadierNode(
}

// Create node for this argument
previousNodes = argument.addArgumentNodes(previousNodes, previousArguments, previousArgumentNames,
previousNodeInformation = argument.addArgumentNodes(previousNodeInformation, previousArguments, previousArgumentNames,
executor.hasAnyExecutors() ? args -> handler.generateBrigadierCommand(args, executor) : null);

// Collect children into our own list
List<RegisteredCommand.Node> childrenNodeInformation = new ArrayList<>();

// 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
// Collect children into our own list
NodeInformation<Source> newPreviousNodeInformation = new NodeInformation<>(
previousNodeInformation.lastCommandNodes(),
children -> childrenNodeInformation.addAll(children)
);

// We need a new list so each branch acts independently
List<Argument> newPreviousArguments = new ArrayList<>(previousArguments);
List<String> newPreviousArgumentNames = new ArrayList<>(previousArgumentNames);

child.buildBrigadierNode(previousNodes, newPreviousArguments, newPreviousArgumentNames);
child.buildBrigadierNode(newPreviousNodeInformation, newPreviousArguments, newPreviousArgumentNames);
}

// Create registered nodes now that all children are generated
previousNodeInformation.childrenConsumer().createNodeWithChildren(childrenNodeInformation);
}
}
101 changes: 34 additions & 67 deletions commandapi-core/src/main/java/dev/jorel/commandapi/AbstractCommandAPICommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
package dev.jorel.commandapi;

import com.mojang.brigadier.Command;
import com.mojang.brigadier.tree.CommandNode;
import com.mojang.brigadier.tree.LiteralCommandNode;
import dev.jorel.commandapi.arguments.AbstractArgument;
import dev.jorel.commandapi.arguments.AbstractArgument.NodeInformation;
import dev.jorel.commandapi.exceptions.MissingCommandExecutorException;
import dev.jorel.commandapi.exceptions.OptionalArgumentException;

Expand Down Expand Up @@ -222,65 +222,6 @@ public void setSubcommands(List<Impl> subcommands) {
//////////////////
// Registration //
//////////////////

@Override
public List<List<String>> getArgumentsAsStrings() {
// Return an empty list if we have no arguments
if (subcommands.isEmpty() && !hasAnyArguments()) {
return List.of(List.of());
}

List<List<String>> argumentStrings = new ArrayList<>();

// Build main path, if it is executable
if (executor.hasAnyExecutors()) {
// Required arguments represent one path
List<List<String>> mainPath = new ArrayList<>();
mainPath.add(new ArrayList<>());
for (Argument argument : requiredArguments) {
argument.appendToCommandPaths(mainPath);
}

List<Integer> slicePositions = new ArrayList<>();
// Note: Assumption that all paths are the same length
slicePositions.add(mainPath.get(0).size());

// Each optional argument is a potential stopping point
for (Argument argument : optionalArguments) {
argument.appendToCommandPaths(mainPath);
slicePositions.add(mainPath.get(0).size());
}

// Return each path as sublists of the main path
for (List<String> path : mainPath) {
for (int slicePos : slicePositions) {
argumentStrings.add(path.subList(0, slicePos));
}
}
}

// Build subcommand paths
for (Impl subCommand : subcommands) {
String[] subCommandArguments = new String[subCommand.aliases.length + 1];
subCommandArguments[0] = subCommand.name;
System.arraycopy(subCommand.aliases, 0, subCommandArguments, 1, subCommand.aliases.length);
for (int i = 0; i < subCommandArguments.length; i++) {
subCommandArguments[i] += ":LiteralArgument";
}

for (List<String> subArgs : subCommand.getArgumentsAsStrings()) {
for (String subCommandArgument : subCommandArguments) {
List<String> newPath = new ArrayList<>();
newPath.add(subCommandArgument);
newPath.addAll(subArgs);
argumentStrings.add(newPath);
}
}
}

return argumentStrings;
}

@Override
protected void checkPreconditions() {
if (!executor.hasAnyExecutors() && (subcommands.isEmpty() || hasAnyArguments())) {
Expand All @@ -297,12 +238,14 @@ protected boolean isRootExecutable() {
}

@Override
protected <Source> void createArgumentNodes(LiteralCommandNode<Source> rootNode) {
protected <Source> List<RegisteredCommand.Node> createArgumentNodes(LiteralCommandNode<Source> rootNode) {
CommandAPIHandler<Argument, CommandSender, Source> handler = CommandAPIHandler.getInstance();

List<RegisteredCommand.Node> childrenNodes = new ArrayList<>();

// Create arguments
if (hasAnyArguments()) {
List<CommandNode<Source>> previousNodes = List.of(rootNode);
NodeInformation<Source> previousNodeInformation = new NodeInformation<>(List.of(rootNode), children -> childrenNodes.addAll(children));
List<Argument> previousArguments = new ArrayList<>();
List<String> previousArgumentNames = new ArrayList<>();

Expand All @@ -316,26 +259,50 @@ protected <Source> void createArgumentNodes(LiteralCommandNode<Source> rootNode)

previousArguments.add(commandNames);

Function<List<Argument>, Command<Source>> executorCreator = executor.hasAnyExecutors() ?
args -> handler.generateBrigadierCommand(args, executor) : null;
// Note that `executor#hasAnyExecutors` must be true here
// If not, then `checkPreconditions` would have thrown a `MissingCommandExecutorException`
Function<List<Argument>, Command<Source>> executorCreator = args -> handler.generateBrigadierCommand(args, executor);

// Stack required arguments
// Only the last required argument is executable
previousNodes = AbstractArgument.stackArguments(requiredArguments, previousNodes, previousArguments, previousArgumentNames, executorCreator);
previousNodeInformation = AbstractArgument.stackArguments(requiredArguments, previousNodeInformation, previousArguments, previousArgumentNames, executorCreator);

// Add optional arguments
for (Argument argument : optionalArguments) {
// All optional arguments are executable
previousNodes = argument.addArgumentNodes(previousNodes, previousArguments, previousArgumentNames, executorCreator);
previousNodeInformation = argument.addArgumentNodes(previousNodeInformation, previousArguments, previousArgumentNames, executorCreator);
}

// Create registered nodes now that all children are generated
previousNodeInformation.childrenConsumer().createNodeWithChildren(List.of());
}

// Add subcommands
for (Impl subCommand : subcommands) {
Nodes<Source> nodes = subCommand.createCommandNodes();
CommandInformation<Source> nodes = subCommand.createCommandInformation("");

// Add root node
rootNode.addChild(nodes.rootNode());

RegisteredCommand.Node rootNodeInformation = nodes.command().rootNode();
childrenNodes.add(rootNodeInformation);

// Add aliases
for (LiteralCommandNode<Source> aliasNode : nodes.aliasNodes()) {
rootNode.addChild(aliasNode);

// Create node information for the alias
String aliasName = aliasNode.getLiteral();
childrenNodes.add(
new RegisteredCommand.Node(
aliasName, rootNodeInformation.className(), aliasName,
rootNodeInformation.executable(), rootNodeInformation.children()
)
);
}
}

// Return node information
return childrenNodes;
}
}
31 changes: 10 additions & 21 deletions commandapi-core/src/main/java/dev/jorel/commandapi/AbstractCommandTree.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.mojang.brigadier.tree.LiteralCommandNode;
import dev.jorel.commandapi.arguments.AbstractArgument;
import dev.jorel.commandapi.arguments.AbstractArgument.NodeInformation;
import dev.jorel.commandapi.exceptions.MissingCommandExecutorException;

import java.util.ArrayList;
Expand Down Expand Up @@ -72,25 +73,6 @@ public void setArguments(List<AbstractArgumentTree<?, Argument, CommandSender>>
//////////////////
// Registration //
//////////////////

@Override
public List<List<String>> getArgumentsAsStrings() {
// Return an empty list if we have no arguments
if (arguments.isEmpty()) return List.of(List.of());

List<List<String>> argumentStrings = new ArrayList<>();

// If this node is executable, no arguments is a valid path
if (this.executor.hasAnyExecutors()) argumentStrings.add(List.of());

// Add branching paths
for (AbstractArgumentTree<?, Argument, CommandSender> argument : arguments) {
argumentStrings.addAll(argument.getBranchesAsStrings());
}

return argumentStrings;
}

@Override
protected void checkPreconditions() {
if (!executor.hasAnyExecutors() && arguments.isEmpty()) {
Expand All @@ -105,9 +87,11 @@ protected boolean isRootExecutable() {
}

@Override
protected <Source> void createArgumentNodes(LiteralCommandNode<Source> rootNode) {
protected <Source> List<RegisteredCommand.Node> createArgumentNodes(LiteralCommandNode<Source> rootNode) {
CommandAPIHandler<Argument, CommandSender, Source> handler = CommandAPIHandler.getInstance();

List<RegisteredCommand.Node> childrenNodes = new ArrayList<>();

// The previous arguments include an unlisted MultiLiteral representing the command name and aliases
// This doesn't affect how the command acts, but it helps represent the command path in exceptions
String[] literals = new String[aliases.length + 1];
Expand All @@ -119,11 +103,16 @@ protected <Source> void createArgumentNodes(LiteralCommandNode<Source> rootNode)
// Build branches
for (AbstractArgumentTree<?, Argument, CommandSender> argument : arguments) {
// We need new previousArguments lists for each branch so they don't interfere
NodeInformation<Source> previousNodeInformation = new NodeInformation<>(List.of(rootNode), children -> childrenNodes.addAll(children));
List<Argument> previousArguments = new ArrayList<>();
List<String> previousArgumentNames = new ArrayList<>();

previousArguments.add(commandNames);

argument.buildBrigadierNode(List.of(rootNode), previousArguments, previousArgumentNames);
argument.buildBrigadierNode(previousNodeInformation, previousArguments, previousArgumentNames);
}

// Return children
return childrenNodes;
}
}
2 changes: 1 addition & 1 deletion commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,6 @@ public static void registerCommand(Class<?> commandClass) {
* registered by the CommandAPI so far. The returned list is immutable.
*/
public static List<RegisteredCommand> getRegisteredCommands() {
return Collections.unmodifiableList(CommandAPIHandler.getInstance().registeredCommands);
return Collections.unmodifiableList(new ArrayList<>(CommandAPIHandler.getInstance().registeredCommands.values()));
}
}
Loading

0 comments on commit c8fe05a

Please sign in to comment.