Skip to content

Commit a3e8531

Browse files
JoltedJonJoltedJon
authored
Remove Singleton Pattern and fix crash in Skeleton3D Editor
* Fixes crash in Skeleton3D editor, issue #1178 * Remove Singleton Pattern in Skeleton3D Editor * Fix potential nullptr dereference on uninitialized variable * More pointer guards
1 parent 779d0e5 commit a3e8531

2 files changed

Lines changed: 40 additions & 36 deletions

File tree

‎editor/scene/3d/skeleton_3d_editor_plugin.cpp‎

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
#include "scene/3d/physics/collision_shape_3d.h"
4747
#include "scene/3d/physics/physical_bone_3d.h"
4848
#include "scene/3d/physics/physical_bone_simulator_3d.h"
49+
#include "scene/3d/skeleton_3d.h"
4950
#include "scene/gui/separator.h"
5051
#include "scene/gui/texture_rect.h"
5152
#include "scene/resources/3d/capsule_shape_3d.h"
@@ -137,10 +138,9 @@ void BonePropertiesEditor::_value_changed(const String &p_property, const Varian
137138
undo_redo->add_undo_property(skeleton, p_property, skeleton->get(p_property));
138139
undo_redo->add_do_property(skeleton, p_property, p_value);
139140

140-
Skeleton3DEditor *se = Skeleton3DEditor::get_singleton();
141-
if (se) {
142-
undo_redo->add_do_method(se, "update_joint_tree");
143-
undo_redo->add_undo_method(se, "update_joint_tree");
141+
if (skeleton_editor) {
142+
undo_redo->add_do_method(skeleton_editor, "update_joint_tree");
143+
undo_redo->add_undo_method(skeleton_editor, "update_joint_tree");
144144
}
145145

146146
undo_redo->commit_action();
@@ -201,7 +201,8 @@ void BonePropertiesEditor::_show_add_meta_dialog() {
201201
add_child(add_meta_dialog);
202202
}
203203

204-
int bone = Skeleton3DEditor::get_singleton()->get_selected_bone();
204+
ERR_FAIL_NULL(skeleton_editor);
205+
int bone = skeleton_editor->get_selected_bone();
205206
StringName dialog_title = skeleton->get_bone_name(bone);
206207

207208
List<StringName> existing_meta_keys;
@@ -210,7 +211,8 @@ void BonePropertiesEditor::_show_add_meta_dialog() {
210211
}
211212

212213
void BonePropertiesEditor::_add_meta_confirm() {
213-
int bone = Skeleton3DEditor::get_singleton()->get_selected_bone();
214+
ERR_FAIL_NULL(skeleton_editor);
215+
int bone = skeleton_editor->get_selected_bone();
214216
String name = add_meta_dialog->get_meta_name();
215217
EditorUndoRedoManager *undo_redo = EditorUndoRedoManager::get_singleton();
216218
undo_redo->create_action(vformat(TTR("Add metadata '%s' to bone '%s'"), name, skeleton->get_bone_name(bone)));
@@ -219,7 +221,8 @@ void BonePropertiesEditor::_add_meta_confirm() {
219221
undo_redo->commit_action();
220222
}
221223

222-
BonePropertiesEditor::BonePropertiesEditor(Skeleton3D *p_skeleton) {
224+
BonePropertiesEditor::BonePropertiesEditor(Skeleton3DEditor *p_skeleton_editor, Skeleton3D *p_skeleton) :
225+
skeleton_editor(p_skeleton_editor) {
223226
create_editors();
224227
set_skeleton(p_skeleton);
225228
}
@@ -287,10 +290,10 @@ void BonePropertiesEditor::_property_keyed(const String &p_path, bool p_advance)
287290
}
288291

289292
void BonePropertiesEditor::_update_properties() {
290-
if (!skeleton) {
293+
if (!skeleton || !skeleton_editor) {
291294
return;
292295
}
293-
int selected = Skeleton3DEditor::get_singleton()->get_selected_bone();
296+
int selected = skeleton_editor->get_selected_bone();
294297
List<PropertyInfo> props;
295298
HashSet<StringName> meta_seen;
296299
skeleton->get_property_list(&props);
@@ -359,8 +362,6 @@ void BonePropertiesEditor::_update_properties() {
359362
}
360363
}
361364

362-
Skeleton3DEditor *Skeleton3DEditor::singleton = nullptr;
363-
364365
void Skeleton3DEditor::set_keyable(const bool p_keyable) {
365366
keyable = p_keyable;
366367
if (p_keyable) {
@@ -1124,7 +1125,7 @@ void Skeleton3DEditor::create_editors() {
11241125
SET_DRAG_FORWARDING_GCD(joint_tree, Skeleton3DEditor);
11251126
s_con->add_child(joint_tree);
11261127

1127-
pose_editor = memnew(BonePropertiesEditor(skeleton));
1128+
pose_editor = memnew(BonePropertiesEditor(this, skeleton));
11281129
pose_editor->set_label(TTR("Bone Transform"));
11291130
pose_editor->set_visible(false);
11301131
add_child(pose_editor);
@@ -1165,6 +1166,9 @@ void Skeleton3DEditor::_notification(int p_what) {
11651166
update_joint_tree();
11661167
} break;
11671168
case NOTIFICATION_PREDELETE: {
1169+
if (editor_plugin) {
1170+
editor_plugin->skeleton_editor = nullptr;
1171+
}
11681172
if (skeleton) {
11691173
select_bone(-1); // Requires that the joint_tree has not been deleted.
11701174
_disconnect_from_skeleton();
@@ -1221,8 +1225,6 @@ void Skeleton3DEditor::edit_mode_toggled(const bool pressed) {
12211225
Skeleton3DEditor::Skeleton3DEditor(EditorInspectorPluginSkeleton *e_plugin, Skeleton3D *p_skeleton) :
12221226
editor_plugin(e_plugin),
12231227
skeleton(p_skeleton) {
1224-
singleton = this;
1225-
12261228
// Handle.
12271229
handle_material.instantiate();
12281230
handle_shader.instantiate();
@@ -1368,10 +1370,7 @@ void Skeleton3DEditor::_subgizmo_selection_change() {
13681370
}
13691371

13701372
int selected = -1;
1371-
Skeleton3DEditor *se = Skeleton3DEditor::get_singleton();
1372-
if (se) {
1373-
selected = se->get_selected_bone();
1374-
}
1373+
selected = get_selected_bone();
13751374

13761375
if (selected >= 0) {
13771376
Vector<Ref<Node3DGizmo>> gizmos = skeleton->get_gizmos();
@@ -1413,8 +1412,6 @@ void Skeleton3DEditor::select_bone(int p_idx) {
14131412
}
14141413

14151414
Skeleton3DEditor::~Skeleton3DEditor() {
1416-
singleton = nullptr;
1417-
14181415
handles_mesh_instance->queue_free();
14191416

14201417
Node3DEditor *ne = Node3DEditor::get_singleton();
@@ -1431,21 +1428,21 @@ void EditorInspectorPluginSkeleton::parse_begin(Object *p_object) {
14311428
Skeleton3D *skeleton = Object::cast_to<Skeleton3D>(p_object);
14321429
ERR_FAIL_NULL(skeleton);
14331430

1434-
skel_editor = memnew(Skeleton3DEditor(this, skeleton));
1435-
add_custom_control(skel_editor);
1431+
skeleton_editor = memnew(Skeleton3DEditor(this, skeleton));
1432+
add_custom_control(skeleton_editor);
14361433
}
14371434

14381435
Skeleton3DEditorPlugin::Skeleton3DEditorPlugin() {
14391436
skeleton_plugin = memnew(EditorInspectorPluginSkeleton);
14401437

14411438
EditorInspector::add_inspector_plugin(skeleton_plugin);
14421439

1443-
Ref<Skeleton3DGizmoPlugin> gizmo_plugin = Ref<Skeleton3DGizmoPlugin>(memnew(Skeleton3DGizmoPlugin));
1440+
Ref<Skeleton3DGizmoPlugin> gizmo_plugin = Ref<Skeleton3DGizmoPlugin>(memnew(Skeleton3DGizmoPlugin(skeleton_plugin)));
14441441
Node3DEditor::get_singleton()->add_gizmo_plugin(gizmo_plugin);
14451442
}
14461443

14471444
EditorPlugin::AfterGUIInput Skeleton3DEditorPlugin::forward_3d_gui_input(Camera3D *p_camera, const Ref<InputEvent> &p_event) {
1448-
Skeleton3DEditor *se = Skeleton3DEditor::get_singleton();
1445+
Skeleton3DEditor *se = skeleton_plugin->skeleton_editor;
14491446
Node3DEditor *ne = Node3DEditor::get_singleton();
14501447
if (se && se->is_edit_mode()) {
14511448
const Ref<InputEventMouseButton> mb = p_event;
@@ -1500,7 +1497,8 @@ int Skeleton3DEditor::get_selected_bone() const {
15001497

15011498
Skeleton3DGizmoPlugin::SelectionMaterials Skeleton3DGizmoPlugin::selection_materials;
15021499

1503-
Skeleton3DGizmoPlugin::Skeleton3DGizmoPlugin() {
1500+
Skeleton3DGizmoPlugin::Skeleton3DGizmoPlugin(EditorInspectorPluginSkeleton *p_skeleton_plugin) :
1501+
skeleton_plugin(p_skeleton_plugin) {
15041502
selection_materials.unselected_mat.instantiate();
15051503
selection_materials.unselected_mat->set_shading_mode(StandardMaterial3D::SHADING_MODE_UNSHADED);
15061504
selection_materials.unselected_mat->set_transparency(StandardMaterial3D::TRANSPARENCY_ALPHA);
@@ -1552,8 +1550,7 @@ int Skeleton3DGizmoPlugin::subgizmos_intersect_ray(const EditorNode3DGizmo *p_gi
15521550
Skeleton3D *skeleton = Object::cast_to<Skeleton3D>(p_gizmo->get_node_3d());
15531551
ERR_FAIL_NULL_V(skeleton, -1);
15541552

1555-
Skeleton3DEditor *se = Skeleton3DEditor::get_singleton();
1556-
1553+
Skeleton3DEditor *se = skeleton_plugin->skeleton_editor;
15571554
if (!se || !se->is_edit_mode()) {
15581555
return -1;
15591556
}
@@ -1629,9 +1626,13 @@ void Skeleton3DGizmoPlugin::commit_subgizmos(const EditorNode3DGizmo *p_gizmo, c
16291626
Skeleton3D *skeleton = Object::cast_to<Skeleton3D>(p_gizmo->get_node_3d());
16301627
ERR_FAIL_NULL(skeleton);
16311628

1632-
Skeleton3DEditor *se = Skeleton3DEditor::get_singleton();
16331629
Node3DEditor *ne = Node3DEditor::get_singleton();
16341630

1631+
Skeleton3DEditor *se = skeleton_plugin->skeleton_editor;
1632+
if (!se) {
1633+
ERR_FAIL_NULL(se);
1634+
}
1635+
16351636
EditorUndoRedoManager *ur = EditorUndoRedoManager::get_singleton();
16361637
ur->create_action(TTR("Set Bone Transform"));
16371638
if (ne->get_tool_mode() == Node3DEditor::TOOL_MODE_SELECT || ne->get_tool_mode() == Node3DEditor::TOOL_MODE_MOVE) {
@@ -1670,8 +1671,9 @@ void Skeleton3DGizmoPlugin::redraw(EditorNode3DGizmo *p_gizmo) {
16701671
return;
16711672
}
16721673

1674+
Skeleton3DEditor *se = skeleton_plugin->skeleton_editor;
1675+
16731676
int selected = -1;
1674-
Skeleton3DEditor *se = Skeleton3DEditor::get_singleton();
16751677
if (se) {
16761678
selected = se->get_selected_bone();
16771679
}

‎editor/scene/3d/skeleton_3d_editor_plugin.h‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class EditorInspectorPluginSkeleton;
4646
class EditorPropertyVector3;
4747
class Joint;
4848
class PhysicalBone3D;
49+
class Skeleton3DEditor;
4950
class Skeleton3DEditorPlugin;
5051
class Button;
5152
class Tree;
@@ -70,6 +71,7 @@ class BonePropertiesEditor : public VBoxContainer {
7071

7172
Rect2 background_rects[5];
7273

74+
Skeleton3DEditor *skeleton_editor = nullptr;
7375
Skeleton3D *skeleton = nullptr;
7476
// String property;
7577

@@ -95,7 +97,7 @@ class BonePropertiesEditor : public VBoxContainer {
9597
void _notification(int p_what);
9698

9799
public:
98-
BonePropertiesEditor(Skeleton3D *p_skeleton);
100+
BonePropertiesEditor(Skeleton3DEditor *p_skeleton_editor, Skeleton3D *p_skeleton);
99101

100102
// Which transform target to modify.
101103
void set_target(const String &p_prop);
@@ -158,8 +160,6 @@ class Skeleton3DEditor : public VBoxContainer {
158160

159161
bool keyable = false;
160162

161-
static Skeleton3DEditor *singleton;
162-
163163
void _on_click_skeleton_option(int p_skeleton_option);
164164
void _file_selected(const String &p_file);
165165
TreeItem *_find(TreeItem *p_node, const NodePath &p_path);
@@ -222,8 +222,6 @@ class Skeleton3DEditor : public VBoxContainer {
222222
void _node_removed(Node *p_node);
223223

224224
public:
225-
static Skeleton3DEditor *get_singleton() { return singleton; }
226-
227225
void select_bone(int p_idx);
228226

229227
int get_selected_bone() const;
@@ -247,8 +245,10 @@ class EditorInspectorPluginSkeleton : public EditorInspectorPlugin {
247245
GDCLASS(EditorInspectorPluginSkeleton, EditorInspectorPlugin);
248246

249247
friend class Skeleton3DEditorPlugin;
248+
friend class Skeleton3DGizmoPlugin;
249+
friend class Skeleton3DEditor;
250250

251-
Skeleton3DEditor *skel_editor = nullptr;
251+
Skeleton3DEditor *skeleton_editor = nullptr;
252252

253253
public:
254254
virtual bool can_handle(Object *p_object) override;
@@ -280,6 +280,8 @@ class Skeleton3DGizmoPlugin : public EditorNode3DGizmoPlugin {
280280
};
281281
static SelectionMaterials selection_materials;
282282

283+
EditorInspectorPluginSkeleton *skeleton_plugin = nullptr;
284+
283285
public:
284286
static Ref<ArrayMesh> get_bones_mesh(Skeleton3D *p_skeleton, int p_selected, bool p_is_selected);
285287

@@ -294,6 +296,6 @@ class Skeleton3DGizmoPlugin : public EditorNode3DGizmoPlugin {
294296

295297
void redraw(EditorNode3DGizmo *p_gizmo) override;
296298

297-
Skeleton3DGizmoPlugin();
299+
Skeleton3DGizmoPlugin(EditorInspectorPluginSkeleton *p_skeleton_plugin);
298300
~Skeleton3DGizmoPlugin();
299301
};

0 commit comments

Comments
 (0)