Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit a signal after animation completes #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Loufe
Copy link
Collaborator

@Loufe Loufe commented Dec 8, 2023

Hey Alex,

I wanted to be able to run some animations during cutscene dialogue and couldn't find a way to limit an action to a single repetition. This was my best solution, though it's not perfect. With this code I'm able to play an animation, then await the new signal to know it's done.

image

I don't like using flags that much but I needed to do it this way to ensure I'm grabbing a sprite with the used animation (in the case of multiple sprites) to not brake functionality for people using the add-on that way. Make sure it's flagged to avoid sending the signal multiple times.

Also to note that I use the add-on purely in code (no scene tree entry) so the signal cannot be linked with a node in the scene-tree to achieve this.

Open to your thoughts.

@alextrevisan
Copy link
Owner

alextrevisan commented Dec 11, 2023

I would say that this should be after the single spritesheet, because the AnimatedSprite2D already have a signal for animation_finished and we should use it instead of creating a new implementation. And it would be only one spritesheet, so only one signal emitted
https://docs.godotengine.org/en/stable/classes/class_animatedsprite2d.html#signals

@Loufe
Copy link
Collaborator Author

Loufe commented Dec 11, 2023

Yeah that's a fair point. I still think the LPC class should funnel back the signal, as other might use a no-scene-tree-node based implementation like I do. Any thoughts?

@alextrevisan
Copy link
Owner

I think this should be merged, just because of this: #20 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants