-
-
Notifications
You must be signed in to change notification settings - Fork 98
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
Add support for all engine types in JSON encoding/decoding #9510
Comments
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. For example adding an optional parameter to JSON.stringify, like full_serialization = false, so default behavior is the same as now. |
@JoNax97 I prefer not, as I stated for many reasons:
|
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. |
@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. |
Should the type identifier be |
@Calinou oh sure, makes sense |
@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. |
@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. |
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];
|
You can already use |
For a use case in validating secure input in an online user generated content system. |
I mean you can use |
Tiny suggestion, how about formatting non-string dicts like this?
|
I know, but there is no point to use JSON then 😓. |
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 |
How are you planning to encode special values such as Infinity or NaN? Also, what about circular references? |
Hi, I actually coded something like that for a game of mine. 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). |
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. |
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! |
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. |
Note that the gltf XMP extension has a neat way of encoding sets vs lists in json if we need that feature. |
I support the proposal, but one aspect I see missing is how to handle I have yet to test godotengine/godot#91503, but I suspect it might not be able to preserve information about the difference between 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. |
@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 godotengine/godot#47502. |
@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. |
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 godotengine/godot#47502 and that changes this (now all floats have a decimal point), but that's indeed not enough to properly parse back 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: |
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. |
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. |
I worked on this a little here godotengine/godot#92656 |
Implemented by godotengine/godot#92656. |
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:
Implementation in Godot:
Changing the JSON Parser to add this seems like bloating it unnecesarily. It seems to me we should add two functions:
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:
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.
The text was updated successfully, but these errors were encountered: