Skip to content

Fix #572: Don't forcefully append .0 decimal when calling toJSON #767

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jan 11, 2019
Merged

Fix #572: Don't forcefully append .0 decimal when calling toJSON #767

merged 2 commits into from
Jan 11, 2019

Conversation

@patrikjuvonen
Copy link
Contributor

@patrikjuvonen patrikjuvonen commented Jan 11, 2019

Would be great to know if someone sees any edge problem with this... feels like an awfully ugly fix, but we've seen those ones before haven't we.

The following behaves like before now:

local strData = "[[[1,1.0,1.1,1.123],[12,12.0,12.1,12.123],[123,123.0,123.1,123.123],[1234,1234.0,1234.1,1234.123],[12345,12345.0,12345.1,12345.123],[123456,123456.0,123456.1,123456.123],[1234567,1234567.0,1234567.1,1234567.123],[12345678,12345678.0,12345678.1,12345678.123],[123456789,123456789.0,123456789.1,123456789.123],[1234567890,1234567890.0,1234567890.1,1234567890.123],[12345678901,12345678901.0,12345678901.1,12345678901.123],[123456789012,123456789012.0,123456789012.1,123456789012.123],[1234567890123,1234567890123.0,1234567890123.1,1234567890123.123],[12345678901234,12345678901234.0,12345678901234.1,12345678901234.123],[123456789012345,123456789012345.0,123456789012345.1,123456789012345.123],[1234567890123456,1234567890123456.0,1234567890123456.1,1234567890123456.123],[12345678901234567,12345678901234567.0,12345678901234567.1,12345678901234567.123],[123456789012345678,123456789012345678.0,123456789012345678.1,123456789012345678.123],[1234567890123456789,1234567890123456789.0,1234567890123456789.1,1234567890123456789.123],[12345678901234567890,12345678901234567890.0,12345678901234567890.1,12345678901234567890.123]]]"
local data = {{1,1.0,1.1,1.123},{12,12.0,12.1,12.123},{123,123.0,123.1,123.123},{1234,1234.0,1234.1,1234.123},{12345,12345.0,12345.1,12345.123},{123456,123456.0,123456.1,123456.123},{1234567,1234567.0,1234567.1,1234567.123},{12345678,12345678.0,12345678.1,12345678.123},{123456789,123456789.0,123456789.1,123456789.123},{1234567890,1234567890.0,1234567890.1,1234567890.123},{12345678901,12345678901.0,12345678901.1,12345678901.123},{123456789012,123456789012.0,123456789012.1,123456789012.123},{1234567890123,1234567890123.0,1234567890123.1,1234567890123.123},{12345678901234,12345678901234.0,12345678901234.1,12345678901234.123},{123456789012345,123456789012345.0,123456789012345.1,123456789012345.123},{1234567890123456,1234567890123456.0,1234567890123456.1,1234567890123456.123},{12345678901234567,12345678901234567.0,12345678901234567.1,12345678901234567.123},{123456789012345678,123456789012345678.0,123456789012345678.1,123456789012345678.123},{1234567890123456789,1234567890123456789.0,1234567890123456789.1,1234567890123456789.123},{12345678901234567890,12345678901234567890.0,12345678901234567890.1,12345678901234567890.123}}

print(inspect(toJSON(data)))
print(inspect(fromJSON(toJSON(data))))

print(inspect(fromJSON(strData)))

print(inspect(toJSON(1234567890000)))
print(inspect(fromJSON(toJSON(1234567890000))))

Before json-c update:

[2019-01-11 11:49:59] INFO: "[ [ [ 1, 1, 1.1, 1.123 ], [ 12, 12, 12.1, 12.123 ], [ 123, 123, 123.1, 123.123 ], [ 1234, 1234, 1234.1, 1234.123 ], [ 12345, 12345, 12345.1, 12345.123 ], [ 123456, 123456, 123456.1, 123456.123 ], [ 1234567, 1234567, 1234567.1, 1234567.123 ], [ 12345678, 12345678, 12345678.1, 12345678.123 ], [ 123456789, 123456789, 123456789, 123456789 ], [ 1234567890, 1234567890, 1234567890, 1234567890 ], [ 12345678901, 12345678901, 12345678901.1, 12345678901.123 ], [ 123456789012, 123456789012, 123456789012.1, 123456789012.123 ], [ 1234567890123, 1234567890123, 1234567890123.1, 1234567890123.123 ], [ 12345678901234, 12345678901234, 12345678901234.1, 12345678901234.12 ], [ 123456789012345, 123456789012345, 123456789012345.1, 123456789012345.1 ], [ 1234567890123456, 1234567890123456, 1234567890123456, 1234567890123456 ], [ 1.234567890123457e+16, 1.234567890123457e+16, 1.234567890123457e+16, 1.234567890123457e+16 ], [ 1.234567890123457e+17, 1.234567890123457e+17, 1.234567890123457e+17, 1.234567890123457e+17 ], [ 1.234567890123457e+18, 1.234567890123457e+18, 1.234567890123457e+18, 1.234567890123457e+18 ], [ 1.234567890123457e+19, 1.234567890123457e+19, 1.234567890123457e+19, 1.234567890123457e+19 ] ] ]"
[2019-01-11 11:49:59] INFO: { { 1, 1, 1.1, 1.123 }, { 12, 12, 12.1, 12.123 }, { 123, 123, 123.1, 123.123 }, { 1234, 1234, 1234.1, 1234.123 }, { 12345, 12345, 12345.1, 12345.123 }, { 123456, 123456, 123456.1, 123456.123 }, { 1234567, 1234567, 1234567.1, 1234567.123 }, { 12345678, 12345678, 12345678.1, 12345678.123 }, { 123456789, 123456789, 123456789, 123456789 }, { 1234567890, 1234567890, 1234567890, 1234567890 }, { 12345678901, 12345678901, 12345678901.1, 12345678901.123 }, { 123456789012, 123456789012, 123456789012.1, 123456789012.12 }, { 1234567890123, 1234567890123, 1234567890123.1, 1234567890123.1 }, { 12345678901234, 12345678901234, 12345678901234, 12345678901234 }, { 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14 }, { 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15 }, { 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16 }, { 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17 }, { 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18 }, { 1.2345678901235e+19, 1.2345678901235e+19, 1.2345678901235e+19, 1.2345678901235e+19 } }
[2019-01-11 11:49:59] INFO: { { 1, 1, 1.1, 1.123 }, { 12, 12, 12.1, 12.123 }, { 123, 123, 123.1, 123.123 }, { 1234, 1234, 1234.1, 1234.123 }, { 12345, 12345, 12345.1, 12345.123 }, { 123456, 123456, 123456.1, 123456.123 }, { 1234567, 1234567, 1234567.1, 1234567.123 }, { 12345678, 12345678, 12345678.1, 12345678.123 }, { 123456789, 123456789, 123456789.1, 123456789.123 }, { 1234567890, 1234567890, 1234567890.1, 1234567890.123 }, { 12345678901, 12345678901, 12345678901.1, 12345678901.123 }, { 123456789012, 123456789012, 123456789012.1, 123456789012.12 }, { 1234567890123, 1234567890123, 1234567890123.1, 1234567890123.1 }, { 12345678901234, 12345678901234, 12345678901234, 12345678901234 }, { 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14 }, { 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15 }, { 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16 }, { 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17 }, { 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18 }, { 9.2233720368548e+18, 1.2345678901235e+19, 1.2345678901235e+19, 1.2345678901235e+19 } }
[2019-01-11 11:49:59] INFO: "[ 1234567890000 ]"
[2019-01-11 11:49:59] INFO: 1234567890000

After json-c update:

[2019-01-11 11:53:48] INFO: "[ [ [ 1, 1, 1.1, 1.123 ], [ 12, 12, 12.1, 12.123 ], [ 123, 123, 123.1, 123.123 ], [ 1234, 1234, 1234.1, 1234.123 ], [ 12345, 12345, 12345.1, 12345.123 ], [ 123456, 123456, 123456.1, 123456.123 ], [ 1234567, 1234567, 1234567.1, 1234567.123 ], [ 12345678, 12345678, 12345678.1, 12345678.123 ], [ 123456789, 123456789, 123456789, 123456789 ], [ 1234567890, 1234567890, 1234567890, 1234567890 ], [ 12345678901.0, 12345678901.0, 12345678901.1, 12345678901.123 ], [ 123456789012.0, 123456789012.0, 123456789012.1, 123456789012.123 ], [ 1234567890123.0, 1234567890123.0, 1234567890123.1, 1234567890123.123 ], [ 12345678901234.0, 12345678901234.0, 12345678901234.1, 12345678901234.12 ], [ 123456789012345.0, 123456789012345.0, 123456789012345.1, 123456789012345.1 ], [ 1234567890123456.0, 1234567890123456.0, 1234567890123456.0, 1234567890123456.0 ], [ 1.234567890123457e+16, 1.234567890123457e+16, 1.234567890123457e+16, 1.234567890123457e+16 ], [ 1.234567890123457e+17, 1.234567890123457e+17, 1.234567890123457e+17, 1.234567890123457e+17 ], [ 1.234567890123457e+18, 1.234567890123457e+18, 1.234567890123457e+18, 1.234567890123457e+18 ], [ 1.234567890123457e+19, 1.234567890123457e+19, 1.234567890123457e+19, 1.234567890123457e+19 ] ] ]"
[2019-01-11 11:53:48] INFO: { { 1, 1, 1.1, 1.123 }, { 12, 12, 12.1, 12.123 }, { 123, 123, 123.1, 123.123 }, { 1234, 1234, 1234.1, 1234.123 }, { 12345, 12345, 12345.1, 12345.123 }, { 123456, 123456, 123456.1, 123456.123 }, { 1234567, 1234567, 1234567.1, 1234567.123 }, { 12345678, 12345678, 12345678.1, 12345678.123 }, { 123456789, 123456789, 123456789, 123456789 }, { 1234567890, 1234567890, 1234567890, 1234567890 }, { 12345678901, 12345678901, 12345678901.1, 12345678901.123 }, { 123456789012, 123456789012, 123456789012.1, 123456789012.12 }, { 1234567890123, 1234567890123, 1234567890123.1, 1234567890123.1 }, { 12345678901234, 12345678901234, 12345678901234, 12345678901234 }, { 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14 }, { 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15 }, { 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16 }, { 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17 }, { 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18 }, { 1.2345678901235e+19, 1.2345678901235e+19, 1.2345678901235e+19, 1.2345678901235e+19 } }
[2019-01-11 11:53:48] INFO: { { 1, 1, 1.1, 1.123 }, { 12, 12, 12.1, 12.123 }, { 123, 123, 123.1, 123.123 }, { 1234, 1234, 1234.1, 1234.123 }, { 12345, 12345, 12345.1, 12345.123 }, { 123456, 123456, 123456.1, 123456.123 }, { 1234567, 1234567, 1234567.1, 1234567.123 }, { 12345678, 12345678, 12345678.1, 12345678.123 }, { 123456789, 123456789, 123456789.1, 123456789.123 }, { 1234567890, 1234567890, 1234567890.1, 1234567890.123 }, { 12345678901, 12345678901, 12345678901.1, 12345678901.123 }, { 123456789012, 123456789012, 123456789012.1, 123456789012.12 }, { 1234567890123, 1234567890123, 1234567890123.1, 1234567890123.1 }, { 12345678901234, 12345678901234, 12345678901234, 12345678901234 }, { 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14 }, { 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15 }, { 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16 }, { 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17 }, { 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18 }, { 9.2233720368548e+18, 1.2345678901235e+19, 1.2345678901235e+19, 1.2345678901235e+19 } }
[2019-01-11 11:53:48] INFO: "[ 1234567890000.0 ]"
[2019-01-11 11:53:48] INFO: 1234567890000

