Godot: Dictionary fails to parse JSON with array as top level

[neikeq]

Operating system or device:
Ubuntu 15.10 64bits

Issue description (what happened, and what was expected):
It's valid for a JSON to have an array as top level, however _Dictionary_ fails to parse it:

ERROR: parse_json: Error parsing JSON: Expected '{' at line: 0
   At: core/dictionary.cpp:206.

Steps to reproduce:

func test_json_top_level_array():
    assert({}.parse_json("[ { \"name\": \"Richard\" }, {\"name\": \"Drahcir\"} ]") == OK)

[blurymind]

This bug affects me :)

[neikeq]

The problem is that a Dictionary needs a key (otherwise it would be an Array). IMO the best solution would be to expose a JSON class to GDScript with a parse method that would return either an Array or a Dictionary.

@reduz WDYT?

[mooglegiant]

top level still needs { } and dictionaries/arrays need a key
it should be something like
{ "objecttype" : [ { "name": "Richard" }, {"name": "Drahcir"} ] }

when it doubt, validate with http://json.parser.online.fr/ (just remove the escape characters)

[neikeq]

@mooglegiant A valid JSON can have an array [] as top level.

[Razzlegames]

But valid Json doesn't necessarily translate to a valid dictionary. Json that maps to a dictionary is a subset of all possible JSON.

How would you reference an element in a hash map (dictionary) without a valid hash (key)? Would you use a blank key? Ok that's fine for one unkeyed value, but what if there are many?

[bojidar-bg]

@Razzlegames ~ I think you didn't actually read @neikeq's proposal above, which is to move JSON parsing to a new JSON class, and out of Dictionary (though the latter should be kept for compatibility). That would allow the parse_json method to return an Array as well as a Dictionary.

[neikeq]

@bojidar-bg I agree. Dictionary could still keep parse_json with the current behaviour.

[punto-]

It could also be on Marshalls, which is where all the serialization stuff
is.

On 6 April 2016 at 04:21, Ignacio Etcheverry [email protected]
wrote:

@bojidar-bg https://github.com/bojidar-bg I agree. Dictionary could
still keep parse_json with the current behaviour.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
https://github.com/godotengine/godot/issues/4232#issuecomment-206176760

[bojidar-bg]

@punto- I think rather not, since in Marshalls most stuff is related to Variant-RawArray conversion, and not JSON or some other human-readable format.

[blurymind]

This will help me port yarn parser to godot. The json files yarn Exports
have [] as root
On 6 Apr 2016 16:04, "Bojidar Marinov" [email protected] wrote:

@punto- https://github.com/punto- I think rather not, since in
Marshalls most stuff is related to Variant-RawArray conversion, and not
JSON or some other human-readable format.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
https://github.com/godotengine/godot/issues/4232#issuecomment-206417785

[vnen]

I never notice before, but it seems that str2var() can understand JSON, be it dictionary or array.

[neikeq]

@vnen That's probably because a Dictionary as string has a similar syntax to JSON. But is it exactly the same in all cases?

[vnen]

I don't know. It certainly manages simple JSON, but not sure how much of it (haven't tested things like escape sequences or true, false, null values).

[reduz]

I've been re-reading the JSON specification, apparently anything can be at
top level in JSON, even strings, ints, etc.
So, adding this ability to Array is not a good idea. It should be added to
String and return a Variant, however due to how C++ works this is not
possible (cross dependency).

So, if anyone is up for fixing this, the plan should be:

For C++ we should just use the JSON singleton instead of Dictionary (make a
static function like Variant JSON::parse(const String&) ), and for GDScript
we can add a binding manually to String in variant_call.cpp

We can keep the current function in Dictionary for compatibility and erase
it in 3.0

On Tue, Jun 14, 2016 at 11:33 AM, George Marques [email protected]
wrote:

I don't know. It certainly manages simple JSON, but not sure how much of
it (haven't tested things like escape sequences or true, false, null
values).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/4232#issuecomment-225900403,
or mute the thread
https://github.com/notifications/unsubscribe/AF-Z21vVGi3Ap6Kb-S-IhkwYrJ3Tr6-1ks5qLruwgaJpZM4IAFTs
.

[reduz]

