Fix ClientboundEntityUpdateAttributes key mapper: missing attributes shift every subsequent attribute's name - #1254
Conversation
…shift every subsequent attribute's name
extremeheat
left a comment
There was a problem hiding this comment.
Automated review from Codex, requested by @extremeheat.
The overall diagnosis and most of this patch look correct, but I found one remaining registry-order issue that should be fixed before merging.
For both Minecraft 1.21.3 and 1.21.4, vanilla registers the final entries in this order:
- 29: sweeping_damage_ratio
- 30: tempt_range
- 31: water_movement_efficiency
The proposed maps omit tempt_range and place water_movement_efficiency at ID 30. Because water_movement_efficiency is actually ID 31, it would still decode incorrectly in those versions. This also means the maps do not yet exactly match those versions' attributes.json files, which include tempt_range.
Vanilla references:
Please add tempt_range immediately before water_movement_efficiency in the 1.21.3 and 1.21.4 proto.yml files and regenerate/update their protocol.json files accordingly.
Everything else I checked against the extracted vanilla sources looks good, including the 1.21.5 camera_distance removal and the 1.21.6+ ordering.
Fix ClientboundEntityUpdateAttributes key mapper: missing attributes shift every subsequent attribute's name
Problem
packet_entity_update_attributes (Java Edition's ClientboundEntityUpdateAttributes) sends each entity attribute as a VarInt id plus a value and modifiers. Since Minecraft 1.20.5, that id is a plain sequential registration-order index into the game's attribute registry — it is not a stable enum.
For every PC version from 1.21.1+, the mapper for this list was missing several attributes that exist in the real game registry:
• burning_time, explosion_knockback_resistance (added early in 1.21)
• mining_efficiency, movement_efficiency, oxygen_bonus, sneaking_speed, submerged_mining_speed, sweeping_damage_ratio, water_movement_efficiency (added in 1.21)
• 1.21.5 additionally had a bogus camera_distance entry that doesn't exist in that version's real registry at all.
Because the mapper is just a flat, indexed list with no version-range checks, every attribute registered after the first gap gets silently mislabeled — the packet still decodes fine (no parse error, no panic), it just reports the wrong attribute name and the correct value/modifiers attached to it. Concretely, on the affected versions, the real generic.movement_speed index was being read back as generic.step_height (1.21.1/1.21.3/1.21.4) or generic.scale (1.21.5 through 1.21.11/latest/26.1) — meaning any consumer reading attributes by name got either the wrong attribute's data, or nothing, for every attribute at or past generic.max_health in the registry.
Root cause
The mapper lists in proto.yml are hand-curated and were evidently never updated in lockstep with the game's own attribute registry as new attributes were added across the 1.21+ line. This is distinct from attributes.json, a separate reference data file (name/min/max/default, not used by the packet decoder) that — for every affected version — already lists every attribute in the correct, real registration order. That gave a reliable, in-repo source of truth to correct the mapper against, without needing to trust any external decompiled-source claims.
Fix
For each affected version, the key: varint => list under packet_entity_update_attributes in proto.yml was reordered/extended to exactly match that version's own attributes.json order (with the minecraft: prefix stripped). Two rules were followed throughout:
The one exception to rule 1: 1.21.5's fabricated camera_distance entry (which doesn't exist in that version's real registry) was removed and replaced with the real attribute at that slot, burning_time — this is a data-correctness fix, not a rename, since camera_distance was never a real 1.21.5 attribute.
Why these versions, and not others
Every PC version's protocol definition for this packet was checked to establish the exact boundary of what could be affected:
• Through 1.20.4: the attribute key is sent as name: string — a literal resource-location string, not an index. There's no sequential mapper at all, so this class of bug is structurally impossible.
• 1.20.5 / 1.20.6 (they share one proto.yml): this is where the index-based key: varint => mapper was introduced, but its attribute list is already complete for that version — none of the attributes missing in the 1.21.x bug existed in the game yet. Not affected.
• 1.21 / 1.21.1, 1.21.3, 1.21.4, 1.21.5, 1.21.6, 1.21.8, 1.21.9, 1.21.10, 1.21.11: affected, per above. (1.21 shares 1.21.1's files; 1.21.10 shares 1.21.9's.)
• latest (and therefore 26.1, and any future version until the next branch-off): has the identical 4-entry gap as the 1.21.6+ group. Left unfixed, every future version bump would inherit the same bug, so it's included even though the original bug report only checked through 1.21.11.
• No proto/protocol files exist for any version between 1.21.11 and 26.1, so there's no gap in coverage there.