PrismarineJS / minecraft-data

Language independent module providing minecraft data for minecraft clients, servers and libraries.
https://prismarinejs.github.io/minecraft-data
644 stars 218 forks source link

varlong 64-bit integer data type (vs varint 32-bit) in protocol data #119

Open deathcap opened 8 years ago

deathcap commented 8 years ago

http://wiki.vg/Protocol#Data_types

Name Size Encodes Notes
VarInt ≥ 1 ≤ 5 An integer between -2147483648 and 2147483647 Protocol Buffer Varint, encoding a two's complement signed 32-bit integer
VarLong ≥ 1 ≤ 10 An integer between -9223372036854775808 and 9223372036854775807 Protocol Buffer Varint, encoding a two's complement signed 64-bit integer

According to http://wiki.vg/Protocol#World_Border this 'varlong' type is only used in the world_border packet (0x44 in 1.8, 0x35 in 1.9), for the speed field, minecraft-data specifies using a varint:

            {
              "name": "speed",
              "type": [
                "switch",
                {
                  "compareTo": "action",
                  "fields": {
                    "1": "varint",
                    "3": "varint"
                  },
                  "default": "void"
                }
              ]
            },

Should the type here be a varlong instead? Looks like protodef varint will shift up to 64, but this could be problematic since JavaScript can only natively support up to 53-bit, for 64-bit integers I think we would need something like https://www.npmjs.com/package/node-int64. Other platforms using minecraft-data may also need to handle 32-bit vs 64-bit in their own way (no difference with Python since it'll promote automatically, but C could use int32_t or int64_t, etc.)

nickelpro commented 5 years ago

What's the status on this? ProtoDef says that ProtoDef-native varints shouldn't be longer than 5 bytes but Minecraft Varlongs can be up to 10 bytes.

Right now minecraft-data is incorrectly labeling protocol varlongs as varints. It's not a huge issue, but I would like to have this corrected upstream rather than hacking around it on my end. I don't know if minecraft style varlongs and varints belong in ProtoDef, but can we just add them as a custom data type in minecraft-data?

rom1504 commented 5 years ago

What protodef calls varint are varlong. Why do we need varints ?

nickelpro commented 5 years ago

Because if minecraft passes a negative number as a 32-bit varint and it's decoded as a 64-bit varlong you'll have a positive number. If you just assume everything is a 32-bit varint you run the risk of overflowing when decoding varlong

roblabla commented 5 years ago

As I said in ProtoDef: What we need is a way to cast a varlong down. It's fine to decode everything as varlongs, but we'd need to specify what the output type is (u8/16/32/64 or s8/16/32/64). We might be able to do this efficiently by using TypedArrays/Buffers. Basically, read the varint as an u64, then write it to a TypedArray and read it back with an array reader of the correct type.

nickelpro commented 5 years ago

I'm not a JS programmer, my only request is that this is solved in a language agnostic way. The type should be annotated correctly in minecraft-data and then the implementations can hash out what that means to them at their level.

roblabla commented 5 years ago

Sure. The idea is that you'd have ["varnum", "u16"] in protodef for a varshort, or ["varnum", "s16"] for a signed varshort.

xTachyon commented 2 years ago

Any update on this? Treating everything as 64 bits doubles the memory you would need to store a number, and require more instructions to handle on non-64 bits systems, which is really unfortunate.

extremeheat commented 2 years ago

Any update on this? Treating everything as 64 bits doubles the memory you would need to store a number, and require more instructions to handle on non-64 bits systems, which is really unfortunate.

In JS, all the numbers are 64 bit doubles. Aside from arbitrary length BigInt, there are no other numeric types to work with.

xTachyon commented 2 years ago

Any update on this? Treating everything as 64 bits doubles the memory you would need to store a number, and require more instructions to handle on non-64 bits systems, which is really unfortunate.

In JS, all the numbers are 64 bit doubles. Aside from arbitrary length BigInt, there are no other numeric types to work with.

JS is not the only language that exists.

extremeheat commented 2 years ago

JS is not the only language that exists.

I don't see the problem here then. This issue is about JS and the potential for overflow. In other languages like C++ you can treat these directly as an i64. If you use a 64-bit processor there is no performance difference between an i64 and an i32 other than the latter holding less data (yes: on a 32-bit processor you have 1 extra instruction). Memory is also not a concern: packet types only matters when serializing and deserializing. Unless you plan to persistently store thousands of packets in memory, I am not understanding the point here. Maybe for documentation purposes however varlong could be an alias to varint.