.NET: Add a warning in the inspector when properties might be out of sync

[paulloz]

Add a small warning in the inspector when C# scripts are out of sync with data we gathered during compilation. The warning is displayed if any C# script in the class hierarchy was modified since the last compilation, and disappears as soon as possible.

Sadly, the inspector is not refreshed ATM when the window is refocused (e.g. if you tab back from your external code editor). From my tests, this behaviour is not language-dependent, so it'd probably warrant a fix higher up (therefore, I didn't look how to do so in this PR).

I gladly take any suggestion for UI and wording. E.g. someone suggested to directly add a button/link to build directly from the warning. But I'm not sure how I feel about that.

[raulsntos]

Thank you so much for working on this.

[raulsntos]

Sadly, the inspector is not refreshed ATM when the window is refocused

That's really unfortunate. When modifying a GDScript in an external editor the inspector updates when refocusing the Godot editor, but I couldn't find any calls to InspectorDock::get_inspector_singleton()->update_tree(); in the GDScript module.

It's probably triggered somewhere in GDScript::_update_exports, maybe we could do the same in CSharpScript::_update_exports.

[paulloz]

When modifying a GDScript in an external editor the inspector updates when refocusing the Godot editor

Uuh, you're right. I'm positive I tried it out and didn't see it refresh 🤔 Looking at it now, it comes from GDScript::update_exports() called by ScriptEditor::_update_modified_scripts_for_external_editor().

This does the trick, but I'm not entirely sure of all the potential implications. The idea being: when we're not sure something changed or not, consider it might have, and call notify_property_list_change() (which, in turn, queue an update).

diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp
index 8e1587997b..933d0a0f84 100644
--- a/modules/mono/csharp_script.cpp
+++ b/modules/mono/csharp_script.cpp
@@ -2246,6 +2246,13 @@ bool CSharpScript::_update_exports(PlaceHolderScriptInstance *p_instance_to_upda
                        } else {
                                p_instance_to_update->update(propnames, values);
                        }
+               } else if (placeholders.size()) {
+                       for (PlaceHolderScriptInstance *instance : placeholders) {
+                               Object *owner = instance->get_owner();
+                               if (owner->get_script_instance() == instance) {
+                                       owner->notify_property_list_changed();
+                               }
+                       }
                }
        }
 #endif

[raulsntos]

It'd be nice to notify only if the file last modified date is newer than the last valid build time, so maybe something like this:

diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp
index 8e1587997b..9759db5a0a 100644
--- a/modules/mono/csharp_script.cpp
+++ b/modules/mono/csharp_script.cpp
@@ -2246,6 +2246,18 @@ bool CSharpScript::_update_exports(PlaceHolderScriptInstance *p_instance_to_upda
 			} else {
 				p_instance_to_update->update(propnames, values);
 			}
+		} else if (placeholders.size()) {
+			uint64_t script_modified_time = FileAccess::get_modified_time(get_path());
+			uint64_t last_valid_build_time = FileAccess::get_modified_time(GodotSharpDirs::get_res_temp_assemblies_dir().path_join(path::get_csharp_project_name() + ".dll"));
+
+			if (script_modified_time > last_valid_build_time) {
+				for (PlaceHolderScriptInstance *instance : placeholders) {
+					Object *owner = instance->get_owner();
+					if (owner->get_script_instance() == instance) {
+						owner->notify_property_list_changed();
+					}
+				}
+			}
 		}
 	}
 #endif

I'm not sure if notify_property_list_change() is really the right thing to use here though. Maybe instead of iterating the placeholders, we could just call InspectorDock::get_inspector_singleton()->update_tree(); like the editor does when a GDExtension is reloaded:

godot/editor/editor_node.cpp

Lines 443 to 446 in 2d0ee20

	void EditorNode::_gdextensions_reloaded() {
	// In case the developer is inspecting an object that will be changed by the reload.
	InspectorDock::get_inspector_singleton()->update_tree();
	}

We also do that when reloading C# assemblies:

godot/modules/mono/csharp_script.cpp

Lines 1144 to 1148 in 2d0ee20

	// FIXME: Hack to refresh editor in order to display new properties and signals. See if there is a better alternative.
	if (Engine::get_singleton()->is_editor_hint()) {
	InspectorDock::get_inspector_singleton()->update_tree();
	NodeDock::get_singleton()->update_lists();
	}

[paulloz]

Yeah, I was pondering the call to update_tree() directly because I don't really know if there's a risk we end up calling that a bunch? Vs. notifying the change where it only dirties a flag, and the inspector will update only once. But if it's not a concern I 100% agree,

[YuriSizov]

As a rule of thumb you should never call update_tree manually. It should be done as little as possible. Calling notify_property_list_changed() is absolutely fine.

[paulloz]

It's probably triggered somewhere in GDScript::_update_exports, maybe we could do the same in CSharpScript::_update_exports.

Humpf, actually the issue is this is only triggered when text_editor/external/use_external_editor is set, but we use dotnet/editor/external_editor for C#.

[paulloz]

Yeah, not sure what the best way to handle this is. Should we have our own version of what's done in ScriptEditor::_update_modified_scripts_for_external_editor to update even when this setting is unset?

[raulsntos]

For me I have both settings unset and it seems to work, I guess it's because I have the C# script open in the Godot editor. But you are right that if I have text_editor/external/use_external_editor unset and dotnet/editor/external_editor set it won't update the inspector on refocusing the Godot window, every other combination seems to work.

Should we have our own version of what's done in ScriptEditor::_update_modified_scripts_for_external_editor to update even when this setting is unset?

Maybe we can just change ScriptEditor::_update_modified_scripts_for_external_editor to also consider scr->get_language()->overrides_external_editor() (like ScriptEditor::edit does).

[raulsntos]

Looks good to me and works as expected. Thanks again for contributing to the usability of the .NET module.

[paulloz]

Last push is simply squashing all commits, and using TTR() on the string used in UI.

[raulsntos]

using TTR() on the string used in UI.

That shouldn't be necessary, Label auto translates the Text property¹.

godot/scene/gui/label.cpp

Line 889 in 13a0d6e

xl_text = atr(p_string);

Footnotes

1. For more information, see the documentation: https://docs.godotengine.org/en/stable/tutorials/i18n/internationalizing_games.html#converting-keys-to-text ↩

[paulloz]

Urh, sorry, I thought it was necessary for the key to be extracted in the first place.

[raulsntos]

I don't think extracting works for C# either way, so all the other strings that we have with TTR() are probably not translated either. For example, I couldn't find the string Generating solution... in Weblate.

Maybe these strings need to be added to https://github.com/godotengine/godot-editor-l10n manually or implement C# support in extract_editor.py.

[paulloz]

So I guess having the TTR() call here is kinda future-proofing for when we add C# support to the extraction script 😅

[akien-mga]

Thanks!

[molingyu]

Can an editor option be provided so that the project can automatically perform C# compilation after the file is changed?

[paulloz]

Hello! There are already other issues discussing this

• Implement background building of project solution godot-proposals#849
• Godot Mono: Compile project on script change only godot-proposals#2231
• Auto (re)build project when C# script changes godot-proposals#2920
• Compile CSharp project on script change only #93431