Allow submitting documentation to the Godot editor

[dsnopek]

Godot PR godotengine/godot#83747 added support for submitting documentation to the Godot editor from a GDExtension.

This PR aims to give godot-cpp users a way to use that, ideally, in much the same way as documentation is provided by modules.

Here's how it works:

• Using PR Generate docs from GDExtensions using --gdextension-docs with --doctool godot#91518, developers can run godot --doctool ../ --gdextension-docs in their demo project (assuming the project is in a directory below their top-level extension code) which will generate a doc_classes/ directory
• Edit the XML files in the doc_classes/ directory
• Add something like this to their SConstruct:

  if env["target"] in ["editor", "template_debug"]:
    doc_data = env.GodotCPPDocData("src/gen/doc_data.gen.cpp", source=Glob("doc_classes/*.xml"))
    sources.append(doc_data)
  

• Recompile
• And that's it!

When the GDExtension is loaded into the editor, it will automatically submit the documentation XML.

I'm not totally sure how to add this in CMake, but I think we can do it in a similar way to how we run binding_generator.py from CMake. I'd really appreciate any help with that!

Original description

I'm not entirely sure how we should expose this to developers. What I have here presently, is mostly about being able to test that the functionality from the Godot PR works. This could certainly use some design help for how to expose the build-system part (both for SCons and cmake) as well as how the developer should trigger the registration.

We'll also probably need some tooling somewhere, to assist in generating the XML files for extension classes. It looks like godot --doctool doesn't include extension classes, so we'll either need to modify it so that it does, or come up with some other process.

[dsnopek]

Alright, I've figured out a reasonably nice way for developers to use this functionality, that is as simple and automatic as I could make it. So, I'm taking this out of draft - it's ready for review!

The one main thing I still need to figure out is handling CMake support, but I don't think that'll be too hard.

Feedback on the overall approach would be appreciated!

[Naros]

Hi @dsnopek, what about being able to read documentation for existing classes? For example, a custom extension/plug-in that may want to display the description associated with a specific Godot native, extension contributed, or script contributed class as part of their UI?

[dsnopek]

@Naros:

what about being able to read documentation for existing classes? For example, a custom extension/plug-in that may want to display the description associated with a specific Godot native, extension contributed, or script contributed class as part of their UI?

I don't think Godot gives us an API to read documentation, unfortunately. That would be something to propose for Godot, before we could do anything in godot-cpp. If it was through classes bound in ClassDB then even GDScript could use it?

Your PR godotengine/godot#90189 is probably the closest thing that exists presently, allowing editor plugins to show particular documentation to the user, but not actually read its content.

[akien-mga]

Tested this PR and the upstream godot one on my Threen extension:
akien-mga/threen#3

It works great, and out of the box! The instructions in the PR were clear.

Some minor issues / potential future work I noticed:

• The first time I loaded the editor with the extension with docs compiled in, the doc page was still not showing descriptions. Restarting the editor fixed it. I couldn't reproduce it again, I tried to remove the descriptions from the XML and recompile, it showed empty descriptions as expected, then I re-added them, recompiled, and it showed descriptions the first time. Might still have some issues playing nice with the editor doc cache or time of first loading the extension.

• const methods don't seem to get the qualifiers="const" attribute, unlike they do in the core engine.

• The XML header has xsi:noNamespaceSchemaLocation="../class.xsd" like upstream, but this is meant to be a path to the class.xsd file, which isn't valid for GDExtension which live outside the Godot repo. It seems like this field expects any URI, so we can maybe use a direct link to the GitHub file: https://raw.githubusercontent.com/godotengine/godot/master/doc/class.xsd

[akien-mga]

Source compatibility

Should we consider the use case of maintaining an extension that may want to make use of this new feature for builds using 4.3+ as baseline, while still keeping the possibility to build for older Godot versions when targeting an older godot-cpp commit?

This may require adding an API in the godot-cpp env like env.has_feature("doc_data") or exposing version information so that one could check if a feature is supported before trying to use it in SConstruct.

diff --git a/SConstruct b/SConstruct
index 4f19de1..781fc98 100644
--- a/SConstruct
+++ b/SConstruct
@@ -7,9 +7,10 @@ env = SConscript("godot-cpp/SConstruct")
 env.Append(CPPPATH=["src/"])
 sources = Glob("src/*.cpp")
 
-if env["target"] in ["editor", "template_debug"]:
-    doc_data = env.GodotCPPDocData("src/gen/doc_data.gen.cpp", source=Glob("doc_classes/*.xml"))
-    sources.append(doc_data)
+if env.has_feature("doc_data"):
+    if env["target"] in ["editor", "template_debug"]:
+        doc_data = env.GodotCPPDocData("src/gen/doc_data.gen.cpp", source=Glob("doc_classes/*.xml"))
+        sources.append(doc_data)
 
 if env["platform"] == "macos":
     platlibname = "{}.{}.{}".format(libname, env["platform"], env["target"])

Of course this wouldn't work right now for versions of godot-cpp before adding this add_feature method, so one would need to check the method exists too with hasattr or similar. But this could be a system for checking feature availability once 4.3+ is reasonably the baseline for most extensions.

Another approach that's already usable for this specific feature is simply a try/except approach like this:

diff --git a/SConstruct b/SConstruct
index 4f19de1..e18ff0e 100644
--- a/SConstruct
+++ b/SConstruct
@@ -8,8 +8,11 @@ env.Append(CPPPATH=["src/"])
 sources = Glob("src/*.cpp")
 
 if env["target"] in ["editor", "template_debug"]:
-    doc_data = env.GodotCPPDocData("src/gen/doc_data.gen.cpp", source=Glob("doc_classes/*.xml"))
-    sources.append(doc_data)
+    try:
+        doc_data = env.GodotCPPDocData("src/gen/doc_data.gen.cpp", source=Glob("doc_classes/*.xml"))
+        sources.append(doc_data)
+    except AttributeError:
+        print("Not including class reference as we're targeting a pre-4.3 baseline.")
 
 if env["platform"] == "macos":
     platlibname = "{}.{}.{}".format(libname, env["platform"], env["target"])

[akien-mga]

Code looks good, and I tested the feature which works great.

[dsnopek]

@akien-mga Thanks for doing such extensive testing and review!

• The first time I loaded the editor with the extension with docs compiled in, the doc page was still not showing descriptions. Restarting the editor fixed it. I couldn't reproduce it again, I tried to remove the descriptions from the XML and recompile, it showed empty descriptions as expected, then I re-added them, recompiled, and it showed descriptions the first time. Might still have some issues playing nice with the editor doc cache or time of first loading the extension.

I also encountered this (but only two times), and I also suspect something with the doc cache. This needs more investigation, but I don't think the problem is on the godot-cpp side, so I think it's out-of-scope for this PR.

• const methods don't seem to get the qualifiers="const" attribute, unlike they do in the core engine.

This turned out to be a godot-cpp bug: I've added a fix for this in my latest push!

Should we consider the use case of maintaining an extension that may want to make use of this new feature for builds using 4.3+ as baseline, while still keeping the possibility to build for older Godot versions when targeting an older godot-cpp commit?

This may require adding an API in the godot-cpp env like env.has_feature("doc_data") or exposing version information so that one could check if a feature is supported before trying to use it in SConstruct.
[snip]
Another approach that's already usable for this specific feature is simply a try/except approach like this:

Maybe we can recommend the try/except approach for now, and consider something more organized for later?

As far as "recommending" the approach, I considered putting it into test/SConstruct, but that isn't really an example anymore, since we converted it into just automated tests (in PR #1101). I guess it should be added to https://github.com/godotengine/godot-cpp-template?

[akien-mga]

• const methods don't seem to get the qualifiers="const" attribute, unlike they do in the core engine.

This turned out to be a godot-cpp bug: I've added a fix for this in my latest push!

Tested and confirmed the fix, I got three methods in Threen with the const qualifier added.

Another approach that's already usable for this specific feature is simply a try/except approach like this:

Maybe we can recommend the try/except approach for now, and consider something more organized for later?

Sounds good yeah.

As far as "recommending" the approach, I considered putting it into test/SConstruct, but that isn't really an example anymore, since we converted it into just automated tests (in PR #1101). I guess it should be added to https://github.com/godotengine/godot-cpp-template?

Yes I think we could add this to godot-cpp-template.

[akien-mga]

Thanks!

[Naros]

Hi @dsnopek, tested this with Cmake and it works like a champ, thanks! 👍

However, I am finding that method and member details are not being rendered, although contributed.

<?xml version="1.0" encoding="UTF-8" ?>
<class name="TestClass" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../class.xsd">
    <brief_description>
        brief description here
    </brief_description>
    <description>
        description here
    </description>
    <tutorials>
        <link title="Math documentation index">$DOCS_URL/tutorials/math/index.html</link>
    </tutorials>
    <methods>
        <method name="test" qualifiers="const">
            <return type="void" />
            <param index="0" name="data" type="int" />
            <description>
                test function
            </description>
        </method>
    </methods>
    <members>
        <member name="text" type="String" setter="" getter="" default="&quot;Hello&quot;">
            The text that will be rendered.
        </member>
    </members>
</class>

[dsnopek]

Hi @dsnopek, tested this with Cmake and it works like a champ, thanks! 👍

Ah, I forgot to add CMake support :-/

I think it'd be a matter of splitting the make_doc_source() function in tools/godotcpp.py into separate script, so that CMake can run it, similar to how it runs binding_generator.py. That doesn't sound too hard? If you have the time to help with that, that'd be great!

However, I am finding that method and member details are not being rendered, although contributed.

Yeah, that sounds like it's not working. :-) Godot will make "fake" docs for GDExtension classes that don't have XML docs, that just show all the methods and members, but without the descriptions, which is what it sounds like you're seeing.

[dsnopek]

I started draft PR #1499, which has the refactoring required to add CMake support. I still need to figure out how to add it to CMake, though.

[Kehom]

