Skip to content

Commit 17248b2

Browse files
60k41p60k41p
committed
Second try of crashes prevention on extension reload by ensuring signal teardown
1 parent acb8cfb commit 17248b2

5 files changed

Lines changed: 202 additions & 1 deletion

File tree

‎stagehand/utilities/disconnect_node_signal_graph.cpp‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
#include "stagehand/utilities/disconnect_node_signal_graph.h"
2+
3+
#include <unordered_set>
4+
5+
#include <godot_cpp/classes/object.hpp>
6+
#include <godot_cpp/variant/array.hpp>
7+
#include <godot_cpp/variant/callable.hpp>
8+
#include <godot_cpp/variant/dictionary.hpp>
9+
10+
namespace {
11+
using VisitedObjects = std::unordered_set<uint64_t>;
12+
13+
void cleanup_configuration_value_signals(const godot::Variant &value, VisitedObjects &visited_objects);
14+
15+
void disconnect_all_signal_connections(godot::Object *object) {
16+
if (object == nullptr) {
17+
return;
18+
}
19+
20+
const godot::TypedArray<godot::Dictionary> signal_list = object->get_signal_list();
21+
for (int signal_index = 0; signal_index < signal_list.size(); ++signal_index) {
22+
const godot::Dictionary signal_info = signal_list[signal_index];
23+
const godot::Variant signal_name_variant = signal_info.get("name", godot::Variant());
24+
if (signal_name_variant.get_type() != godot::Variant::STRING && signal_name_variant.get_type() != godot::Variant::STRING_NAME) {
25+
continue;
26+
}
27+
28+
const godot::StringName signal_name = signal_name_variant;
29+
const godot::TypedArray<godot::Dictionary> connections = object->get_signal_connection_list(signal_name);
30+
for (int connection_index = 0; connection_index < connections.size(); ++connection_index) {
31+
const godot::Dictionary connection_info = connections[connection_index];
32+
const godot::Variant callable_variant = connection_info.get("callable", godot::Variant());
33+
if (callable_variant.get_type() != godot::Variant::CALLABLE) {
34+
continue;
35+
}
36+
37+
const godot::Callable callable = callable_variant;
38+
if (object->is_connected(signal_name, callable)) {
39+
object->disconnect(signal_name, callable);
40+
}
41+
}
42+
}
43+
}
44+
45+
void disconnect_signals_in_object(godot::Object *object, VisitedObjects &visited_objects) {
46+
if (object == nullptr) {
47+
return;
48+
}
49+
50+
const uint64_t instance_id = object->get_instance_id();
51+
if (!visited_objects.insert(instance_id).second) {
52+
return;
53+
}
54+
55+
disconnect_all_signal_connections(object);
56+
57+
if (godot::Node *node = godot::Object::cast_to<godot::Node>(object)) {
58+
const godot::TypedArray<godot::Node> child_nodes = node->get_children();
59+
for (int child_index = 0; child_index < child_nodes.size(); ++child_index) {
60+
disconnect_signals_in_object(godot::Object::cast_to<godot::Node>(child_nodes[child_index]), visited_objects);
61+
}
62+
}
63+
64+
const godot::TypedArray<godot::Dictionary> property_list = object->get_property_list();
65+
for (int property_index = 0; property_index < property_list.size(); ++property_index) {
66+
const godot::Dictionary property_info = property_list[property_index];
67+
const uint32_t usage = static_cast<uint32_t>(property_info.get("usage", 0));
68+
if ((usage & (godot::PROPERTY_USAGE_STORAGE | godot::PROPERTY_USAGE_EDITOR)) == 0) {
69+
continue;
70+
}
71+
72+
const godot::Variant property_name_variant = property_info.get("name", godot::Variant());
73+
if (property_name_variant.get_type() != godot::Variant::STRING && property_name_variant.get_type() != godot::Variant::STRING_NAME) {
74+
continue;
75+
}
76+
77+
const godot::StringName property_name = property_name_variant;
78+
if (property_name == godot::StringName("script")) {
79+
continue;
80+
}
81+
82+
cleanup_configuration_value_signals(object->get(property_name), visited_objects);
83+
}
84+
}
85+
86+
void cleanup_configuration_value_signals(const godot::Variant &value, VisitedObjects &visited_objects) {
87+
switch (value.get_type()) {
88+
case godot::Variant::OBJECT:
89+
disconnect_signals_in_object(value.get_validated_object(), visited_objects);
90+
break;
91+
case godot::Variant::ARRAY: {
92+
const godot::Array array = value;
93+
for (int item_index = 0; item_index < array.size(); ++item_index) {
94+
cleanup_configuration_value_signals(array[item_index], visited_objects);
95+
}
96+
break;
97+
}
98+
case godot::Variant::DICTIONARY: {
99+
const godot::Dictionary dictionary = value;
100+
const godot::Array keys = dictionary.keys();
101+
for (int key_index = 0; key_index < keys.size(); ++key_index) {
102+
const godot::Variant key = keys[key_index];
103+
cleanup_configuration_value_signals(key, visited_objects);
104+
cleanup_configuration_value_signals(dictionary[key], visited_objects);
105+
}
106+
break;
107+
}
108+
default:
109+
break;
110+
}
111+
}
112+
} // namespace
113+
114+
void utilities::disconnect_node_signal_graph(godot::Node *root) {
115+
if (root == nullptr) {
116+
return;
117+
}
118+
119+
VisitedObjects visited_objects;
120+
const godot::TypedArray<godot::Node> child_nodes = root->get_children();
121+
for (int child_index = 0; child_index < child_nodes.size(); ++child_index) {
122+
disconnect_signals_in_object(godot::Object::cast_to<godot::Node>(child_nodes[child_index]), visited_objects);
123+
}
124+
}

