Allow anyState transitions to interrupt crossFade & fix transition bugs#2893
Allow anyState transitions to interrupt crossFade & fix transition bugs#2893GuoLei1990 wants to merge 4 commits intodev/2.0from
Conversation
WalkthroughCentralizes 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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
🤖 Augment PR SummarySummary: This PR fixes Animator transition bookkeeping and reduces duplicated cross-fade interrupt logic. Changes:
Tests:
🤖 Was this summary useful? React with 👍 or 👎 |
…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>
f29cfde to
9201b22
Compare
Codecov Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- 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>
- 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>
There was a problem hiding this comment.
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.
…ansitionCollection
Further optimizations and fixes based on #2885, which had redundant code, could be more concise, and missed a bug in
updateTransitionsIndex.Summary
From #2885 (re-implemented with cleaner code)
CrossFadingorFixedCrossFading, anyState noExitTime transitions can interrupt the current crossFade and redirect to a new statecanTransitionToSelf=falsebehavior, matching Unity)_addTransitionsorting boundary: HasExitTime transitions are now correctly sorted only within the[noExitTimeCount, count)range, avoiding comparison with noExitTime transitions_checkNoExitTimeTransitionsloop range: Loop now usesnoExitTimeCountinstead ofcount, preventing hasExitTime transitions from being incorrectly triggered before their exitTimeNew in this PR
updateTransitionsIndexnoExitTimeCount bug: When togglinghasExitTimeat runtime,noExitTimeCountwas not decremented, causing the noExitTime scan to read past its boundary_tryCrossFadeInterrupthelper: Deduplicate identical interrupt check logic from_updateCrossFadeStateand_updateCrossFadeFromPoseState, also absorbs the removed_checkCrossFadeInterruptand_checkNoExitTimeTransitionwrapperCompared to #2885, this PR achieves the same functionality with ~80 fewer lines of code.
Test plan