Polish & fix editor help cache generation #84354
Conversation
|
The Mono glue generation is crashing: https://github.com/godotengine/godot/actions/runs/6731199052/job/18295441262?pr=84354 It reports that it's missing GlobalScope, not sure if that's the source of the crash. |
- Isolated the generation of extensions's docs. They're now not cached and refreshed as needed. - Removed superfluous sorting of the class list. - Removed some superfluous/unused elements. - Renamed some items for clarity.
RandomShaper
force-pushed
the
gdext_no_cache_doc
branch
from
f75509a
to
a1d8fc1
Compare
|
Should be fixed now. |
| @@ -2415,14 +2419,15 @@ void EditorHelp::_gen_doc_thread(void *p_udata) { | |||
| } | |||
| } | |||
|
|
|||
| void EditorHelp::_gen_extensions_docs() { | |||
There was a problem hiding this comment.
I think this can be a public method (called generate_extension_docs), and I think to fully support extensions this needs to be called whenever the GDExtension manager hot reloads any library. IIUC, this right now only loads extension docs when loading the cache or generating it anew.
Though that said, we would have to also invalidate old records somehow. DocData should keep track if it's an extension class or not, if it doesn't already, and we should be able to nuke it all.
There was a problem hiding this comment.
In @Riteo's PR to allow GDExtensions to add fuller documentation, he's got a separate DocTools * just for the extension documentation in order to keep track of which is from extensions:
Perhaps that approach could work here?
(May not actually make sense, I don't know the documentation system that well :-))
There was a problem hiding this comment.
I don't think this would be necessary for this particular purpose (and besides, when we generate everything anew, we don't differentiate between sources of each class). ClassDB already has the API type stored, we can just read it and add it to the documentation entry.
There was a problem hiding this comment.
A bit late to the party but yeah, the separate DocTools is actually just a way to persist that non-generated data between regens.
There was a problem hiding this comment.
I can confirm this fixes the issue, if we don't consider hot reloading. Code changes also look good to me.
I think hot reloading can be fixed separately in a future PR (for 4.3 + maybe a 4.2.x cherry-pick). The implementation would be pretty straightforward, IMO. We can utilize DocTools::remove_doc and remove all docs related to extensions at the start of GDExtensionManager::reload_extension, and then regenerate them all after it is done. A bit excessive, but will get the job done.
We could probably use DocTools::remove_doc on individual classes before we reload the extension, if we can figure out which classes belong to it. But I believe a full (extension only) regen would be quick enough here already. If we do manage to remove individual classes, though, we should also make sure to regenerate only the newly added classes.
|
Thanks! |
FTR, this approach is implemented #83747. Note that the individual regen part is not implemented in said PR but I can make one after GodotCon. |
Fixes #82817.