fix: send the action ids a protocol maps to names - #4077
Closed
u9g wants to merge 2 commits into
Closed
Conversation
26.1's protocol maps client_command's actionId and use_entity's hand to strings, so writing the numeric id throws "SizeOf error ... is not in the mappings value" and the packet never leaves the client: the bot cannot respawn or right-click an entity there. entity_action has been mapped since 1.21.6, which bed.js's wake() still missed. Needs the two features added in PrismarineJS/minecraft-data#1279.
Contributor
|
Tests more than 1.5x slower than master (durations are noisy, so this is informational): |
Member
Author
|
#4079 is the same fix. If PrismarineJS/minecraft-data#1278 lands, |
… from the schema Replaces the supportFeature checks with a lib/protocol_ids.js helper that asks the schema whether the field is a protodef mapper, so the fix is right whether minecraft-data keeps 26.1's mappers or drops them, and needs no unreleased data. The three tests now resolve the packet out of the server's read path before asserting: throwing inside the packet listener wedged the connection and surfaced as an afterEach timeout on versions whose use_entity carries no hand at all (1.8-1.11), and those versions now only assert that the packet arrives.
Member
Author
|
#4076 (opened first) covers |
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.
On 26.1
client_command.actionIdanduse_entity.handaremapperfields:Writing the numeric id there throws inside the serializer:
The packet is dropped on the floor, so on 26.1 the bot cannot respawn (
bot.respawn(), the death handler, the win_game handler), cannot request stats, andbot.activateEntity()does nothing at all — right-clicking an NPC on a 26.1 server looked like the server ignoring us.entity_actionhas been string-mapped since 1.21.6 andphysics.jsalready handles it, butbed.js'swake()still wroteactionId: 2, which both fails to serialize there and meansstop_sprintingunder the new mapping.game.jsalso wrote{ action: 0 }, a field name that does not exist in any protocol version, so the win_game respawn never serialized anywhere.lib/protocol_ids.jsasks the schema whether the field is a mapper rather than going through a feature, so this is right whether minecraft-data keeps 26.1's mappers (PrismarineJS/minecraft-data#1279) or drops them (PrismarineJS/minecraft-data#1278), and it needs no unreleased data.bed.jskeepssupportFeature('entityActionUsesStringMapper')to matchphysics.js, which already uses it for the same packet.Verified live: with this change
bot.activateEntity()on a 26.1 server's NPC producesuse_entity {target, hand: 'main_hand', location, sneaking}and the server answers, where before nothing went out.Tests: three cases in
test/internalTest.jscovering every tested version. They resolve the packet out of the server's read path before asserting — an assertion thrown inside the packet listener wedges the connection and shows up as anafterEachtimeout — and on 1.8-1.11, whoseuse_entityhas nohandfield at all, the assertion is just that the packet arrives.