From 11036b6345a5b3b5f7a34cadbc1b5e9eefbfddd6 Mon Sep 17 00:00:00 2001 From: willkroboth <46540330+willkroboth@users.noreply.github.com> Date: Tue, 14 Nov 2023 10:24:20 -0500 Subject: [PATCH] Make sure arguments are preserved after redirects See https://github.com/Mojang/brigadier/pull/138#issuecomment-1807144181 --- .../mojang/brigadier/CommandDispatcher.java | 1 + .../context/CommandContextBuilder.java | 5 +++++ .../brigadier/CommandDispatcherTest.java | 17 ++++++++++++++++ .../brigadier/CommandSuggestionsTest.java | 20 +++++++++++++++++++ .../brigadier/context/CommandContextTest.java | 13 ++++++++++++ 5 files changed, 56 insertions(+) diff --git a/src/main/java/com/mojang/brigadier/CommandDispatcher.java b/src/main/java/com/mojang/brigadier/CommandDispatcher.java index 69a2a83d..b7d2dd1f 100644 --- a/src/main/java/com/mojang/brigadier/CommandDispatcher.java +++ b/src/main/java/com/mojang/brigadier/CommandDispatcher.java @@ -328,6 +328,7 @@ private ParseResults parseNodes(final CommandNode node, final StringReader reader.skip(); if (child.getRedirect() != null) { final CommandContextBuilder childContext = new CommandContextBuilder<>(this, source, child.getRedirect(), reader.getCursor()); + childContext.withArguments(context.getArguments()); final ParseResults parse = parseNodes(child.getRedirect(), reader, childContext); context.withChild(parse.getContext()); return new ParseResults<>(context, parse.getReader(), parse.getExceptions()); diff --git a/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java b/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java index 1af2cb83..1f290c6d 100644 --- a/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java +++ b/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java @@ -50,6 +50,11 @@ public CommandContextBuilder withArgument(final String name, final ParsedArgu return this; } + public CommandContextBuilder withArguments(Map> arguments) { + this.arguments.putAll(arguments); + return this; + } + public Map> getArguments() { return arguments; } diff --git a/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java b/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java index ff81f41b..69c0c84b 100644 --- a/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java +++ b/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java @@ -10,6 +10,7 @@ import com.mojang.brigadier.context.CommandContext; import com.mojang.brigadier.context.CommandContextBuilder; import com.mojang.brigadier.exceptions.CommandSyntaxException; +import com.mojang.brigadier.tree.ArgumentCommandNode; import com.mojang.brigadier.tree.LiteralCommandNode; import com.mojang.brigadier.tree.RootCommandNode; import org.hamcrest.CustomMatcher; @@ -409,6 +410,22 @@ public void testRedirectModifierEmptyResult() throws CommandSyntaxException { assertThat(result, is(0)); // No commands executed, so result is 0 } + @Test + public void testRedirectPreservesPreviousArguments() throws CommandSyntaxException { + final LiteralCommandNode ending = literal("ending") + .executes(context -> context.getArgument("number", int.class)).build(); + final ArgumentCommandNode lowNumber = argument("number", integer(1, 10)) + .then(ending).build(); + final ArgumentCommandNode highNumber = argument("number", integer(11, 20)) + .redirect(lowNumber).build(); + subject.register(literal("base") + .then(literal("low").then(lowNumber)) + .then(literal("high").then(highNumber))); + + assertThat(subject.execute("base low 5 ending", source), is(5)); + assertThat(subject.execute("base high 15 ending", source), is(15)); + } + @Test public void testExecuteOrphanedSubcommand() { subject.register(literal("foo").then( diff --git a/src/test/java/com/mojang/brigadier/CommandSuggestionsTest.java b/src/test/java/com/mojang/brigadier/CommandSuggestionsTest.java index 80018f49..6487ec87 100644 --- a/src/test/java/com/mojang/brigadier/CommandSuggestionsTest.java +++ b/src/test/java/com/mojang/brigadier/CommandSuggestionsTest.java @@ -253,6 +253,26 @@ public void getCompletionSuggestions_redirect_lots() throws Exception { assertThat(result.getList(), equalTo(Lists.newArrayList(new Suggestion(StringRange.at(33), "loop")))); } + @Test + public void getCompletionSuggestions_redirectPreservesArguments() throws Exception { + subject.register(literal("command") + .then( + argument("first", integer()) + .then( + argument("second", integer()) + .suggests((context, builder) -> { + builder.suggest(String.valueOf(context.getLastChild().getArgument("first", int.class) + 1)); + return builder.buildFuture(); + }) + ) + )); + subject.register(literal("redirect").redirect(subject.getRoot())); + + testSuggestions("command 1 ", 10, StringRange.at(10), "2"); + testSuggestions("redirect command 1 ", 19, StringRange.at(19), "2"); + testSuggestions("redirect redirect command 1 ", 28, StringRange.at(28), "2"); + } + @Test public void getCompletionSuggestions_execute_simulation() throws Exception { final LiteralCommandNode execute = subject.register(literal("execute")); diff --git a/src/test/java/com/mojang/brigadier/context/CommandContextTest.java b/src/test/java/com/mojang/brigadier/context/CommandContextTest.java index 95d58a05..bcf363a4 100644 --- a/src/test/java/com/mojang/brigadier/context/CommandContextTest.java +++ b/src/test/java/com/mojang/brigadier/context/CommandContextTest.java @@ -14,6 +14,9 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import java.util.HashMap; +import java.util.Map; + import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.mock; @@ -51,6 +54,16 @@ public void testGetArgument() throws Exception { assertThat(context.getArgument("foo", int.class), is(123)); } + @Test + public void testGetArguments() throws Exception { + Map> arguments = new HashMap<>(); + arguments.put("foo", new ParsedArgument<>(0, 1, 123)); + arguments.put("bar", new ParsedArgument<>(0, 1, "123")); + final CommandContext context = builder.withArguments(arguments).build("123"); + assertThat(context.getArgument("foo", int.class), is(123)); + assertThat(context.getArgument("bar", String.class), is("123")); + } + @Test public void testSource() throws Exception { assertThat(builder.build("").getSource(), is(source));