Skip to content

Comments

Allow anyState transitions to interrupt crossFade & fix transition bugs#2893

Open
GuoLei1990 wants to merge 4 commits intodev/2.0from
fix/animator-inter
Open

Allow anyState transitions to interrupt crossFade & fix transition bugs#2893
GuoLei1990 wants to merge 4 commits intodev/2.0from
fix/animator-inter

Conversation

@GuoLei1990
Copy link
Member

@GuoLei1990 GuoLei1990 commented Feb 22, 2026

Further optimizations and fixes based on #2885, which had redundant code, could be more concise, and missed a bug in updateTransitionsIndex.

Note: This PR is built upon the work of @luzhuang in #2885. Thanks for the initial implementation!
Co-Authored-By: luzhuang <luzhuang@aspect3d.com>

Summary

From #2885 (re-implemented with cleaner code)

  • Allow anyState noExitTime transitions to interrupt crossFade: During CrossFading or FixedCrossFading, anyState noExitTime transitions can interrupt the current crossFade and redirect to a new state
  • Prevent re-triggering to same dest state: Skip anyState transitions whose destination matches the current crossFade target (canTransitionToSelf=false behavior, matching Unity)
  • Fix _addTransition sorting boundary: HasExitTime transitions are now correctly sorted only within the [noExitTimeCount, count) range, avoiding comparison with noExitTime transitions
  • Fix _checkNoExitTimeTransitions loop range: Loop now uses noExitTimeCount instead of count, preventing hasExitTime transitions from being incorrectly triggered before their exitTime

New in this PR

  • Fix updateTransitionsIndex noExitTimeCount bug: When toggling hasExitTime at runtime, noExitTimeCount was not decremented, causing the noExitTime scan to read past its boundary
  • Extract _tryCrossFadeInterrupt helper: Deduplicate identical interrupt check logic from _updateCrossFadeState and _updateCrossFadeFromPoseState, also absorbs the removed _checkCrossFadeInterrupt and _checkNoExitTimeTransition wrapper

Compared to #2885, this PR achieves the same functionality with ~80 fewer lines of code.

Test plan

  • All 30 Animator tests pass
  • New tests: anyState transition interrupts crossFade, noExitTime scan ignores exitTime transitions, hasExitTime runtime toggle maintains correct noExitTimeCount

@coderabbitai
Copy link

coderabbitai bot commented Feb 22, 2026

Walkthrough

Centralizes cross-fade interrupt logic into a new helper, replaces single-transition no-exit-time checks with a multi-transition scanner, fixes noExitTime bookkeeping in the transition collection, and adds tests covering anyState interrupts and hasExitTime toggles.

Changes

Cohort / File(s) Summary
Animator core changes
packages/core/src/animation/Animator.ts
Adds _tryCrossFadeInterrupt and _checkNoExitTimeTransitions; invokes interrupt checks before progressing cross-fades; replaces legacy single-transition no-exit-time checks; supports excluding the current cross-fade destination when scanning.
Transition collection logic
packages/core/src/animation/AnimatorStateTransitionCollection.ts
Fixes updateTransitionsIndex to adjust noExitTimeCount when re-adding transitions; _addTransition keeps no-exit-time transitions at the front and bounds insertion/search to the exit-time region.
Tests
tests/src/core/Animator.test.ts
Adds afterEach cleanup of state transitions; new tests for AnyState interrupting cross-fade, noExitTime scan ignoring exit-time transitions, and toggling hasExitTime validating noExitTimeCount and ordering.
Manifest
package.json
Manifest referenced/updated in diff.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇
I hopped through fades and checked each gate,
Skipped the exits, rearranged their state,
Counts in front, the transitions sing,
A little hop — interrupt! — and spring.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the two main changes: allowing anyState transitions to interrupt crossFade and fixing transition-related bugs in the animation system.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/animator-inter

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@augmentcode
Copy link

augmentcode bot commented Feb 22, 2026

🤖 Augment PR Summary

Summary: This PR fixes Animator transition bookkeeping and reduces duplicated cross-fade interrupt logic.

