hollow-cube/command-parser-fixes (#54)

* Rewrite CommandParserImpl recursively (fix #1327)

* Fix for tests: CommandManagerTest, CommandParseTest

* Make attributes final inside Chain

* fix #1295 with argument type priority

* Don't include command name in list of arguments

* Add test for #1327

* Add test for #1295

* Fix suggestions with bad syntax

* Fix #1916

* Add test for #1916

* add failing test

* mess with arg order

* Fix `GraphImpl` causing syntax order issues

---------

Co-authored-by: Spanner <spanner77@protonmail.com>

(cherry picked from commit e9d0098418)
This commit is contained in:
Matt Worzala 2023-08-30 19:04:11 -04:00
parent 0c328a4e7b
commit 6805c903f3
7 changed files with 96 additions and 80 deletions

View File

@ -59,6 +59,7 @@ public class Main {
commandManager.register(new DebugGridCommand());
commandManager.register(new DisplayCommand());
commandManager.register(new NotificationCommand());
commandManager.register(new TestCommand2());
commandManager.setUnknownCommandCallback((sender, command) -> sender.sendMessage(Component.text("Unknown command", NamedTextColor.RED)));

View File

@ -132,6 +132,14 @@ public class PlayerInit {
})
.addListener(PlayerBlockPlaceEvent.class, event -> {
// event.setDoBlockUpdates(false);
})
.addListener(PlayerBlockInteractEvent.class, event -> {
var block = event.getBlock();
var rawOpenProp = block.getProperty("open");
if (rawOpenProp == null) return;
block = block.withProperty("open", String.valueOf(!Boolean.parseBoolean(rawOpenProp)));
event.getInstance().setBlock(event.getBlockPosition(), block);
});
static {

View File

@ -0,0 +1,20 @@
package net.minestom.demo.commands;
import net.minestom.server.command.builder.Command;
import net.minestom.server.command.builder.arguments.ArgumentType;
public class TestCommand2 extends Command {
public TestCommand2() {
super("test2");
var argA = ArgumentType.String("a");
var argB = ArgumentType.String("b");
addSyntax((sender, context) -> {
sender.sendMessage("a only");
}, argA);
addSyntax((sender, context) -> {
sender.sendMessage("a and b");
}, argB, argA);
}
}

View File

@ -18,7 +18,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
final class CommandParserImpl implements CommandParser {
@ -71,10 +70,6 @@ final class CommandParserImpl implements CommandParser {
return nodeResults.stream().map(x -> x.node.argument()).collect(Collectors.toList());
}
int size() {
return nodeResults.size();
}
Chain() {}
Chain(CommandExecutor defaultExecutor,
@ -102,19 +97,18 @@ final class CommandParserImpl implements CommandParser {
NodeResult result = parseNode(sender, parent, chain, reader);
chain = result.chain;
NodeResult lastNodeResult = chain.nodeResults.peekLast();
if (lastNodeResult == null) return UnknownCommandResult.INSTANCE;
Node lastNode = lastNodeResult.node;
if (result.argumentResult instanceof ArgumentResult.Success<?>) {
NodeResult lastNodeResult = chain.nodeResults.peekLast();
Node lastNode = lastNodeResult.node;
CommandExecutor executor = nullSafeGetter(lastNode.execution(), Graph.Execution::executor);
if (executor != null) return ValidCommand.executor(input, chain, executor);
}
// If here, then the command failed or didn't have an executor
// Look for a default executor, or give up if we got nowhere
if (lastNode.equals(parent)) return UnknownCommandResult.INSTANCE;
NodeResult lastNode = chain.nodeResults.peekLast();
if (lastNode.node.equals(parent)) return UnknownCommandResult.INSTANCE;
if (chain.defaultExecutor != null) {
return ValidCommand.defaultExecutor(input, chain);
}
@ -138,7 +132,7 @@ final class CommandParserImpl implements CommandParser {
NodeResult nodeResult = new NodeResult(node, chain, (ArgumentResult<Object>) result, suggestionCallback);
chain.append(nodeResult);
if (suggestionCallback != null) chain.suggestionCallback = suggestionCallback;
if (chain.size() == 1) { // If this is the root node (usually "Literal<>")
if (chain.nodeResults.size() == 1) { // If this is the root node (usually "Literal<>")
reader.cursor(start);
} else {
if (!(result instanceof ArgumentResult.Success<?>)) {
@ -175,11 +169,11 @@ final class CommandParserImpl implements CommandParser {
// Assume that there is only one successful node for a given chain of arguments
return childResult;
} else {
if (error == null || error.chain.size() < childResult.chain.size()) {
if (error == null) {
// If this is the base argument (e.g. "teleport" in /teleport) then
// do not report an argument to be incompatible, since the more
// correct thing would be to say that the command is unknown.
if (!(childResult.chain.size() == 2 && childResult.argumentResult instanceof ArgumentResult.IncompatibleType<?>)) {
if (!(childResult.chain.nodeResults.size() == 2 && childResult.argumentResult instanceof ArgumentResult.IncompatibleType<?>)) {
error = childResult;
}
}

View File

@ -21,7 +21,6 @@ public class EntityActionListener {
private static void setSneaking(Player player, boolean sneaking) {
boolean oldState = player.isSneaking();
player.setSneaking(sneaking);
if (oldState != sneaking) {

View File

@ -1,7 +1,10 @@
package net.minestom.server.command;
import net.minestom.server.command.builder.Command;
import net.minestom.server.command.builder.CommandResult;
import net.minestom.server.command.builder.arguments.ArgumentType;
import net.minestom.server.network.packet.server.play.DeclareCommandsPacket;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;
import java.util.concurrent.atomic.AtomicBoolean;
@ -45,6 +48,62 @@ public class CommandManagerTest {
assertTrue(check.get());
}
@Test
public void testSharedArgumentSyntaxABFirst() {
var manager = new CommandManager();
var checkA = new AtomicBoolean(false);
var checkAB = new AtomicBoolean(false);
var cmd = new Command("cmd");
var argA = ArgumentType.String("a");
var argB = ArgumentType.String("b");
cmd.addSyntax((sender, context) -> checkAB.set(true), argA, argB);
cmd.addSyntax((sender, context) -> checkA.set(true), argA);
manager.register(cmd);
var result = manager.executeServerCommand("cmd a");
assertEquals(CommandResult.Type.SUCCESS, result.getType());
assertTrue(checkA.get());
assertFalse(checkAB.get());
checkA.set(false); // these should be different tests
checkAB.set(false);
result = manager.executeServerCommand("cmd a b");
assertEquals(CommandResult.Type.SUCCESS, result.getType());
assertFalse(checkA.get());
assertTrue(checkAB.get());
}
@Test
public void testSharedArgumentSyntaxAFirst() {
var manager = new CommandManager();
var checkA = new AtomicBoolean(false);
var checkAB = new AtomicBoolean(false);
var cmd = new Command("cmd");
var argA = ArgumentType.String("a");
var argB = ArgumentType.String("b");
cmd.addSyntax((sender, context) -> checkA.set(true), argA);
cmd.addSyntax((sender, context) -> checkAB.set(true), argA, argB);
manager.register(cmd);
var result = manager.executeServerCommand("cmd a");
assertEquals(CommandResult.Type.SUCCESS, result.getType());
assertTrue(checkA.get());
assertFalse(checkAB.get());
checkA.set(false); // these should be different tests
checkAB.set(false);
result = manager.executeServerCommand("cmd a b");
assertEquals(CommandResult.Type.SUCCESS, result.getType());
assertFalse(checkA.get());
assertTrue(checkAB.get());
}
private static void assertNodeEquals(DeclareCommandsPacket.Node node, byte flags, int[] children, int redirectedNode,
String name, String parser, byte[] properties, String suggestionsType) {
assertEquals(flags, node.flags);

View File

@ -75,69 +75,4 @@ public class CommandSuggestionIntegrationTest {
assertEquals(List.of(new TabCompletePacket.Match("suggestion", null)), tabCompletePacket.matches());
});
}
@Test
public void suggestionWithSubcommand(Env env) {
var instance = env.createFlatInstance();
var connection = env.createConnection();
var player = connection.connect(instance, new Pos(0, 42, 0)).join();
var command = new Command("foo");
var subCommand = new Command("bar");
var wordArg1 = Word("wordArg1").setSuggestionCallback((sender, context, suggestion) -> {
suggestion.addEntry(new SuggestionEntry("suggestionA"));
});
var wordArg2 = Word("wordArg2").setSuggestionCallback((sender, context, suggestion) -> {
suggestion.addEntry(new SuggestionEntry("suggestionB"));
});
subCommand.addSyntax((sender, context) -> {}, wordArg1, wordArg2);
command.addSyntax((sender,context)->{}, Literal("literal"), wordArg2);
command.addSubcommand(subCommand);
env.process().command().register(command);
var listener = connection.trackIncoming(TabCompletePacket.class);
player.addPacketToQueue(new ClientTabCompletePacket(1, "foo bar "));
player.interpretPacketQueue();
listener.assertSingle(tabCompletePacket -> {
assertEquals(List.of(new TabCompletePacket.Match("suggestionA", null)), tabCompletePacket.matches());
});
}
@Test
public void suggestionWithTwoLiterals(Env env) {
var instance = env.createFlatInstance();
var connection = env.createConnection();
var player = connection.connect(instance, new Pos(0, 42, 0)).join();
var command = new Command("foo");
var wordArg1 = Word("wordArg1").setSuggestionCallback((sender, context, suggestion) -> {
suggestion.addEntry(new SuggestionEntry("suggestionA"));
});
var wordArg2 = Word("wordArg2").setSuggestionCallback((sender, context, suggestion) -> {
suggestion.addEntry(new SuggestionEntry("suggestionB"));
});
command.addSyntax((sender,context)->{}, Literal("literal1"), wordArg1);
command.addSyntax((sender,context)->{}, Literal("literal2"), wordArg2);
env.process().command().register(command);
var listener = connection.trackIncoming(TabCompletePacket.class);
player.addPacketToQueue(new ClientTabCompletePacket(1, "foo literal2 "));
player.interpretPacketQueue();
listener.assertSingle(tabCompletePacket -> {
assertEquals(List.of(new TabCompletePacket.Match("suggestionB", null)), tabCompletePacket.matches());
});
}
}