Skip to content
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.

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

Fix JSON Parsing to Differentiate Integers and Floats #100454

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Ebbo
Copy link

@Ebbo Ebbo commented Dec 15, 2024

This change enhances JSON compatibility by correctly parsing numeric types, aligning the behavior with expectations for strict JSON type handling.

Data:

{
	"key1": 42,
	"key2": 3.14,
	"key3": 100,
	"key4": 541.,
	"key5": 0.0,
	"key6": 0102
}

Example GDScript:

func _ready() -> void:
	var data = FileAccess.get_file_as_string("res://data.json")

	var json_data = JSON.parse_string(data)
	print(json_data["key1"])
	print(json_data["key2"])
	print(json_data["key3"])
	print(json_data["key4"])
	print(json_data["key5"])
	print(json_data["key6"])

Current:
42.0
3.14
100.0
541.0
0.0
102.0
Patch:
42
3.14
100
541.0
0.0
102

@Ebbo Ebbo requested a review from a team as a code owner December 15, 2024 22:48
@RedMser
Copy link
Contributor

RedMser commented Dec 15, 2024

Thanks for the PR! You need to open a design proposal as well, since this is not a bug fix but rather a (breaking) change in behavior. It's especially important to know what problem you are trying to solve, since JSON itself does not distinguish numbers vs integers to begin with.

@Ebbo
Copy link
Author

Ebbo commented Dec 15, 2024

Thanks for the information! I will have a look at that - I simply realized that parsing json data behaves differently between 4.3 to 4.4. If eg. 100 was parsed it was also used as integer instead of double. Now this behavior changed. That's why I wanted to provide this change.

@RedMser
Copy link
Contributor

RedMser commented Dec 15, 2024

I simply realized that parsing json data behaves differently between 4.3 to 4.4.

Ah gotcha! 4.4 made it so stringifying floats always appends the .0 now, instead of only doing so whenever there's an actual decimal part. This also applies to print(). So I believe it was always parsing floats, but it is made more visible now.

@akien-mga
Copy link
Member

akien-mga commented Dec 16, 2024

{
"key1": 42,
"key2": 3.14,
"key3": 100,
"key4": 541.,
"key5": 0.0,
"key6": 0102
}

This isn't valid JSON, so it's not a proper example, but actually validating it will also show why .0 can't be relied on.

Try to input it in https://jsonlint.com/

"key4": 541.,

This is invalid, JSON doesn't support 541. syntax.

"key6": 0102

This also appears to be invalid, leading zeroes are not supported.

After fixing those two issues, you'll see that jsonlint.com will clean it to:

{
    "key1": 42,
    "key2": 3.14,
    "key3": 100,
    "key4": 541,
    "key5": 0,
    "key6": 102
}

Other tools such as https://codebeautify.org/jsonminifier will minify it to:

{"key1":42,"key2":3.14,"key3":100,"key4":541,"key5":0,"key6":102}

I.e. no trailing .0 for floats.

That's because the JSON specification only supports Number, and doesn't make a distinction between floats and integers. For JSON, 1.0 and 1 are the same Number and the .0 is irrelevant information.

So you can't expect it to be significant and preserve and use it for serialization with Godot, or any engine that needs separate integers and floats.

The change you noticed in 4.4 is that we now always print whole floats with .0 to make it clear they're floats. They were still floats in 4.3, just not printed explicitly as such.

If you want to preserve Godot type information in JSON, the recommended way would be to use the new JSON.from_native and JSON.to_native methods, which encode all Godot types in a way that can be parsed back to the original Variant.

@Chaosus Chaosus added this to the 4.x milestone Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants