GDExtension: Add an interface for loading extra documentation

[deralmas]

This is the culmination of quite a lot of attempts at interfacing with the editor documentation from GDExtension's API. It's not perfect but I think that it's finally usable enough considering the way that the doc system currently works.

GDExtensions can call editor_help_load_xml_from_utf8_chars or editor_help_load_xml_from_utf8_chars_and_len to register new "external" doc data in the form of an XML-formatted UTF-8 buffer. This should work with both workflows of using multiple calls of smaller data or a single call with everything inside. A single class' data will be automatically cleared when calling classdb_unregister_extension_class.

This is done through an extra documentation container in EditorHelp that stores a parsed copy of everything added/removed which is then appended to the main object when needed so that it can persist after a full doc regeneration. I can't think of a cleaner way with the current state of the doc system.

Some new methods got added to assist in this, namely: DocTools::load_xml, EditorHelp::load_xml_buffer and EditorHelp::remove_doc.

I'm not very experienced with GDExt's API conventions so feedback on it is appreciated.

A test extension is available over https://github.com/Riteo/threen/tree/doc (includes already an api dump, just call scons gdextension_dir=gdextension)

Based on the work of @akien-mga, supersedes #75415.

Production edit: closes godotengine/godot-roadmap#61

[dsnopek]

Thanks, this looks great! At a high-level, I think this is working exactly the right way :-)

There is only one more tricky thing we need to solve here, which I describe in one of my code comments.

[dsnopek]

Thanks! The latest updates look great to me - including the stuff you were worried about in the 2nd commit! Just looking at the code, I don't have any more notes. :-) Personally, I think it'd be fine to squash the commits and take out of "draft" so that it can get some wider review.

The only thing left from my end is that I'd like to do some testing before hitting the "Approve" button.

(There will also be a non-trivial amount of work figuring out how best to integrate this functionality into godot-cpp, but that doesn't need to hold up this PR.)

[deralmas]

All right! Squashed and ready for review :)

[DmitriySalnikov]

Can it be merged before the 4.2 release?
It would be really cool to finally add documentation inside libraries.

[akien-mga]

No, 4.2 is in feature freeze and even at RC stage, it's not the time to add new features.

[dsnopek]

Sorry for taking so long to get back to this - I'm going to try to test this soon, hopefully this week. But just wanted to point out that this needs a rebase now.

[dsnopek]

Alright!

I did my own local rebase and tested this. With the modification I describe in one of my review comments (about moving some code into EditorHelp::_gen_extension_docs()), it works great! 🎉

In case it's helpful, I pushed my rebase to this branch in my fork. I made the one functional change from my review there, but I didn't make the renaming changes.

And draft PR godotengine/godot-cpp#1374 for godot-cpp can be used for testing. We'll need to do a bunch of design work and iteration to evolve it into the right solution for godot-cpp, but that shouldn't hold up this PR.

In any case, after the rebase and the renames, I'd be happy to approve this :-)

[deralmas]

@dsnopek I've not yet rebased this thing because #84354 did some stuff that overlaps with this PR, namely separating extension generation, so now there's some "parallel work" and the feeling of "two different people behind" if you get what I mean.

I'd like to rework this a tiny bit to properly acknowledge the new changes, such as renaming ext_doc to custom_ext_doc, but other than that it actually looks like luckily not much else has to be done. I expected a way worse situation lol.

Regarding the rebased branch, thanks for sharing it! Not sure if I'll need it, but it's appreciated nonetheless :D

Let's see what I can do ;)

[deralmas]

Ok, update: I gave it a good look and yes, I thought it was going to be way worse.

I pretty much only did what you asked (the var rename was unnecessary) and what you already did in your branch.

Note that I put the doc append block only once, in the ext doc generation routine, instead of both the normal and ext routines, so it's not exactly identical to your branch.

The only thing left is updating the example extension, which is trivial, although it would probably be useful to make a new "native" godot-cpp based branch with the draft API you've linked.

Other than that, I think that this is ready! 🎉

[dsnopek]

The code changes look great! I updated the changes in the draft PR for godot-cpp, and re-tested, and it worked great too.

IMO, this one is ready to merge :-)

[akien-mga]

Thanks!

[mihe]

I've been testing this functionality for a bit now, and I'm starting to realize that you can't actually document project settings added by extensions when using this.

I'm assuming this is due to the fact that project settings are effectively documented as properties on the ProjectSettings class, which presumably can't be declared (and ideally merged) multiple times? I've tried naively declaring it in my extension anyway, but that seems to just get ignored.

Even if that did work I guess it wouldn't work well with --doctool ../ --gdextension-docs since that file would get deleted every time.

Any thoughts/advice?

[dsnopek]

Hm. Yeah, we may need a special way to handle project settings.

Having a ProjectSettings.xml that is merged into the existing properties isn't a terrible way to handle it. If that could be made to work, then maybe we just need an additional argument that can be used with --doctool --gdextension-docs to list which project settings you want exported?

[mihe]

then maybe we just need an additional argument that can be used with --doctool --gdextension-docs to list which project settings you want exported?

Could that maybe be inferred from the property info of those project settings instead, seeing as how that's a Dictionary already, by adding some new entry to it, like "origin": "extension" or something? It would be nice not to have to pass them to --doctool manually.

[dsnopek]

Could that maybe be inferred from the property info of those project settings instead, seeing as how that's a Dictionary already, by adding some new entry to it, like "origin": "extension" or something?

Perhaps? It gets converted to a PropertyInfo struct internally, but maybe this info could be kept on the ProjectSettings singleton and the doc tool could get it from there?

It also occurs to me that we have the same problem with export settings, which are treated as properties of the export platform (ex EditorExportPlatformAndroid).

And, we have the same problem with GDScript based editor plugins too: they can also add project settings and export settings, and it would be nice to be able to document them.

So, maybe we need an API that can be used by either GDExtension or GDScript for this case? The documentation XML loading is GDExtension only.

[elementbound]

@mihe @dsnopek are you aware of any task or proposal about documenting custom ProjectSettings entries? I've recently ran into needing the same in foxssake/netfox#244 and in foxssake/netfox#314, both to document existing settings and to notify users that some settings are now deprecated and will be removed in the future.

[dsnopek]

There is a proposal about it: godotengine/godot-proposals#10652