erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(motion): #39 — skip transition link for cyclic→cyclic locomotion in SetCycle

Browse source 
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) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-05-06 07:28:21 +02:00
parent 5660f3483d
commit 863d96bb23
1 changed files with 67 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

68
src/AcDream.Core/Physics/AnimationSequencer.cs
View file

@ -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;
}
/// <summary>
/// True if the given motion-low-byte names a locomotion cycle —
/// WalkForward (0x05), WalkBackward (0x06), RunForward (0x07),
/// SideStepRight (0x0F), or SideStepLeft (0x10).
/// Used by <see cref="SetCycle"/> to recognise cyclic→cyclic
/// direct transitions and bypass the transition link in that case
/// (retail's observed add_to_queue semantics).
/// </summary>
private static bool IsLocomotionCycleLowByte(uint lowByte)
{
return lowByte == 0x05u || lowByte == 0x06u || lowByte == 0x07u
|| lowByte == 0x0Fu || lowByte == 0x10u;
}
/// <summary>
/// Quaternion slerp matching the retail client's <c>FUN_005360d0</c>
/// (chunk_00530000.c:4799-4846):