rayxuln / spine-runtime-for-godot

This project is a module for godot that allows it to load/play Spine skeleton animation.
MIT License
93 stars 22 forks source link

[Bug] Setting skin resource from code #17

Closed yuraj11 closed 4 years ago

yuraj11 commented 4 years ago

Calling this when skin skin resource is already set:

spine.animation_state_data_res = load(type)
spine.packed_skin_resource = load(skin)

causes this issue in logs:

connect: Signal 'property_changed' is already connected to given method '_on_skin_property_changed' in that object.
scottkunkel commented 4 years ago

We can disconnect the signal before assigning the new skin. Alternatively, we can set the skin, not do the signal connection when a skin was already there and trigger the signal in the c code itself.

@rayxuln What is the purpose of the signal? The skin will not change by itself but has to be assigned manually, meaning the programmer can run the signal itself. Looking over the code, I think we can remove the signal completely since setting the skin will also run update_runtime_skin.

rayxuln commented 4 years ago

It's for notifying the SpineSprite to change the skin when the user changed the skin names in the packed skin resources in the editor.

Don't know why this error happened, but it's not fetal.

I'll fix it later.

scottkunkel commented 4 years ago

You just need to disconnect signal from the old skin if it’s there.

Thomas

On Jul 20, 2020, at 8:15 PM, Rayxuln notifications@github.com wrote:

 It's for noticing the SpineSprite to change the skin when the user changed the skin names in the packed skin resources in the editor.

Don't know why this error happened, but it's not fetal.

I'll fix it later.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

scottkunkel commented 4 years ago

If that signal is needed, then my pr did it wrong by removing it entirely.

Thomas

On Jul 20, 2020, at 8:15 PM, Rayxuln notifications@github.com wrote:

 It's for noticing the SpineSprite to change the skin when the user changed the skin names in the packed skin resources in the editor.

Don't know why this error happened, but it's not fetal.

I'll fix it later.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

yuraj11 commented 4 years ago

Thanks. Fixed!