Skip to content

fix: write the enum names newer protocols declare for client_command and entity_action - #4076

Closed
u9g wants to merge 1 commit into
PrismarineJS:masterfrom
u9g:fix/string-mapper-enum-writes
Closed

fix: write the enum names newer protocols declare for client_command and entity_action#4076
u9g wants to merge 1 commit into
PrismarineJS:masterfrom
u9g:fix/string-mapper-enum-writes

Conversation

@u9g

@u9g u9g commented Sep 7, 2026

Copy link
Copy Markdown
Member

packet_client_command.actionId is a string mapper from 26.1 and packet_entity_action.actionId from 1.21.6, while older versions declare a plain varint. protodef's interpreted mapper writer has always thrown 0 is not in the mappings value on the numeric form, and since ProtoDef-io/node-protodef#176 the compiled writer throws too, which ends the connection: on 26.1 that takes out bot.respawn() (so a bot never respawns after dying), creative.getStats(), and bot.wake() on any version from 1.21.6.

lib/protocol_enum.js reads which form the loaded version declares. game.js also wrote the field as action, which no version has, so the win_game respawn threw on every version.

Tests: internalTest now asserts the respawn and wake actions match what the version's schema declares, for every tested version.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Tests more than 1.5x slower than master (durations are noisy, so this is informational):

   4197ms ->    9656ms  mineflayer_external 1.11.2v nether
   8247ms ->   14067ms  mineflayer_external 1.19.4v nether
   8457ms ->   15295ms  mineflayer_external 1.21.4v nether
  10012ms ->   15261ms  mineflayer_external 1.21.5v nether

@u9g

u9g commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

#4077 does the same for client_command/entity_action and additionally fixes use_entity.hand, which this PR does not touch: on 26.1 that field is a mapper too, so bot.activateEntity() throws in the serializer and sends nothing. Whichever of the two lands, that hunk should be in it.

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.

1 participant