godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.83k stars 21.13k forks source link

In Godot 4.4 dev 2, typed dictionaries not compatible with JSON.parse_string #97137

Open YahelBaram opened 1 month ago

YahelBaram commented 1 month ago

Tested versions

-Reproducible in Godot 4.4 dev 2

System information

Windows 10, Godot 4.4 dev 2, Forward+

Issue description

(I'm sorry if there is an open issue about this case and I didn't find it before opening this issue)

When using JSON.parse_string as an assignment to a typed dictionary that is not declared as Dictionary[Variant, Variant] (Example from my use case: Dictionary[String, Dictionary]) I get the error:

Invalid assignment of property or key with value of type 'Dictionary'

Steps to reproduce

var d : Dictionary[String, Variant] = {}

var s : String = '{"A": "Hi","B": "Hey"}' """

func _ready() -> void: d = JSON.parse_string(s)



- Run the script (I got an error)
- Modify the dictionary to be `Dictionary[Variant, Variant]`
- Run the code again (Worked for me)

There is a MRP that has this script, with an extra line (commented by default) that loads the data from a .json file

### Minimal reproduction project (MRP)

[MRP.zip](https://github.com/user-attachments/files/17039753/MRP.zip)
chocola-mint commented 1 month ago

I think it makes sense that JSON.parse_string returns an untyped Dictionary instead of trying to guess the JSON string's typing.

IMO there should be a new method JSON.parse_string_typed instead that would allow the user to ensure that the JSON dictionary obtained from the string satisfies a given key/value type pair. This method would then be able to be used in this issue's use case.

The function signature could be something like this:

static Variant JSON::parse_string_typed(const String &p_json_string, Variant::Type p_key_type, Variant::Type p_value_type);
chocola-mint commented 1 month ago

To add to the discussion, another approach to solving this problem could be to just add infer_typed_key() and infer_typed_value() methods to allow users to make untyped dictionaries typed. This would be easier to implement and can be used to verify JSON schema containing typed dictionaries. (The above parse_string_typed would only be able to enforce typing on the top level of the JSON's hierarchy. Plus, it also has to consider the fact that JSON strings can also just evaluate into a single float/string instead of a dictionary)

extends Node

var d : Dictionary[String, String] = {}

var s : String = '{"A": "Hi","B": "Hey"}'

func _ready() -> void:
  var dd = JSON.parse_string(s)
  if dd.infer_typed_key(): # If d's key type is Variant, you can just not infer the key's type here.
    if dd.infer_typed_value(): # If d's value type is Variant, you can just not infer the value's type here.
      if dd.is_same_typed(d):
        d = dd
        print("Success")
      else:
        print("Failed: Dictionary is not Dictionary[String, String]")
    else:
      print("Failed: Not all values share the same type.")
  else:
    print("Failed: Not all keys share the same type.")

And a version with no error checking:

func _ready() -> void:
  var dd = JSON.parse_string(s)
  if dd.infer_typed_key() and dd.infer_typed_value() and dd.is_same_typed(d):
    d = dd
friendnard commented 1 month ago

I believe the method that works with typed Arrays is .assign(), which handles type conversions. It would be good if typed Dictionaries used the same method name.

EDIT: assign() is in the latest docs. Try d.assign(dd).

Also, json is a bit awkward with its limited types. It has no integer type, even; so any exported Dictionary using type int will be imported with type float. So checking the type of the imported Dictionary is not reliable.