diff --git a/api/src/main/java/com/velocitypowered/api/command/CommandMeta.java b/api/src/main/java/com/velocitypowered/api/command/CommandMeta.java index b2612c4b68..693f0f8aee 100644 --- a/api/src/main/java/com/velocitypowered/api/command/CommandMeta.java +++ b/api/src/main/java/com/velocitypowered/api/command/CommandMeta.java @@ -41,6 +41,13 @@ public interface CommandMeta { */ @Nullable Object getPlugin(); + /** + * Returns whether partial invocations of this command are forwarded to the backend. + * + * @return whether to forward partial invocations + */ + boolean forwardPartial(); + /** * Provides a fluent interface to create {@link CommandMeta}s. */ @@ -73,6 +80,18 @@ interface Builder { */ Builder plugin(Object plugin); + /** + * Specifies whether partial matches to this command are forwarded to the backend. + * + *

For example with the registered command "rootcommand -> subcommand" where only the subcommand is executable, this + * specifies whether invocations such as "/rootcommand" or "/rootcommand nonexistant" should be forwarded to the + * backend, or be handled on the proxy. + * + * @param fowardPartial whether to forward partial matches + * @return this builder, for chaining + */ + Builder forwardPartial(boolean fowardPartial); + /** * Returns a newly-created {@link CommandMeta} based on the specified parameters. * diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java index 49dceb319d..0cebd233cd 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -59,7 +59,10 @@ import java.util.stream.Collectors; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.ComponentLike; +import net.kyori.adventure.text.TextComponent; +import net.kyori.adventure.text.event.ClickEvent; import net.kyori.adventure.text.format.NamedTextColor; +import net.kyori.adventure.text.format.TextDecoration; import org.checkerframework.checker.lock.qual.GuardedBy; import org.checkerframework.checker.nullness.qual.Nullable; import org.jetbrains.annotations.VisibleForTesting; @@ -238,22 +241,30 @@ private boolean executeImmediately0(final CommandSource source, final ParseResul result = executed ? CommandResult.EXECUTED : CommandResult.FORWARDED; return executed; } catch (final CommandSyntaxException e) { - boolean isSyntaxError = !e.getType().equals( - CommandSyntaxException.BUILT_IN_EXCEPTIONS.dispatcherUnknownCommand()); + boolean isSyntaxError = !e.getType().equals(CommandSyntaxException.BUILT_IN_EXCEPTIONS.dispatcherUnknownCommand()) + && !e.getType().equals(CommandSyntaxException.BUILT_IN_EXCEPTIONS.dispatcherUnknownArgument()); + if (isSyntaxError) { - final Message message = e.getRawMessage(); - if (message instanceof ComponentLike componentLike) { - source.sendMessage(componentLike.asComponent().applyFallbackStyle(NamedTextColor.RED)); - } else { - source.sendMessage(Component.text(e.getMessage(), NamedTextColor.RED)); - } - result = com.velocitypowered.api.command.CommandResult.SYNTAX_ERROR; + sendError(source, e); + result = CommandResult.SYNTAX_ERROR; // This is, of course, a lie, but the API will need to change... return true; - } else { - result = CommandResult.FORWARDED; - return false; } + + boolean existsOnProxy = !parsed.getContext().getNodes().isEmpty(); + if (existsOnProxy) { + String invokedAlias = parsed.getContext().getNodes().get(0).getRange().get(parsed.getReader()); + CommandMeta meta = this.commandMetas.get(invokedAlias); + if (meta != null && !meta.forwardPartial()) { + // mark command handled and send error to source if command meta specifies partial matches should not be forwarded + sendError(source, e); + result = CommandResult.SYNTAX_ERROR; + return true; + } + } + // Command does not exist or was not handled on the proxy, let the backend server handle it + result = CommandResult.FORWARDED; + return false; } catch (final Throwable e) { // Ugly, ugly swallowing of everything Throwable, because plugins are naughty. throw new RuntimeException("Unable to invoke command " + parsed.getReader().getString() + " for " + source, e); @@ -262,6 +273,36 @@ private boolean executeImmediately0(final CommandSource source, final ParseResul } } + private void sendError(CommandSource source, CommandSyntaxException e) { + TextComponent.Builder error = Component.text(); + final Message message = e.getRawMessage(); + if (message instanceof ComponentLike componentLike) { + error.append(componentLike.asComponent().applyFallbackStyle(NamedTextColor.RED)); + } else { + error.append(Component.text(message.toString(), NamedTextColor.RED)); + } + + if (e.getInput() != null && e.getCursor() > 0) { + int min = Math.min(e.getInput().length(), e.getCursor()); + TextComponent.Builder details = Component.text() + .color(NamedTextColor.GRAY) + .clickEvent(ClickEvent.suggestCommand("/" + e.getInput())); + + if (min > 10) { + details.append(Component.text("...")); + } + details.append(Component.text(e.getInput().substring(Math.max(0, min - 10), min))); + + if (e.getInput().length() > min) { + details.append(Component.text(e.getInput().substring(min), NamedTextColor.RED, TextDecoration.UNDERLINED)); + } + details.append(Component.translatable("command.context.here", NamedTextColor.RED, TextDecoration.ITALIC)); + error.append(Component.newline(), details); + } + + source.sendMessage(error); + } + @Override public CompletableFuture executeAsync(final CommandSource source, final String cmdLine) { Preconditions.checkNotNull(source, "source"); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandMeta.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandMeta.java index 39b2439999..4d5aac3ba1 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandMeta.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandMeta.java @@ -43,12 +43,14 @@ static final class Builder implements CommandMeta.Builder { private final ImmutableSet.Builder aliases; private final ImmutableList.Builder> hints; private @MonotonicNonNull Object plugin; + private boolean forwardPartial; public Builder(final String alias) { Preconditions.checkNotNull(alias, "alias"); this.aliases = ImmutableSet.builder() .add(alias.toLowerCase(Locale.ENGLISH)); this.hints = ImmutableList.builder(); + this.forwardPartial = false; this.plugin = null; } @@ -83,9 +85,15 @@ public CommandMeta.Builder plugin(Object plugin) { return this; } + @Override + public CommandMeta.Builder forwardPartial(boolean fowardPartial) { + this.forwardPartial = fowardPartial; + return this; + } + @Override public CommandMeta build() { - return new VelocityCommandMeta(this.aliases.build(), this.hints.build(), this.plugin); + return new VelocityCommandMeta(this.aliases.build(), this.hints.build(), this.plugin, this.forwardPartial); } } @@ -126,15 +134,18 @@ public static Stream> copyHints(final CommandMeta met private final Set aliases; private final List> hints; private final Object plugin; + private final boolean forwardPartial; private VelocityCommandMeta( final Set aliases, final List> hints, - final @Nullable Object plugin + final @Nullable Object plugin, + final boolean forwardPartial ) { this.aliases = aliases; this.hints = hints; this.plugin = plugin; + this.forwardPartial = forwardPartial; } @Override @@ -152,6 +163,11 @@ public Collection> getHints() { return plugin; } + @Override + public boolean forwardPartial() { + return forwardPartial; + } + @Override public boolean equals(final Object o) { if (this == o) { diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java index abbfbc422f..28dacbf2ba 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java @@ -26,6 +26,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; +import com.mojang.brigadier.arguments.IntegerArgumentType; import com.mojang.brigadier.arguments.StringArgumentType; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; @@ -233,6 +234,92 @@ void testExecuteAsyncCompletesExceptionallyOnRequirementException() { assertSame(expected, wrapper.getCause()); } + @Test + void testForwardPartialMatch() { + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> fail()) + .then(LiteralArgumentBuilder + .literal("world") + .executes(context -> fail()) + ) + .build(); + + var meta = manager.metaBuilder(new BrigadierCommand(node)).forwardPartial(true).build(); + manager.register(meta, new BrigadierCommand(node)); + + assertForwarded("hello badargument"); + } + + @Test + void testForwardMissingRequirement() { + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> fail()) + .then(LiteralArgumentBuilder + .literal("world") + .executes(context -> fail()) + .requires(src -> false) + ) + .build(); + + var meta = manager.metaBuilder(new BrigadierCommand(node)).forwardPartial(true).build(); + manager.register(meta, new BrigadierCommand(node)); + + assertForwarded("hello world"); + } + + @Test + void testDontForwardHasRequirement() { + final var callCount = new AtomicInteger(); + + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> fail()) + .then(LiteralArgumentBuilder + .literal("world") + .executes(context -> { + callCount.incrementAndGet(); + return 1; + }) + .requires(src -> true) + ) + .build(); + + var meta = manager.metaBuilder(new BrigadierCommand(node)).forwardPartial(true).build(); + manager.register(meta, new BrigadierCommand(node)); + + assertHandled("hello world"); + assertEquals(1, callCount.get()); + } + + @Test + void testDontForwardArgumentParseException() { + final var callCount = new AtomicInteger(); + + final var node = LiteralArgumentBuilder + .literal("number") + .executes(context -> fail()) + .then(RequiredArgumentBuilder + .argument("numberInRange", IntegerArgumentType.integer(1, 5)) + .executes(context -> { + callCount.incrementAndGet(); + return 1; + }) + .requires(src -> true) + ) + .build(); + + var meta = manager.metaBuilder(new BrigadierCommand(node)).forwardPartial(true).build(); + manager.register(meta, new BrigadierCommand(node)); + + assertHandled("number 1"); + assertHandled("number 235"); + assertHandled("number -5"); + assertHandled("number word"); // Is it desirable for this to be forwarded? + assertEquals(1, callCount.get()); + } + // Suggestions @Test