Skip to content

Commit 9b0eb01

Browse files
badlogicbadlogic
committed
[godot] Fix wrapper crash issues
Refs #3133.
1 parent 4498500 commit 9b0eb01

8 files changed

Lines changed: 60 additions & 28 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,8 @@
312312
- Added `SpineTrackEntry.get_additive()` / `set_additive()` for additive blending per track entry.
313313

314314
- **Bug fixes**
315+
- Fixed Godot wrapper crashes from invalid skin attachment lookups, wrong constraint type filtering, stale cached bone/slot wrappers after skeleton rebuilds, dangling `SpineSlotNode` connections after unparenting, and stale Godot 3 wrapper signal connections.
316+
- Fixed `SpineSprite` debug drawing and global bone transform helpers to work when a skeleton exists without an animation state.
315317
- Fixed draw order timelines not mixing out to the setup pose.
316318
- Fixed editor crashes when assigning skeleton data with slider animations that key slots or constraints.
317319
- Fixed attachment timelines so hidden setup-pose attachments remain hidden while mixing out, preserving deform behavior.

‎spine-godot/spine_godot/SpineCommon.h‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,9 @@ class SpineObjectWrapper : public REFCOUNTED {
128128

129129
Object *spine_owner;
130130
void *spine_object;
131+
#if VERSION_MAJOR <= 3
132+
ObjectID spine_owner_id;
133+
#endif
131134

132135
protected:
133136
static void _bind_methods() {
@@ -141,11 +144,27 @@ class SpineObjectWrapper : public REFCOUNTED {
141144
#else
142145
spine_owner->disconnect(SNAME("_internal_spine_objects_invalidated"), this, SNAME("_internal_spine_objects_invalidated"));
143146
#endif
147+
spine_owner = nullptr;
144148
}
145149

146-
SpineObjectWrapper() : spine_owner(nullptr), spine_object(nullptr) {
150+
SpineObjectWrapper() : spine_owner(nullptr), spine_object(nullptr)
151+
#if VERSION_MAJOR <= 3
152+
, spine_owner_id(0)
153+
#endif
154+
{
147155
}
148156

157+
#if VERSION_MAJOR <= 3
158+
~SpineObjectWrapper() {
159+
if (!spine_object) return;
160+
auto owner = ObjectDB::get_instance(spine_owner_id);
161+
if (!owner) return;
162+
if (owner->is_connected(SNAME("_internal_spine_objects_invalidated"), this, SNAME("_internal_spine_objects_invalidated"))) {
163+
owner->disconnect(SNAME("_internal_spine_objects_invalidated"), this, SNAME("_internal_spine_objects_invalidated"));
164+
}
165+
}
166+
#endif
167+
149168
template<typename OWNER, typename OBJECT>
150169
void _set_spine_object_internal(const OWNER *_owner, OBJECT *_object) {
151170
if (spine_owner) {
@@ -162,6 +181,9 @@ class SpineObjectWrapper : public REFCOUNTED {
162181
}
163182

164183
spine_owner = (Object *) _owner;
184+
#if VERSION_MAJOR <= 3
185+
spine_owner_id = spine_owner->get_instance_id();
186+
#endif
165187
spine_object = _object;
166188
#if VERSION_MAJOR > 3
167189
spine_owner->connect(SNAME("_internal_spine_objects_invalidated"), callable_mp(this, &SpineObjectWrapper::spine_objects_invalidated));

‎spine-godot/spine_godot/SpineSkeleton.cpp‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ SpineSkeleton::~SpineSkeleton() {
9999
void SpineSkeleton::set_spine_sprite(SpineSprite *_sprite) {
100100
delete skeleton;
101101
skeleton = nullptr;
102+
_cached_bones.clear();
103+
_cached_slots.clear();
102104
sprite = _sprite;
103105
if (!sprite || !sprite->get_skeleton_data_res().is_valid() || !sprite->get_skeleton_data_res()->is_skeleton_data_loaded()) return;
104106
skeleton = new spine::Skeleton(*sprite->get_skeleton_data_res()->get_skeleton_data());
@@ -310,10 +312,10 @@ Array SpineSkeleton::get_ik_constraints() {
310312
int size = 0;
311313
for (int i = 0; i < constraints.size(); ++i) {
312314
auto constraint = constraints[i];
313-
if (!constraint->getRTTI().isExactly(spine::IkConstraint::rtti) == false) continue;
315+
if (!constraint->getRTTI().isExactly(spine::IkConstraint::rtti)) continue;
314316
Ref<SpineIkConstraint> constraint_ref(memnew(SpineIkConstraint));
315317
constraint_ref->set_spine_object(sprite, static_cast<spine::IkConstraint *>(constraint));
316-
result[i] = constraint_ref;
318+
result[size] = constraint_ref;
317319
size++;
318320
}
319321
result.resize(size);
@@ -328,10 +330,10 @@ Array SpineSkeleton::get_transform_constraints() {
328330
int size = 0;
329331
for (int i = 0; i < constraints.size(); ++i) {
330332
auto constraint = constraints[i];
331-
if (!constraint->getRTTI().isExactly(spine::TransformConstraint::rtti) == false) continue;
333+
if (!constraint->getRTTI().isExactly(spine::TransformConstraint::rtti)) continue;
332334
Ref<SpineTransformConstraint> constraint_ref(memnew(SpineTransformConstraint));
333335
constraint_ref->set_spine_object(sprite, static_cast<spine::TransformConstraint *>(constraint));
334-
result[i] = constraint_ref;
336+
result[size] = constraint_ref;
335337
size++;
336338
}
337339
result.resize(size);
@@ -346,10 +348,10 @@ Array SpineSkeleton::get_path_constraints() {
346348
int size = 0;
347349
for (int i = 0; i < constraints.size(); ++i) {
348350
auto constraint = constraints[i];
349-
if (!constraint->getRTTI().isExactly(spine::PathConstraint::rtti) == false) continue;
351+
if (!constraint->getRTTI().isExactly(spine::PathConstraint::rtti)) continue;
350352
Ref<SpinePathConstraint> constraint_ref(memnew(SpinePathConstraint));
351353
constraint_ref->set_spine_object(sprite, static_cast<spine::PathConstraint *>(constraint));
352-
result[i] = constraint_ref;
354+
result[size] = constraint_ref;
353355
size++;
354356
}
355357
result.resize(size);
@@ -376,9 +378,9 @@ Array SpineSkeleton::get_sliders() {
376378
auto &constraints = skeleton->getConstraints();
377379
for (int i = 0; i < constraints.size(); ++i) {
378380
auto constraint = constraints[i];
379-
if (!constraint->getRTTI().isExactly(spine::Slider::rtti) == false) continue;
381+
if (!constraint->getRTTI().isExactly(spine::Slider::rtti)) continue;
380382
Ref<SpineSlider> slider_ref(memnew(SpineSlider));
381-
slider_ref->set_spine_object(sprite, (spine::Slider *) constraint);
383+
slider_ref->set_spine_object(sprite, static_cast<spine::Slider *>(constraint));
382384
result.append(slider_ref);
383385
}
384386
return result;

‎spine-godot/spine_godot/SpineSkin.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ void SpineSkin::set_attachment(int slot_index, const String &placeholder, Ref<Sp
8181
Ref<SpineAttachment> SpineSkin::get_attachment(int slot_index, const String &placeholder) {
8282
SPINE_CHECK(get_spine_object(), nullptr)
8383
auto attachment = get_spine_object()->getAttachment(slot_index, SPINE_STRING(placeholder));
84-
if (attachment) return nullptr;
84+
if (!attachment) return nullptr;
8585
Ref<SpineAttachment> attachment_ref(memnew(SpineAttachment));
8686
attachment_ref->set_spine_object(get_spine_owner(), attachment);
8787
return attachment_ref;

‎spine-godot/spine_godot/SpineSlotNode.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,20 +65,20 @@ void SpineSlotNode::_bind_methods() {
6565
"get_screen_material");
6666
}
6767

68-
SpineSlotNode::SpineSlotNode() : slot_index(-1) {
68+
SpineSlotNode::SpineSlotNode() : slot_index(-1), spine_sprite(nullptr) {
6969
}
7070

7171
void SpineSlotNode::_notification(int what) {
7272
switch (what) {
7373
case NOTIFICATION_PARENTED: {
74-
SpineSprite *sprite = cast_to<SpineSprite>(get_parent());
75-
if (sprite) {
74+
spine_sprite = cast_to<SpineSprite>(get_parent());
75+
if (spine_sprite) {
7676
#if VERSION_MAJOR > 3
77-
sprite->connect(SNAME("world_transforms_changed"), callable_mp(this, &SpineSlotNode::on_world_transforms_changed));
77+
spine_sprite->connect(SNAME("world_transforms_changed"), callable_mp(this, &SpineSlotNode::on_world_transforms_changed));
7878
#else
79-
sprite->connect(SNAME("world_transforms_changed"), this, SNAME("_on_world_transforms_changed"));
79+
spine_sprite->connect(SNAME("world_transforms_changed"), this, SNAME("_on_world_transforms_changed"));
8080
#endif
81-
update_transform(sprite);
81+
update_transform(spine_sprite);
8282
#if VERSION_MAJOR == 3
8383
_change_notify("transform/translation");
8484
_change_notify("transform/rotation");
@@ -95,13 +95,13 @@ void SpineSlotNode::_notification(int what) {
9595
break;
9696
}
9797
case NOTIFICATION_UNPARENTED: {
98-
SpineSprite *sprite = cast_to<SpineSprite>(get_parent());
99-
if (sprite) {
98+
if (spine_sprite) {
10099
#if VERSION_MAJOR > 3
101-
sprite->disconnect(SNAME("world_transforms_changed"), callable_mp(this, &SpineSlotNode::on_world_transforms_changed));
100+
spine_sprite->disconnect(SNAME("world_transforms_changed"), callable_mp(this, &SpineSlotNode::on_world_transforms_changed));
102101
#else
103-
sprite->disconnect(SNAME("world_transforms_changed"), this, SNAME("_on_world_transforms_changed"));
102+
spine_sprite->disconnect(SNAME("world_transforms_changed"), this, SNAME("_on_world_transforms_changed"));
104103
#endif
104+
spine_sprite = nullptr;
105105
}
106106
break;
107107
}

‎spine-godot/spine_godot/SpineSlotNode.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class SpineSlotNode : public Node2D {
4444
protected:
4545
String slot_name;
4646
int slot_index;
47+
SpineSprite *spine_sprite;
4748
Ref<Material> normal_material;
4849
Ref<Material> additive_material;
4950
Ref<Material> multiply_material;

‎spine-godot/spine_godot/SpineSprite.cpp‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,7 +1056,7 @@ void createLinesFromMesh(Vector<Vector2> &scratch_points, spine::Array<unsigned
10561056
}
10571057

10581058
void SpineSprite::draw() {
1059-
if (!animation_state.is_valid() && !skeleton.is_valid()) return;
1059+
if (!skeleton.is_valid()) return;
10601060
if (!Engine::get_singleton()->is_editor_hint() && !get_tree()->is_debugging_collisions_hint()) return;
10611061

10621062
auto &statics = SpineSpriteStatics::instance();
@@ -1288,14 +1288,16 @@ void SpineSprite::draw() {
12881288
draw_set_transform(mouse_position + Vector2(20, 0), -get_global_rotation(),
12891289
Vector2(inverse_zoom * (1 / global_scale.x), inverse_zoom * (1 / global_scale.y)));
12901290

1291-
Ref<Font> default_font;
1292-
auto control = memnew(Control);
1291+
if (!debug_font.is_valid()) {
1292+
auto control = memnew(Control);
12931293
#if VERSION_MAJOR > 3
1294-
default_font = control->get_theme_default_font();
1294+
debug_font = control->get_theme_default_font();
12951295
#else
1296-
default_font = control->get_font(SNAME("font"), SNAME("Label"));
1296+
debug_font = control->get_font(SNAME("font"), SNAME("Label"));
12971297
#endif
1298-
memdelete(control);
1298+
memdelete(control);
1299+
}
1300+
const Ref<Font> &default_font = debug_font;
12991301

13001302
#if VERSION_MAJOR > 3
13011303
#ifdef SPINE_GODOT_EXTENSION
@@ -1389,7 +1391,7 @@ void SpineSprite::callback(spine::AnimationState *state, spine::EventType type,
13891391
}
13901392

13911393
Transform2D SpineSprite::get_global_bone_transform(const String &bone_name) {
1392-
if (!animation_state.is_valid() && !skeleton.is_valid()) return get_global_transform();
1394+
if (!skeleton.is_valid()) return get_global_transform();
13931395
auto bone = skeleton->find_bone(bone_name);
13941396
if (!bone.is_valid()) {
13951397
return get_global_transform();
@@ -1398,7 +1400,7 @@ Transform2D SpineSprite::get_global_bone_transform(const String &bone_name) {
13981400
}
13991401

14001402
void SpineSprite::set_global_bone_transform(const String &bone_name, Transform2D transform) {
1401-
if (!animation_state.is_valid() && !skeleton.is_valid()) return;
1403+
if (!skeleton.is_valid()) return;
14021404
auto bone = skeleton->find_bone(bone_name);
14031405
if (!bone.is_valid()) return;
14041406
bone->set_global_transform(transform);

‎spine-godot/spine_godot/SpineSprite.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@
3737
#include <godot_cpp/templates/vector.hpp>
3838
#include <godot_cpp/classes/rendering_server.hpp>
3939
#include <godot_cpp/classes/canvas_item_material.hpp>
40+
#include <godot_cpp/classes/font.hpp>
4041
#else
4142
#include "scene/2d/node_2d.h"
43+
#include "scene/resources/font.h"
4244
#if VERSION_MAJOR > 3
4345
#include "servers/rendering/rendering_server.h"
4446
#endif
@@ -168,6 +170,7 @@ class SpineSprite : public Node2D, public spine::AnimationStateListenerObject {
168170

169171
spine::Array<spine::Array<SpineSlotNode *>> slot_nodes;
170172
Vector<SpineMesh2D *> mesh_instances;
173+
Ref<Font> debug_font;
171174
Ref<Material> normal_material;
172175
Ref<Material> additive_material;
173176
Ref<Material> multiply_material;

0 commit comments

Comments
 (0)