Add redirects to the equality checks for nodes.

They are added as reference equality checks rather than standard object equality checks because of a stack overflow that can occur if a redirect is to a parent of the current node. If two nodes are redirected to two different nodes that otherwise look the same, then they will not be equal, which is fine because if one of the nodes gains a new child then they won't be equal any more - so this is a sensible check to have.

Test included to demonstrate the change.
This commit is contained in:
Daniel Naylor 2021-04-25 13:05:01 +01:00
parent 559d8f3972
commit 073ee9aa29
2 changed files with 17 additions and 3 deletions

View file

@ -128,13 +128,18 @@ public abstract class CommandNode<S> implements Comparable<CommandNode<S>> {
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<S> getRequirement() {

View file

@ -15,8 +15,10 @@ import org.junit.Before;
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 class ArgumentCommandNodeTest extends AbstractCommandNodeTest {
.testEquals();
}
@Test
public void testNodesWithDifferentRedirectsAreNotEqual() throws Exception {
final CommandNode<Object> 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<Object, Integer> builder = node.createBuilder();