Prevent crashes on extension reload by ensuring signal teardown

Author: 60k41p

stagehand/nodes/instanced_renderer_3d.cpp

@@ -185,28 +185,44 @@ void InstancedRenderer3D::set_prefabs_rendered(const godot::PackedStringArray &p
     update_configuration_warnings();
 }
 
-void InstancedRenderer3D::set_lod_levels(const godot::TypedArray<InstancedRenderer3DLODConfiguration> &p_lod_levels) {
+void InstancedRenderer3D::connect_configuration_change_signals() {
     const godot::Callable warnings_callable = callable_mp(static_cast<godot::Node *>(this), &godot::Node::update_configuration_warnings);
 
     for (int i = 0; i < lod_levels.size(); ++i) {
-        godot::Ref<InstancedRenderer3DLODConfiguration> previous_lod_level = lod_levels[i];
-        if (previous_lod_level.is_valid() && previous_lod_level->is_connected("changed", warnings_callable)) {
-            previous_lod_level->disconnect("changed", warnings_callable);
+        godot::Ref<InstancedRenderer3DLODConfiguration> lod_level = lod_levels[i];
+        if (lod_level.is_valid() && !lod_level->is_connected("changed", warnings_callable)) {
+            lod_level->connect("changed", warnings_callable);
         }
     }
+}
 
-    lod_levels = p_lod_levels;
+void InstancedRenderer3D::disconnect_configuration_change_signals() {
+    const godot::Callable warnings_callable = callable_mp(static_cast<godot::Node *>(this), &godot::Node::update_configuration_warnings);
 
     for (int i = 0; i < lod_levels.size(); ++i) {
         godot::Ref<InstancedRenderer3DLODConfiguration> lod_level = lod_levels[i];
-        if (lod_level.is_valid() && !lod_level->is_connected("changed", warnings_callable)) {
-            lod_level->connect("changed", warnings_callable);
+        if (lod_level.is_valid() && lod_level->is_connected("changed", warnings_callable)) {
+            lod_level->disconnect("changed", warnings_callable);
         }
     }
+}
+
+void InstancedRenderer3D::set_lod_levels(const godot::TypedArray<InstancedRenderer3DLODConfiguration> &p_lod_levels) {
+    disconnect_configuration_change_signals();
+
+    lod_levels = p_lod_levels;
+
+    connect_configuration_change_signals();
 
     update_configuration_warnings();
 }
 
+void InstancedRenderer3D::_notification(int p_what) {
+    if (p_what == godot::Node::NOTIFICATION_EXIT_TREE || p_what == godot::Object::NOTIFICATION_PREDELETE) {
+        disconnect_configuration_change_signals();
+    }
+}
+
 godot::PackedStringArray InstancedRenderer3D::_get_configuration_warnings() const {
     godot::PackedStringArray warnings;
 

stagehand/nodes/instanced_renderer_3d.h

@@ -92,8 +92,12 @@ class InstancedRenderer3D : public godot::Node3D {
 
   protected:
     static void _bind_methods();
+    void _notification(int p_what);
 
   private:
+    void connect_configuration_change_signals();
+    void disconnect_configuration_change_signals();
+
     godot::PackedStringArray prefabs_rendered;
     godot::TypedArray<InstancedRenderer3DLODConfiguration> lod_levels;
     godot::Ref<godot::Material> material;

tests/integration/tests/instanced_renderer_lod_cleanup/instanced_renderer_lod_cleanup.gd

@@ -0,0 +1,46 @@
+extends Node3D
+
+
+func _ready() -> void:
+	print("Test: InstancedRenderer3D LOD signal cleanup")
+
+	var renderer = $InstancedRenderer3D
+	var lod_levels = renderer.get_lod_levels()
+	assert_eq(lod_levels.size(), 1, "Single LOD level configured")
+
+	renderer.set_lod_levels(lod_levels)
+
+	var lod_resource = lod_levels[0]
+	assert_true(lod_resource.get_signal_connection_list("changed").size() > 0, "LOD resource changed signal connected while renderer is alive")
+
+	renderer.queue_free()
+	await get_tree().process_frame
+	await get_tree().process_frame
+
+	assert_true(not is_instance_valid(renderer), "Renderer freed cleanly")
+	assert_eq(lod_resource.get_signal_connection_list("changed").size(), 0, "LOD resource changed signal disconnected after renderer teardown")
+
+	lod_resource.set_mesh(SphereMesh.new())
+	print("  PASS: LOD resource updates remain stable after renderer teardown")
+
+	print("InstancedRenderer3D LOD signal cleanup test passed!")
+	get_tree().quit(0)
+
+
+func assert_eq(actual, expected, label: String) -> void:
+	if actual != expected:
+		_fail("%s: expected %s, got %s" % [label, str(expected), str(actual)])
+	else:
+		print("  PASS: %s" % label)
+
+
+func assert_true(value: bool, label: String) -> void:
+	if not value:
+		_fail(label)
+	else:
+		print("  PASS: %s" % label)
+
+
+func _fail(msg: String) -> void:
+	print("FAIL: %s" % msg)
+	get_tree().quit(1)

tests/integration/tests/instanced_renderer_lod_cleanup/instanced_renderer_lod_cleanup.tscn

@@ -0,0 +1,16 @@
+[gd_scene format=3]
+
+[ext_resource type="Script" path="res://tests/instanced_renderer_lod_cleanup/instanced_renderer_lod_cleanup.gd" id="1_script"]
+
+[sub_resource type="SphereMesh" id="SphereMesh_lod0"]
+
+[sub_resource type="InstancedRenderer3DLODConfiguration" id="LODResource_0"]
+mesh = SubResource("SphereMesh_lod0")
+visibility_range_begin = 0.0
+visibility_range_end = 50.0
+
+[node name="InstancedRendererLODSignalCleanup" type="Node3D"]
+script = ExtResource("1_script")
+
+[node name="InstancedRenderer3D" type="InstancedRenderer3D" parent="."]
+lod_levels = [SubResource("LODResource_0")]