-
-
Notifications
You must be signed in to change notification settings - Fork 22.7k
GDExtension: Store source of gdextension_interface.h
in JSON
#107845
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
base: master
Are you sure you want to change the base?
Conversation
fd610ba
to
7abb091
Compare
Somehow, I completely missed this huge change! 😅 For completeness, I'd also like to mention potential downsides. Some are inherent to this sort of migration and not a flaw of the chosen approach, so they're not necessarily arguments against moving to JSON 🙂 long-term, this will likely simplify a lot!
I wonder if it would be possible to convert header files of older Godot versions to the JSON format and make them available? This would only need to happen for each minor version (as the GDExtension header doesn't change in-between), and would completely solve point (4). I'll look into the detailed changes at a later point. Just one request: could we keep the field names and structure as close as possible to the
|
Another question: if the JSON becomes now the source-of-truth, how do contributors modify it? If manually, is there tooling to ensure indentation, order of keys etc., to keep some consistency? |
@Bromeon Thanks, those are some very good points :-)
I think it makes sense to always keep the option of generating the C header. So, for language bindings that use premade tooling that can parse a C header, they can keep using that. But with the JSON, they can also do their own additional code generation, without having to make their own parser for the C header.
I agree that dealing with the whole thing at once is kind of hard to read. But I think with individual PRs that only add a couple things at a time, it should be fine. And, it'll also make it harder for certain errors to slip in, especially if we add some validation of the JSON, which you also bring up, and I agree would be a good idea.
Actually, bindings can always use the latest version of the JSON! (And this is true of the current We never change or remove old signatures. You just need to ignore any interface functions with a "since" field that's newer than the Godot version you are targeting. So, a binding could use the JSON from Godot 4.6 to build the Godot 4.1-4.5 compatible versions as well.
That makes sense! I'll update the PR when I have a chance.
Yes, manually. We could (and should!) add some tooling for formatting and validation. I think this would help to prevent the sort of errors and inconsistencies that have already cropped up in the I'll dig into this |
I just want to mention that |
I agree that JSON isn't the best format for human editing. (If I recall correctly, I think @vnen was advocating for using XML for this data at a meeting in the past?) I went for JSON largely because (a) parsing it in Godot is very easy and (b) developers already need to work with the I suppose we could keep the data in some other format (yaml, toml, xml, whatever) and then convert it to JSON to distribute it? That's a lot of converting, though |
Very good point. I was actually considering this at some point for the
Yes, or non-C code creeping in 🙂 #96408
It would make sense if both Personal preference:
I'm not sure if it's even an option to change the format for now, but for a potential Godot 5, I could totally imagine something like TOML. |
7abb091
to
2070c09
Compare
In my latest push, I've attempted to align the structure with
I've decided to go with "kind" (so, we have "a kind of type", which is literally what I would say out loud in normal conversation). And I've also renamed "simple" to "alias", which I think is more descriptive of what it is
I added some simple validation for types, so it shouldn't be possible to accidentally sneak a There's some more validation I'd like to add, including around formatting, which'll come in my next update! |
For the C# bindings we have no plans to move to the new JSON, so as long as the C header is still available it doesn't really affect us what the source of truth is. In the C# bindings we use ClangSharp which can generate C# bindings from the C header. It uses Clang directly, so as long as the C header is valid C/C++ code it should work fine. For other languages, keep in mind they may not have great support for TOML, whereas JSON and XML tend to be ubiquitous. It makes sense to use JSON since they'll need to support it for But as mentioned earlier, I don't plan to consume this new format, so I don't have a vested interest in the choice of format. |
I don't remember when I suggested XML as a format. It was probably because of the class ref docs, so we could use a similar format. But we usually rely on doctool creating the stuff for us, writing by hand is not really great. Not that I think JSON is much better (at least compared writing plain C code), but since all the tooling is based on JSON already, it makes sense to keep in the same format. Speaking of which, it might be interesting to keep a JSON Schema too, which also helps with validation (though I'm not sure how easy it would be to integrate a schema validator in our system). |
I think making a JSON Schema is a great idea, even if it only ends up as documentation Since we already have Python code in SCons parsing the JSON and looping over it, it may be easier just to validate its structure in there, rather than adding JSON schema validation tooling. But I'll take a look at it! If the schema validator can run as part of the pre-commit hooks, then we can check its format before SCons even runs |
This PR converts the canonical source of truth for the GDExtension interface from the
gdextension_interface.h
header, to a new JSON file (gdextension_interface.json
).The header can still be generated from the JSON, and
godot --dump-gdextension-interface
will still create a header. (This will be the "legacy header" - more on that below.)But now there's a new
godot --dump-gdextension-interface-json
that will dump the JSON data.(When we eventually do Godot 5 and can break compat, I'd like to make
--dump-gdextension-interface
output the JSON, and a new--dump-gdextension-interface-header
to do the header. This will make it clear the JSON is the canonical source, but probably lots of folks will still want to use the header.)Advantages
This is something we've talked about at GDExtension team meetings for a long time (years!), which has a number of advantages:
There are already bindings that parse the
gdextension_interface.h
to power their own code generation. They will no longer need to deal with the complexities of parsing C! They can just use the JSON data. (FYI, I initially made the JSON in this PR with a script that extracted the data from the header into JSON, and now no one needs to do that ever again :-))We can iterate on the header used within Godot and godot-cpp. Another change we've discussed in the past is replacing some of the
typedef void *
s withstruct {}
s so that the compiler can help prevent us from mixing up types, but we've been unable to do that, because we don't want to break developers using the "legacy header". Well, now Godot and godot-cpp can generate a modified header from the JSON to use internally, without affecting external users of the legacy headerUp until now we've had to enforce restrictions on what can go into
gdextension_interface.h
and its format because we know some bindings parse it. This has meant trying to catch PRs that attempt to add "real code" to the header in review. We won't have to worry about that anymore! Anything added to the JSON will automatically be added to the "legacy header" in the correct format.Result
You can compare the new header generated by Godot, with the old header - or look at this convenient diff!
All the changes are cosmetic, mostly related to spacing or comment type. (I decided to standardize on C-style comments, and putting comments before what they comment, rather than inline.)
I've run the godot-cpp tests with the legacy header generated by Godot, and everything seems to work fine!
Notes
Note 1: Generating the header
There are two places in the PR that take the JSON and generate a C header: in Python code run by SCons to make the header used by Godot, and in Godot itself to generate the "legacy header".
This is intentional!
Right now, they do basically the same thing (with the exception of some type-o's in type names maintained in the legacy header, but fixed in the Python version).
But, as explained above, the goal is that we can continue to iterate on the Python version, so that we can make some improvements to the header used by Godot and godot-cpp. Whereas the header generation code in Godot will stay the roughly same for backwards compatibility.
Note 2: The docs
This PR doesn't attempt to improve the docs at all! It just transfers what we already have into JSON.
Since this PR will conflict with any other changes to
gdextension_interface.h
, it runs the risk of stalling any other GDExtension work that needs to touch that file.So, while there may be a temptation to iterate on the docs in this PR, instead, I think we should attempt to merge it with the existing docs as soon as possible in the Godot 4.6 dev cycle. The goal is for the docs to be no worse than the docs we already have.
Afterwards, we can make follow-up PRs to improve the docs.
I'd like for review of this PR to focus on the format of the JSON file, and correctly generating the header.