Implement signed byte conversion for checksum to fix #3765 #1442
Conversation
|
@copilot Review ? (idk if it works like that) |
There was a problem hiding this comment.
Pull request overview
This PR implements signed byte conversion for chat checksums to fix issue #3765. The Minecraft 1.21.5+ protocol expects checksums to be signed bytes (range -128 to 127) rather than unsigned bytes (range 0-255), which was causing protocol validation issues.
- Adds a
toSignedByteutility function to convert unsigned bytes to signed bytes - Applies the conversion to the checksum field in chat command packets for version 1.21.5+
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const toSignedByte = (value) => { | ||
| const byte = value & 0xff | ||
| return byte > 127 ? byte - 256 : byte | ||
| } |
There was a problem hiding this comment.
Consider moving the toSignedByte function to src/datatypes/checksums.js and exporting it alongside computeChatChecksum. This would improve code organization by grouping related checksum functionality together and make the function reusable if needed elsewhere. The function is specifically designed to handle the signed byte requirement for checksums in the Minecraft protocol.
There was a problem hiding this comment.
@copilot coauthor pr #1442 based on this this feedback
|
Thanks |
| argumentSignatures: canSign ? signaturesForCommand(command, options.timestamp, options.salt, options.preview, acknowledgements) : [], | ||
| messageCount: client._lastSeenMessages.pending, | ||
| checksum: computeChatChecksum(client._lastSeenMessages), // 1.21.5+ | ||
| checksum: toSignedByte(computeChatChecksum(client._lastSeenMessages)), // 1.21.5+ |
There was a problem hiding this comment.
This branch specifically covers the chat commands only. Is this intentional? Your linked issue PrismarineJS/mineflayer#3765 seems to hint at normal chat messages too
|
It's also already clamped here https://github.com/PrismarineJS/node-minecraft-protocol/blob/master/src/datatypes/checksums.js#L16 |
2 participants
Fix PrismarineJS/mineflayer#3765