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

parsing Vector2/3 array from and to a json file without need to str2var and var2str #11866

Closed
blurymind opened this issue Oct 5, 2017 · 23 comments

Comments

@blurymind
Copy link

blurymind commented Oct 5, 2017

I had to deal with this recently and was surprised to find out that godot needs to convert any vector2/3 variables to strings with var2str before writing them to json file and vice versa.

It becomes a bigger pain in the neck, when your variables are inside an array that is inside another and mix with other type variables that are not a problem to write to json directly. I had to construct for loops to convert any vector2's contained within an array within an array into stringified vector2s. Then recreate the entire original array structure and put the stringified vector2s in placeof the vector2's...

Then json only takes dictionaries , so I had to further put my array inside a dictionary.

Is there a particular reason why var2str is not just done automatically by the function that parses them to the json string? And also str2var when reading them from the json string?

Feature request:
It would save quite a few lines of code to just let godot do that- detect that its a damned vector2/3 and turn it into a var2str-inged string automatically by the json r/w parsers!!
It would in my opinion take away ALOT of complexity from gdscript. Recreating an array structure to write to json can be very labour intensive and it can leave you with a lot of code that is double the effort to amend

It would be nice if it could do it on a mixed array that contains both vector2/3s and strings.
I guess the complexity there would be in having arrays within arrays. Having to make the json parser go through the entire hierarchy, find vector2s and vector3s and apply a var2str operation on them - to turn them into something appropriate for the json file

@blurymind
Copy link
Author

blurymind commented Oct 6, 2017

To add to injury, I found that this detail is not documented in the json parser and there is even wrong information in the community as to how to handle it:
#11438 (comment)

Here is an example of a guy who gave a broken code example to another guy:
https://www.reddit.com/r/godot/comments/4t25y1/string_to_vector2/
arr = string.split(','), then arr[0] for x and arr[1] for y
because he didn't know that str2var and var2str must be used in this case

@Zylann
Copy link
Contributor

Zylann commented Oct 7, 2017

I had to deal with this in another engine and the thing is... Deserializing a vector from json is possible only if you know it's a vector3, no matter how hard you try. You could set up some kind of convention but then the fact you use JSON limits you. Guessing object types straight from json is impossible, unless you provide a schema (like, which structure and destination types are expected)

@Zylann
Copy link
Contributor

Zylann commented Oct 7, 2017

I also didn't know Godot did that stringification on non-json-translatable objects... Well, what I said above applies, then. Anything non-json that you serialize into json will become non-standard to parse back, unless a schema you provide can do the translation in a standard way.

@connorjclark
Copy link

connorjclark commented Oct 16, 2017

Guessing object types straight from json is impossible, unless you provide a schema (like, which structure and destination types are expected)

The cleanest way I've seen it handled is introducing a __class property when serializing objects.

@blurymind
Copy link
Author

blurymind commented Oct 16, 2017

@Zylann right now godot saves the vector2 values as "(123,123)" strings in the json file by default, which is useless if you later want to load them in godot. You have to first convert them to str2var in order to make them useful.
My request here is to eliminate the need to do that and simply add it as an option to dict.parse_json(text,true) (where true tells the parser to write any Vector 2 values in the dictionary as "Vector2(123,123)" instead of "(123,123)")

Right now I have to literally recreate the entire dictionary and run all vector2 through var2str before I can parse it to json :/ This is crazy!

Then when we read from the json file, godot can recognise all vector2 strings automatically with a regular expression and convert them to vector2. You already have str2var, my request is to simply also incorporate it in the to_json function - optionally if you want

We can keep str2var and var2str, but also add their functionality in the parse_json and to_json methods! That will result in infinitely less hassle when dealing with saving these values and loading them!!

@Zylann
Copy link
Contributor

Zylann commented Oct 16, 2017

@blurymind what about Colors? Vector2? Vector3? Basis? Litterally any type which is not JSON? Sure, you could indeed have an option to load and save "special" JSON that Godot will recognize, but that won't be JSON anymore since you reserve special values to be Godot things, which raises the question "why would you use JSON then". This format isn't mandatory, it's popular and easy but, you don't have to use it specifically for that. That said, I guess it's fine as long as standard JSON can still properly be parsed.

@blurymind
Copy link
Author

blurymind commented Oct 16, 2017

@Zylann thus why I suggest we make it an optional parameter in both parse_json and to_json methods :)

@bojidar-bg
Copy link
Contributor

Wouldn't it be better if to_json/parse_json save Vector2 as a dictionary instead of string?

{
    "x": 123,
    "y": 345
}

Or, if we must have an explicit type there:

{
    "type": "Vector2",
    "x": 123,
    "y": 345
}

Colors would be saved as r/g/b/a, Basis as either x/y/z vectors or as xx/xy/xz/../zz, StringArray would be converted to Array of String, and so on.

@Zylann
Copy link
Contributor

Zylann commented Oct 17, 2017

@bojidar-bg whatever convention is provided in-engine, please keep it an option ;)
This issue got me interested into making a JSON serialization utility script to do what @blurymind expects^^ Which basically would process all JSON and transform any string (value or key) into its potential typed equivalent

@blurymind what was your use case initially? Are you trying to talk to a web server or it a local savegame?

@raymoo
Copy link
Contributor

raymoo commented Oct 17, 2017

Seems like a bug that parsing gives back something other than the original data. Godot should just error if it can't represent a value unambiguously.

@blurymind
Copy link
Author

blurymind commented Oct 17, 2017 via email

@Zylann
Copy link
Contributor

Zylann commented Oct 18, 2017

FYI, this is the printed representation of types in 2.1.4:

Vector2: (0, 0)
Vector3: (0, 0, 0)
Color: (0, 0, 0, 1)
Quat: 0,0,0,1 (no brackets and no spaces)
Matrix3: ((1, 0, 0), (0, 1, 0), (0, 0, 1))
Rect2: (0, 0, 0, 0)
AABB: 0, 0, 0 - 0, 0, 0 (no brackets, and minus sign in the middle)

Note that these representations were chosen to be readable, not easily parsable from a file ;)

@blurymind
Copy link
Author

blurymind commented Oct 18, 2017 via email

@raymoo
Copy link
Contributor

raymoo commented Oct 18, 2017

The way I see it, parse/to_json is meant for conversion between strings and a variant representation of JSON. So to_json should error on anything that doesn't represent a JSON value and there should be a separate set of functions meant to serialize/deserialize variants to JSON, and which deserializing a serialized value gives back something like the original. (I say "like" because it probably would only be implemented to support tree structures. It is possible to support cyclic connections and I would prefer if it did.)

@Zylann
Copy link
Contributor

Zylann commented Oct 18, 2017

@raymoo I started sketching a GDScript helper that does this. But the problem with cyclic connections is that they just can't be translated into pure JSON, or you will either loop infinitely or loose data. Conventions exist to do it (like formatting into a database-like representation where all references are associated to IDs to emulate pointers, I've done this in my own engine https://github.com/Zylann/SnowfeetEngine/blob/master/doc/Scenes.md#scene-format), but at this point JSON becomes a transport format rather than a 1:1 format, because an additional data processing is required.
But if it were to be raised to this point in complexity, I would question the point of using JSON in the first place, to be honest...

@raymoo
Copy link
Contributor

raymoo commented Oct 18, 2017

That's why I said it should be a separate function, because it's no longer 1:1 with JSON.

@Zylann
Copy link
Contributor

Zylann commented Oct 19, 2017

Here is a GDScript helper in the meantime (written in 2.1.4): Serialize.zip
It should support the usual extra types to be encoded to and from JSON, even if they are used as dictionary keys. I've done very little testing though, feel free to improve it if you crave that feature :p
Keep in mind that it will slow down parsing a lot by design (even if ported to C++).

@akien-mga akien-mga changed the title parsing Vector2/3 array from and to a json file without need to str2var and var2str - feature request parsing Vector2/3 array from and to a json file without need to str2var and var2str Jan 15, 2018
@vicguedez
Copy link

vicguedez commented Sep 27, 2019

Just bumped into this using 3.1. I think bojidar's idea would be a more explicit (maybe better) way of storing godot types on JSON.

{
    "type": "Vector2",
    "x": 123,
    "y": 345
}

Edit:

For other people that might be looking at this, I'm using this in any dict or array that I'm storing/sending over network.

NOTE: This is a temporary solution. Theese are recursive functions, so only use them directly on variables or root level array/dict.


func array_json(array : Array, encode : bool) -> Array:
	var json_array = []
	
	for value in array:
		var type : int = typeof(value)
		var new_value
		
		if type == TYPE_ARRAY:
			new_value = array_json(value, encode)
		elif type == TYPE_DICTIONARY:
			new_value = dict_json(value, encode)
		else:
			new_value = var2str(value) if encode == true else str2var(value)
		
		json_array.append(new_value)
	
	return json_array

func dict_json(dictionary : Dictionary, encode : bool) -> Dictionary:
	var json_dict = {}
	
	for key in dictionary:
		var value = dictionary[key]
		var type : int = typeof(value)
		var new_value
		
		if type == TYPE_ARRAY:
			new_value = array_json(value, encode)
		elif type == TYPE_DICTIONARY:
			new_value = dict_json(value, encode)
		else:
			new_value = var2str(value) if encode == true else str2var(value)
		
		json_dict[key] = new_value
	
	return json_dict