After test fix:

[2019-01-11 11:51:07] INFO: "[ [ [ 1, 1, 1.1, 1.123 ], [ 12, 12, 12.1, 12.123 ], [ 123, 123, 123.1, 123.123 ], [ 1234, 1234, 1234.1, 1234.123 ], [ 12345, 12345, 12345.1, 12345.123 ], [ 123456, 123456, 123456.1, 123456.123 ], [ 1234567, 1234567, 1234567.1, 1234567.123 ], [ 12345678, 12345678, 12345678.1, 12345678.123 ], [ 123456789, 123456789, 123456789, 123456789 ], [ 1234567890, 1234567890, 1234567890, 1234567890 ], [ 12345678901, 12345678901, 12345678901.1, 12345678901.123 ], [ 123456789012, 123456789012, 123456789012.1, 123456789012.123 ], [ 1234567890123, 1234567890123, 1234567890123.1, 1234567890123.123 ], [ 12345678901234, 12345678901234, 12345678901234.1, 12345678901234.12 ], [ 123456789012345, 123456789012345, 123456789012345.1, 123456789012345.1 ], [ 1234567890123456, 1234567890123456, 1234567890123456, 1234567890123456 ], [ 1.234567890123457e+16, 1.234567890123457e+16, 1.234567890123457e+16, 1.234567890123457e+16 ], [ 1.234567890123457e+17, 1.234567890123457e+17, 1.234567890123457e+17, 1.234567890123457e+17 ], [ 1.234567890123457e+18, 1.234567890123457e+18, 1.234567890123457e+18, 1.234567890123457e+18 ], [ 1.234567890123457e+19, 1.234567890123457e+19, 1.234567890123457e+19, 1.234567890123457e+19 ] ] ]"
[2019-01-11 11:51:07] INFO: { { 1, 1, 1.1, 1.123 }, { 12, 12, 12.1, 12.123 }, { 123, 123, 123.1, 123.123 }, { 1234, 1234, 1234.1, 1234.123 }, { 12345, 12345, 12345.1, 12345.123 }, { 123456, 123456, 123456.1, 123456.123 }, { 1234567, 1234567, 1234567.1, 1234567.123 }, { 12345678, 12345678, 12345678.1, 12345678.123 }, { 123456789, 123456789, 123456789, 123456789 }, { 1234567890, 1234567890, 1234567890, 1234567890 }, { 12345678901, 12345678901, 12345678901.1, 12345678901.123 }, { 123456789012, 123456789012, 123456789012.1, 123456789012.12 }, { 1234567890123, 1234567890123, 1234567890123.1, 1234567890123.1 }, { 12345678901234, 12345678901234, 12345678901234, 12345678901234 }, { 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14 }, { 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15 }, { 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16 }, { 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17 }, { 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18 }, { 1.2345678901235e+19, 1.2345678901235e+19, 1.2345678901235e+19, 1.2345678901235e+19 } }
[2019-01-11 11:51:07] INFO: { { 1, 1, 1.1, 1.123 }, { 12, 12, 12.1, 12.123 }, { 123, 123, 123.1, 123.123 }, { 1234, 1234, 1234.1, 1234.123 }, { 12345, 12345, 12345.1, 12345.123 }, { 123456, 123456, 123456.1, 123456.123 }, { 1234567, 1234567, 1234567.1, 1234567.123 }, { 12345678, 12345678, 12345678.1, 12345678.123 }, { 123456789, 123456789, 123456789.1, 123456789.123 }, { 1234567890, 1234567890, 1234567890.1, 1234567890.123 }, { 12345678901, 12345678901, 12345678901.1, 12345678901.123 }, { 123456789012, 123456789012, 123456789012.1, 123456789012.12 }, { 1234567890123, 1234567890123, 1234567890123.1, 1234567890123.1 }, { 12345678901234, 12345678901234, 12345678901234, 12345678901234 }, { 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14, 1.2345678901235e+14 }, { 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15, 1.2345678901235e+15 }, { 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16, 1.2345678901235e+16 }, { 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17, 1.2345678901235e+17 }, { 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18, 1.2345678901235e+18 }, { 9.2233720368548e+18, 1.2345678901235e+19, 1.2345678901235e+19, 1.2345678901235e+19 } }
[2019-01-11 11:51:07] INFO: "[ 1234567890000 ]"
[2019-01-11 11:51:07] INFO: 1234567890000

@patrikjuvonen
Fix #572: Don't forcefully append .0 decimal when calling toJSON
6b583a6 
There probably exists a better way to do this...
@patrikjuvonen patrikjuvonen added the bug Something isn't working label Jan 11, 2019
@patrikjuvonen patrikjuvonen added this to the 1.5.7 milestone Jan 11, 2019
@qaisjp
Copy link
Contributor

qaisjp commented Jan 11, 2019

In #572 you said:

According to @ccw808 it indeed has to do with the initially referenced ShouldUseInt math method which initially wasn't made to support doubles, but should be adjusted to support them properly. So this is a separate issue, but good to realize now while messing with the stuff.

Is that no longer a solution?

@patrikjuvonen
Copy link
Contributor Author

patrikjuvonen commented Jan 11, 2019

In #572 you said:

According to @ccw808 it indeed has to do with the initially referenced ShouldUseInt math method which initially wasn't made to support doubles, but should be adjusted to support them properly. So this is a separate issue, but good to realize now while messing with the stuff.

Is that no longer a solution?

That's a different issue:

{
    123456789,    -- becomes 123456789
    123456789.0,  -- becomes 123456789
    123456789.1,  -- becomes 123456789
    123456789.123 -- becomes 123456789
},
{
    1234567890,    -- becomes 1234567890
    1234567890.0,  -- becomes 1234567890
    1234567890.1,  -- becomes 1234567890
    1234567890.123 -- becomes 1234567890
}

Not related to the regression, but needs to be fixed (separate issue though). This was an issue also before the json-c update, which I only found when testing the original .0 decimal issue.

@qaisjp
Copy link
Contributor

qaisjp commented Jan 11, 2019 •
edited
Loading

That's a different issue:
...
Not related to the regression, but needs to be fixed (separate issue though). This was an issue also before the json-c update, which I only found when testing the original .0 decimal issue.

Ah, ok. 👍

I can't really see the difference between before update and after update, but since the difference is only a trailing .0 (i.e "[ 1234567890000.0 ]" vs "[ 1234567890000 ]") imo I don't think this really needs to be fixed. My reasoning for this is:

The JSON data is the same, it doesn't matter that the string representation is different. As long as data in machine form (after parsing) is identical, it should be fine.

However, since we're hoping to move away from json-c, we might as well apply your fix just to keep consistency until that big update. That way we can deal with any changes all in one go.

What do you think?

tl;dr looks fine to merge. what do you think?

@patrikjuvonen
Copy link
Contributor Author

patrikjuvonen commented Jan 11, 2019

I agree that there is no need to fix it, and in my opinion the issue is trivial at best and could even be closed.

I'll leave the decision of merging this PR and closing the original issue to your discretion. In my opinion it is ok if we leave it as it is in the current release build -- but if it affects some few developers still today, we might as well merge it in and worry about these issues once we swap out the json-c library.

@qaisjp qaisjp requested a review from a team January 11, 2019 14:10
@qaisjp qaisjp merged commit 400c920 into multitheftauto:master Jan 11, 2019
@patrikjuvonen patrikjuvonen deleted the github-issue-572 branch February 12, 2019 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ccw808 ccw808 ccw808 approved these changes

+1 more reviewer

@qaisjp qaisjp qaisjp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

1.5.7

Development

Successfully merging this pull request may close these issues.

3 participants

@patrikjuvonen @qaisjp @ccw808