From 863d96bb23a608ce0fadfe4eb3fcb690caea43e2 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 07:28:21 +0200 Subject: [PATCH] =?UTF-8?q?fix(motion):=20#39=20=E2=80=94=20skip=20transit?= =?UTF-8?q?ion=20link=20for=20cyclic=E2=86=92cyclic=20locomotion=20in=20Se?= =?UTF-8?q?tCycle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause identified by research-agent read of AnimationSequencer.SetCycle + Advance + the per-tick TickAnimations call site: - SetCycle enqueues transition link + new cycle, then forces _currNode onto firstNew (the LINK), per the 357dcc0 fix that pinned _currNode to the most-recently-enqueued node. - Advance plays the link to completion (~100–300 ms at Framerate 30 × link runSpeed) before AdvanceToNextAnimation moves _currNode forward to the cycle. - For Walk↔Run direct toggles faster than the link's drain time, the next UM arrives, SetCycle restarts _currNode on a fresh link, and the cycle node at the queue tail is never reached. - BuildBlendedFrame returns frames from the link the entire time — user observes the link's interpolation pose ("blips forward in walking animation"), never the new Walk or Run cycle. Confirmed by [SCFULL] currNodeIsCyclic=False after every direct Walk↔Run transition in launch-39-candidate.log. Fix: when prev motion AND new motion are both locomotion cycles (WalkForward, WalkBackward, RunForward, SideStep L/R), land _currNode on _firstCyclic (the new cycle node) instead of firstNew (the link), and remove the just-enqueued link from the queue. Conditional on BOTH being locomotion to avoid regressing cases that DO need the link to play: - Idle→Run (link is the wind-up pose) - Falling→Ready (landing animation) - Ready→Sitting/Crouching/Sleeping - Combat substates (attack/parry/ready transitions) Reverted commit c06b6c5 demonstrated that unconditional link skip breaks all of those — this fix is narrower. Retail reference: cdb live trace 2026-05-03 of a Walk→Run direct transition logged add_to_queue(45000005) followed by add_to_queue(44000007) with truncate_animation_list never firing — matching the observed semantics this fix implements. 42/42 AnimationSequencer tests pass. The 8 pre-existing test failures elsewhere on the branch (BSPStepUp, MotionInterpreter WalkBackward, etc.) are unrelated to this change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Physics/AnimationSequencer.cs | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/src/AcDream.Core/Physics/AnimationSequencer.cs b/src/AcDream.Core/Physics/AnimationSequencer.cs index 9687fea..56e03e4 100644 --- a/src/AcDream.Core/Physics/AnimationSequencer.cs +++ b/src/AcDream.Core/Physics/AnimationSequencer.cs @@ -525,7 +525,59 @@ public sealed class AnimationSequencer // newly-added node; if preEnqueueTail was null (queue was empty // before enqueue), the first new node is _queue.First. var firstNew = preEnqueueTail is null ? _queue.First : preEnqueueTail.Next; - if (firstNew is not null) + + // #39 Fix B (2026-05-06): for direct cyclic-locomotion → + // cyclic-locomotion transitions (Walk↔Run on Shift toggle, + // W↔S direct flip, A↔D, Forward↔Strafe), land _currNode on + // the new CYCLE (_firstCyclic), NOT on the link (firstNew), + // and remove the just-enqueued link from the queue. + // + // Why: the transition link's drain time (~100–300 ms at + // Framerate 30 × link runSpeed) gets restarted before it can + // end if the user toggles Shift faster than that. _currNode + // sits on a fresh link every UM and Advance never reaches + // the cycle. User observes "blips forward in walking + // animation" — what they're seeing is the link's + // interpolation pose, never the new cycle. + // + // Conditional on BOTH old AND new being locomotion cycles to + // avoid regressing the cases where the link IS the right + // animation: + // - Idle (Ready) → any cycle: link is the wind-up pose + // - Falling → Ready: landing animation + // - Ready → Sitting/Crouching: pose-change links + // - Combat substates (attack/parry/ready transitions) + // Commit c06b6c5 (reverted in a2ae2ae) demonstrated that + // unconditionally skipping the link breaks all of these. + // + // Retail reference: cdb live trace 2026-05-03 of a Walk→Run + // direct transition logged + // add_to_queue(45000005, looping=1) walk + // add_to_queue(44000007, looping=1) run + // with truncate_animation_list never firing — i.e. retail + // appends the new cycle directly without a separate link + // enqueue or visible link pose for cyclic→cyclic. Our + // structural mismatch was always enqueueing link+cycle and + // forcing _currNode onto the link; this fix matches retail's + // observed semantics for the locomotion subset. + bool prevIsLocomotion = IsLocomotionCycleLowByte(CurrentMotion & 0xFFu); + bool newIsLocomotion = IsLocomotionCycleLowByte(motion & 0xFFu); + if (prevIsLocomotion && newIsLocomotion && _firstCyclic is not null) + { + // Drop the just-enqueued link node (firstNew) from the + // queue if it's distinct from the cycle — nothing should + // ever play it, and leaving stale non-cyclic nodes ahead + // of _currNode contributes to the unbounded queue growth + // observed in [SCFULL] (qCount climbing past 49 over + // ~30 transitions). + if (firstNew is not null && firstNew != _firstCyclic) + { + _queue.Remove(firstNew); + } + _currNode = _firstCyclic; + _framePosition = _firstCyclic.Value.GetStartFramePosition(); + } + else if (firstNew is not null) { _currNode = firstNew; _framePosition = _currNode.Value.GetStartFramePosition(); @@ -1384,6 +1436,20 @@ public sealed class AnimationSequencer return result; } + /// + /// True if the given motion-low-byte names a locomotion cycle — + /// WalkForward (0x05), WalkBackward (0x06), RunForward (0x07), + /// SideStepRight (0x0F), or SideStepLeft (0x10). + /// Used by to recognise cyclic→cyclic + /// direct transitions and bypass the transition link in that case + /// (retail's observed add_to_queue semantics). + /// + private static bool IsLocomotionCycleLowByte(uint lowByte) + { + return lowByte == 0x05u || lowByte == 0x06u || lowByte == 0x07u + || lowByte == 0x0Fu || lowByte == 0x10u; + } + /// /// Quaternion slerp matching the retail client's FUN_005360d0 /// (chunk_00530000.c:4799-4846):