Fix varlong serialization

[valaphee]

readVarLong uses an int instead of a long for its value. This PR also removes the multiplications.

Original PR GeyserMC/PacketLib#48

[Tim203]

It seems like our current (and your changes) of at least the readVarInt is wrong.
The if (size > 5) and if (size > 35) should become >=. This is because of the & 0x80) = 0x80 bit in combination with the placement of the size check, as it currently allows varints with 6 bytes instead of the max 5. I personally btw prefer the old (length must be <= 5).
Maybe we can replace this with a do while loop? I personally think that the current while condition makes it unnecessarily hard to read.

[valaphee]

The implementation is correct, it has to be greater-than as a VarInt should be able to consist out of 35, or 70 bits to be able to fully represent 32 bit or 64 bit. (and therefore has to run 5 iterations, at the 6th iteration it should fail, which it doe)

Using bits has just the slight advantage of not having to multiply by 7 each iteration, even though it's definitely not a bottleneck.

[Tim203]

You're right that the max length of a varint is 5 bytes to represent 4 bytes. But look at the condition in the while.
The body of the while is only executed when there is at least one more byte to read, because it checks if the continue bit is set.
This means that the total bytes it can read is the while body + 1 (because the continue bit is not set for the final byte). Resulting in this method accepting a max length of 6 bytes instead of 5.

This is why I recommend switching to a do while loop variant, because it has to read at least 1 byte and continue until there is no continue bit set.

[Konicai]

Some unit tests would be nice

[valaphee]

I mean you are theoretically right, but even the implementation Minecraft uses in both Bedrock and Java Edition post and prior Netty rewrite has this flaw.

byte b;
do {
	b = buf.readByte(); // will also do the 6th iteration
	i |= (b & 127) << j++ * 7;
	if (j > 5) {
		throw new RuntimeException("VarInt too big");
	}
} while (shouldContinueRead(b));

But I agree, it shouldn't consume the extra byte (even though it doesn't match the Vanilla impl) and I'll add a unit test

[valaphee]

The writeVarLong method seems to be also wrong
0x80_80_80_80_80_80_80_80 should not be encoded as 0x80_81_82_84_88_90_e0_40_80_01 (as the 0x40 byte misses the marker, etc.)

But there are no serverbound varlong writes at least