Dinnerbone / mcstatus

A Python class for checking the status of an enabled Minecraft server
http://dinnerbone.com/minecraft/tools/status/
1.11k stars 146 forks source link

Fix varint reading and writing. Closes #141 #142

Closed CoolCat467 closed 3 years ago

CoolCat467 commented 3 years ago

Make varint reading and writing functions use C int 32's with ctypes module. Important: Writing converts value to unsigned_int32, so that signed bit get forced to turn into a part of the number for writing, sort of breaking the number so python handles it like Java and C would. Reading converts final value back to signed_int32 so that the number is correct when python tries to use it.

CoolCat467 commented 3 years ago

I think your tests are not working quite right my friend. Your first test my change failed is looking for the value 34359738367, which is 2 35 - 1. This value is far larger than 2 32 - 1. The second test my change failed is looking for the code to fail at values greater than 2 35 - 1, in particular it's feeding it exactly 2 35, 1 off from the previous bugged limit. 32 bit signed integers can only represent integers from -2147483648 to 2147483647.

kevinkjt2000 commented 3 years ago

Of course, you may change the tests to be correct. When I say that I can't accept code without tests passing, I mean that if the tests can't pass I can't run the release script which runs the tests:

https://github.com/Dinnerbone/mcstatus/blob/5320c870c59cf1d52d8e986f0da2e8a8b55aa4e5/release.sh#L4

CoolCat467 commented 3 years ago

144 should fix tests to look for feasible values and also test negative varints

kevinkjt2000 commented 3 years ago

Released in v6.1.2 Thanks for the contribution ❤️