fix: stabilize item display metadata packing - #408
Open
mvanhorn wants to merge 1 commit into
Open
Conversation
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
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.
Order each Bukkit NMS adapter's reflected
EntityDataAccessorcollection by accessor id before any existing positional selection, keeping the current field mapping and packet-building behavior otherwise unchanged. Apply the same minimal change across all maintained Bukkit NMS versions because these duplicated adapters are required to remain behaviorally equivalent; the Fabric implementation already uses named mixin accessors and is out of scope. Add a focusedv26_R2regression test that checks accessor ordering and the completeTransformationData.pack()output, including unique expected ids and the correct serializer/value category for interpolation delay, integer duration, translation, scale, and rotation. BetterModel 3.3.0 can send an ItemDisplay metadata entry whose field id is the integer interpolation-duration field while its serializer and payload are for a float, causing vanilla 26.2 clients to disconnect with a network protocol error. The reported failure is intermittent under rapid model spawn/despawn load, but the field/type pair and thev26_R2transformation packing path are concrete. Bukkit NMS adapters currently collectEntityDataAccessorfields in rawClass.getDeclaredFields()order and then use positional indexes to select duration, translation, scale, and rotation accessors. That JVM field order is not a supported contract, while the accessors' numeric metadata ids provide the stable ordering the packet format requires.Testing: With reflected
Displayaccessors returned fromaccessors(), assert their ids are strictly ordered so positional selection cannot bind a later float accessor to the interpolation-duration slot; Pack a transformation with duration20and non-default translation, scale, and rotation; assert the duration entry uses the interpolation-duration id and integer serializer/value rather than a float serializer/value.Fixes #405