node-minecraft-protocol-forge icon indicating copy to clipboard operation
node-minecraft-protocol-forge copied to clipboard

[1.7] varshort for node-minecraft-protocol-forge

Open deathcap opened this issue 9 years ago • 5 comments

http://wiki.vg/Minecraft_Forge_Handshake#Definitions

Minecraft Forge uses an additional type not covered in data types for some things -- a varshort. This is essentially the same as a regular short, except that if the top byte (what is normally sign) is set, it is followed by an additional byte. This allows forge to retain backwards compatibility but extend the length of certain numbers -- the varshort is only used in places where, in vanilla Minecraft, the sign bit would not have been set. It is implemented in ByteBufUtils.

http://wiki.vg/Minecraft_Forge_Handshake#Differences_from_Forge_1.7.10

Forge for Minecraft 1.8 made some changes from the 1.7.10 version. The most important thing to keep track of isn't (entirely) a forge change. In 1.7.10, plugin channel packets are length prefixed, while in 1.8 they are not. However, forge makes some more changes to the server to client packet (but not the client to server packet): Rather than using a short for the length, a varshort is used. Due to the way that varshorts work, this is backwards compatible, but if this is not accounted for you may receive forge packets that seem corrupted as there is an extra byte that appears seemingly randomly. Warning: While it may seem like you can get away with not handling varshorts, on servers with lots of mods (EG FTB), this will appear.

http://wiki.vg/Minecraft_Forge_Handshake#Definitions

https://github.com/MinecraftForge/MinecraftForge/blob/ebe9b6d4cbc4a5281c386994f1fbda04df5d2e1f/src/main/java/net/minecraftforge/fml/common/network/ByteBufUtils.java#L58-L89

deathcap avatar Jan 25 '16 16:01 deathcap

Tagging as "bug" because of wiki.vg's warning "While it may seem like you can get away with not handling varshorts…" (which node-minecraft-protocol-forge isn't, and has gotten away with so far, but I haven't tested it on servers with lots of mods)

deathcap avatar Feb 13 '16 08:02 deathcap

https://github.com/PrismarineJS/minecraft-data/pull/117 adds a short prefix to 1.7 - to resolve this issue, would need to change it to varshort (not yet implemented). Doesn't apply to 1.8+ because plugin channel packets do not have a data length prefix (data is restBuffer, length is implied by the packet length)

deathcap avatar Feb 19 '16 02:02 deathcap

PR'd in https://github.com/roblabla/ProtoDef/pull/52

deathcap avatar Feb 19 '16 22:02 deathcap

To see about moving varshort here, since it is Forge-specific, but would need some way to alter the protocol specification before it is passed to the serializer (related https://github.com/PrismarineJS/node-minecraft-protocol/issues/367)

deathcap avatar Feb 20 '16 20:02 deathcap

@deathcap it is possible using https://github.com/PrismarineJS/node-minecraft-protocol/blob/master/examples/client_custom_packets/client_custom_packets.js ;)

(just give the "custom" packet the same name as the old one)

rom1504 avatar Feb 20 '16 20:02 rom1504