Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
deathcap opened this issue Jan 25, 2016 · 5 comments
Open

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

deathcap opened this issue Jan 25, 2016 · 5 comments
Labels

Comments

@deathcap
Copy link
Member

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 deathcap added the bug label Feb 13, 2016
@deathcap
Copy link
Member Author

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
Copy link
Member Author

PrismarineJS/minecraft-data#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 deathcap changed the title varshort for node-minecraft-protocol-forge [1.7] varshort for node-minecraft-protocol-forge Feb 19, 2016
@deathcap
Copy link
Member Author

PR'd in ProtoDef-io/node-protodef#52

@deathcap deathcap reopened this Feb 20, 2016
@deathcap
Copy link
Member Author

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 PrismarineJS/node-minecraft-protocol#367)

@rom1504
Copy link
Member

rom1504 commented Feb 20, 2016

@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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants