Skip to content

Compare block state property values with equals, not reference identity - #5122

Merged
leijurv merged 1 commit into
cabaletta:1.21.4from
pirateben820:fix/blockstate-property-equality
Sep 16, 2026
Merged

leijurv merged 1 commit into
cabaletta:1.21.4from
pirateben820:fix/blockstate-property-equality

Conversation

@pirateben820

@pirateben820 pirateben820 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

What

BuilderProcess.sameBlockstate compares each property's value with !=:

Map<Property<?>, Comparable<?>> map1 = first.getValues();
Map<Property<?>, Comparable<?>> map2 = second.getValues();
for (Property<?> prop : map1.keySet()) {
    if (map1.get(prop) != map2.get(prop)

The values are boxed Comparables, so this is reference identity. It is fine for EnumProperty and
BooleanProperty, whose values are singletons. For an IntegerProperty it happens to work today because every
vanilla integer property stays inside the -128..127 range the JVM's Integer cache interns — I checked: the largest,
age, tops out at 25. Nothing in the comparison guarantees that, and identity is not what is meant here; equality is.

Objects.equals to match the three other comparisons in this file. Plain equals would also be correct, since both
states share one StateDefinition and the lookups cannot return null.

This branch is only reached when buildIgnoreDirection or buildIgnoreProperties is set (both default off);
otherwise the early first.equals(second) return is taken.

Verification

Compiles on this branch (./gradlew compileJava, Gradle 8.7, Temurin JDK 21.0.12.1). GitHub's own runs are waiting on first-contributor approval. Not run in game - it is a one-line change to a comparison.

Authorship

This patch and this description were written by an AI assistant (Claude, via Claude Code) working under my direction,
while I was debugging my own mod, which drives Baritone from a language model. I am submitting it and I answer for it.
If the project would rather not take AI-written changes, say so and I will close it.

🤖 Generated with Claude Code

sameBlockstate compares each property's value with !=. The values come out of
BlockState.getValues() as boxed Comparable, so this is reference identity. It
is correct for enum and Boolean properties, whose values are singletons, and
for an IntegerProperty it happens to work today because every vanilla integer
property stays inside the -128..127 range the JVM's Integer cache interns
(the largest, age, tops out at 25). Nothing in the comparison guarantees that,
and identity is not what is meant here; equality is.

Objects.equals to match the three other comparisons in this file. Plain
equals would also be correct: both states share one StateDefinition, so the
lookups cannot return null.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@pirateben820
pirateben820 force-pushed the fix/blockstate-property-equality branch from a31caee to 4aa223d Compare September 16, 2026 14:01
@pirateben820

Copy link
Copy Markdown
Contributor Author

Retargeted to 1.21.4 as requested on #5123. The change had to be re-expressed rather than rebased: on 1.21.4 the comparison is map1.get(prop) != map2.get(prop) over getValues(), so it is Objects.equals there. The description now describes that code; the earlier one described the 26.2 shape. Compiles on 1.21.4 (Gradle 8.7, JDK 21).

@leijurv
leijurv merged commit 35f9f6f into cabaletta:1.21.4 Sep 16, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants