fix #203: preserve animation on appearance updates

docs/ISSUES.md

@@ -46,6 +46,28 @@ Copy this block when adding a new issue:
 
 ---
 
+## #203 — Unequipping armor freezes character and world animations — FIXED
+
+**Status:** DONE — 2026-07-11 (this change)
+**Severity:** HIGH
+**Component:** live entities / animation / appearance updates
+
+**Description:** After armor was removed, the local character stopped animating
+and animation-dependent world interactions such as doors appeared frozen.
+
+**Root cause:** `0xF625 ObjDescEvent` was incorrectly handled as a full
+despawn/respawn. That discarded the live `WorldEntity`, animation manager,
+physics host, and collision registration while the player movement controller
+still referenced the discarded sequencer. Named retail instead applies
+`DoObjDescChangesFromDefault` to the existing object.
+
+**Resolution:** Appearance hydration now mutates only the existing entity's
+meshes, palette, part overrides, and bounds. Entity identity, animation playback,
+movement, physics, collision, selection, and dead-reckoning state survive. The
+classification cache is invalidated in place. A regression test pins entity and
+sequencer identity across the update. User verified the exact equip/unequip flow
+in the Release client.
+
 ## #191 — Tapping W (brief forward press) glides forward without playing the step animation
 ## #194 — WbDrawDispatcher._groups is never pruned (minor)
 
@@ -7161,7 +7183,7 @@ threshold.
 
 **Component:** rendering / `WbDrawDispatcher` / `EntityClassificationCache` / `LandblockLoader`
 
-**Resolution.** New `EntityClassificationCache` keyed by `(entityId, landblockHint)` tuple in `src/AcDream.App/Rendering/Wb/EntityClassificationCache.cs`. The dispatcher routes static entities (NOT in `_animatedEntities`) through the cache — first-frame slow-path populates flat `CachedBatch[]` (one entry per (partIdx, batchIdx) with the part-relative `RestPose` and resolved `BindlessTextureHandle`); subsequent-frame cache hits skip classification entirely and append `cached.RestPose * entityWorld` to each matching group. Animated entities bypass. Invalidation fires from `RemoveLiveEntityByServerGuid` (per-entity, `0xF747`/`0xF625`) and `RemoveEntitiesFromLandblock` (per-LB, Near→Far demote + unload).
+**Resolution.** New `EntityClassificationCache` keyed by `(entityId, landblockHint)` tuple in `src/AcDream.App/Rendering/Wb/EntityClassificationCache.cs`. The dispatcher routes static entities (NOT in `_animatedEntities`) through the cache — first-frame slow-path populates flat `CachedBatch[]` (one entry per (partIdx, batchIdx) with the part-relative `RestPose` and resolved `BindlessTextureHandle`); subsequent-frame cache hits skip classification entirely and append `cached.RestPose * entityWorld` to each matching group. Animated entities bypass. Invalidation fires from `RemoveLiveEntityByServerGuid` for real despawns (`0xF747`), directly from the in-place `0xF625` appearance mutation, and from `RemoveEntitiesFromLandblock` (per-LB, Near→Far demote + unload).
 
 **Perf result.** Entity dispatcher cpu_us **median ~1200 µs, p95 ~1500 µs** at horizon-safe + High preset on AMD Radeon RX 9070 XT @ 1440p. Pre-Tier-1 baseline was ~3500m / ~4000p95. ~66% reduction in median, ~63% in p95. Well under the A.5 spec budget (median ≤ 2.0 ms, p95 ≤ 2.5 ms). No `BUDGET_OVER` flag observed.
 

docs/architecture/acdream-architecture.md

@@ -292,6 +292,13 @@ Target state: every entity in the world — player, NPC, monster, lifestone,
 door, chest — becomes a `GameEntity`. The renderer iterates them and draws.
 The plugin API exposes them as `WorldEntitySnapshot`. GameWindow becomes thin.
 
+Lifecycle invariant in both the current split model and the target `GameEntity`:
+an `ObjDescEvent` changes appearance in place. It may replace resolved meshes,
+palette ranges, part overrides, and visual bounds, but it must preserve entity
+identity plus animation, motion, physics, collision, selection, and
+dead-reckoning owners. Only a real delete/despawn tears those owners down. This
+matches retail's `CPhysicsObj::DoObjDescChangesFromDefault` behavior.
+
 ---
 
 ## Per-Frame Update Order (current runtime)

docs/research/2026-05-10-tier1-mutation-audit.md

@@ -3,6 +3,13 @@
 **Created:** 2026-05-10, opening move of the ISSUE #53 retry session.
 **Purpose:** enumerate every code path that writes to `WorldEntity.MeshRefs` (the dispatcher's load-bearing per-entity input) and every adjacent state read by `WbDrawDispatcher.ClassifyBatches` / model-matrix composition, classify each as STATIC or DYNAMIC, and design the cache invalidation surface BEFORE touching renderer code.
 
+> **Superseded detail (2026-07-11):** this historical audit described
+> `0xF625 ObjDescEvent` as despawn + respawn. That was an acdream implementation
+> artifact, not retail behavior, and caused animation state to freeze after
+> equip/unequip. Appearance updates now mutate the existing entity and invalidate
+> its classification cache in place, matching
+> `CPhysicsObj::DoObjDescChangesFromDefault`. Real despawns remain unchanged.
+
 This audit is the load-bearing prerequisite the prior Tier 1 attempt (commit `3639a6f`, reverted at `9b49009`) skipped. Cache design follows from the audit, not the other way around.
 
 ---

docs/superpowers/specs/2026-05-10-issue-53-tier1-cache-design.md

@@ -6,6 +6,11 @@
 **Originating issue:** [docs/ISSUES.md](../../ISSUES.md) §#53.
 **Phase context:** Phase Post-A.5 polish, Priority 3 (only remaining priority after #52 + #54 closed).
 
+> **Historical correction (2026-07-11):** this spec inherited the old assumption
+> that `0xF625 ObjDescEvent` was a despawn/respawn. The cache behavior remains
+> valid, but appearance updates now preserve entity identity and invalidate the
+> entity's cache entry in place, matching retail.
+
 ---
 
 ## §1. Problem

memory/project_animation_runtime.md

@@ -0,0 +1,29 @@
+# Animation Runtime Crib
+
+## Live entity lifecycle invariant
+
+`CreateObject` creates a live entity and a real `DeleteObject` destroys it.
+`ObjDescEvent (0xF625)` is neither: it changes appearance on the existing object.
+
+For an appearance update, preserve:
+
+- `WorldEntity` object identity and local `Id`
+- `AnimatedEntity`, `AnimationSequencer`, frame/range/rate state
+- movement-controller and animation-manager bindings
+- physics host, collision registration, dead reckoning, and selection state
+
+Replace only resolved `MeshRefs`, palette override, part overrides, and visual
+bounds, then invalidate the entity-classification cache. The paperdoll may clone
+the updated appearance separately.
+
+Retail oracle chain:
+
+`SmartBox::HandleObjDescEvent` → `SmartBox::UpdateVisualDesc` →
+`ACCObjectMaint::SetVisualDesc` →
+`CPhysicsObj::DoObjDescChangesFromDefault` →
+`CPartArray::DoObjDescChanges`.
+
+Regression history: treating `0xF625` as despawn/respawn severed the player
+controller from the active animation state, so unequipping armor froze character
+animation and made animation-dependent door interaction appear frozen. Fixed as
+issue #203 on 2026-07-11 and user-gated in a Release client.

src/AcDream.App/Rendering/GameWindow.cs

@@ -318,6 +318,10 @@ public sealed class GameWindow : IDisposable
         public readonly List<AcDream.Core.World.MeshRef> MeshRefsScratch = new();
     }
 
+    private sealed record AppearanceUpdateState(
+        AcDream.Core.World.WorldEntity Entity,
+        AnimatedEntity? Animation);
+
     private AcDream.Core.Physics.DatCollectionLoader? _animLoader;
 
     // Phase E.1: central fan-out for animation hooks. Audio (E.2),
@@ -2850,7 +2854,9 @@ public sealed class GameWindow : IDisposable
     /// </summary>
     private static bool IsDoorName(string? name) => name == "Door";
 
-    private void OnLiveEntitySpawnedLocked(AcDream.Core.Net.WorldSession.EntitySpawn spawn)
+    private void OnLiveEntitySpawnedLocked(
+        AcDream.Core.Net.WorldSession.EntitySpawn spawn,
+        AppearanceUpdateState? appearanceUpdate = null)
     {
         _liveSpawnReceived++;
 
@@ -2877,6 +2883,7 @@ public sealed class GameWindow : IDisposable
         // For a respawn, drop the previous rendering state here before we
         // build the new one. `_entitiesByServerGuid` is the canonical map,
         // its value is the live WorldEntity we need to dispose.
+        if (appearanceUpdate is null)
             RemoveLiveEntityByServerGuid(spawn.Guid);
 
         // When requested, log every spawn that arrives so we can inventory what the server
@@ -3430,6 +3437,19 @@ public sealed class GameWindow : IDisposable
                     animPartChanges[i].PartIndex, animPartChanges[i].NewModelId);
         }
 
+        if (appearanceUpdate is { } visualUpdate)
+        {
+            AcDream.Core.World.WorldEntity existing = visualUpdate.Entity;
+            existing.ApplyAppearance(meshRefs, paletteOverride, entityPartOverrides);
+            if (liveBounds.TryGet(out var appearanceMin, out var appearanceMax))
+                existing.SetLocalBounds(appearanceMin, appearanceMax);
+            if (visualUpdate.Animation is { } animation)
+                RebindAnimatedEntityForAppearance(animation, existing, setup, scale, meshRefs);
+            _classificationCache.InvalidateEntity(existing.Id);
+            _lastSpawnByGuid[spawn.Guid] = spawn;
+            return;
+        }
+
         var entity = new AcDream.Core.World.WorldEntity
         {
             Id = _liveEntityIdCounter++,
@@ -3541,8 +3561,6 @@ public sealed class GameWindow : IDisposable
         else if (idleCycle.Animation.PartFrames.Count <= 1)
             _liveAnimRejectPartFrames++;
 
-
-
         if (idleCycle is not null && idleCycle.Framerate != 0f
             && idleCycle.HighFrame > idleCycle.LowFrame
             && idleCycle.Animation.PartFrames.Count > 1)
@@ -3770,11 +3788,11 @@ public sealed class GameWindow : IDisposable
     /// appearance changed (equip / unequip / tailoring / recipe result /
     /// character option toggle). The wire payload only carries the new
     /// ModelData (palette + texture + animpart changes), not position or
-    /// motion, so we splice it onto the cached spawn and replay through
-    /// <see cref="OnLiveEntitySpawned"/>. The dedup at the start of
-    /// <see cref="OnLiveEntitySpawnedLocked"/> tears down the previous
-    /// rendering state (GpuWorldState entry, animated entity, collision
-    /// registration) before rebuilding.
+    /// motion, so we splice it onto the cached spawn and rehydrate only the
+    /// visual projection. Retail <c>SmartBox::UpdateVisualDesc</c> calls
+    /// <c>CPhysicsObj::DoObjDescChangesFromDefault</c> on the existing object;
+    /// accordingly, entity identity and all animation/movement/physics state
+    /// survive while MeshRefs, palette, and part overrides are replaced.
     /// </summary>
     private void OnLiveAppearanceUpdated(AcDream.Core.Net.Messages.ObjDescEvent.Parsed update)
     {
@@ -3796,15 +3814,49 @@ public sealed class GameWindow : IDisposable
             SubPalettes = md.SubPalettes,
             BasePaletteId = md.BasePaletteId,
         };
-        OnLiveEntitySpawned(newSpawn);
+        lock (_datLock)
+        {
+            AppearanceUpdateState? appearanceState = CaptureLiveAppearanceState(update.Guid);
+            OnLiveEntitySpawnedLocked(newSpawn, appearanceState);
+        }
 
-        // Slice 2: a player appearance change (equip / unequip) rebuilt _entitiesByServerGuid[player]
-        // above; flag the paperdoll doll to re-clone from it on the next doll pass (the C# analog of
-        // RedressCreature). Cheap flag — the rebuild is deferred to the pre-UI hook when visible.
+        // Slice 2: flag the paperdoll doll to re-clone the player's newly
+        // mutated appearance on the next doll pass (the C# analog of
+        // RedressCreature). The rebuild is deferred until the doll is visible.
         if (update.Guid == _playerServerGuid)
             _paperdollDollDirty = true;
     }
 
+    /// <summary>
+    /// Capture the runtime identity that retail preserves across
+    /// <c>DoObjDescChangesFromDefault</c>. Replacement visual data is hydrated
+    /// first and then assigned to this same entity instance.
+    /// </summary>
+    private AppearanceUpdateState? CaptureLiveAppearanceState(uint serverGuid)
+    {
+        if (!_entitiesByServerGuid.TryGetValue(serverGuid, out var entity))
+            return null;
+
+        _animatedEntities.TryGetValue(entity.Id, out var animation);
+        return new AppearanceUpdateState(entity, animation);
+    }
+
+    internal static void RebindAnimatedEntityForAppearance(
+        AnimatedEntity animation,
+        AcDream.Core.World.WorldEntity entity,
+        DatReaderWriter.DBObjs.Setup setup,
+        float scale,
+        IReadOnlyList<AcDream.Core.World.MeshRef> meshRefs)
+    {
+        animation.Entity = entity;
+        animation.Setup = setup;
+        animation.Scale = scale;
+        var template = new (uint, IReadOnlyDictionary<uint, uint>?)[meshRefs.Count];
+        for (int i = 0; i < meshRefs.Count; i++)
+            template[i] = (meshRefs[i].GfxObjId, meshRefs[i].SurfaceOverrides);
+        animation.PartTemplate = template;
+    }
+
     /// <summary>
     /// Rebuilds the paperdoll doll from the live player entity (retail makeObject(player) +
     /// DoObjDescChangesFromDefault). Clones the player's already-resolved Setup / MeshRefs / palette /

src/AcDream.Core/World/WorldEntity.cs

@@ -4,6 +4,9 @@ namespace AcDream.Core.World;
 
 public sealed class WorldEntity
 {
+    private PaletteOverride? _paletteOverride;
+    private IReadOnlyList<PartOverride> _partOverrides = Array.Empty<PartOverride>();
+
     public required uint Id { get; init; }
     /// <summary>
     /// Server-assigned GUID (from CreateObject). Zero for dat-hydrated
@@ -34,7 +37,11 @@ public sealed class WorldEntity
     /// colors, creature recolors (e.g. stone-colored drudge statue),
     /// and team colors. Non-palette-indexed textures ignore this field.
     /// </summary>
-    public PaletteOverride? PaletteOverride { get; init; }
+    public PaletteOverride? PaletteOverride
+    {
+        get => _paletteOverride;
+        init => _paletteOverride = value;
+    }
 
     /// <summary>
     /// EnvCell or outdoor cell ID that owns this entity (room geometry, static
@@ -94,7 +101,30 @@ public sealed class WorldEntity
     /// <c>AnimatedEntityState</c>'s override map at spawn time. Empty for atlas-
     /// tier entities.
     /// </summary>
-    public IReadOnlyList<PartOverride> PartOverrides { get; init; } = Array.Empty<PartOverride>();
+    public IReadOnlyList<PartOverride> PartOverrides
+    {
+        get => _partOverrides;
+        init => _partOverrides = value;
+    }
+
+    /// <summary>
+    /// Replaces the render-only appearance carried by an ObjDesc update while
+    /// preserving this entity's identity, spatial state, and runtime owners.
+    /// Retail performs the equivalent mutation through
+    /// <c>CPhysicsObj::DoObjDescChangesFromDefault</c>.
+    /// </summary>
+    public void ApplyAppearance(
+        IReadOnlyList<MeshRef> meshRefs,
+        PaletteOverride? paletteOverride,
+        IReadOnlyList<PartOverride> partOverrides)
+    {
+        ArgumentNullException.ThrowIfNull(meshRefs);
+        ArgumentNullException.ThrowIfNull(partOverrides);
+
+        MeshRefs = meshRefs;
+        _paletteOverride = paletteOverride;
+        _partOverrides = partOverrides;
+    }
 
     /// <summary>
     /// Bitmask of hidden Setup parts. Bit <c>i</c> set hides part <c>i</c> at

tests/AcDream.App.Tests/Rendering/LiveAppearanceAnimationTests.cs

@@ -0,0 +1,65 @@
+using System.Numerics;
+using AcDream.App.Rendering;
+using AcDream.Core.Physics;
+using AcDream.Core.World;
+using DatReaderWriter.DBObjs;
+
+namespace AcDream.App.Tests.Rendering;
+
+public sealed class LiveAppearanceAnimationTests
+{
+    [Fact]
+    public void RebindAppearance_PreservesEntitySequencerAndPlaybackState()
+    {
+        var setup = new Setup();
+        var animation = new Animation();
+        var sequencer = new AnimationSequencer(setup, new MotionTable(), new NullLoader());
+        var oldEntity = Entity(0x70000001u, 0x01000001u);
+        var state = new GameWindow.AnimatedEntity
+        {
+            Entity = oldEntity,
+            Setup = setup,
+            Animation = animation,
+            LowFrame = 2,
+            HighFrame = 9,
+            Framerate = 30f,
+            Scale = 1f,
+            PartTemplate = [(0x01000001u, null)],
+            CurrFrame = 6.5f,
+            Sequencer = sequencer,
+        };
+        IReadOnlyList<MeshRef> dressedParts =
+        [
+            new MeshRef(0x01000002u, Matrix4x4.Identity),
+            new MeshRef(0x01000003u, Matrix4x4.CreateTranslation(1f, 2f, 3f)),
+        ];
+
+        oldEntity.ApplyAppearance(dressedParts, paletteOverride: null, partOverrides: []);
+        GameWindow.RebindAnimatedEntityForAppearance(
+            state, oldEntity, setup, 1.25f, dressedParts);
+
+        Assert.Same(oldEntity, state.Entity);
+        Assert.Same(dressedParts, state.Entity.MeshRefs);
+        Assert.Same(sequencer, state.Sequencer);
+        Assert.Equal(6.5f, state.CurrFrame);
+        Assert.Equal((2, 9, 30f), (state.LowFrame, state.HighFrame, state.Framerate));
+        Assert.Equal(1.25f, state.Scale);
+        Assert.Equal(new[] { 0x01000002u, 0x01000003u },
+            state.PartTemplate.Select(part => part.GfxObjId));
+    }
+
+    private static WorldEntity Entity(uint id, uint gfxObjId) => new()
+    {
+        Id = id,
+        ServerGuid = 0x50000001u,
+        SourceGfxObjOrSetupId = 0x02000001u,
+        Position = Vector3.Zero,
+        Rotation = Quaternion.Identity,
+        MeshRefs = [new MeshRef(gfxObjId, Matrix4x4.Identity)],
+    };
+
+    private sealed class NullLoader : IAnimationLoader
+    {
+        public Animation? LoadAnimation(uint id) => null;
+    }
+}

tests/AcDream.Core.Tests/Rendering/Wb/EntityClassificationCacheTests.cs

@@ -232,13 +232,11 @@ public class EntityClassificationCacheTests
     }
 
     [Fact]
-    public void DespawnRespawn_UnderReusedId_RepopulatesFresh()
+    public void InvalidateRepopulate_UnderReusedId_RepopulatesFresh()
     {
-        // Pins the audit's ObjDescEvent contract (audit section 1):
-        //   ObjDescEvent is despawn + respawn (with a NEW local entity.Id),
-        //   never an in-place mutation. Even when an id IS reused
-        //   (theoretical — _liveEntityIdCounter is monotonic in practice),
-        //   the cache must serve fresh data after invalidation.
+        // Appearance updates preserve the entity Id and invalidate this cache
+        // before the updated MeshRefs are classified. This lower-level test
+        // pins invalidate-then-repopulate behavior for any reused Id.
         var cache = new EntityClassificationCache();
         var batchesV1 = new[] { MakeCachedBatch(1, 0, 6, 0xAA) };
         var batchesV2 = new[] { MakeCachedBatch(2, 6, 12, 0xCC) };