currently, the JSON::parse function expects a { token, as it calls
parse_object(). It should be changed to use _parse_value()

On Tue, Jun 14, 2016 at 11:43 AM, Juan Linietsky [email protected] wrote:

I've been re-reading the JSON specification, apparently anything can be at
top level in JSON, even strings, ints, etc.
So, adding this ability to Array is not a good idea. It should be added to
String and return a Variant, however due to how C++ works this is not
possible (cross dependency).

So, if anyone is up for fixing this, the plan should be:

For C++ we should just use the JSON singleton instead of Dictionary (make
a static function like Variant JSON::parse(const String&) ), and for
GDScript we can add a binding manually to String in variant_call.cpp

We can keep the current function in Dictionary for compatibility and erase
it in 3.0

On Tue, Jun 14, 2016 at 11:33 AM, George Marques <[email protected]

wrote:

I don't know. It certainly manages simple JSON, but not sure how much of
it (haven't tested things like escape sequences or true, false, null
values).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/4232#issuecomment-225900403,
or mute the thread
https://github.com/notifications/unsubscribe/AF-Z21vVGi3Ap6Kb-S-IhkwYrJ3Tr6-1ks5qLruwgaJpZM4IAFTs
.

[neikeq]

OK, I tried validating "foo" with https://jsonformatter.curiousconcept.com/

Invalid JSON (RFC 4627)
Valid JSON (RFC 7159)
Invalid JSON (ECMA-404)

The last two are the competing standards. Would supporting RFC 7159 break compatibility with ECMA-404?

BTW, I can make the PR.

[vnen]

According to Ecma 404:

A JSON text is a sequence of tokens formed from Unicode code points that conforms to the JSON value grammar.

and in the value grammar:

A JSON value can be an object, array, number, string, true, false, or null.

So I'd say it actually agrees with RFC 7159 ans that JSON formatter tool misinterpreted it. Also, this RFC states:

Implementations that generate only objects or arrays where a JSON text is called for will be interoperable in the sense that all implementations will accept these as conforming JSON texts.

If we try to parse and it's an array or object, it'll be valid. The major concern would be about JSON _generartion_, but I think we can assume the user will pass a valid dictionary/array if they want such format (and this will be documented as such).

Also, by both specs (as I understand) "foo","bar" is valid JSON. The formatter says this is valid according to RFC 7159. This might be interpreted as an array for our purposes.

[reduz]

I don't think I would go that far, let's just expect arrays to have [] but
accept any token such as "helo" or 25 as valid JSON

On Tue, Jun 14, 2016 at 1:24 PM, George Marques [email protected]
wrote:

According to Ecma 404:

A JSON text is a sequence of tokens formed from Unicode code points that
conforms to the JSON value grammar.

and in the value grammar:

A JSON value can be an object, array, number, string, true, false, or
null.

So I'd say it actually agrees with RFC 7159 ans that JSON formatter tool
misinterpreted it. Also, this RFC states:

Implementations that generate only objects or arrays where a JSON text is
called for will be interoperable in the sense that all implementations will
accept these as conforming JSON texts.

If we try to parse and it's an array or object, it'll be valid. The major
concern would be about JSON _generartion_, but I think we can assume the
user will pass a valid dictionary/array if they want such format (and this
will be documented as such).

Also, by both specs (as I understand) "foo","bar" is valid JSON. The
formatter says this is valid according to RFC 7159. This might be
interpreted as an array for our purposes.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/4232#issuecomment-225936547,
or mute the thread
https://github.com/notifications/unsubscribe/AF-Z2980M50GpkDfDLX2I_HW4ov7fv4Mks5qLtXFgaJpZM4IAFTs
.

[Razzlegames]

As long as it's documented, I don't think there's any problem with only
supporting a subset of the JSON standard.

On Tue, Jun 14, 2016 at 11:01 AM, Juan Linietsky [email protected]
wrote:

I don't think I would go that far, let's just expect arrays to have [] but
accept any token such as "helo" or 25 as valid JSON

On Tue, Jun 14, 2016 at 1:24 PM, George Marques [email protected]
wrote:

According to Ecma 404:

A JSON text is a sequence of tokens formed from Unicode code points that
conforms to the JSON value grammar.

and in the value grammar:

A JSON value can be an object, array, number, string, true, false, or
null.

So I'd say it actually agrees with RFC 7159 ans that JSON formatter tool
misinterpreted it. Also, this RFC states:

Implementations that generate only objects or arrays where a JSON text is
called for will be interoperable in the sense that all implementations
will
accept these as conforming JSON texts.

If we try to parse and it's an array or object, it'll be valid. The major
concern would be about JSON _generartion_, but I think we can assume the
user will pass a valid dictionary/array if they want such format (and
this
will be documented as such).

Also, by both specs (as I understand) "foo","bar" is valid JSON. The
formatter says this is valid according to RFC 7159. This might be
interpreted as an array for our purposes.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/godotengine/godot/issues/4232#issuecomment-225936547
,
or mute the thread
<
https://github.com/notifications/unsubscribe/AF-Z2980M50GpkDfDLX2I_HW4ov7fv4Mks5qLtXFgaJpZM4IAFTs

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/4232#issuecomment-225964370,
or mute the thread
https://github.com/notifications/unsubscribe/AIWVf8W3wyGKb-qi1m0xa_g7oOxW-LkIks5qLuxrgaJpZM4IAFTs
.

[reduz]

I wrote the parser based on this spec:
http://www.json.org/
though I clearly misunderstood that everything had to be inside of a dictionary because 99.999999% of JSON is used like that :P

over there, it doesn't show that you can do 1,2,3 and treat it as array

[akien-mga]

Is this fixed with 62273e5?

[neikeq]

Tested on 62273e5 with the following code:

extends Node

var json_list = [
    "0",
    "\"hello\"",
    "{\"hello\":10}",
    "[\"hello\", 10]",
    "[{\"hello\": 10}]"
]

func _ready():
    for json in json_list:
        assert(validate_json(json).empty())
        var parsed = parse_json(json)
        print(parsed)
        var back = to_json(parsed)
        print(back)

Output:

0
0
hello
"hello"
(hello:10)
{"hello":10}
[hello, 10]
["hello", 10]
[(hello:10)]
[{"hello":10}]

It works!