godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.8k stars 20.91k forks source link

JSON Parser modifies long numbers #24119

Open gokudomatic opened 5 years ago

gokudomatic commented 5 years ago

Godot version: Version 3.0.6 stable official

OS/device including version: Windows 10 Home 64 NVidia 1060 gtx

Issue description: When a Json string containing a long number of 17 characters or more, it modifies the last numbers after the 16th position and replace them with 0, except for the 17th character which seems to be rounded in a strange way. For instance, the text {"sessionid":"2178736780526359416"} will return for sessionid the value 2178736780526359300 When the number is a string, with quotes, the problem doesn't occur.

Steps to reproduce: run the following code: print(JSON.parse('{"sessionid":2178736780526359412}').result.sessionid) It will return 2178736780526359300

Calinou commented 5 years ago

Make sure it's not the print() function rounding the number on display. For instance, try setting the result of JSON.parse('{"sessionid":2178736780526359412}').result.sessionid as the text of a label:

$Label.text = str(JSON.parse('{"sessionid":2178736780526359412}').result.sessionid)
gokudomatic commented 5 years ago

Make sure it's not the print() function rounding the number on display. For instance, try setting the result of JSON.parse('{"sessionid":2178736780526359412}').result.sessionid as the text of a label:

$Label.text = str(JSON.parse('{"sessionid":2178736780526359412}').result.sessionid)

I tested your code right now and it displays the same wrong result in the label. The debugger's inspector evaluates also the same wrong value.

girng commented 5 years ago

w10, 64-bit, gtx 950: $Label.text = str(JSON.parse('{"sessionid":2178736780526359412}').result.sessionid) gives me: 2178736780526359296

eon-s commented 5 years ago

Ubuntu, Godot 3.0.6

print(JSON.parse('{"sessionid":2178736780526359412}'
    ).result.sessionid)

Shows 2178736780526359296

This

print(2178736780526359412)

Prints the correct number

And this

print(JSON.parse('{"sessionid":2178736780526359412}'
    ).result.sessionid == 2178736780526359412)

Says true.

:upside_down_face:

bojidar-bg commented 5 years ago

Probably related to this comment: https://github.com/godotengine/godot/blob/aa311320d92b2982dd4606156882686cc354e551/core/ustring.cpp#L1898-L1901

DevinPentecost commented 5 years ago

Hi,

I just ran into this same issue, caused a lot of headache and I had to create a weird workaround as a solution. Can we confirm the bug? Doesn't need to be fixed immediately but I'd like to have it added to the list. I'm sure other people will run into it in the future.

If you need more information to show it's reproduction, I can help with that, but it looks like the comments above have it covered.

Thanks

ThakeeNathees commented 4 years ago

Probably related to this comment: https://github.com/godotengine/godot/blob/aa311320d92b2982dd4606156882686cc354e551/core/ustring.cpp#L1898-L1901

this comment is only valid when mantissa > 18 and (decimal point)decPt < 18, unless we can't just ignore the trailing digits. but fixing this cause overflow and the number become negative, I thing we should document the maximum value it could hold if we haven't already.

usix79 commented 10 months ago

The issue is still reproduced in Version 4.1.1 stable official

AThousandShips commented 10 months ago

I'm not sure if this is a bug, 18 digits of precision is beyond the capacity of double precision floating points

This is indeed simply a limitation, if you want to load this as an integer you will first have to parse it as a string and then convert that go an integer

This should be made a little more clear in the documentation though

Calinou commented 10 months ago

I'm not sure if this is a bug, 18 digits of precision is beyond the capacity of double precision floating points

Indeed, the value range for a precision of 1.0 with doubles is ±9,007,199,254,740,992, which is 16 digits. With single-precision floats, it's ±16,777,216 (7 digits).

This is indeed simply a limitation, if you want to load this as an integer you will first have to parse it as a string and then convert that go an integer

I wonder if Godot could do this automatically for integers outside the 1.0 precision range of a double-precision float. This would come at a performance cost when parsing or writing such large numbers, but it's likely better than rounding the integer in an unexpected way.

usix79 commented 10 months ago

In my case, the issue reproduced when receiving an int64 number from the server.

An automatic conversion of long numbers to strings before parsing could be a viable solution.

Alternatively, an exception would be acceptable behavior, as it would allow for proper error handling.

Either way, a transparent approach would be greatly appreciated to prevent such silent data alterations.

AThousandShips commented 10 months ago

JSON only using float is documented (4.x) and documented (3.x)

lyuma commented 9 months ago

This is technically not a bug since the behavior is documented, and brute-force "fixing" this bug without a proposal lead to compat breakage.

Therefore, I have opened a feature implementation proposal at godotengine/godot-proposals#8830 to prevent data loss caused by long -> float conversion in the JSON parser and serializer.

Let me know what you all think. I think the fact that the official Java org.json implementation supports lossless int64 (Java long type) in this way is a strong argument in favor of adding this functionality, but we should continue the discussion in the proposal.