godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.08k stars 69 forks source link

Add support for all engine types in JSON encoding/decoding #9510

Open reduz opened 3 months ago

reduz commented 3 months ago

Describe the project you are working on

Godot

Describe the problem or limitation you are having in your project

Several users who want to use JSON for tasks such as:

have issues with the JSON support not being available to parse all types of data Godot supports, either writing to JSON and loading from it.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

The idea is to add two functions to Godot:

To make it clear, for simplicity (it would become super bloated otherwise) this would not affect the JSON parser per se, but would be functions you can before encoding to JSON and after parsing from it.

Additionally, in order to carry this process perfectly, Godot should detect when numbers are integer and floating point in JSON and parse them as such (currently only parses them as float.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

This is an example of how JSON would look like in a Godot compatible format:

# Godot Elements
# Encoded as dictionary. If dictionary has _gtype key then it has a Godot type encoded with the type encoded as value.
{ 
  "__gtype":"Vector3",
  "values":[0,1,2]
},
{ 
  "__gtype":"Vector4",
  "values":[0,1,2,4]
}

# Or alternatively as members

{ 
  "__gtype":"Vector3",
  "x":0,
  "y":1,
  "z":2
},
{ 
  "__gtype":"Vector4",
  "x":0,
  "y":1,
  "z":2
  "w":4  
}

# Poll can be made to see what preference community has.

# For the rest, examples:

{ 
  "__gtype":"NodePath",
  "path":["a","b","c"]
  "subpath":["a","b","c"]
},
{ 
  "__gtype":"StringName",
  "name":"Hello" 
}

# Callable, Signal, RID not encodeable
# OBJECT can encode classes (optionally) basically using the same process as inst2dict 
# (in fact can just use the same code), but adding __gtype:"ClassName"

# Dictionaries are a problem because in Godot they can have non-string keys.
# So my recommendation here is to simply check if the dictionary has non-string keys. 
# If does not, simply use a regular JSON Object. Otherwise
# encode like this:

{
  "__gtype":"Dictionary",
  "pairs" : [ key, value, key, value, etc]
}

# The "pairs" format as seen above would be rare to see because Godot very rarely (if ever) serializes information using non String keys, but it needs to exist in case a Dictionary containing non-String keys is used.

# Arrays are just regular JSON arrays

# Packed arrays need to specify the type

{
  "__gtype":"PackedByteArray",
  "base64" : "<base64_data>"
},
{
  "__gtype":"PackedIntArray",
  "values" : [1,2,3,4]
}

Implementation in Godot:

Changing the JSON Parser to add this seems like bloating it unnecesarily. It seems to me we should add two functions:

Variant JSON::native_to_json(Variant p_native,bool p_allow_classes = false, bool p_allow_scripts = false);
Variant JSON::json_to_native(Variant p_native,bool p_allow_classes = false, bool p_allow_scripts = false);

These operate on Variant, not on text. So you would need to use this to encode a Variant before converting it to text, and after converting it from text.

It would be used like this:


json_string = JSON.stringify( JSON.native_to_json( godot_type) )

# and

godot_type = JSON.json_to_native( JSON.parse_string( json_string ) )

Implementing like this would make the implementation trivial and secure.

If this enhancement will not be used often, can it be worked around with a few lines of script?

No

Is there a reason why this should be core and not an add-on in the asset library?

An add-on should be possible, but given this is something that will need to be eventually audited (as it can become an exploitable surface), it is ideal the engine supports it internally.

JoNax97 commented 3 months ago

I like the idea but I'm not a fan or the API. I think having a 2-step process can be confusing for some users. Could a convenience API be added that combines both steps?

For example adding an optional parameter to JSON.stringify, like full_serialization = false, so default behavior is the same as now.

reduz commented 3 months ago

@JoNax97 I prefer not, as I stated for many reasons:

fire commented 3 months ago

I agree with making all engine types json encodable/decodable. We worked with a contributor @AdrienQuillet to implement a diff-ing system too. Perhaps he can join the discussion.

reduz commented 3 months ago

@fire right, remember again that the idea of this proposal is that we don't change our JSON encoder/decoder since users may actually not want this, (as they are working with pure JSON), but we implement this as an extra, optional layer.

Calinou commented 3 months ago

Should the type identifier be __gdtype instead? We generally use "gd" as short for Godot rather than just "g".

reduz commented 3 months ago

@Calinou oh sure, makes sense

fire commented 3 months ago

@reduz We wanted to save the godot engine scene as json and resources which then be easily diffable. https://github.com/V-Sekai/godot_json_scene_merge was our last proof of concept.

reduz commented 3 months ago

@fire yeah nothing should avoid you to save a PackedScene as JSON but I have the feeling this is not what you are after. That said you should be able to encode and decode it with some extra code on your end with this.

fire commented 3 months ago

I made a diagram of our proposed process which would benefit with this feature to lossless encode and decode engine JSON types.

graph TD;
    A[PackedScene Resource] -->|Convert to Variant| B[Variant Representation];
    B --> C[Apply native_to_json];
    C --> D[JSON Dictionary with __gdtype];
    D --> E[Convert Dictionary to JSON String];
    E -->|Serialize to Binary JSON| F[Binary JSON Storage];

    G[Binary JSON Storage] -->|Deserialize from Binary JSON| H[JSON String];
    H --> I[JSON Dictionary with __gdtype];
    I --> J[Apply json_to_native];
    J --> K[Variant Representation];
    K --> L[Reconstruct PackedScene Resource];
KoBeWi commented 3 months ago

You can already use var_to_str to achieve the same thing.

fire commented 3 months ago

var_to_str is not an acceptable replacement feature because parsing strings is not very efficient and the str representation representation quality varies compared to json.

For a use case in validating secure input in an online user generated content system.

KoBeWi commented 3 months ago

I mean you can use var_to_str to convert native type to String and then store it inside JSON.

foolmoron commented 3 months ago

Tiny suggestion, how about formatting non-string dicts like this?

{
  "__gtype":"Dictionary",
  "keys" : [ key, key ],
  "values": [ value, value ]
}
reduz commented 3 months ago

@KoBeWi

You can already use var_to_str to achieve the same thing.

I know, but there is no point to use JSON then :sweat:.

adriano-sudario commented 3 months ago

Naming and using suggestion:

json_string = JSON.variant_stringify( godot_type )

and

godot_type = JSON.to_variant( json_string ) # 'JSON.parse_string' could be used internally

RichoM commented 3 months ago

How are you planning to encode special values such as Infinity or NaN?

Also, what about circular references?

SelennLamson commented 3 months ago

Hi, I actually coded something like that for a game of mine. One of the biggest problems of Godot serialization imo is the ability for loaded objects to contain code, which might be immediately ran by the engine. This is a safety breach, so I wanted to make sure I have something as simple as possible to serialize data, without having to go through manual serialization of my classes.

It's in C#, and it works from a custom class inheriting "Godot.Resource". It simply converts the custom resource to a Godot Dictionary in a way that can be serialized to JSON using Godot functions, and reverted back to the initial custom resource (as long as you know the root type and it matches the content of the serialized string).

It can currently:

Current limitations:

I'm new to the Godot community and would be glad to participate in making something that others could actually use (I mean, something that meets the quality standard).

MMUTFX2053 commented 3 months ago

maybe its better to have a new format that replaces JSON, lets call it GDSON

Calinou commented 3 months ago

maybe its better to have a new format that replaces JSON, lets call it GDSON

The goal of this proposal is to keep using a standard format, as it's what other applications (such as databases) use. Using a custom format would defeat the point of this proposal. Also, Godot already has its own file formats such as Resource (text or binary) and ConfigFile (text).

MMUTFX2053 commented 3 months ago

maybe its better to have a new format that replaces JSON, lets call it GDSON

The goal of this proposal is to keep using a standard format, as it's what other applications (such as databases) use. Using a custom format would defeat the point of this proposal. Also, Godot already has its own file formats such as Resource (text or binary) and ConfigFile (text).

im an amateur/hobbyist gamedev, so im not as experienced or knowledgeable as you guys, but from my experience, its very frustrating that there are limitations to things like JSON, because i did find it easier to work with then with resource, but then i noticed i couldnt store all the data types available.

that is why i thought maybe its better to deprecate JSON and introduce a new format that supports all data types

willnationsdev commented 3 months ago

maybe its better to have a new format that replaces JSON, lets call it GDSON

The goal of this proposal is to keep using a standard format, as it's what other applications (such as databases) use. Using a custom format would defeat the point of this proposal. Also, Godot already has its own file formats such as Resource (text or binary) and ConfigFile (text).

im an amateur/hobbyist gamedev, so im not as experienced or knowledgeable as you guys, but from my experience, its very frustrating that there are limitations to things like JSON, because i did find it easier to work with then with resource, but then i noticed i couldnt store all the data types available.

that is why i thought maybe its better to deprecate JSON and introduce a new format that supports all data types

@MMUTFX2053 There is no singular data format that can support all possible software. If each app tried to push its needs onto a common format, then all apps would endlessly spend their days adapting to ever-shifting standards rather than being productive.

That's why instead people convert to a simple & flexible format (like JSON) that can convey ideas from any domain (albeit with some inefficiencies / added metadata). The other applications will already have working systems that know how to parse and understand its structure, even if it may not know the meaning of certain keys or values initially.

Godot's traditional JSON serialization just builds a JSON structure out of Godot data without ensuring that it is capable of fully/safely converting the data back (it leaves out needed metadata). This proposal is for clarifying how a JSON structure could be mutated to provide that metadata and by what means the Godot API should convert to/from the metadata-filled vs. metadata-less JSON.

WolfgangSenff commented 3 months ago

I really like this idea and would help my GodotFirebase plugin out a lot - we have some automatic conversion stuff in our codebase that could be removed as not needed. I do think there may be some difficulty with storing stuff as "{data : [1, 2, 3, 4]}" for instance with it though, and would prefer if possible to use the member names rather than an array for vectors and things. But maybe in the end we'll need custom conversions for arrays anyway, since there's a limitation in Firebase of not supporting arrays directly (in one of its technologies, works fine in others). Conversely, if it just ends up being a string of an array, then that's totally fine no matter what, so as long as it parses back to the correct thing, I and my users would be very happy to have this!

reduz commented 2 months ago

@MMUTFX2053

maybe its better to have a new format that replaces JSON, lets call it GDSON

This is what you have already when you call var_to_str and str_to_var , and it fully supports all Godot types.

This proposed feature is more of a requirement for inter-operation with existing servers and protocols to allow storing the data in a way that something like databases and back-end languages and platforms can understand and optimize for.

fire commented 2 months ago

Note that the gltf XMP extension has a neat way of encoding sets vs lists in json if we need that feature.

https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Khronos/KHR_xmp_json_ld/README.md#lists-and-sets

akien-mga commented 2 months ago

I support the proposal, but one aspect I see missing is how to handle int and float types to ensure that they get properly converted. This is currently a major pain point with JSON which doesn't have these two types, but only Number.

I have yet to test https://github.com/godotengine/godot/pull/91503, but I suspect it might not be able to preserve information about the difference between var i: int = 1 and var f: float = 1.0. I'll test to confirm.

CC @AThousandShips as we discussed this at length a few weeks ago, I think there might be related proposals / issues that could possibly be folded into this one as a common solution for preserving as much Godot-specific type information as possible.

reduz commented 2 months ago

@akien-mga The int/float thing to be honest is a problem, but not entirely related to this proposal (which just works on a higher level). My feeling regarding int/float is that we should explicitly add decimals to floats and parse them as such.

This is not a warranty of course that other implementations you talk to will do this, but at least if you are saving/loading this within Godot it remains consistent.

akien-mga commented 2 months ago

@akien-mga The int/float thing to be honest is a problem, but not entirely related to this proposal (which just works on a higher level). My feeling regarding int/float is that we should explicitly add decimals to floats and parse them as such.

This is not a warranty of course that other implementations you talk to will do this, but at least if you are saving/loading this within Godot it remains consistent.

Right, this is something that could be done if we go ahead with https://github.com/godotengine/godot/pull/47502.

reduz commented 2 months ago

@akien-mga afaik we already are forcing decimals when writing floats, I meant more about changes to the JSON parser (that only has a token for numbers, not necessarily int or float).

Adding explicit support for ints may break compatibility if users are expecting numbers to always be float, so it also may need to be an extra option for parsing.

akien-mga commented 2 months ago

Currently we don't force decimals when stringifying floats to JSON, here's an example output from your PR:

                "_float": 42,
                "_float_var": 42,

I just tested https://github.com/godotengine/godot/pull/47502 and that changes this (now all floats have a decimal point), but that's indeed not enough to properly parse back 42 specifically as int, this should be changed in the parser. And indeed this would likely be considered compat breaking if we change it.

But I agree that's not directly related to this proposal. We seem to have this proposal open already, so we can move that part of the discussion there:

h0lley commented 1 month ago

I am struggling with the point. You are basically wrapping a Godot specific format into JSON. You still have the Godot specific format. Nothing other than Godot can do anything with this Godot-JSON without extra steps. Sure, other software can parse the JSON, but then it would need a special Godot type parser to do anything with it - just like it would need with TRES. The same goes for generating this Godot-JSON, of course.

Calinou commented 1 month ago

but then it would need a special Godot type parser to do anything with it - just like it would need with TRES. The same goes for generating this Godot-JSON, of course.

It's much easier to read the custom Godot datatypes within JSON than to implement a TRES parser in another language. TRES looks a bit like TOML, but a lot of its syntax is incompatible with it.

fire commented 1 month ago

I worked on this a little here https://github.com/godotengine/godot/pull/92656