@Xrayez
Copy link
Contributor

Xrayez commented Nov 1, 2019

See my implementation in C++: #33241. The original use case was to allow to serialize dictionaries inside dictionaries (as keys) when using inst2dict and dict2inst recursively, but I think I've managed to generalize this.

@xix-xeaon
Copy link

I've wasted way too much time figuring this out already, which is kind of the opposite of what I want from a game engine. The current JSON implementation is faulty and this fact should be made clear in the documentation until it's fixed. Additionally, var2str should be clearly recommended in the documentation.

  1. When searching for "GDScript JSON" you find the class but it has a non standard interface and the helper functions (to_json and parse_json) are not mentioned at all.
  2. Neither the functions nor the class describe any behavior for non JSON datatypes such as Vector2 etc.
  3. The actual behavior is to turn the Vector2 into a nondescript string "(0, 0)" like it's Aprils Fools day. An error is preferred since the string representation is essentially useless anyway.
  4. The correct (minimal) behavior is to turn it into an array like [0, 0]. It can't be recovered to Vector2 etc but it can actually be used after reading the JSON without special parsing.
  5. Since this is a game engine, Vector2 and others are central enough that it makes sense to extend JSON with support for these types like others have mentioned {"__class":"Vector2", "x":0, "y":0} or similar. But it is also true that JSON is JSON and maybe something else should be used instead when we desire to preserve datatypes not actually supported by JSON.
  6. In the documentation for JSON it says "Useful for serializing data to store or send over the network." but this is not generally good advice, as long as it does not support Vector2 etc which are rather central for game data.
  7. var2str, on the other hand, is directly comparable to JSON in that it's simple, handles dicts, arrays etc, human readable and looks the same for the standard datatypes, but it also handles Vector2 etc. It also has a binary version. These functions should be recommended anywhere it says JSON as the superior alternative unless you actually need JSON because you have to talk to some other system which only speaks JSON. var2str etc should have the "Useful for serializing.." advice.

@Xrayez
Copy link
Contributor

Xrayez commented Dec 14, 2019

  1. Since this is a game engine, Vector2 and others are central enough that it makes sense to extend JSON with support for these types like others have mentioned {"__class":"Vector2", "x":0, "y":0} or similar. But it is also true that JSON is JSON and maybe something else should be used instead when we desire to preserve datatypes not actually supported by JSON.

The problem with this approach is that the JSON parser would also have to be rewritten as well, but if you just use the proper serializing procedures internally (var2str/str2var), adapting JSON writer is the only thing which needs a major overhaul currently (which is less work).

But there are certain Godot types (Pool*Vectors specifically) which can be currently converted to a fully compatible JSON, so go figure.

@Zylann
Copy link
Contributor

Zylann commented Dec 14, 2019

Some time ago I made this script: https://github.com/Zylann/godot_texture_generator/blob/master/addons/zylann.tgen/util/jsonify.gd
It converts some of the Godot types into a very close JSON representation (i.e Vector2 becomes {"x": 1, "y": 2}, and back to Vector2 if a given object only has an x and an y field). It is slow but it's JSON so speed isn't really the point. Since I don't introduce any kind of "special" typing fields there are some limitations, but it was enough for my needs. I think if I ever need to support types that would be ambiguous such as int/float, custom classes, dictionary keys and typed arrays, I would probably rely on a JSON Schema of some sort in order to parse it back to Godot in its original form (while currently I do a small extra pass myself in the application layer). Or, in the most widely general case, switch to a database representation which would barely need to be json anymore.
Note that typed GDScript and class reflection could serve as a schema ;)

@Calinou
Copy link
Member

Calinou commented May 26, 2020

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

@Calinou Calinou closed this as completed May 26, 2020
mikemike37 added a commit to mikemike37/JsonClassConverter that referenced this issue Aug 19, 2024
(I'm new to github, so apologies if I'm missing some best practice here!)

I've just started using your JsonClassConverter - thanks for sharing! I noticed it suffers from the very old problem that json serialising/deserialising of vectors just doesn't work: godotengine/godot#11866

I have implemented a fix for that, that I thought could be interesting to take to mainline.

Another significant change is the requirement that the variable have the @export annotation. At least in my case bringing over all the variables was not needed and was causing me problems.

I also split the load function into two parts: json_file_to_dict and json_file_to_class, since I needed to handle that a bit specially.

Take or leave any parts of this, but in any case thanks for your work :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants