API updates to the AnimationPlayer #9002

DevinLeamy · 2023-06-30T05:07:20Z

Objective

Added AnimationPlayer API UX improvements.

Succestor to added some simple functions to AnimationPlayer #5912
Fixes More erganomic AnimationPlayer API #5848

(Credits to @asafigan for filing #5848, creating the initial pull request, and the discussion in #5912)

Solution

Created RepeatAnimation enum to describe an animation repetition behavior.
Added is_finished(), set_repeat(), and is_playback_reversed() methods to the animation player.
~~Made the animation clip optional as per the comment from added some simple functions to AnimationPlayer #5912~~

My problem is that the default handle [used the initialize a PlayingAnimation] could actually refer to an actual animation if an AnimationClip is set for the default handle, which leads me to ask, "Should animation_clip should be an Option?"

Added an accessor for the animation clip animation_clip() to the animation player.

To determine if an animation is finished, we use the number of times the animation has completed and the repetition behavior. If the animation is playing in reverse then elapsed < 0.0 counts as a completion. Otherwise, elapsed > animation.duration counts as a completion. This is what I would expect, personally. If there's any ambiguity, perhaps we could add some AnimationCompletionBehavior, to specify that kind of completion behavior to use.

Update: Previously PlayingAnimation::elapsed was being used as the seek time into the animation clip. This was misleading because if you increased the speed of the animation it would also increase (or decrease) the elapsed time. In other words, the elapsed time was not actually the elapsed time. To solve this, we introduce PlayingAnimation::seek_time to serve as the value we manipulate the move between keyframes. Consequently, elapsed() now returns the actual elapsed time, and is not effected by the animation speed. Because set_elapsed was being used to manipulate the displayed keyframe, we introduce AnimationPlayer::seek_to and AnimationPlayer::replay to provide this functionality.

Migration Guide

Removed set_elapsed.
Removed stop_repeating in favour of AnimationPlayer::set_repeat(RepeatAnimation::Never).
Introduced seek_to to seek to a given timestamp inside of the animation.
Introduced seek_time accessor for the PlayingAnimation::seek_to.
Introduced AnimationPlayer::replay to reset the PlayingAnimation to a state where no time has elapsed.

…ates

crates/bevy_animation/src/lib.rs

alice-i-cecile · 2023-06-30T17:07:14Z

@DevinLeamy the conflicts aren't trivial; could you resolve them so we can re-review and merge?

DevinLeamy · 2023-06-30T18:03:05Z

@DevinLeamy the conflicts aren't trivial; could you resolve them so we can re-review and merge?

@alice-i-cecile I'm not seeing any conflicts?

alice-i-cecile · 2023-06-30T19:43:00Z

Oops, I confused this with #5912!

DevinLeamy · 2023-07-01T13:54:56Z

Updated to incorporate the changes from #5912.

crates/bevy_animation/src/lib.rs

Pietrek14

LGTM

Shatur

I think we should remove old repeat and stop_repeating methods or update their descriptions to point to specific RepeatAnimation.

mockersf · 2023-08-12T15:25:24Z

Made the animation clip optional as per the comment from added some simple functions to AnimationPlayer #5912

I don't think the handle should be an option

crates/bevy_animation/src/lib.rs

…to animation-api

…ion-api

github-actions · 2023-08-18T03:01:32Z

Example alien_cake_addict failed to run, please try running it locally and check the result.

github-actions · 2023-08-18T03:01:41Z

Example alien_cake_addict failed to run, please try running it locally and check the result.

github-actions · 2023-08-18T03:08:52Z

Example alien_cake_addict failed to run, please try running it locally and check the result.

crates/bevy_animation/src/lib.rs

Co-authored-by: François <mockersf@gmail.com>

# Objective After #9002, it seems that "single shot" animations were broken. When completing, they would reset to their initial value. Which is generally not what you want. - Fixes #10480 ## Solution Avoid `%`-ing the animation after the number of completions exceeds the specified one. Instead, we early-return. This is also true when the player is playing in reverse. --- ## Changelog - Avoid resetting animations after `Repeat::Never` animation completion.

@asafigan

# Objective Added `AnimationPlayer` API UX improvements. - Succestor to bevyengine#5912 - Fixes bevyengine#5848 _(Credits to @asafigan for filing bevyengine#5848, creating the initial pull request, and the discussion in bevyengine#5912)_ ## Solution - Created `RepeatAnimation` enum to describe an animation repetition behavior. - Added `is_finished()`, `set_repeat()`, and `is_playback_reversed()` methods to the animation player. - ~~Made the animation clip optional as per the comment from bevyengine#5912~~ > ~~My problem is that the default handle [used the initialize a `PlayingAnimation`] could actually refer to an actual animation if an AnimationClip is set for the default handle, which leads me to ask, "Should animation_clip should be an Option?"~~ - Added an accessor for the animation clip `animation_clip()` to the animation player. To determine if an animation is finished, we use the number of times the animation has completed and the repetition behavior. If the animation is playing in reverse then `elapsed < 0.0` counts as a completion. Otherwise, `elapsed > animation.duration` counts as a completion. This is what I would expect, personally. If there's any ambiguity, perhaps we could add some `AnimationCompletionBehavior`, to specify that kind of completion behavior to use. Update: Previously `PlayingAnimation::elapsed` was being used as the seek time into the animation clip. This was misleading because if you increased the speed of the animation it would also increase (or decrease) the elapsed time. In other words, the elapsed time was not actually the elapsed time. To solve this, we introduce `PlayingAnimation::seek_time` to serve as the value we manipulate the move between keyframes. Consequently, `elapsed()` now returns the actual elapsed time, and is not effected by the animation speed. Because `set_elapsed` was being used to manipulate the displayed keyframe, we introduce `AnimationPlayer::seek_to` and `AnimationPlayer::replay` to provide this functionality. ## Migration Guide - Removed `set_elapsed`. - Removed `stop_repeating` in favour of `AnimationPlayer::set_repeat(RepeatAnimation::Never)`. - Introduced `seek_to` to seek to a given timestamp inside of the animation. - Introduced `seek_time` accessor for the `PlayingAnimation::seek_to`. - Introduced `AnimationPlayer::replay` to reset the `PlayingAnimation` to a state where no time has elapsed. --------- Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com> Co-authored-by: François <mockersf@gmail.com>

# Objective After bevyengine#9002, it seems that "single shot" animations were broken. When completing, they would reset to their initial value. Which is generally not what you want. - Fixes bevyengine#10480 ## Solution Avoid `%`-ing the animation after the number of completions exceeds the specified one. Instead, we early-return. This is also true when the player is playing in reverse. --- ## Changelog - Avoid resetting animations after `Repeat::Never` animation completion.

DevinLeamy added 3 commits June 30, 2023 00:48

RepeatAnimation, is_playback_reversed(), is_finished(), animation upd…

e663e51

…ates

Apply modulo to elapsed time

a6488df

Greater than or equals

169a94c

jdm reviewed Jun 30, 2023

View reviewed changes

crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved

DevinLeamy added 3 commits June 30, 2023 01:17

lint

1400461

Updated names and comments

6e3b15c

Typo

eec1240

alice-i-cecile requested a review from mockersf June 30, 2023 17:04

alice-i-cecile added C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Animation Make things move and change over time labels Jun 30, 2023

DevinLeamy mentioned this pull request Jun 30, 2023

added some simple functions to AnimationPlayer #5912

Closed

DevinLeamy added 2 commits July 1, 2023 09:45

Added animation_clip() accessor and made animation_clip optional

a9f82a3

lint

a920e90

DevinLeamy added 2 commits July 1, 2023 09:59

Updated comment

57df0ac

Conditionally update the elapsed time

5af3277

mockersf reviewed Jul 1, 2023

View reviewed changes

crates/bevy_animation/src/lib.rs Show resolved Hide resolved

DevinLeamy added 2 commits July 1, 2023 11:03

repeat_mode() and completions() public accessors

1d829db

Updated comment

3ad8d27

Pietrek14 approved these changes Jul 4, 2023

View reviewed changes

DevinLeamy requested a review from mockersf July 7, 2023 03:36

alice-i-cecile requested a review from james7132 July 7, 2023 18:12

Shatur approved these changes Aug 12, 2023

View reviewed changes

Shatur suggested changes Aug 12, 2023

View reviewed changes

mockersf reviewed Aug 12, 2023

View reviewed changes

crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved

DevinLeamy added 3 commits August 17, 2023 22:51

Merge branch 'animation-api' of https://github.com/DevinLeamy/bevy in…

5b8da3f

…to animation-api

Merge branch 'main' of https://github.com/bevyengine/bevy into animat…

f52c991

…ion-api

nit: format

7910f05

Shatur approved these changes Aug 18, 2023

View reviewed changes

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 18, 2023

DevinLeamy added 2 commits August 22, 2023 19:35

Reformat comments + re-run CI, post moving run-examples over to windows

6527d72

nit: cargo fmt --all

f50c8c4

mockersf reviewed Aug 23, 2023

View reviewed changes

crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved

mockersf reviewed Aug 23, 2023

View reviewed changes

crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved

mockersf reviewed Aug 23, 2023

View reviewed changes

crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved

mockersf reviewed Aug 23, 2023

View reviewed changes

crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved

mockersf reviewed Aug 23, 2023

View reviewed changes

crates/bevy_animation/src/lib.rs Show resolved Hide resolved

mockersf reviewed Aug 23, 2023

View reviewed changes

crates/bevy_animation/src/lib.rs Show resolved Hide resolved

Applied suggestions (updates to comments and inlines)

8d3b642

Co-authored-by: François <mockersf@gmail.com>

DevinLeamy requested a review from mockersf August 24, 2023 00:53

alice-i-cecile added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Aug 28, 2023

alice-i-cecile added this pull request to the merge queue Aug 28, 2023

Merged via the queue into bevyengine:main with commit db5f80b Aug 28, 2023

DevinLeamy deleted the animation-api branch August 28, 2023 17:09

nicopap mentioned this pull request Oct 12, 2023

Batch skinned meshes #10094

Open

cart mentioned this pull request Oct 13, 2023

News: Release 0.12 bevyengine/bevy-website#754

Merged

43 tasks

nicopap mentioned this pull request Nov 13, 2023

Fix animations resetting after repeat count #10540

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API updates to the AnimationPlayer #9002

API updates to the AnimationPlayer #9002

DevinLeamy commented Jun 30, 2023 •

edited

Loading

alice-i-cecile commented Jun 30, 2023

DevinLeamy commented Jun 30, 2023

alice-i-cecile commented Jun 30, 2023

DevinLeamy commented Jul 1, 2023

Pietrek14 left a comment

Shatur left a comment

mockersf commented Aug 12, 2023

github-actions bot commented Aug 18, 2023

github-actions bot commented Aug 18, 2023

github-actions bot commented Aug 18, 2023

API updates to the AnimationPlayer #9002

API updates to the AnimationPlayer #9002

Conversation

DevinLeamy commented Jun 30, 2023 • edited Loading

Objective

Solution

Migration Guide

alice-i-cecile commented Jun 30, 2023

DevinLeamy commented Jun 30, 2023

alice-i-cecile commented Jun 30, 2023

DevinLeamy commented Jul 1, 2023

Pietrek14 left a comment

Choose a reason for hiding this comment

Shatur left a comment

Choose a reason for hiding this comment

mockersf commented Aug 12, 2023

github-actions bot commented Aug 18, 2023

github-actions bot commented Aug 18, 2023

github-actions bot commented Aug 18, 2023

DevinLeamy commented Jun 30, 2023 •

edited

Loading