‎stagehand/utilities/disconnect_node_signal_graph.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#pragma once
2+
3+
#include <godot_cpp/classes/node.hpp>
4+
5+
namespace utilities {
6+
/// Disconnects outgoing signal connections from a node's child object graph.
7+
/// This walks child nodes plus reachable stored Object, Array, and Dictionary values.
8+
void disconnect_node_signal_graph(godot::Node *root);
9+
} // namespace utilities

‎stagehand/world.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "stagehand/nodes/instanced_renderer_3d.h"
2121
#include "stagehand/nodes/multi_mesh_renderer.h"
2222
#include "stagehand/registry.h"
23+
#include "stagehand/utilities/disconnect_node_signal_graph.h"
2324
#include "stagehand/utilities/platform.h"
2425

2526
namespace stagehand {
@@ -33,7 +34,7 @@ namespace stagehand {
3334
#if defined(DEBUG_ENABLED)
3435
godot::UtilityFunctions::print(godot::String("Debug build. Enabling extra logging and Flecs Explorer: https://www.flecs.dev/explorer/?host=localhost"));
3536
world.set<flecs::Rest>({});
36-
world.import<flecs::stats>();
37+
world.import <flecs::stats>();
3738
// flecs::log::set_level(1);
3839
#endif
3940

@@ -524,6 +525,8 @@ namespace stagehand {
524525
enter_tree_setup_completed = false;
525526
post_tree_setup_completed = false;
526527

528+
utilities::disconnect_node_signal_graph(this);
529+
527530
if (is_initialised) {
528531
cleanup_instanced_renderer_rids();
529532
is_initialised = false;

‎tests/integration/tests/world_init/teardown_disconnects_child_resource_signals/teardown_disconnects_child_resource_signals.gd‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
extends Node
2+
3+
4+
func _ready() -> void:
5+
print("Test: FlecsWorld teardown disconnects child resource signals")
6+
7+
var world = $FlecsWorld
8+
var renderer = world.get_node_or_null("InstancedRenderer3D")
9+
assert_true(renderer != null, "InstancedRenderer3D child exists")
10+
11+
var lod_levels = renderer.get_lod_levels()
12+
assert_eq(lod_levels.size(), 1, "Single LOD level configured")
13+
14+
var lod_resource = lod_levels[0]
15+
assert_true(lod_resource.get_signal_connection_list("changed").size() > 0, "LOD resource changed signal connected while world is alive")
16+
17+
world.queue_free()
18+
await get_tree().process_frame
19+
await get_tree().process_frame
20+
21+
assert_true(not is_instance_valid(world), "FlecsWorld freed cleanly")
22+
assert_eq(lod_resource.get_signal_connection_list("changed").size(), 0, "LOD resource changed signal disconnected during world teardown")
23+
24+
lod_resource.set_mesh(SphereMesh.new())
25+
print(" PASS: LOD resource updates remain stable after FlecsWorld teardown")
26+
27+
print("FlecsWorld teardown signal cleanup test passed!")
28+
get_tree().quit(0)
29+
30+
31+
func assert_eq(actual, expected, label: String) -> void:
32+
if actual != expected:
33+
_fail("%s: expected %s, got %s" % [label, str(expected), str(actual)])
34+
else:
35+
print(" PASS: %s" % label)
36+
37+
38+
func assert_true(value: bool, label: String) -> void:
39+
if not value:
40+
_fail(label)
41+
else:
42+
print(" PASS: %s" % label)
43+
44+
45+
func _fail(msg: String) -> void:
46+
print("FAIL: %s" % msg)
47+
get_tree().quit(1)

‎tests/integration/tests/world_init/teardown_disconnects_child_resource_signals/teardown_disconnects_child_resource_signals.tscn‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
[gd_scene format=3]
2+
3+
[ext_resource type="Script" path="res://tests/world_init/teardown_disconnects_child_resource_signals/teardown_disconnects_child_resource_signals.gd" id="1_script"]
4+
5+
[sub_resource type="SphereMesh" id="SphereMesh_lod0"]
6+
7+
[sub_resource type="InstancedRenderer3DLODConfiguration" id="LODResource_0"]
8+
mesh = SubResource("SphereMesh_lod0")
9+
visibility_range_begin = 0.0
10+
visibility_range_end = 50.0
11+
12+
[node name="TeardownDisconnectsChildResourceSignals" type="Node"]
13+
script = ExtResource("1_script")
14+
15+
[node name="FlecsWorld" type="FlecsWorld" parent="."]
16+
17+
[node name="InstancedRenderer3D" type="InstancedRenderer3D" parent="FlecsWorld"]
18+
lod_levels = [SubResource("LODResource_0")]

0 commit comments

Comments
 (0)