I realize this is probably not the best place to ask this, however I'm at this point first trying to make sure I'm using it correctly. The thing is, no matter what, when attempting to use --doctool here Godot crashes if my extension is in the project. And it crashes even if I don't use --gdextension-docs. If I remove the extension binary from the project then Godot does generate the docs as it normally would do.

What I have attempted:

1. From the demo project directory, which is inside the "root" of the extension project, run godot --doctool ../ --gdextension-docs.
2. From the demo project directory, run godot --doctool <full_path_to_the_extension_project> --gdetension-docs
3. From the "root" of the extension project, run godot --path ./demo --doctool ./ --gdextension-docs
4. From the "root" of the extension project, run godot --path <full_path_to_demo> --doctool <full_path_to_extension_project> --gdextension-docs

I have repeated all of the previous commands after also copying the extension binaries to the root of the demo project. I also attempted to run all of those commands in an empty project containing only the extension binary. All attempts have resulted in the crash, both on Windows and Linux. At this point I'm wishing I'm not using it correctly!

That said, if I'm indeed using it correctly them I will open a bug report in the core Godot repository and if necessary compile Godot with debug symbols to get a proper stack trace report.

By the way, I have attempted this in beta 1 and beta 2 of Godot 4.3

[Kehom]

OK! I have found out the culprit and it was indeed on my end!

I have a singleton in my extension that attaches a node into the tree root as soon as a function that requires said node is called. In other words, I'm "lazy loading" that Node. However I forgot to verify the validity of the root. Now doc generation works.

Yet, before the fix:

• Loading Godot with that extension worked without any problems, including using those singletons
• Hot loading crashed Godot (however I never thought it could be the singletons)
• Attempting to generate documentation XML for the extension crashed Godot
• Attempting to generate mono-glue for the extension crashed Godot (albeit now I'm getting a completely different set of errors).

I realize the tree root might not be created at all during doc generation, but this puzzle me a little bit. Do the extension functions get called during this doc (and mono-glue) generation process?

[dsnopek]

Do the extension functions get called during this doc (and mono-glue) generation process?

Yes, GDExtensions are still initialized and all their classes registered when generating documentation, otherwise Godot wouldn't know that the classes from the GDExtensions exist, and couldn't generate documentation for them.

I'm less familiar with the mono-glue generation, but it sounds like GDExtensions are initialized in that case as well.

[Kehom]

Yes, GDExtensions are still initialized and all their classes registered when generating documentation, otherwise Godot wouldn't know that the classes from the GDExtensions exist, and couldn't generate documentation for them.

Ahh, sorry! I poorly formulated my question! Initialization I knew it did. My question was related to the functions registered in the _bind_methods(). In none of the "boilerplate" functions I'm attempting to access the tree root, only when explicitly calling those "custom functions". Which would be something like the doctool calling set_text() in the LineEdit control, as an example. That is what was puzzling me.

I'm less familiar with the mono-glue generation, but it sounds like GDExtensions are initialized in that case as well.

I'm not familiar with mono-glue at all. I just wanted to allow C# projects to also use my extension. However the set of errors I have encountered seem completely different and I will probably open a dedicated issue for that. From my limited knowledge and testing it seems mono-glue thing is generating a package for the entire engine, without having any way to tell it "hey, I want only the GDExtension stuff". Yet, I still need to perform some more tests here, so yeah...

Thank you very much the answer!

[dsnopek]

In none of the "boilerplate" functions I'm attempting to access the tree root, only when explicitly calling those "custom functions". Which would be something like the doctool calling set_text() in the LineEdit control, as an example.

It shouldn't call any of the property setters, but it will make one instance of every class and call all of their property getters, in order to figure out the default value of every property.

So, if your GDExtension is accessing the tree root in one of the constructors or property getters, then that would be called.

[Kehom]

Oh! Ok! Now everything makes sense! Thank you very much!

[Naros]

It shouldn't call any of the property setters, but it will make one instance of every class and call all of their property getters, in order to figure out the default value of every property.

Hi @dsnopek, I know for the inspector, it uses the property_can_revert and property_get_revert methods for this purpose, does this stage you're referring not use the same to avoid calling the getter, if the class implements those two methods? I would expect the getter to be called if not, but if those exist I wouldn't expect that behavior.

[dsnopek]

I believe this is done by ClassDB::class_get_property_default_value(), which appears to just be calling Object::get_property_list(), looping over all properties and then calling Object::get().

Just skimming through that code, it doesn't look like property_can_revert() or property_get_revert() are used, but I'm not 100% sure.

[Naros]

Ah, I was referring to code in the Inspector.

If I recall correctly, if you don't implement the property_can_revert() and property_get_revert() virtual overrides when you inspect objects, then somewhere in that code path it creates an instance of the inspected object for the sole purpose of calling the getters to determine the "default values" to handle the Inspector's revert functionality as this method does in ClassDB.

What I am curious (I don't have the code in front of me) is why this method doesn't attempt to do the same?

[dsnopek]

What I am curious (I don't have the code in front of me) is why this method doesn't attempt to do the same?

I don't know! However this works is how it has worked for quite a long time, and isn't something we changed for GDExtension - this is the same behavior modules have also had.