Repository navigation
Conversation
UnorderedPairMap stored values in a HashMap<Int, V> keyed by the combined hash of the two keys. Two distinct pairs whose combined hashes collide therefore shared one entry: put() overwrote the earlier pair's value, get() returned the wrong pair's value, and remove() on one pair evicted the other. PhysicsWorld keeps active collisions in this map, so a hash collision between two entity pairs silently dropped a real collision and onCollision under- reported. Identity hash codes vary by JVM, platform, and allocation history, which is why this surfaced as an intermittent, platform-specific collision count rather than a deterministic failure. Key the map by an UnorderedPair that implements equals as well as hashCode, keeping the existing order-independent hash. Collisions now land in the same bucket and are separated by equals, as HashMap intends. Adds a regression with a key type whose hashCode is constant, so every pair collides, and asserts distinct pairs stay distinct through get, values and remove. Also labels the two PhysicsWorldTest collision assertions so a future failure names which strategy and which phase produced the wrong count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoGxx6nHtCDdeiQrTEWXFz
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1484
The map is now keyed by an
UnorderedPairthat implementsequalsas well ashashCode, keeping the same order-independent hash, so colliding pairs share a bucket but stay separate entries.Why
UnorderedPairMapstored values in aHashMap<Int, V>keyed by the combined hash of the two keys, so two distinct pairs whose hashes collided shared one entry.put()overwrote,get()returned the other pair's value, andremove()evicted the other pair.PhysicsWorldkeeps active collisions in this map, which is why the collision count came up short intermittently and only on some platforms.Tests
UnorderedPairMapTestgets a regression using a key type with a constanthashCode, so every pair collides, and checksget,valuesandremovekeep pairs distinct.PhysicsWorldTestcollision assertions now carry a label naming the strategy, so a future failure says which one.Locally on JDK 25:
UnorderedPairMapTest3/3,PhysicsWorldTest14/14.Pull Request (PR) prerequisites
Discussed and confirmed in #1484 before opening. No refactoring beyond the fix and its tests.