Skip to content
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

Optionally include documentation in GDExtension API dump #82331

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

rburing
Copy link
Member

@rburing rburing commented Sep 25, 2023

Language bindings implemented in GDExtension that generate glue code (in the future, C#?) may want to include documentation (for classes, properties, methods, signals, etc.) to improve the user experience in code editors and REPLs.

This PR adds an option to include that documentation in the extension_api.json file.

This is much more convenient than having to obtain and parse ~829 XML files from the Godot source code.

(Ideally the build system for a GDExtension should depend just on having the correct API file, and not on having to run Godot or having the Godot source code.)

  • Size of extension_api.json generated using --dump-extension-api: 5559324 bytes.
  • Size of extension_api.json generated using --dump-extension-api-with-docs: 9121481 bytes.
  • The size increase is 65%.

Feedback welcome.

There are quite some loops that could be hidden away by adding new core methods to ClassDoc, if desired.

Copy link
Contributor

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a heavy user of extension_api.json, I would welcome this addition. It would mean one less data source and one less format (XML) that needs to be understood.

The XML files also contain some meta-information that is exclusive to them, for example whether a class is experimental. Do you think it makes sense to add this to the JSON as well -- maybe even the base (non-doc) version?

core/extension/extension_api_dump.cpp Show resolved Hide resolved
@YuriSizov YuriSizov added this to the 4.x milestone Sep 26, 2023
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a great idea!

I tested locally, and it seems to be working, although I only skimmed through the JSON, I didn't scrutinize it in depth.

The code looks good to me as well! I only have some minor notes about the command-line help info.

main/main.cpp Outdated Show resolved Hide resolved
main/main.cpp Outdated Show resolved Hide resolved
@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Sep 26, 2023
@rburing
Copy link
Member Author

rburing commented Sep 26, 2023

Thanks for the timely reviews!

@Bromeon

The XML files also contain some meta-information that is exclusive to them, for example whether a class is experimental. Do you think it makes sense to add this to the JSON as well -- maybe even the base (non-doc) version?

It makes sense to me, but I would prefer not to touch it in this PR.

We can discuss it with the GDExtension team on RocketChat.

@akien-mga akien-mga merged commit 251fb83 into godotengine:master Sep 26, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@rburing rburing deleted the gdextension_dump_docs branch September 26, 2023 21:06
@maiself
Copy link
Contributor

maiself commented Sep 26, 2023

Working with python:

Screenshot from 2023-09-26 17-12-48

Parsing the bbcode is a little awkward, but not too bad. This is really cool, thanks!

Edit: Just to add a bit of info: compressed json went from ~400kb to ~1.2mb.

@akien-mga
Copy link
Member

Parsing the bbcode is a little awkward, but not too bad.

I assume many language bindings will use Python for codegen from the json. Maybe we could have a small python library that can be used for parsing the bbcode in each language binding?

@GeorgeS2019
Copy link

@rburing
Thanks for your contribution

We are starting to see a path to create GDExtension C# Bindings using your merge

@rburing
Copy link
Member Author

rburing commented Oct 18, 2023

For those who found this PR and want to use the feature: note that in #83318 we changed the key name from "documentation" to "description" (for internal consistency) and we added "brief_description" to classes and built-in classes.

This will allow generating even more detailed documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants