fix: emit animation_finished only when all loops complete - #222
Merged
Merged
Conversation
`ss_runtime_is_end_frame_reached` means "the last frame of a cycle is showing", not "playback finished". With a finite loop count the runtime raises it once per cycle, and under the default infinite count it is never raised at all -- so wiring `animation_finished` straight to it fired the signal once per loop iteration. `set_loop_count(3)` emitted three `animation_finished` instead of `looped, looped, finished`. Switch to `ss_runtime_is_finished`, the terminal signal added upstream. It is a sticky state rather than a pulse; the existing `is_playing` early-return is what keeps it from re-emitting, so note that dependency where it matters. `animation_looped` is unchanged -- it still fires on every wrap, including under an infinite loop count, which is where cycle hooks are most useful. Also corrects the docs, which claimed `0` plays once (it is a synonym for infinite) and that `animation_finished` is emitted for non-looping animations only. Verified headless against this SDK branch: set_loop_count(3) -> looped, looped, finished set_loop_count(1) -> finished set_loop_count(-1) -> looped, looped, ... (never finishes) set_loop_count(0) -> identical to -1 The SDK submodule is bumped to the branch of cri-middleware/SpriteStudio-SDK#307 so CI can build the runtime; it needs re-pointing at develop once that merges.
Picks up the end-frame event-duplication fix, so CI builds the runtime this PR is actually written against. Still re-point at the default branch once #307 lands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
animation_finishedwas wired straight toss_runtime_is_end_frame_reached, which does not mean "playback finished" — it means "the last frame of a cycle is showing". The runtime raises it once per cycle with a finite loop count, and never under the default infinite count.So
set_loop_count(3)emittedfinished, finished, finishedinstead oflooped, looped, finished.Switches to
ss_runtime_is_finished, the terminal signal added in cri-middleware/SpriteStudio-SDK#307.Notes for review
is_finishedis a sticky state, not a pulse. The existingif (!ss_runtime_is_playing(runtime_ctx)) return;at the top ofupdate()is what keeps it from re-emitting, since onlyplay()clears it. That dependency is now called out in a comment rather than left implicit.animation_loopedis deliberately unchanged. It still fires on every wrap, including under an infinite loop count — that is where cycle hooks (idle/walk beats, lap counting) are most useful, and it costs one signal per cycle, not per frame.Doc corrections
Both were wrong before this PR, independently of the code:
set_loop_count(0)was documented as "plays once with no repeat" / 「0で1回だけ再生」 and0 for no playback.0is a synonym for infinite — the runtime normalizes every value<= 0to-1.animation_finishedwas documented as "non-looping only", which is not what it did (it fired per cycle) nor what it does now (it fires once all loops complete).Verification
Built against the SDK branch and run headless (
examples/overall, module build):set_loop_count3looped, looped, finished1finished-1looped, looped, …(never finishes)0-1Depends on cri-middleware/SpriteStudio-SDK#307
Merge #307 first. This PR does not build without it:
ss_runtime_is_end_frame_reachedis removed there, andss_runtime_is_finishedonly exists there.The SDK submodule pins
dc9b987, the head of #307's branch — deliberately, and it stays that way. CI builds the runtime from the submodule (scripts/build-runtime.sh), so this PR is green as-is, and it is verified against exactly the SDK revision it is written for. #307 merges as a merge commit (the SDK repo keepsdeleteBranchOnMerge=falseand every recent PR landed that way), sodc9b987remains reachable from the default branch afterwards and the pin does not rot.scripts/SDK_VERSION.txtstays atv7.0.0-alpha.1: it can only name a release that exists, and none shipsss_runtime_is_finishedyet. It is read by the download path (scripts/download-sdk.sh, used byweekly.yml), not by PR CI. Bump it when that release is cut — until then that path is on the pre-fix SDK.