diff --git a/src/main/java/com/mojang/brigadier/tree/CommandNode.java b/src/main/java/com/mojang/brigadier/tree/CommandNode.java index 15b584aa..c522c31e 100644 --- a/src/main/java/com/mojang/brigadier/tree/CommandNode.java +++ b/src/main/java/com/mojang/brigadier/tree/CommandNode.java @@ -128,13 +128,18 @@ public boolean equals(final Object o) { if (!children.equals(that.children)) return false; if (command != null ? !command.equals(that.command) : that.command != null) return false; - - return true; + // Because the primary use of a redirect is to be able to create commands that effectively loop, + // we can't do a standard equality check on them as we could very well end up with a stack overflow + // if a branch loops back to this node. However, they are still important here as two nodes that + // only differ by where they redirect to should not be considered the same node... + // + // Hence, we do a reference equality check. + return redirect == that.redirect; } @Override public int hashCode() { - return 31 * children.hashCode() + (command != null ? command.hashCode() : 0); + return 31 * children.hashCode() + (command != null ? command.hashCode() : 0) + System.identityHashCode(redirect); } public Predicate getRequirement() { diff --git a/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java b/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java index e7525e55..55ad255b 100644 --- a/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java +++ b/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java @@ -15,8 +15,10 @@ import org.junit.Test; import static com.mojang.brigadier.arguments.IntegerArgumentType.integer; +import static com.mojang.brigadier.builder.LiteralArgumentBuilder.literal; import static com.mojang.brigadier.builder.RequiredArgumentBuilder.argument; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.mock; @@ -87,6 +89,13 @@ public void testEquals() throws Exception { .testEquals(); } + @Test + public void testNodesWithDifferentRedirectsAreNotEqual() throws Exception { + final CommandNode redirectTarget = argument("baz", integer()).build(); + assertThat(argument("foo", integer()).redirect(redirectTarget).build(), not(argument("foo", integer()).build())); + assertThat(literal("foo").redirect(redirectTarget).build(), not(literal("foo").build())); + } + @Test public void testCreateBuilder() throws Exception { final RequiredArgumentBuilder builder = node.createBuilder();