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

Make varint switchable between signed and unsigned #2

Closed
hansihe opened this issue Mar 27, 2016 · 5 comments
Closed

Make varint switchable between signed and unsigned #2

hansihe opened this issue Mar 27, 2016 · 5 comments

Comments

@hansihe
Copy link
Member

hansihe commented Mar 27, 2016

The original ProtoBuf varint implementation seems to be unsigned, while the Minecraft one is signed. We should at the very least add a flag that decides if the varint is signed or not to improve compatibility with other protocols.

@rom1504
Copy link
Member

rom1504 commented Apr 2, 2016

kind of related : ProtoDef-io/node-protodef#51

@roblabla
Copy link
Contributor

roblabla commented Apr 2, 2016

I'm very much for the deprecation of varint in ProtoDef's core spec, and moving it to minecraft-specific datatype (or a separate ProtoDef extension).

Also, protodef's "varint" can encode up to 64-bit by default. Even if a Protocol Definition is int32, on the wire this information is lost. As such, varint should, in theory, support 64-bit by default (relevant: #1).

@hansihe
Copy link
Member Author

hansihe commented Apr 2, 2016

How about having a core spec, and then additional extension specs providing more specialized or less useful types? This could also apply to #1.

If this is a good way of going at it, should extension specs be optional or manditory? How would a spec declare usage of an extension?

@roblabla
Copy link
Contributor

roblabla commented Apr 2, 2016

Protocol files already declare what types are necessary for it to function properly ("i16": "native").

Some protocols might require types that are missing for the given language. For instance, if we take minecraft-protocol, the target language probably doesn't already define nbt datatype. Does that mean we should create an extension spec for nbt ?

The spec defines what a certain (native) type outputs for cross-language operability. This allows usage of its output inside switch cases, etc... What it outputs to the user of the protocol file doesn't really matter too much. What matters is that a given protocol file is internally consistent.

The question, then, is whether having implementation-defined types as part as the core spec a good idea or not. My guess is "probably not", since those still don't allow cross-language internally consistent protocol files.

Having "extension specs" however doesn't really help. I think the idea is that this spec will provide a cross-language, lowest-common-denominator core from which people can build more specialized stuff. If someone wants to define "nbt" and what its (protocol-internal) output is, so he can switch on it, he can do so. Then implementors just have to respect his spec.

@roblabla
Copy link
Contributor

Closing because #14. I intend to deprecate varint from the core spec, it's a minecraft-specific thing.

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

No branches or pull requests

3 participants