Skip to content

Write registry ids for sounds, not id + 1 - #91

Open
Pix3lPirat3 wants to merge 1 commit into
PrismarineJS:mainfrom
Pix3lPirat3:fix/sound-ids-zero-based
Open

Pix3lPirat3 wants to merge 1 commit into
PrismarineJS:mainfrom
Pix3lPirat3:fix/sound-ids-zero-based

Conversation

@Pix3lPirat3

@Pix3lPirat3 Pix3lPirat3 commented Sep 19, 2026 •

Copy link
Copy Markdown

The 1.19.2+ folders added one to every sound id because the packet encodes registry id + 1 (0 = inline). node-minecraft-protocol's registryEntryHolder already hands consumers the registry id and node-minecraft-data indexes sounds by the id field, so mineflayer looked up the neighbouring sound on 1.19.2 - 26.1. Write the registry id in all twelve folders, like the 1.19 and 1.20.4 modules do.


Companion PRs: regenerated data is in PrismarineJS/minecraft-data#1316.

The 1.19.2+ folders added one to every sound id because the packet encodes registry id + 1 (0 = inline). node-minecraft-protocol's registryEntryHolder already hands consumers the registry id and node-minecraft-data indexes sounds by the id field, so mineflayer looked up the neighbouring sound on 1.19.2 - 26.1. Write the registry id in all twelve folders, like the 1.19 and 1.20.4 modules do.

@rom1504 rom1504 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Astra agent review — AI-generated, not manually written by the maintainer.

The producer correction looks sound: raw registry IDs belong in sounds.json; the packet-specific +1 belongs to the registry-holder codec. I verified successful per-version CI jobs at this exact head and compared their emitted sounds.json with #1316 for 1.19.2, 1.20, 1.21.3, 1.21.8 and 26.1: all five match exactly. Production NMP codec → current Mineflayer sound-plugin probes then resolve chicken.hurt correctly across those protocol versions, including 26.1 ID355.

These are specific successful jobs in an overall cancelled matrix, not a claim that the full matrix passed. I found no new code blocker; preserve the companion data publication so existing consumers receive the corrected IDs.

Skills used: prismarine-protocol-data-review separated registry IDs from wire sentinels and checked generated outputs through actual consumers; prismarine-architecture-review placed the normalization in the producer; prismarine-review checked exact-head CI evidence and scope.

@rom1504 rom1504 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Astra agent review — AI-generated, not manually written by the maintainer.

The sound-registry correction remains correct in the reviewed scope: the producer exports zero-based registry IDs, while the codec owns the wire sentinel offset. The earlier exact-head artifacts and production NMP → Mineflayer probes remain applicable; see the existing review.

The remaining landing condition is integration: the refreshed matrix contains failed Build 1.14, 1.20.4, and 1.21 jobs. Diagnose or rerun those jobs before treating the whole generator PR as ready; successful targeted artifacts do not clear the rest of the matrix. I have no new code defect to repeat, and did not rerun Java generation locally.

Skills used: prismarine-review checked the current revision and discussion; prismarine-protocol-data-review checked the version-selected producer/consumer contract; prismarine-architecture-review checked package ownership and integration scope.

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