godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.15k stars 97 forks source link

Use cJSON for JSON (un)marshalling instead of a custom implementation #1833

Open jonbonazza opened 3 years ago

jonbonazza commented 3 years ago

Describe the project you are working on: Any game that uses JSON (especially online games that communicate with webapis). Also relevant for the Godot engine code itself.

Describe the problem or limitation you are having in your project: While fleshing out unit tests for Godot's JSON facilities, @Calinou has found several issues with the Godot JSON parser not being conformant to spec (PR Link) (TODO: Link to tracking issue once created). This is very bad in my opinion, especially for online games that rely on communications with various webapis (first and 3rd party) for various auxiliary (read: non-gameplay) functionality, such as matchmaking, database apis, commerce, etc... since these apis almost always use JSON for their content types.

Describe the feature / enhancement and how it helps to overcome the problem or limitation: From my understanding, Godot's JSON facilities are fairly ancient, and have existed long before Godot was even open sourced. Since then some very tiny, well reputed, battle tested, and portable json libraries have sprung up. One of such libs, cJSON has pretty much become the defacto JSON library for anything written in C or C++. It's used pretty much everywhere is incredibly tiny, fast and easy to use, is as portable as can be, and is 100% conformant to spec.

I suggest we add cjson as a Godot core dependency and use it for Godot's JSON needs.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:

Godot's JSON apis are already very isolated, so changing them to use cjson would be extremely easy. As I already mentioned, cjson is as portable as portable can be, and since most (all?) consoles and platforms already support or use it anyway, this should be enough. If we really wanted to be safe, however, we could make the json stuff part of the platform specific code, which would allow us to implement different json backends depending on platform, but I really don't think this is necessary.

If this enhancement will not be used often, can it be worked around with a few lines of script?: No. JSON is a core feature

Is there a reason why this should be core and not an add-on in the asset library?: Technically speaking, there's nothing stopping someone from creating a GDNative plugin for cjson, however since JSON is so integral to gamedev--especially online games as mentioned before, I believe that this makes the most sense as an enhancement to core.

silverkorn commented 3 years ago

Might simdjson be a potentially better choice? It seems to be cross-platform and it's under Apache 2.0 license. It might not be as mature as cJSON and easily implementable but it's worth to give it a look! 😄

Calinou commented 3 years ago

Might simdjson be a potentially better choice? It seems to be cross-platform and it's under Apache 2.0 license. It might not be as mature as cJSON and easily implementable but it's worth to give it a look! smile

Performance isn't critical for our use cases. Portability is more important, as we need something that works well on x86, ARM and possibly more (consoles).

PS: Small reminder about implementing cJSON if we go for it:

Case Sensitivity

When cJSON was originally created, it didn't follow the JSON standard and didn't make a distinction between uppercase and lowercase letters. If you want the correct, standard compliant, behavior, you need to use the CaseSensitive functions where available.