Fix subcommands not being added to the graph and (maybe) better separate responsibilities of the relevant classes

This commit is contained in:
Noel Németh 2022-07-05 01:36:42 +02:00
parent ab5734334c
commit 2a1abdbaf4
5 changed files with 179 additions and 61 deletions

View File

@ -3,9 +3,6 @@ package net.minestom.server.command;
import net.minestom.server.command.builder.Command;
import net.minestom.server.command.builder.CommandDispatcher;
import net.minestom.server.command.builder.CommandResult;
import net.minestom.server.command.builder.CommandSyntax;
import net.minestom.server.command.builder.arguments.Argument;
import net.minestom.server.command.builder.condition.CommandCondition;
import net.minestom.server.entity.Player;
import net.minestom.server.event.EventDispatcher;
import net.minestom.server.event.player.PlayerCommandEvent;
@ -160,43 +157,6 @@ public final class CommandManager {
* @return the {@link DeclareCommandsPacket} for {@code player}
*/
public @NotNull DeclareCommandsPacket createDeclareCommandsPacket(@NotNull Player player) {
final GraphBuilder factory = new GraphBuilder();
for (Command command : this.dispatcher.getCommands()) {
// Check if user can use the command
final CommandCondition condition = command.getCondition();
if (condition != null && !condition.canUse(player, null)) continue;
// Add command to the graph
// Create the command's root node
final Node cmdNode = factory.createLiteralNode(command.getName(), true,
command.getDefaultExecutor() != null, command.getAliases(), null);
// Add syntax to the command
for (CommandSyntax syntax : command.getSyntaxes()) {
boolean executable = false;
Node[] lastArgNodes = new Node[] {cmdNode}; // First arg links to cmd root
@NotNull Argument<?>[] arguments = syntax.getArguments();
for (int i = 0; i < arguments.length; i++) {
Argument<?> argument = arguments[i];
// Determine if command is executable here
if (executable && argument.getDefaultValue() == null) {
// Optional arg was followed by a non-optional
throw new RuntimeException("");//todo exception
}
if (!executable && i < arguments.length-1 && arguments[i+1].getDefaultValue() != null || i+1 == arguments.length) {
executable = true;
}
// Append current node to previous
final Node[] argNodes = factory.createArgumentNode(argument, executable);
for (Node lastArgNode : lastArgNodes) {
lastArgNode.addChild(argNodes);
}
lastArgNodes = argNodes;
}
}
}
return factory.createCommandPacket();
return GraphBuilder.forPlayer(this.dispatcher.getCommands(), player).createPacket();
}
}

View File

