ProtoDef-io / ProtoDef

ProtoDef specification: describe your protocol, and read it with ease.
37 stars 8 forks source link

Deprecate varint #14

Open roblabla opened 7 years ago

roblabla commented 7 years ago

Varint is really a minecraft-specific datatype that never should have been in ProtoDef to begin with. It is under-specified, and is adding cruft. Furthermore, it created issues like https://github.com/ProtoDef-io/ProtoDef/issues/12 when it wasn't behaving as expected because of the under-specification.

I propose we remove varint.

Saiv46 commented 3 years ago

Actually, varints are also used by ProtoBuf.

roblabla commented 3 years ago

yes, but using protodef to parse protobufs is kinda weird. I expect people using protodef to be parsing custom protocols that are unlikely to use Protocol Buffers. In practice, not many protocols use this specific kind of varint. QUIC for instance, uses a different variable integer scheme where the first two bits are used to decide on the length. See https://github.com/quicwg/base-drafts/blob/f3b76d0adce066194a43a7560284fdc1a78c4ab7/draft-ietf-quic-transport.md#variable-length-integer-encoding-integer-encoding

Because this type is neither obvious nor fundamental, I do believe it should be declared an extension instead of being part of the core specification.

EDIT: if it remains in core specification, having it renamed to pbvarint would at least give a subtle hint as to the kind of varint we're providing.

Saiv46 commented 3 years ago

@rom1504 Please create a repo like ProtoDef-Extensions, so such specific types like varint and int could be moved to

Saiv46 commented 3 years ago

@roblabla How about moving it to https://github.com/Saiv46/protodef-extensions as pbvarint?