From f329f927e76689a6bddc4839d8d6ea2c3c8465f2 Mon Sep 17 00:00:00 2001 From: Nathan Adams Date: Tue, 16 Jan 2018 13:36:25 +0100 Subject: [PATCH] Improved the speed of redirected commands --- .../mojang/brigadier/CommandDispatcher.java | 35 ++++++++++--------- .../brigadier/builder/ArgumentBuilder.java | 5 ++- .../brigadier/context/CommandContext.java | 13 ++++++- .../context/CommandContextBuilder.java | 5 ++- .../brigadier/CommandDispatcherTest.java | 2 +- .../CommandDispatcherUsagesTest.java | 4 +-- .../builder/ArgumentBuilderTest.java | 9 ++--- 7 files changed, 43 insertions(+), 30 deletions(-) diff --git a/src/main/java/com/mojang/brigadier/CommandDispatcher.java b/src/main/java/com/mojang/brigadier/CommandDispatcher.java index 805580c..fb532aa 100644 --- a/src/main/java/com/mojang/brigadier/CommandDispatcher.java +++ b/src/main/java/com/mojang/brigadier/CommandDispatcher.java @@ -1,7 +1,6 @@ package com.mojang.brigadier; import com.google.common.collect.Iterables; -import com.google.common.collect.Iterators; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -92,29 +91,33 @@ public class CommandDispatcher { int successfulForks = 0; boolean forked = false; boolean foundCommand = false; - final Deque> contexts = new ArrayDeque<>(); - contexts.add(parse.getContext()); + final Deque> contexts = new ArrayDeque<>(8); + final String command = parse.getReader().getString(); + contexts.add(parse.getContext().build(command)); while (!contexts.isEmpty()) { - final CommandContextBuilder builder = contexts.removeFirst(); - final CommandContextBuilder child = builder.getChild(); - final CommandContext context = builder.build(parse.getReader().getString()); + final CommandContext context = contexts.removeFirst(); + final CommandContext child = context.getChild(); if (child != null) { if (!child.getNodes().isEmpty()) { foundCommand = true; - final RedirectModifier modifier = Iterators.getLast(builder.getNodes().keySet().iterator()).getRedirectModifier(); - final Collection results = modifier.apply(context); - if (results.size() > 1) { - forked = true; - } - for (final S source : results) { - contexts.add(child.copy().withSource(source)); + final RedirectModifier modifier = context.getRedirectModifier(); + if (modifier == null) { + contexts.add(child); + } else { + final Collection results = modifier.apply(context); + if (results.size() > 1) { + forked = true; + } + for (final S source : results) { + contexts.add(child.copyFor(source)); + } } } - } else if (builder.getCommand() != null) { + } else if (context.getCommand() != null) { foundCommand = true; try { - final int value = builder.getCommand().run(context); + final int value = context.getCommand().run(context); result += value; consumer.onCommandComplete(context, true, value); successfulForks++; @@ -128,7 +131,7 @@ public class CommandDispatcher { } if (!foundCommand) { - consumer.onCommandComplete(parse.getContext().build(parse.getReader().getString()), false, 0); + consumer.onCommandComplete(parse.getContext().build(command), false, 0); throw ERROR_UNKNOWN_COMMAND.createWithContext(parse.getReader()); } diff --git a/src/main/java/com/mojang/brigadier/builder/ArgumentBuilder.java b/src/main/java/com/mojang/brigadier/builder/ArgumentBuilder.java index ea2ad15..a8ef68f 100644 --- a/src/main/java/com/mojang/brigadier/builder/ArgumentBuilder.java +++ b/src/main/java/com/mojang/brigadier/builder/ArgumentBuilder.java @@ -6,7 +6,6 @@ import com.mojang.brigadier.tree.CommandNode; import com.mojang.brigadier.tree.RootCommandNode; import java.util.Collection; -import java.util.Collections; import java.util.function.Predicate; public abstract class ArgumentBuilder> { @@ -14,7 +13,7 @@ public abstract class ArgumentBuilder> { private Command command; private Predicate requirement = s -> true; private CommandNode target; - private RedirectModifier modifier = s -> Collections.singleton(s.getSource()); + private RedirectModifier modifier = null; protected abstract T getThis(); @@ -57,7 +56,7 @@ public abstract class ArgumentBuilder> { } public T redirect(final CommandNode target) { - return redirect(target, modifier); + return redirect(target, null); } public T redirect(final CommandNode target, final RedirectModifier modifier) { diff --git a/src/main/java/com/mojang/brigadier/context/CommandContext.java b/src/main/java/com/mojang/brigadier/context/CommandContext.java index 04346f2..1f01b1f 100644 --- a/src/main/java/com/mojang/brigadier/context/CommandContext.java +++ b/src/main/java/com/mojang/brigadier/context/CommandContext.java @@ -3,6 +3,7 @@ package com.mojang.brigadier.context; import com.google.common.collect.Iterables; import com.google.common.primitives.Primitives; import com.mojang.brigadier.Command; +import com.mojang.brigadier.RedirectModifier; import com.mojang.brigadier.tree.CommandNode; import java.util.Map; @@ -15,8 +16,9 @@ public class CommandContext { private final Map, StringRange> nodes; private final StringRange range; private final CommandContext child; + private final RedirectModifier modifier; - public CommandContext(final S source, final String input, final Map> arguments, final Command command, final Map, StringRange> nodes, final StringRange range, final CommandContext child) { + public CommandContext(final S source, final String input, final Map> arguments, final Command command, final Map, StringRange> nodes, final StringRange range, final CommandContext child, final RedirectModifier modifier) { this.source = source; this.input = input; this.arguments = arguments; @@ -24,6 +26,11 @@ public class CommandContext { this.nodes = nodes; this.range = range; this.child = child; + this.modifier = modifier; + } + + public CommandContext copyFor(final S source) { + return new CommandContext<>(source, input, arguments, command, nodes, range, child, modifier); } public CommandContext getChild() { @@ -88,6 +95,10 @@ public class CommandContext { return result; } + public RedirectModifier getRedirectModifier() { + return modifier; + } + public StringRange getRange() { return range; } diff --git a/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java b/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java index dac2c0b..d84f4cd 100644 --- a/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java +++ b/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java @@ -3,6 +3,7 @@ package com.mojang.brigadier.context; import com.google.common.collect.Maps; import com.mojang.brigadier.Command; import com.mojang.brigadier.CommandDispatcher; +import com.mojang.brigadier.RedirectModifier; import com.mojang.brigadier.tree.CommandNode; import java.util.Map; @@ -15,6 +16,7 @@ public class CommandContextBuilder { private Command command; private CommandContextBuilder child; private StringRange range; + private RedirectModifier modifier = null; public CommandContextBuilder(final CommandDispatcher dispatcher, final S source, final int start) { this.dispatcher = dispatcher; @@ -48,6 +50,7 @@ public class CommandContextBuilder { public CommandContextBuilder withNode(final CommandNode node, final StringRange range) { nodes.put(node, range); this.range = StringRange.encompassing(this.range, range); + this.modifier = node.getRedirectModifier(); return this; } @@ -87,7 +90,7 @@ public class CommandContextBuilder { } public CommandContext build(final String input) { - return new CommandContext<>(source, input, arguments, command, nodes, range, child == null ? null : child.build(input)); + return new CommandContext<>(source, input, arguments, command, nodes, range, child == null ? null : child.build(input), modifier); } public CommandDispatcher getDispatcher() { diff --git a/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java b/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java index c6122c5..e20bc95 100644 --- a/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java +++ b/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java @@ -250,7 +250,7 @@ public class CommandDispatcherTest { @Test public void testExecuteRedirectedMultipleTimes() throws Exception { subject.register(literal("actual").executes(command)); - subject.register(literal("redirected").redirect(subject.getRoot(), Collections::singleton)); + subject.register(literal("redirected").redirect(subject.getRoot())); final String input = "redirected redirected actual"; final ParseResults parse = subject.parse(input, source); diff --git a/src/test/java/com/mojang/brigadier/CommandDispatcherUsagesTest.java b/src/test/java/com/mojang/brigadier/CommandDispatcherUsagesTest.java index 20c19a8..a95ee5d 100644 --- a/src/test/java/com/mojang/brigadier/CommandDispatcherUsagesTest.java +++ b/src/test/java/com/mojang/brigadier/CommandDispatcherUsagesTest.java @@ -90,11 +90,11 @@ public class CommandDispatcherUsagesTest { ); subject.register( literal("j") - .redirect(subject.getRoot(), Collections::singleton) + .redirect(subject.getRoot()) ); subject.register( literal("k") - .redirect(get("h"), Collections::singleton) + .redirect(get("h")) ); } diff --git a/src/test/java/com/mojang/brigadier/builder/ArgumentBuilderTest.java b/src/test/java/com/mojang/brigadier/builder/ArgumentBuilderTest.java index e7501d5..a84d6db 100644 --- a/src/test/java/com/mojang/brigadier/builder/ArgumentBuilderTest.java +++ b/src/test/java/com/mojang/brigadier/builder/ArgumentBuilderTest.java @@ -3,9 +3,6 @@ package com.mojang.brigadier.builder; import com.mojang.brigadier.tree.CommandNode; import org.junit.Before; import org.junit.Test; -import org.mockito.Mockito; - -import java.util.Collections; import static com.mojang.brigadier.arguments.IntegerArgumentType.integer; import static com.mojang.brigadier.builder.LiteralArgumentBuilder.literal; @@ -37,7 +34,7 @@ public class ArgumentBuilderTest { @Test public void testRedirect() throws Exception { final CommandNode target = mock(CommandNode.class); - builder.redirect(target, Collections::singleton); + builder.redirect(target); assertThat(builder.getRedirect(), is(target)); } @@ -45,13 +42,13 @@ public class ArgumentBuilderTest { public void testRedirect_withChild() throws Exception { final CommandNode target = mock(CommandNode.class); builder.then(literal("foo")); - builder.redirect(target, Collections::singleton); + builder.redirect(target); } @Test(expected = IllegalStateException.class) public void testThen_withRedirect() throws Exception { final CommandNode target = mock(CommandNode.class); - builder.redirect(target, Collections::singleton); + builder.redirect(target); builder.then(literal("foo")); }