Merge pull request #65266 from raulsntos/dotnet/reload-non-tool-scripts

Create script instance of reloaded scripts even if they're not tools

3 files changed, 44 insertions, 6 deletions

diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp
index 64792a795f..990a95821e 100644
--- a/modules/mono/csharp_script.cpp
+++ b/modules/mono/csharp_script.cpp
@@ -784,6 +784,13 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) {
 		for (Object *obj : script->instances) {
 			script->pending_reload_instances.insert(obj->get_instance_id());
 
+			// Since this script instance wasn't a placeholder, add it to the list of placeholders
+			// that will have to be eventually replaced with a script instance in case it turns into one.
+			// This list is not cleared after the reload and the collected instances only leave
+			// the list if the script is instantiated or if it was a tool script but becomes a
+			// non-tool script in a rebuild.
+			script->pending_replace_placeholders.insert(obj->get_instance_id());
+
 			RefCounted *rc = Object::cast_to<RefCounted>(obj);
 			if (rc) {
 				rc_instances.push_back(Ref<RefCounted>(rc));
@@ -836,6 +843,7 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) {
 			obj->set_script(Ref<RefCounted>()); // Remove script and existing script instances (placeholder are not removed before domain reload)
 		}
 
+		script->was_tool_before_reload = script->tool;
 		script->_clear();
 	}
 
@@ -924,24 +932,34 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) {
 
 				ScriptInstance *si = obj->get_script_instance();
 
+				// Check if the script must be instantiated or kept as a placeholder
+				// when the script may not be a tool (see #65266)
+				bool replace_placeholder = script->pending_replace_placeholders.has(obj->get_instance_id());
+				if (!script->is_tool() && script->was_tool_before_reload) {
+					// The script was a tool before the rebuild so the removal was intentional.
+					replace_placeholder = false;
+					script->pending_replace_placeholders.erase(obj->get_instance_id());
+				}
+
 #ifdef TOOLS_ENABLED
 				if (si) {
 					// If the script instance is not null, then it must be a placeholder.
 					// Non-placeholder script instances are removed in godot_icall_Object_Disposed.
 					CRASH_COND(!si->is_placeholder());
 
-					if (script->is_tool() || ScriptServer::is_scripting_enabled()) {
-						// Replace placeholder with a script instance
+					if (replace_placeholder || script->is_tool() || ScriptServer::is_scripting_enabled()) {
+						// Replace placeholder with a script instance.
 
 						CSharpScript::StateBackup &state_backup = script->pending_reload_state[obj_id];
 
-						// Backup placeholder script instance state before replacing it with a script instance
+						// Backup placeholder script instance state before replacing it with a script instance.
 						si->get_property_state(state_backup.properties);
 
 						ScriptInstance *script_instance = script->instance_create(obj);
 
 						if (script_instance) {
 							script->placeholders.erase(static_cast<PlaceHolderScriptInstance *>(si));
+							script->pending_replace_placeholders.erase(obj->get_instance_id());
 							obj->set_script_instance(script_instance);
 						}
 					}
@@ -951,8 +969,24 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) {
 #else
 				CRASH_COND(si != nullptr);
 #endif
-				// Re-create script instance
-				obj->set_script(script); // will create the script instance as well
+
+				// Re-create the script instance.
+				if (replace_placeholder || script->is_tool() || ScriptServer::is_scripting_enabled()) {
+					// Create script instance or replace placeholder with a script instance.
+					ScriptInstance *script_instance = script->instance_create(obj);
+
+					if (script_instance) {
+						script->pending_replace_placeholders.erase(obj->get_instance_id());
+						obj->set_script_instance(script_instance);
+						continue;
+					}
+				}
+				// The script instance could not be instantiated or wasn't in the list of placeholders to replace.
+				obj->set_script(script);
+#if DEBUG_ENABLED
+				// If we reached here, the instantiated script must be a placeholder.
+				CRASH_COND(!obj->get_script_instance()->is_placeholder());
+#endif
 			}
 		}
 
diff --git a/modules/mono/csharp_script.h b/modules/mono/csharp_script.h
index 3509a5c87d..f2844a051d 100644
--- a/modules/mono/csharp_script.h
+++ b/modules/mono/csharp_script.h
@@ -90,6 +90,9 @@ class CSharpScript : public Script {
 
 	HashSet<ObjectID> pending_reload_instances;
 	RBMap<ObjectID, StateBackup> pending_reload_state;
+
+	bool was_tool_before_reload = false;
+	HashSet<ObjectID> pending_replace_placeholders;
 #endif
 
 	String source;
diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs
index 3884781988..092724a6b1 100644
--- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs
+++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs
@@ -130,7 +130,6 @@ namespace Godot.Bridge
             {
                 // Performance is not critical here as this will be replaced with source generators.
                 Type scriptType = _scriptTypeBiMap.GetScriptType(scriptPtr);
-                var obj = (Object)FormatterServices.GetUninitializedObject(scriptType);
 
                 var ctor = scriptType
                     .GetConstructors(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
@@ -151,6 +150,8 @@ namespace Godot.Bridge
                     }
                 }
 
+                var obj = (Object)FormatterServices.GetUninitializedObject(scriptType);
+
                 var parameters = ctor.GetParameters();
                 int paramCount = parameters.Length;