Skip to content

Fix TypedKey equals/hashCode symmetry with Key#14025

Open
gunjanjaswal wants to merge 2 commits into
PaperMC:mainfrom
gunjanjaswal:fix/13678-typedkey-equals-symmetry
Open

Fix TypedKey equals/hashCode symmetry with Key#14025
gunjanjaswal wants to merge 2 commits into
PaperMC:mainfrom
gunjanjaswal:fix/13678-typedkey-equals-symmetry

Conversation

@gunjanjaswal

Copy link
Copy Markdown
Contributor

Summary

Fixes #13678.

TypedKeyImpl is a record implementing Key, but it never overrides equals/hashCode, so Java generates them from both record components and only matches another TypedKeyImpl. Adventure's KeyImpl.equals compares any Key by namespace + value, so equality is one-directional:

Key key = Key.key("minecraft:stone");
TypedKey<ItemType> typedKey = TypedKey.create(RegistryKey.ITEM, key);
key.equals(typedKey);   // true
typedKey.equals(key);   // false

That breaks the symmetry clause of Object.equals, and the two hash differently, so a TypedKey can go missing in a HashSet<Key> built from plain keys, and contains/remove turn order dependent when both types are mixed.

This overrides equals/hashCode on TypedKeyImpl:

  • Against another TypedKey, compare underlying key and registry key (same as before, so keys from different registries stay distinct).
  • Against a plain Key, compare namespace + value to match adventure and stay symmetric.
  • hashCode returns the underlying key's hash so equal keys share a hash.

One tradeoff: a TypedKey now equals its plain Key, so two TypedKeys with the same key but different registries stay unequal while both equal that plain key, which can break transitivity in that mixed case. That is inherent to TypedKey extends Key (Effective Java item 10). This follows the approach suggested in the issue; if you'd rather have strict transitivity, I can make equality key-only instead.

Changes

  • TypedKeyImpl: override equals/hashCode.
  • Add TypedKeyEqualityTest covering symmetry, hashCode consistency, and registry distinction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Awaiting review

Development

Successfully merging this pull request may close these issues.

Equivalence between KeyImpl and TypedKeyImpl is not symmetric

1 participant