@ -2,12 +2,19 @@ package net.minestom.server.command;
import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet;
import it.unimi.dsi.fastutil.objects.ObjectSet;
import net.minestom.server.command.builder.Command;
import net.minestom.server.command.builder.CommandSyntax;
import net.minestom.server.command.builder.arguments.*;
import net.minestom.server.network.packet.server.play.DeclareCommandsPacket;
import net.minestom.server.command.builder.condition.CommandCondition;
import net.minestom.server.command.builder.exception.IllegalCommandStructureException;
import net.minestom.server.entity.Player;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Comparator;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
import java.util.Stack;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;
import java.util.stream.Stream;
@ -18,33 +25,37 @@ final class GraphBuilder {
private final ObjectSet<Supplier<Boolean>> redirectWaitList = new ObjectOpenHashSet<>();
private final Node root = rootNode();
private GraphBuilder() {
//no instance
}
private Node rootNode() {
final Node rootNode = new Node(idSource.getAndIncrement());
nodes.add(rootNode);
return rootNode;
}
public Node createLiteralNode(String name, boolean addToRoot, boolean executable, @Nullable String[] aliases, @Nullable Integer redirectTo) {
private Node createLiteralNode(String name, @Nullable Node parent, boolean executable, @Nullable String[] aliases, @Nullable Integer redirectTo) {
if (aliases != null) {
final Node node = createLiteralNode(name, addToRoot, executable, null, null);
final Node node = createLiteralNode(name, parent, executable, null, null);
for (String alias : aliases) {
createLiteralNode(alias, addToRoot, false, null, node.getId());
createLiteralNode(alias, parent, executable, null, node.id());
}
return node;
} else {
final Node literalNode = new Node(idSource.getAndIncrement(), name, redirectTo);
literalNode.setExecutable(executable);
nodes.add(literalNode);
if (addToRoot) root.addChild(literalNode);
if (parent != null) parent.addChild(literalNode);
return literalNode;
}
}
public Node[] createArgumentNode(Argument<?> argument, boolean executable) {
private Node[] createArgumentNode(Argument<?> argument, boolean executable) {
final Node[] nodes;
Integer overrideRedirectTarget = null;
if (argument instanceof ArgumentEnum<?> argumentEnum) {
nodes = argumentEnum.entries().stream().map(x -> createLiteralNode(x, false, executable, null, null)).toArray(Node[]::new);
nodes = argumentEnum.entries().stream().map(x -> createLiteralNode(x, null, executable, null, null)).toArray(Node[]::new);
} else if (argument instanceof ArgumentGroup argumentGroup) {
nodes = argumentGroup.group().stream().map(x -> createArgumentNode(x, executable)).flatMap(Stream::of).toArray(Node[]::new);
} else if (argument instanceof ArgumentLoop<?> argumentLoop) {
@ -52,7 +63,7 @@ final class GraphBuilder {
nodes = argumentLoop.arguments().stream().map(x -> createArgumentNode(x, executable)).flatMap(Stream::of).toArray(Node[]::new);
} else {
if (argument instanceof ArgumentCommand) {
return new Node[]{createLiteralNode(argument.getId(), false, false, null, 0)};
return new Node[]{createLiteralNode(argument.getId(), null, false, null, 0)};
}
final int id = idSource.getAndIncrement();
nodes = new Node[] {argument instanceof ArgumentLiteral ? new Node(id, argument.getId(), null) : new Node(id, argument)};
@ -77,7 +88,7 @@ final class GraphBuilder {
private int tryResolveId(String[] path) {
if (path.length == 0) {
return root.getId();
return root.id();
} else {
Node target = root;
for (String next : path) {
@ -90,19 +101,98 @@ final class GraphBuilder {
target = result.get();
}
}
return target.getId();
return target.id();
}
}
private void finalizeStructure() {
redirectWaitList.removeIf(Supplier::get);
if (redirectWaitList.size() > 0)
throw new RuntimeException("Could not set redirects for all arguments! Did you provide a correct id path which doesn't rely on redirects?");
throw new IllegalCommandStructureException("Could not set redirects for all arguments! Did you provide a " +
"correct id path which doesn't rely on redirects?");
}
public DeclareCommandsPacket createCommandPacket() {
finalizeStructure();
return new DeclareCommandsPacket(nodes.stream().sorted(Comparator.comparingInt(Node::getId))
.map(Node::getPacketNode).toList(), root.getId());
/**
* Creates the nodes for the given command
*
* @param command the command to add
* @param parent where to append the command's root (literal) node
* @param player a player if we should filter commands
*/
private void createCommand(Command command, Node parent, @Nullable Player player) {
if (player != null) {
// Check if user can use the command
final CommandCondition condition = command.getCondition();
if (condition != null && !condition.canUse(player, null)) return;
}
// Create the command's root node
final Node cmdNode = createLiteralNode(command.getName(), parent,
command.getDefaultExecutor() != null, command.getAliases(), null);
// Add syntax to the command
for (CommandSyntax syntax : command.getSyntaxes()) {
boolean executable = false;
Node[] lastArgNodes = new Node[] {cmdNode}; // First arg links to cmd root
@NotNull Argument<?>[] arguments = syntax.getArguments();
for (int i = 0; i < arguments.length; i++) {
Argument<?> argument = arguments[i];
// Determine if command is executable here
if (executable && argument.getDefaultValue() == null) {
// Optional arg was followed by a non-optional
throw new IllegalCommandStructureException("Optional argument was followed by a non-optional one.");
}
if (!executable && i < arguments.length-1 && arguments[i+1].getDefaultValue() != null || i+1 == arguments.length) {
executable = true;
}
// Append current node to previous
final Node[] argNodes = createArgumentNode(argument, executable);
for (Node lastArgNode : lastArgNodes) {
lastArgNode.addChild(argNodes);
}
lastArgNodes = argNodes;
}
}
// Add subcommands
for (Command subcommand : command.getSubcommands()) {
createCommand(subcommand, cmdNode, player);
}
}
public static NodeGraph forPlayer(@NotNull Set<Command> commands, Player player) {
final GraphBuilder builder = new GraphBuilder();
if (GraphBuilder.class.desiredAssertionStatus()) {
// Detect infinite recursion
for (Command command : commands) {
final HashSet<Command> processed = new HashSet<>();
final Stack<Command> stack = new Stack<>();
stack.push(command);
while (!stack.isEmpty()) {
final Command pop = stack.pop();
if (!processed.add(pop)) {
throw new IllegalCommandStructureException("Infinite recursion detected in command: "+command.getName());
} else {
stack.addAll(pop.getSubcommands());
}
}
builder.createCommand(command, builder.root, player);
}
} else {
for (Command command : commands) {
builder.createCommand(command, builder.root, player);
}
}
builder.finalizeStructure();
return new NodeGraph(builder.nodes, builder.root.id());
}
public static NodeGraph forServer(@NotNull Set<Command> commands) {
return forPlayer(commands, null);
}
}

View File

@ -2,12 +2,14 @@ package net.minestom.server.command;
import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
import it.unimi.dsi.fastutil.ints.IntSet;
import it.unimi.dsi.fastutil.ints.IntSets;
import net.minestom.server.command.builder.arguments.Argument;
import net.minestom.server.network.packet.server.play.DeclareCommandsPacket;
final class Node {
private final int id;
private final IntSet children;
private final IntSet children = new IntOpenHashSet();
private final IntSet childrenView = IntSets.unmodifiable(children);
private final DeclareCommandsPacket.NodeType type;
private String name;
private Integer redirectTarget;
@ -16,7 +18,6 @@ final class Node {
Node(int id, DeclareCommandsPacket.NodeType type) {
this.id = id;
this.children = new IntOpenHashSet();
this.type = type;
}
@ -59,13 +60,29 @@ final class Node {
}
public boolean isParentOf(Node node) {
return children.contains(node.getId());
return children.contains(node.id());
}
public int getId() {
public int id() {
return id;
}
public DeclareCommandsPacket.NodeType type() {
return type;
}
public IntSet children() {
return childrenView;
}
public Integer redirectTarget() {
return redirectTarget;
}
public boolean isRoot() {
return type == DeclareCommandsPacket.NodeType.ROOT;
}
public DeclareCommandsPacket.Node getPacketNode() {
final DeclareCommandsPacket.Node node = new DeclareCommandsPacket.Node();
node.children = children.toIntArray();

View File

@ -0,0 +1,41 @@
package net.minestom.server.command;
import it.unimi.dsi.fastutil.objects.ObjectImmutableList;
import it.unimi.dsi.fastutil.objects.ObjectList;
import it.unimi.dsi.fastutil.objects.ObjectSet;
import net.minestom.server.network.packet.server.play.DeclareCommandsPacket;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.Nullable;
import java.util.Comparator;
import java.util.List;
class NodeGraph {
private final ObjectList<Node> nodes;
private final Node root;
NodeGraph(ObjectSet<Node> nodes, int rootId) {
this.nodes = new ObjectImmutableList<>(nodes.stream().sorted(Comparator.comparing(Node::id)).toList());
this.root = this.nodes.get(rootId);
assert root.isRoot() : "rootId doesn't point to the root node";
assert this.nodes.stream().filter(Node::isRoot).count() == 1 : "Invalid root node count!";
}
public Node resolveId(int id) {
return nodes.get(id);
}
public List<Node> getChildren(Node node) {
return node.children().intStream().mapToObj(this::resolveId).toList();
}
public @Nullable Node getRedirectTarget(Node node) {
final Integer target = node.redirectTarget();
return target == null ? null : resolveId(target);
}
@Contract("-> new")
public DeclareCommandsPacket createPacket() {
return new DeclareCommandsPacket(nodes.stream().map(Node::getPacketNode).toList(), root.id());
}
}

View File

@ -0,0 +1,10 @@
package net.minestom.server.command.builder.exception;
public class IllegalCommandStructureException extends RuntimeException {
public IllegalCommandStructureException() {
}
public IllegalCommandStructureException(String message) {
super(message);
}
}