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

JSON integers are parsed as float #61464

Closed
gustavi opened this issue May 27, 2022 · 16 comments
Closed

JSON integers are parsed as float #61464

gustavi opened this issue May 27, 2022 · 16 comments

Comments

@gustavi
Copy link

gustavi commented May 27, 2022

Godot version

v4.0.alpha8.official [cc3ed63]

System information

Windows 10 64bit - Vulkan API 1.2.141 - GeForce GTX 1660 SUPER

Issue description

int are converted to float with Json.parse() method.

Godot 3.4 and 4.0 affected.

Steps to reproduce

See code in "Minimal reproduction project"

Minimal reproduction project

Based on documentation example (https://docs.godotengine.org/en/latest/classes/class_json.html):

func _ready():
	var data_to_send = ["a", "b", "c", 42]
	print("BEFORE = ", typeof(data_to_send[3]))
	var json = JSON.new()
	var json_string = json.stringify(data_to_send)
	# Save data
	# ...
	# Retrieve data
	var error = json.parse(json_string)
	if error == OK:
		var data_received = json.get_data()
		if typeof(data_received) == TYPE_ARRAY:
			print("AFTER = ", typeof(data_received[3]))
		else:
			print("Unexpected data")
	else:
		print("JSON Parse Error: ", json.get_error_message(), " in ", json_string, " at line ", json.get_error_line())

Output:

BEFORE = 2
AFTER = 3

Godot 3.4 poc:

func _ready():
    var data_to_send = ["a", "b", "c", 42]
    print("BEFORE = ", typeof(data_to_send[3]))
    var result = JSON.parse(JSON.print(data_to_send)).result
    print("AFTER = ", typeof(result[3]))
@Calinou
Copy link
Member

Calinou commented May 27, 2022

As per its specification, JSON does not have an integer or float type. It only has a number type, so all JSON numbers are parsed as floats for compatibility reasons.

@Mickeon
Copy link
Contributor

Mickeon commented May 27, 2022

If it hasn't been noted down already, it should probably be specified in the documentation, even if it... typically doesn't matter much? If assigned to integers, floats are casted automatically.

@gustavi
Copy link
Author

gustavi commented May 27, 2022

To be honest I was not aware of this since all langages I worked with preserve type for numbers. Since it's by design in JSON spec I think we can close this issue.

@Mickeon for the context I'm writing and API description where a type is expected (I use something inspired by JSON Schema) but I'll make a "number" type which match both.

@Calinou
Copy link
Member

Calinou commented May 27, 2022

If it hasn't been noted down already, it should probably be specified in the documentation, even if it... typically doesn't matter much? If assigned to integers, floats are casted automatically.

This is already documented here:

[b]Note:[/b] The JSON specification does not define integer or float types, but only a [i]number[/i] type. Therefore, converting a Variant to JSON text will convert all numerical values to [float] types.

Do you know of other places where this information could be added?

@Mickeon
Copy link
Contributor

Mickeon commented May 27, 2022

Do you know of other places where this information could be added?

Not really, JSON.print() makes note of it, as well as JSONParseResult and parse_json(). Really seems like that's it?

@timothyqiu
Copy link
Member

To be honest I was not aware of this since all langages I worked with preserve type for numbers.

Some languages like Python will make the number a int if there is no decimal point in the JSON text, and will always dump float with a decimal point. But it'll face the same issue if the JSON text comes from another system that does not have this kind of special logic, e.g. JavaScript.

@lyuma
Copy link
Contributor

lyuma commented Apr 18, 2023

Just ran into this one today when parsing glTF node indices and finding out they were all floats.

For example, currently if I attempt to call json_list.has(1) if json_list was parsed from [1, 2, 3], it returns false because json_list decodes to [1.0, 2.0, 3.0]

It is common practice in many JSON libraries to decode values as int64 if they do not contain a decimal place, and provided that the value fits in an int64.

I understand the default behavior can't be changed for compat breakage, but it might be nice to consider adding an option the JSON class to permit decoding numbers as int64.

Should this be an implementation proposal?

@YuriSizov
Copy link
Contributor

YuriSizov commented Apr 18, 2023

Yes, if you think there is a use case for such a flag, you need to make a proposal.

And this issue should be closed by now, as there is nothing left to discuss: JSON doesn't support integers.

I understand the default behavior can't be changed for compat breakage

PS. In this case the default should not be changed because it would produce unpredictable results and violate the specification. Consider an array of floating point values where some of them are perfectly valid integers. You don't expect to get a mixed array out of this as a user.

So the default behavior should be the least unpredictable one.

@YuriSizov YuriSizov closed this as not planned Won't fix, can't repro, duplicate, stale Apr 18, 2023
@testman42
Copy link

I just encountered this issue because I was reading data from JSON file and tried to use it to get values out of a Dictionary.
Despite my if value_from_JSON == 9001: checks, Godot threw an error when trying to get value:
Invalid get index '9001' (on base: 'Dictionary').
As this is closed issue, I wonder, where to continue this discussion in order to come up with something, that will help people avoid this in the future. Is there any relevant proposal open?
Few ideas:

  1. Better error message: if engine errors out, then game stops running and engine probably has time to analyse what went wrong. Could just go through all the keys in dictionary and check their type. If no match is found, make error message say "lol none of the keys are of the type that you just tried to use (float)"
  2. add optional argument to JSON's ˙parse_string() ̇, which when given as true, would convert all floats without decimal places to int.

@AThousandShips
Copy link
Member

AThousandShips commented Dec 15, 2023

No that I know of, but as already established this is not a bug or even lacking details, nothing is broken here, this is as expected, and based on how JSON works

JSON is intended as a format to communicate with different software, so it needs to be standardized, and we need to follow that standard as well as we can in Godot, if you need to store data for Godot only use some other format like ConfigFile, var_to_string, var_to_bytes, etc., they are much better suited for that

Please open a proposal if you are interested in this, otherwise please don't revive this thread as there's nothing further to do here 🙂

@timkrief
Copy link

Hi. I recently encountered this issue. I have 0 as a value in my json file. then I put this value inside an array and I tried to check if 0 was in it. It wasn't. It took me quite some time to find out that the parsing casted it as a float, so 0 wasn't in it, 0.0 was. I think adding a warning or a note in the parse_string function could be useful so that this kind of issues doesn't lead people to this closed issue :) .

@AThousandShips
Copy link
Member

AThousandShips commented Mar 26, 2024

It's documented, and there wouldn't be possible to add a warning for this, it couldn't tell that you didn't really want to do that check, and it can't know what data is there without adding some specific code just for that, as it just knows it's some data

The parsing doesn't cast it to a float, it was a float, JSON only stores floats

@timkrief
Copy link

I see, however I think it could be helpful to mention this. We already mention some specs that can be misleading, for instance csv escape of the doublequotes. "Double quotes within a text value can be escaped by doubling their occurrence." https://docs.godotengine.org/en/stable/classes/class_fileaccess.html#class-fileaccess-method-get-csv-line

@lyuma
Copy link
Contributor

lyuma commented Mar 26, 2024

Personally I think Godot should improve its handling of JSON numbers which are integer. Many other languages parse JSON numbers without decimal points as int64, but this needs to be discussed as a proposal, not a bug issue. Please see here for the proposal:

As for the equality of 0 and 0.0 within an array, I do think that is something that GDScript could improve, but it would be very difficult to do this without compatibility break, so again this is something that can improve in terms of documentation or needs a proposal

@Mickeon
Copy link
Contributor

Mickeon commented Mar 27, 2024

As for the equality of 0 and 0.0 within an array, I do think that is something that GDScript could improve, but it would be very difficult to do this without compatibility break

If all non-typed array methods accounted for this they would be slowed down by a pretty large margin. The simpler workaround involves making use of static typing whenever applicable.

@lyuma
Copy link
Contributor

lyuma commented Mar 27, 2024

the problem is arrays and dictionaries read from JSON won't use static typing, so there is no way to trivially cast a loaded JSON Dictionary into a particular schema

Case in point, if I want to quickly scan the "children" array in a glTF node for a particular node index (integer), that will fail due to this issue because children are parsed as floats

But I think the best mitigation is to implement the above proposal 8830 to allow parsing integers from JSON documents as int64. It will solve most of the common cases of type mismatch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants