From 5666e05a85b6c1905bc0ea4543e899c0fcb90950 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 12 Apr 2026 00:05:23 +0200 Subject: [PATCH] =?UTF-8?q?fix(core+app):=20Phase=206.8=20=E2=80=94=20keep?= =?UTF-8?q?=20NPCs=20animated=20when=20post-spawn=20motion=20update=20is?= =?UTF-8?q?=20unmappable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User reported that some NPCs (Pathwarden, Town Crier) didn't breathe. Diagnostic logging revealed: 1. Both NPCs are correctly registered as animated at CreateObject time with the standard 30fps human breath cycle (anim=0x03000001). 2. Immediately after spawn the server sends an UpdateMotion with stance=0x0003 cmd=0x0000 for these NPCs. 3. MotionResolver.GetIdleCycle returned NULL for that combination, because StyleDefaults didn't have an entry for stance=3. 4. OnLiveMotionUpdated treated the NULL as "switch to a static pose" and removed the entity from _animatedEntities. Two fixes: A. MotionResolver.ResolveIdleCycleInternal — when stance is set but StyleDefaults has no entry for it, fall back to the table's DefaultStyle/DefaultSubstate instead of returning null. The server-supplied stance was just an unmappable override; the table default is the correct "I have no better information" answer. Pulled the table-default lookup into a small TryGetTableDefault helper so both fallback paths use the same code. B. OnLiveMotionUpdated — never REMOVE an animated entity. If the re-resolved cycle is bad (null, framerate=0, or single-frame), leave the existing cycle running so the entity continues to breathe with whatever it already had. The defensive "remove on re-resolve failure" was the bug — it silently un-registered NPCs the moment the server sent any partial motion update. Together these mean: any NPC that successfully registers as animated at spawn stays animated, even if the server's subsequent motion updates are incomplete or use stance values our resolver doesn't have a mapping for. Strips the [BREATHE] and [MOTION] diagnostic spew added during the investigation now that the cause is identified. 220 tests still green. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/AcDream.App/Rendering/GameWindow.cs | 27 +++++++++++-------- src/AcDream.Core/Meshing/MotionResolver.cs | 30 +++++++++++++++++++--- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 53e7e60..09328d9 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -689,6 +689,7 @@ public sealed class GameWindow : IDisposable _liveAnimRejectPartFrames++; + if (idleCycle is not null && idleCycle.Framerate != 0f && idleCycle.HighFrame > idleCycle.LowFrame && idleCycle.Animation.PartFrames.Count > 1) @@ -783,18 +784,22 @@ public sealed class GameWindow : IDisposable stanceOverride: stance, commandOverride: command); - if (newCycle is null || newCycle.Framerate == 0f - || newCycle.HighFrame <= newCycle.LowFrame - || newCycle.Animation.PartFrames.Count <= 1) - { - // New pose is a static one — stop animating and leave the - // entity on its last rendered frame. Removing from the map - // means the tick no longer updates it. - _animatedEntities.Remove(entity.Id); - return; - } + // If the new cycle is bad (null, framerate=0, or single-frame), do + // NOT remove the entity from the animated set. Keep its existing + // cycle running so it continues to breathe / idle. Removing on + // re-resolve failure was a bug that silently unregistered NPCs the + // moment the server sent a motion update with a stance/command + // pair the resolver couldn't translate cleanly. Defensive: switch + // only when we have a clearly better cycle. + bool newCycleIsGood = newCycle is not null + && newCycle.Framerate != 0f + && newCycle.HighFrame > newCycle.LowFrame + && newCycle.Animation.PartFrames.Count > 1; - ae.Animation = newCycle.Animation; + if (!newCycleIsGood) + return; + + ae.Animation = newCycle!.Animation; ae.LowFrame = Math.Max(0, newCycle.LowFrame); ae.HighFrame = Math.Min(newCycle.HighFrame, newCycle.Animation.PartFrames.Count - 1); ae.Framerate = newCycle.Framerate; diff --git a/src/AcDream.Core/Meshing/MotionResolver.cs b/src/AcDream.Core/Meshing/MotionResolver.cs index 6e7e9b8..09d3d4a 100644 --- a/src/AcDream.Core/Meshing/MotionResolver.cs +++ b/src/AcDream.Core/Meshing/MotionResolver.cs @@ -162,6 +162,21 @@ public static class MotionResolver uint styleVal; uint substateVal; + // Helper: pick the table's default (style, substate) — used as the + // ultimate fallback when caller-supplied overrides don't resolve. + bool TryGetTableDefault(out uint styleOut, out uint substateOut) + { + if (mtable.StyleDefaults.TryGetValue(mtable.DefaultStyle, out var defaultSubstate)) + { + styleOut = (uint)mtable.DefaultStyle; + substateOut = (uint)defaultSubstate; + return true; + } + styleOut = 0; + substateOut = 0; + return false; + } + if (stanceOverride is { } stance && stance != 0) { styleVal = stance; @@ -175,15 +190,22 @@ public static class MotionResolver } else { - return null; + // The server gave us a stance the motion table doesn't recognize + // (e.g. NPCs that re-broadcast a "ready" stance with a substate + // value where the table only has a style entry). Don't return + // null — fall back to the table default so the entity at least + // animates with its idle cycle. Returning null here was the + // bug that silently un-registered Pathwarden / Town Crier from + // _animatedEntities the moment ACE sent a post-spawn motion + // update with stance=0x0003 cmd=0x0000. + if (!TryGetTableDefault(out styleVal, out substateVal)) + return null; } } else { - if (!mtable.StyleDefaults.TryGetValue(mtable.DefaultStyle, out var defaultSubstate)) + if (!TryGetTableDefault(out styleVal, out substateVal)) return null; - styleVal = (uint)mtable.DefaultStyle; - substateVal = (uint)defaultSubstate; } // ACViewer's cycle key encoding (Physics/Animation/MotionTable.cs:191):