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.

Sign up for GitHub

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

Jump to bottom

Add onLoop to FlxAnimationController #3216

Merged
merged 6 commits into from
Jul 13, 2024
Merged

Add onLoop to FlxAnimationController #3216

merged 6 commits into from
Jul 13, 2024

Conversation

richTrash21
Copy link
Contributor

@richTrash21 richTrash21 commented Jul 12, 2024 •
edited
Loading

Solves #3212 by adding onLoop signal field to the FlxAnimationController that dispatches each time the looped animation starts over.

@richTrash21 richTrash21 changed the title Add onLoopComplete in FlxAnimationController Add onLoopComplete to FlxAnimationController Jul 12, 2024
@Geokureli
Copy link
Member

Geokureli commented Jul 13, 2024 •
edited
Loading

what is the behavior when an animation has 1 frame and is set to loop? Does it fire once, never or repeatedly?

What are your thoughts on onLoop rather than onLoopComplete?

Geokureli
Geokureli reviewed Jul 13, 2024
View reviewed changes
flixel/animation/FlxAnimation.hx Outdated
parent.fireLoopCallback(name);
parent.onLoopComplete.dispatch(name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's revert this since it's how all the others work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted

@richTrash21
Copy link
Contributor Author

richTrash21 commented Jul 13, 2024 •
edited
Loading

what is the behavior when an animation has 1 frame and is set to loop? Does it fire once, never or repeatedly?

Currently testing this, but it will probably just repeatedly fire the callback.
Edit: yes, it repeatedly fires the callback, multiple times at once even. Currently adjusting the behavior

@Geokureli
Copy link
Member

Geokureli commented Jul 13, 2024 •
edited
Loading

yes, it repeatedly fires the callback, multiple times at once even. Currently adjusting the behavior

That's fine, and what I expected

Lastly, thoughts on this (edited so might have missed)

What are your thoughts on onLoop rather than onLoopComplete?

@richTrash21
Copy link
Contributor Author

Lastly, thoughts on this (edited so might have missed)

What are your thoughts on onLoop rather than onLoopComplete?

I think that's a good change

@richTrash21 richTrash21 changed the title Add onLoopComplete to FlxAnimationController Add onLoop to FlxAnimationController Jul 13, 2024
@richTrash21
Copy link
Contributor Author

richTrash21 commented Jul 13, 2024 •
edited
Loading

That's fine, and what I expected

Should I push fix then?
Edit: nvmd already merged so it should be fine

@Geokureli Geokureli added this to the 5.9.0 milestone Jul 13, 2024
@Geokureli Geokureli merged commit c762d99 into HaxeFlixel:dev Jul 13, 2024
11 checks passed
@richTrash21
Copy link
Contributor Author

richTrash21 commented Jul 13, 2024 •
edited
Loading

@Geokureli shouldn't there be "since 5.9.0" in the doc?

@richTrash21 richTrash21 mentioned this pull request Jul 13, 2024
Merged
@richTrash21 richTrash21 deleted the animation-loop-callback branch December 11, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Geokureli Geokureli Geokureli left review comments

Assignees
No one assigned
Labels
Animation/Frames New Feature
Projects
None yet
Milestone
5.9.0
Development

Successfully merging this pull request may close these issues.
2 participants
@richTrash21 @Geokureli