Fix properties being lost when reloading placeholder GDScript instance

During reloading in `GDScriptLanguage::reload_all_scripts` a placeholder instance that must remain so is replaced with a new placeholder instance. The state is then restored by calling `ScriptInstance::set` for each property. This does not work if the script is missing the properties due to build/parse failing.
The fix for such cases is to call `placeholder_set_fallback` instead of `set` on the script instance.

I took this chance to move the `build_failed` flag from `PlaceHolderScriptInstance` to `Script`. That improves the code a lot. I also renamed it to `placeholder_fallback_enabled` which is a much better name (`build_failed` could lead to misunderstandings).

2 files changed, 12 insertions, 7 deletions

diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp
index 3c818898e6..294597d433 100644
--- a/modules/mono/csharp_script.cpp
+++ b/modules/mono/csharp_script.cpp
@@ -783,7 +783,7 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) {
 
 				// Even though build didn't fail, this tells the placeholder to keep properties and
 				// it allows using property_set_fallback for restoring the state without a valid script.
-				placeholder->set_build_failed(true);
+				scr->placeholder_fallback_enabled = true;
 
 				// Restore Variant properties state, it will be kept by the placeholder until the next script reloading
 				for (List<Pair<StringName, Variant> >::Element *G = scr->pending_reload_state[obj_id].properties.front(); G; G = G->next()) {
@@ -1830,12 +1830,10 @@ void CSharpScript::_update_exports_values(Map<StringName, Variant> &values, List
 bool CSharpScript::_update_exports() {
 
 #ifdef TOOLS_ENABLED
-	if (!valid) {
-		for (Set<PlaceHolderScriptInstance *>::Element *E = placeholders.front(); E; E = E->next()) {
-			E->get()->set_build_failed(true);
-		}
+	placeholder_fallback_enabled = true; // until proven otherwise
+
+	if (!valid)
 		return false;
-	}
 
 	bool changed = false;
 
@@ -1944,6 +1942,8 @@ bool CSharpScript::_update_exports() {
 		tmp_object = NULL;
 	}
 
+	placeholder_fallback_enabled = false;
+
 	if (placeholders.size()) {
 		// Update placeholders if any
 		Map<StringName, Variant> values;
@@ -1951,7 +1951,6 @@ bool CSharpScript::_update_exports() {
 		_update_exports_values(values, propnames);
 
 		for (Set<PlaceHolderScriptInstance *>::Element *E = placeholders.front(); E; E = E->next()) {
-			E->get()->set_build_failed(false);
 			E->get()->update(propnames, values);
 		}
 	}
@@ -2666,6 +2665,7 @@ CSharpScript::CSharpScript() :
 
 #ifdef TOOLS_ENABLED
 	source_changed_cache = false;
+	placeholder_fallback_enabled = false;
 	exports_invalidated = true;
 #endif
 
diff --git a/modules/mono/csharp_script.h b/modules/mono/csharp_script.h
index 501e0d9d6d..7177ed7d4a 100644
--- a/modules/mono/csharp_script.h
+++ b/modules/mono/csharp_script.h
@@ -115,6 +115,7 @@ class CSharpScript : public Script {
 	Map<StringName, Variant> exported_members_defval_cache; // member_default_values_cache
 	Set<PlaceHolderScriptInstance *> placeholders;
 	bool source_changed_cache;
+	bool placeholder_fallback_enabled;
 	bool exports_invalidated;
 	void _update_exports_values(Map<StringName, Variant> &values, List<PropertyInfo> &propnames);
 	virtual void _placeholder_erased(PlaceHolderScriptInstance *p_placeholder);
@@ -180,6 +181,10 @@ public:
 
 	virtual int get_member_line(const StringName &p_member) const;
 
+#ifdef TOOLS_ENABLED
+	virtual bool is_placeholder_fallback_enabled() const { return placeholder_fallback_enabled; }
+#endif
+
 	Error load_source_code(const String &p_path);
 
 	StringName get_script_name() const;