Changes:

  • Fixes AnimatorStateTransitionCollection.updateTransitionsIndex() to correctly decrement noExitTimeCount when a transition is toggled to hasExitTime=true, preventing out-of-bounds/no-exit-time scans from reading exit-time transitions.
  • Removes the redundant _checkNoExitTimeTransition passthrough wrapper and calls _checkNoExitTimeTransitions directly.
  • Extracts cross-fade interrupt checking into Animator._tryCrossFadeInterrupt() and reuses it in both cross-fade update paths.
  • Maintains the “check before any update()” behavior so interrupts preserve events and let the new state consume the current deltaTime.

Tests:

  • Simplifies test cleanup by relying on suite afterEach reset logic.
  • Adds a new unit test validating noExitTimeCount stays correct when toggling hasExitTime at runtime.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

…ix transition bugs

Re-apply PR #2885 changes on top of reverted dev/2.0.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Feb 22, 2026

Codecov Report

❌ Patch coverage is 92.85714% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.21%. Comparing base (ff88e8d) to head (4a42aa9).

Files with missing lines Patch % Lines
packages/core/src/animation/Animator.ts 94.44% 2 Missing ⚠️
...src/animation/AnimatorStateTransitionCollection.ts 83.33% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           dev/2.0    #2893   +/-   ##
========================================
  Coverage    83.21%   83.21%           
========================================
  Files          796      796           
  Lines        90398    90431   +33     
  Branches      9489     9499   +10     
========================================
+ Hits         75221    75254   +33     
  Misses       15095    15095           
  Partials        82       82           
Flag Coverage Δ
unittests 83.21% <92.85%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Fix updateTransitionsIndex not maintaining noExitTimeCount when toggling hasExitTime
- Remove redundant _checkNoExitTimeTransition wrapper
- Extract _tryCrossFadeInterrupt to deduplicate interrupt check logic
- Simplify test cleanup by leveraging afterEach, add hasExitTime toggle test

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@GuoLei1990 GuoLei1990 changed the title fix: optimize Animator crossFade interrupt and fix noExitTimeCount bug fix(animation): optimize crossFade interrupt and fix noExitTimeCount bug Feb 22, 2026
@GuoLei1990 GuoLei1990 changed the title fix(animation): optimize crossFade interrupt and fix noExitTimeCount bug fix(animation): allow anyState transitions to interrupt crossFade & fix transition bugs Feb 22, 2026
@GuoLei1990 GuoLei1990 changed the title fix(animation): allow anyState transitions to interrupt crossFade & fix transition bugs Allow anyState transitions to interrupt crossFade & fix transition bugs Feb 22, 2026
- Use multi-line if/return for _tryCrossFadeInterrupt calls
- Reorder transitionDuration declaration for consistency between
  _updateCrossFadeState and _updateCrossFadeFromPoseState

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/src/animation/Animator.ts`:
- Around line 1165-1183: The anyState exit-transition path can mutate
layerData.layerState via _applyTransition/_checkAnyAndEntryState and cause
_tryCrossFadeInterrupt to continue with stale state; after invoking
_checkNoExitTimeTransitions (or inside it) verify whether layerData.layerState
changed from the original currentDestState and, if so, call
_updateState(layerData, deltaTime, aniUpdate) and return true to abort the
crossfade; alternatively make _checkNoExitTimeTransitions return false when
_applyTransition returns null (indicating a state mutation) so
_tryCrossFadeInterrupt treats that as an interrupt—update the logic around
_tryCrossFadeInterrupt, _checkNoExitTimeTransitions, _applyTransition and
_checkAnyAndEntryState to perform this post-check using the layerData.layerState
and currentDestState symbols.

@GuoLei1990 GuoLei1990 added bug Something isn't working Animation Built-in animation system related functions labels Feb 22, 2026
@GuoLei1990 GuoLei1990 added this to the 2.0 milestone Feb 22, 2026
@GuoLei1990 GuoLei1990 self-assigned this Feb 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Animation Built-in animation system related functions bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant