diff --git a/docs/ISSUES.md b/docs/ISSUES.md index dd37ab9a..be9431f9 100644 --- a/docs/ISSUES.md +++ b/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. diff --git a/docs/architecture/acdream-architecture.md b/docs/architecture/acdream-architecture.md index 07790044..0ca8fd7a 100644 --- a/docs/architecture/acdream-architecture.md +++ b/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) diff --git a/docs/research/2026-05-10-tier1-mutation-audit.md b/docs/research/2026-05-10-tier1-mutation-audit.md index f206bf47..95edce7a 100644 --- a/docs/research/2026-05-10-tier1-mutation-audit.md +++ b/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. --- diff --git a/docs/superpowers/specs/2026-05-10-issue-53-tier1-cache-design.md b/docs/superpowers/specs/2026-05-10-issue-53-tier1-cache-design.md index dfe7a84b..a7e81bf2 100644 --- a/docs/superpowers/specs/2026-05-10-issue-53-tier1-cache-design.md +++ b/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 diff --git a/memory/project_animation_runtime.md b/memory/project_animation_runtime.md new file mode 100644 index 00000000..00bf091c --- /dev/null +++ b/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. diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 2e0795b4..2bbcf765 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -318,6 +318,10 @@ public sealed class GameWindow : IDisposable public readonly List 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 /// 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,7 +2883,8 @@ 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. - RemoveLiveEntityByServerGuid(spawn.Guid); + if (appearanceUpdate is null) + RemoveLiveEntityByServerGuid(spawn.Guid); // When requested, log every spawn that arrives so we can inventory what the server // sends (including the ones we can't render yet). The Name field @@ -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 - /// . The dedup at the start of - /// 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 SmartBox::UpdateVisualDesc calls + /// CPhysicsObj::DoObjDescChangesFromDefault on the existing object; + /// accordingly, entity identity and all animation/movement/physics state + /// survive while MeshRefs, palette, and part overrides are replaced. /// 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; } + /// + /// Capture the runtime identity that retail preserves across + /// DoObjDescChangesFromDefault. Replacement visual data is hydrated + /// first and then assigned to this same entity instance. + /// + 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 meshRefs) + { + animation.Entity = entity; + animation.Setup = setup; + animation.Scale = scale; + var template = new (uint, IReadOnlyDictionary?)[meshRefs.Count]; + for (int i = 0; i < meshRefs.Count; i++) + template[i] = (meshRefs[i].GfxObjId, meshRefs[i].SurfaceOverrides); + animation.PartTemplate = template; + } + /// /// Rebuilds the paperdoll doll from the live player entity (retail makeObject(player) + /// DoObjDescChangesFromDefault). Clones the player's already-resolved Setup / MeshRefs / palette / diff --git a/src/AcDream.Core/World/WorldEntity.cs b/src/AcDream.Core/World/WorldEntity.cs index b26e9a4d..a5151f05 100644 --- a/src/AcDream.Core/World/WorldEntity.cs +++ b/src/AcDream.Core/World/WorldEntity.cs @@ -4,6 +4,9 @@ namespace AcDream.Core.World; public sealed class WorldEntity { + private PaletteOverride? _paletteOverride; + private IReadOnlyList _partOverrides = Array.Empty(); + public required uint Id { get; init; } /// /// 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. /// - public PaletteOverride? PaletteOverride { get; init; } + public PaletteOverride? PaletteOverride + { + get => _paletteOverride; + init => _paletteOverride = value; + } /// /// EnvCell or outdoor cell ID that owns this entity (room geometry, static @@ -94,7 +101,30 @@ public sealed class WorldEntity /// AnimatedEntityState's override map at spawn time. Empty for atlas- /// tier entities. /// - public IReadOnlyList PartOverrides { get; init; } = Array.Empty(); + public IReadOnlyList PartOverrides + { + get => _partOverrides; + init => _partOverrides = value; + } + + /// + /// 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 + /// CPhysicsObj::DoObjDescChangesFromDefault. + /// + public void ApplyAppearance( + IReadOnlyList meshRefs, + PaletteOverride? paletteOverride, + IReadOnlyList partOverrides) + { + ArgumentNullException.ThrowIfNull(meshRefs); + ArgumentNullException.ThrowIfNull(partOverrides); + + MeshRefs = meshRefs; + _paletteOverride = paletteOverride; + _partOverrides = partOverrides; + } /// /// Bitmask of hidden Setup parts. Bit i set hides part i at diff --git a/tests/AcDream.App.Tests/Rendering/LiveAppearanceAnimationTests.cs b/tests/AcDream.App.Tests/Rendering/LiveAppearanceAnimationTests.cs new file mode 100644 index 00000000..92f5fc59 --- /dev/null +++ b/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 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; + } +} diff --git a/tests/AcDream.Core.Tests/Rendering/Wb/EntityClassificationCacheTests.cs b/tests/AcDream.Core.Tests/Rendering/Wb/EntityClassificationCacheTests.cs index 16b6e878..16c1907e 100644 --- a/tests/AcDream.Core.Tests/Rendering/Wb/EntityClassificationCacheTests.cs +++ b/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) };