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

Add varshort data type #52

Closed
wants to merge 2 commits into from

Conversation

@deathcap
Copy link

commented Feb 19, 2016

Extended 'short' type, see http://wiki.vg/Minecraft_Forge_Handshake#Definitions

With this PR and minecraft-data 1.7 plugin_channel 0x3f updated to varshort, able to login to an FTBInfinityServer-2.3.5-1.7.10 server (although it later crashes due to use of other custom packets)

ref PrismarineJS/node-minecraft-protocol-forge#7

Closes PrismarineJS/node-minecraft-protocol-forge#1 (couldn't make the change in nmp-f because it requires changing the protocol definition in minecraft-data unfortunately, but it is backwards-compatible with vanilla 1.7)

@rom1504

This comment has been minimized.

Copy link
Member

commented Feb 20, 2016

@deathcap

This comment has been minimized.

Copy link
Author

commented Feb 20, 2016

Added

@rom1504

This comment has been minimized.

Copy link
Member

commented Feb 20, 2016

I'm not 100% sure whether this should be in ProtoDef or in nmp though. Does this exist anywhere but in minecraft protocol ?

@rom1504

This comment has been minimized.

Copy link
Member

commented Feb 20, 2016

Now that nmp has a customPackets option, this could even move to nmp-forge.

We can always move things to nmp or protodef if it's useful in general anyway.

Or can you think of an other thing that forge that would use this ?

@rom1504

This comment has been minimized.

Copy link
Member

commented Feb 20, 2016

(trying not to bloat protodef with too many default types)

@roblabla

This comment has been minimized.

Copy link
Collaborator

commented Feb 20, 2016

stop

Could we please have generalized varnum that takes the maximum number of bytes as arguments ? Instead of bloating protodef with more and more and more types that do exactly the same thing...

@deathcap

This comment has been minimized.

Copy link
Author

commented Feb 20, 2016

No varshort is completely different than varint/varlong despite the similar name. It's backwards-compatible with short for positive values, and for negative adds an extra byte.

But to rom's point, I think it would make sense to move this to nmp-forge, as that is all that uses it (Forge invented this "varshort" data type): PrismarineJS/node-minecraft-protocol-forge#1 (though I'll need some means to modify the 0x3f packet definition before passed to the serializer)

deathcap added a commit to PrismarineJS/node-minecraft-protocol-forge that referenced this pull request Mar 6, 2016
@rom1504

This comment has been minimized.

Copy link
Member

commented Aug 24, 2019

seems like we haven't needed this
if someone need this one day, he can reopen

@rom1504 rom1504 closed this Aug 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.