Do not cache the doc information written by user in the script in Inspector

[Rindbee]

This helps to resolve (partially) #70217 , but doesn't completely resolve it.

The inspector will be updated when * appears after modification, and then the inspector will not be updated again after continuing to save.

Fixed (partially) #63997.

Needs another PR to have the latest doc data ready before updating the tree for a full fix. See #71843 (comment).

[anvilfolk]

Why do you say it fixes it partially but not completely? It looks like it works fully!

[anvilfolk]

Since script classes aren't registered with ClassDB as far as I can tell, then this does exactly what it says on the tin. Stuff that is static, like native classes, can be cached. Scripts, on the other hand, will never be cached here and updated docs information will always be obtained.

This does mean more work with checking whether EditorHelp's thread is running, but it should be negligible.

Highly recommend merging :)

[Rindbee]

Why do you say it fixes it partially but not completely? It looks like it works fully!

GDScript::_update_doc() relies on data parsed during GDScript::reload(), so it should be called after GDScript::reload() to update the docs. But GDScript::update_exports() is not, it will be parsed separately, so it does not depend on GDScript::reload().

GDScript::reload() is currently called after the file is saved, while GDScript::update_exports() is not, and GDScript::update_exports() may eventually cause the property_list_changed signal to be emitted, which will casue EditorInspector::update_tree() to be called at the end of the frame.

godot/editor/plugins/script_text_editor.cpp

Line 476 in a7276f1

script->update_exports();

godot/modules/gdscript/gdscript.cpp

Lines 768 to 773 in a7276f1

	if (changed) {
	for (PlaceHolderScriptInstance *E : placeholders) {
	E->update(propnames, values);
	}
	} else {
	p_instance_to_update->update(propnames, values);

godot/core/object/script_language.cpp

Lines 609 to 611 in a7276f1

	if (owner && owner->get_script_instance() == this) {
	owner->notify_property_list_changed();
	}

The call to EditorInspector::update_tree() does not seem to be directly related to GDScript::reload().

[anvilfolk]

Oh, interesting! I wonder whether this is related to the issue I'm trying to track down, where if you have two different scripts attached to nodes, and all have documentation, when opening the project tooltips work for one script but not for the other.

I think this PR will fix the symptom though: empty docs lead to empty caches, but the script eventually should have docs registered in DocTools... maybe :)

Very cool detective work, this also gives me some more information to follow up on. Basically the issue seems to be that some forms of loading/using exports do not rely on reload(), which is what generates documentation. I believe that once #72095 is merged, we'll be able to get easier methods for generating documentation that don't rely on reload() :)

[Rindbee]

Yes, this PR hands over to other PRs the job of preparing the latest data for the document before calling EditorInspector::update_tree().

Considering the situation of #67203, perhaps it is better to cache separately?

[anvilfolk]

I believe this would fix #63997 as well. I feel like part of the linked issue here is a duplicate of that one?

[Rindbee]

I believe this would fix #63997 as well. I feel like part of the linked issue here is a duplicate of that one?

Yes, both are essentially caching issues.

[anvilfolk]

Just a quick thought - extension tends to refer to a file extension, like .txt. I'm not sure it's the most appropriate term for this. Maybe do not cache member doc data or something along those lines would be more accurate? It'd be neat to rename that - unless I'm totally misinterpreting the issue :)

I keep trying to get someone to look into this one and merge in the chat but no luck so far.

[Rindbee]

Now it seems to generate script doc information in GDScript::reload().

godot/modules/gdscript/gdscript.cpp

Lines 800 to 802 in ee86505

	// Done after compilation because it needs the GDScript object's inner class GDScript objects,
	// which are made by calling make_scripts() within compiler.compile() above.
	GDScriptDocGen::generate_docs(this, parser.get_tree());

So in order to update the doc information of the script in the Inspector, we need to call EditorInspector::update_tree() after the script is reloaded. This is the same as before.

[anvilfolk]

Now it seems to generate script doc information in GDScript::reload().

godot/modules/gdscript/gdscript.cpp

Lines 800 to 802 in ee86505

	// Done after compilation because it needs the GDScript object's inner class GDScript objects,
	// which are made by calling make_scripts() within compiler.compile() above.
	GDScriptDocGen::generate_docs(this, parser.get_tree());

So in order to update the doc information of the script in the Inspector, we need to call EditorInspector::update_tree() after the script is reloaded. This is the same as before.

Yes, I pulled out the documentation generation into its own class, which gets called in GDScript::reload() :) If we somehow manage to get access to the GDScript object, it's likely not too hard to make a method similar to GDScript::reload() that just docgens.

That feels like it could be a future PR though, doesn't need to be for this one!

[YuriSizov]

Thanks!

[anvilfolk]

Yessssssssssss, I am so happy!!! Everyone update their tooltips all the time now 🎉 🎉 🎉

Thanks @Rindbee!