From 3b7dc46219bcfd0113cb5aeeb8c3db7b6088b321 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 24 May 2026 18:47:04 +0200 Subject: [PATCH] fix(phys): GetNearbyObjects dedup-by-entityId silently drops multi-part shadows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apparatus test (DoorCollisionApparatusTests) loads door GfxObj 0x010044B5 from the real dat, builds the door entity's shape list via ShadowShapeBuilder, registers via RegisterMultiPart, and sweeps a player sphere into the door from three angles. Pre-fix: all three assertions fail — the sphere walks straight through. The [cyl-test] probe fires every tick (the small Sphere shape is queried) but no [resolve-bldg] — the per-Part BSP entry is never reached. Root cause: ShadowObjectRegistry.GetNearbyObjects deduplicates on entry.EntityId via HashSet. Pre-RegisterMultiPart each entity had exactly one shadow row, so dedup-by-entityId correctly suppressed multi-cell duplication. After Task 4's RegisterMultiPart introduced multi-shape rows (1 Sphere + 1 per-Part-BSP for doors; potentially more for creatures + items), the dedup silently drops everything after the first. ShadowShapeBuilder emits Sphere shapes before Part-BSPs, so the Sphere wins and the BSP is dropped — exactly the "Task 7 produced zero [resolve-bldg] hits" finding from the 2026-05-24 evening handoff. Fix: dedup on the full ShadowEntry. record-struct equality compares all fields (EntityId, GfxObjId, Position, Rotation, Radius, CollisionType, CylHeight, Scale, State, Flags, LocalPosition, LocalRotation). Distinct shapes of the same entity are not equal and make it through; the same shape registered in multiple cells (its fields identical across calls) dedups exactly as before. Apparatus verification post-fix: all 4 tests pass. - Dead-center front approach: BLOCKED at Y=11.5 normal=(0,-1,0). - 50 cm off-center: BLOCKED at Y=11.5 normal=(0,-1,0). - Back approach from inside: BLOCKED at Y=12.8 normal=(0,+1,0). - Diagnostic dump: BSP fires at tick 5. What this fix DOES NOT do: switch live RegisterLiveEntityCollision to use ShadowShapeBuilder + RegisterMultiPart. That's Task 7 of the original plan, still reverted. With this foundation fix in place, Task 7 should now actually deliver door blocking in production. Test impact: 44/44 in the shape/registry/door scope pass. The broader Physics suite shows the pre-existing PhysicsResolveCapture static-state flakiness documented in CLAUDE.md — 6 baseline failures without my new tests, 10 with them (4 extra are my apparatus tests' IsPlayer-flag resolves getting captured by a concurrent Capture-test race). Independent of this fix; verified by isolating each test class. Findings + apparatus reasoning: docs/research/2026-05-24-door-dat-inspection-findings.md Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Physics/ShadowObjectRegistry.cs | 19 +- .../Physics/DoorCollisionApparatusTests.cs | 327 ++++++++++++++++++ 2 files changed, 343 insertions(+), 3 deletions(-) create mode 100644 tests/AcDream.Core.Tests/Physics/DoorCollisionApparatusTests.cs diff --git a/src/AcDream.Core/Physics/ShadowObjectRegistry.cs b/src/AcDream.Core/Physics/ShadowObjectRegistry.cs index 002ebfe..c8b53ba 100644 --- a/src/AcDream.Core/Physics/ShadowObjectRegistry.cs +++ b/src/AcDream.Core/Physics/ShadowObjectRegistry.cs @@ -434,7 +434,20 @@ public sealed class ShadowObjectRegistry uint primaryCellId = 0u) { results.Clear(); - var seen = new HashSet(); + // A6.P4 door fix (2026-05-24): dedup on the full ShadowEntry rather + // than entity id. Pre-RegisterMultiPart each entity had exactly one + // shadow, so dedup-by-entityId correctly suppressed multi-cell + // duplication. With multi-part entities (a door has 1 Sphere + 1 + // per-Part-BSP = 2 entries with the same EntityId; creatures can + // have more), an entityId dedup silently dropped every shape after + // the first — the door's BSP slab never reached BSPQuery in the + // 2026-05-24 apparatus reproduction. ShadowEntry's record-struct + // equality compares all fields (incl. GfxObjId, LocalPosition, + // CollisionType) so distinct shapes of the same entity make it + // through, while a single shape registered across multiple cells + // (its position + radius equal across calls) deduplicates exactly + // as before. + var seen = new HashSet(); // Cells reachable from the sphere's primary cell via the portal graph // (output of CellTransit.FindCellSet). This set holds the primary @@ -450,7 +463,7 @@ public sealed class ShadowObjectRegistry if (!_cells.TryGetValue(cellId, out var list)) continue; foreach (var entry in list) { - if (seen.Add(entry.EntityId)) + if (seen.Add(entry)) results.Add(entry); } } @@ -503,7 +516,7 @@ public sealed class ShadowObjectRegistry foreach (var entry in list) { - if (seen.Add(entry.EntityId)) + if (seen.Add(entry)) results.Add(entry); } } diff --git a/tests/AcDream.Core.Tests/Physics/DoorCollisionApparatusTests.cs b/tests/AcDream.Core.Tests/Physics/DoorCollisionApparatusTests.cs new file mode 100644 index 0000000..183d0a8 --- /dev/null +++ b/tests/AcDream.Core.Tests/Physics/DoorCollisionApparatusTests.cs @@ -0,0 +1,327 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Numerics; +using AcDream.Core.Physics; +using DatReaderWriter; +using DatReaderWriter.DBObjs; +using DatReaderWriter.Enums; +using DatReaderWriter.Options; +using Xunit; +using Xunit.Abstractions; +using Env = System.Environment; +using Plane = System.Numerics.Plane; + +namespace AcDream.Core.Tests.Physics; + +/// +/// A6.P4 door fix (2026-05-24) — apparatus test. Loads door Setup +/// 0x020019FF + GfxObj 0x010044B5 (the collision-bearing +/// frame/slab part — see +/// ) from the real dat, +/// registers a synthetic door entity at a known world position via +/// , and sweeps a +/// player sphere into the door from multiple angles. +/// +/// +/// Purpose: with the dat inspection having confirmed that +/// 0x010044B5 has a complete 1.9 × 0.26 × 2.5 m door-slab +/// PhysicsBSP, this test isolates whether our +/// + the +/// registration path actually fires +/// collisions against that BSP. If it does, the live Holtburg door +/// bug is somewhere DOWNSTREAM (live registration race, entity +/// rotation, cell scoping). If it doesn't, the bug is here in +/// the integration we can fix with confidence. +/// +/// +/// +/// The test skips gracefully when the dat directory isn't present so +/// CI stays green. Local developer machines always have it. +/// +/// +public class DoorCollisionApparatusTests +{ + private readonly ITestOutputHelper _out; + public DoorCollisionApparatusTests(ITestOutputHelper output) => _out = output; + + private const uint DoorSetupId = 0x020019FFu; + private const uint DoorFrameGfxObjId = 0x010044B5u; + private const uint DoorEntityId = 0xF424Fu; + private const uint TestLandblockId = 0xA9B40000u; + + // Cell (0, 0) in landblock 0xA9B40000 — outdoor 24-m cell, low byte + // = (0 * 8) + 0 + 1 = 1. + private const uint TestCellId = TestLandblockId | 0x0001u; + + private const float SphereRadius = 0.48f; // ~retail player capsule radius + private const float SphereHeight = 1.20f; + private const float StepUpHeight = 0.60f; + private const float StepDownHeight = 0.04f; + + /// + /// Walk the sphere toward the closed door, dead-center, from + /// the door's -Y front face approach. Expect a CollisionNormalValid + /// = true AND the sphere stops before passing through the door. + /// + [Fact] + public void Apparatus_DeadCenter_FrontApproach_BlocksOnBSP() + { + if (!TryBuildScenario(out var ctx)) return; + + // Door at world (12, 12, 0). The frame's BSP slab extends + // entity-local Y∈[-0.009, 0.252] (poly Y range plus frame[0] offset). + // Approach from -Y at sphere Y=11 (1 m before the front face), + // walk +Y at 0.10 m/tick. + var start = new Vector3(12f, 11f, 0.5f); + var perTick = new Vector3(0f, 0.10f, 0f); + + var (blocked, finalPos, normal, ticks) = SweepUntilBlocked( + ctx.engine, start, perTick, maxTicks: 30); + + _out.WriteLine($"Final pos = ({finalPos.X:F3}, {finalPos.Y:F3}, {finalPos.Z:F3}) " + + $"after {ticks} ticks; blocked={blocked} normal=({normal.X:F3},{normal.Y:F3},{normal.Z:F3})"); + + Assert.True(blocked, + $"Expected collision before passing through the door. Sphere reached " + + $"({finalPos.X:F3},{finalPos.Y:F3},{finalPos.Z:F3}) after {ticks} ticks " + + $"without firing CollisionNormalValid."); + Assert.True(finalPos.Y < 12.0f, + $"Sphere should stop before the door's front face (Y ≈ 11.99). " + + $"Got Y={finalPos.Y:F3}"); + } + + /// + /// 50 cm off-center: the small Sphere shape (r=0.10) can't catch + /// this, but the BSP slab (1.9 m wide) MUST. This is the live + /// regression — pre-fix the user can walk around the cylinder. + /// + [Fact] + public void Apparatus_50cmOffCenter_FrontApproach_BlocksOnBSP() + { + if (!TryBuildScenario(out var ctx)) return; + + // Same setup, but start 0.5 m off-center in X. Slab X range + // is approximately [11.05, 12.97] (frame[0].X offset = -0.006). + var start = new Vector3(12.5f, 11f, 0.5f); + var perTick = new Vector3(0f, 0.10f, 0f); + + var (blocked, finalPos, normal, ticks) = SweepUntilBlocked( + ctx.engine, start, perTick, maxTicks: 30); + + _out.WriteLine($"Final pos = ({finalPos.X:F3}, {finalPos.Y:F3}, {finalPos.Z:F3}) " + + $"after {ticks} ticks; blocked={blocked} normal=({normal.X:F3},{normal.Y:F3},{normal.Z:F3})"); + + Assert.True(blocked, + $"Expected BSP collision off-center. Sphere reached " + + $"({finalPos.X:F3},{finalPos.Y:F3},{finalPos.Z:F3}) after {ticks} ticks " + + $"without firing CollisionNormalValid."); + Assert.True(finalPos.Y < 12.0f, + $"Sphere should stop before door front face. Got Y={finalPos.Y:F3}"); + } + + /// + /// Reverse approach: walk from the door's +Y back side toward -Y. + /// SidesType=Landblock polys are two-sided, so this must also block. + /// This pins the live "walking out from inside passes through" bug + /// in its simplest geometric form. + /// + [Fact] + public void Apparatus_DeadCenter_BackApproach_BlocksOnBSP() + { + if (!TryBuildScenario(out var ctx)) return; + + // Start past the door at Y=13, walk back toward -Y. + var start = new Vector3(12f, 13f, 0.5f); + var perTick = new Vector3(0f, -0.10f, 0f); + + var (blocked, finalPos, normal, ticks) = SweepUntilBlocked( + ctx.engine, start, perTick, maxTicks: 30); + + _out.WriteLine($"Final pos = ({finalPos.X:F3}, {finalPos.Y:F3}, {finalPos.Z:F3}) " + + $"after {ticks} ticks; blocked={blocked} normal=({normal.X:F3},{normal.Y:F3},{normal.Z:F3})"); + + Assert.True(blocked, + $"Expected BSP collision from back side (two-sided Landblock poly). " + + $"Sphere reached ({finalPos.X:F3},{finalPos.Y:F3},{finalPos.Z:F3}) " + + $"after {ticks} ticks."); + Assert.True(finalPos.Y > 12.30f, + $"Sphere should stop after the door's back face (Y ≈ 12.25). " + + $"Got Y={finalPos.Y:F3}"); + } + + /// + /// Diagnostic dump: drive 5 ticks with probes on, just to see + /// what the engine reports per-tick. Useful when the assertion + /// tests above fail — the diagnostic test shows the FULL log + /// without yielding a green/red verdict. + /// + [Fact] + public void Apparatus_DiagnosticDump_FrontApproach() + { + if (!TryBuildScenario(out var ctx)) return; + + PhysicsDiagnostics.ProbeResolveEnabled = true; + PhysicsDiagnostics.ProbeBuildingEnabled = true; + try + { + var pos = new Vector3(12f, 11f, 0.5f); + var perTick = new Vector3(0f, 0.10f, 0f); + uint cellId = TestCellId; + bool isOnGround = false; + + for (int tick = 0; tick < 8; tick++) + { + Vector3 target = pos + perTick; + var result = ctx.engine.ResolveWithTransition( + pos, target, cellId, + SphereRadius, SphereHeight, + StepUpHeight, StepDownHeight, + isOnGround, + body: null, + moverFlags: ObjectInfoState.IsPlayer, + movingEntityId: 0); + _out.WriteLine($"tick={tick} pos=({pos.X:F3},{pos.Y:F3},{pos.Z:F3}) → " + + $"({result.Position.X:F3},{result.Position.Y:F3},{result.Position.Z:F3}) " + + $"hit={result.CollisionNormalValid} " + + $"normal=({result.CollisionNormal.X:F3},{result.CollisionNormal.Y:F3},{result.CollisionNormal.Z:F3})"); + pos = result.Position; + cellId = result.CellId; + isOnGround = result.IsOnGround; + } + + Assert.True(true, "Diagnostic test — always passes; check stdout."); + } + finally + { + PhysicsDiagnostics.ProbeResolveEnabled = false; + PhysicsDiagnostics.ProbeBuildingEnabled = false; + } + } + + // ─────────────────────────────────────────────────────────────── + // Apparatus setup + // ─────────────────────────────────────────────────────────────── + + private sealed record Context( + PhysicsEngine engine, + PhysicsDataCache cache, + Setup setup, + IReadOnlyList registeredShapes); + + /// + /// Build the engine + cache + landblock + registered door entity. + /// Returns false if the dat directory is missing (test skips). + /// + private bool TryBuildScenario(out Context ctx) + { + ctx = default!; + var datDir = Env.GetEnvironmentVariable("ACDREAM_DAT_DIR") + ?? Path.Combine(Env.GetFolderPath(Env.SpecialFolder.UserProfile), + "Documents", "Asheron's Call"); + if (!Directory.Exists(datDir)) + { + _out.WriteLine($"SKIP: dat directory not found at {datDir}"); + return false; + } + + using var dats = new DatCollection(datDir, DatAccessType.Read); + + // Load + cache the door's collision-bearing part. + var doorFrameGfx = dats.Get(DoorFrameGfxObjId); + Assert.NotNull(doorFrameGfx); + var cache = new PhysicsDataCache(); + cache.CacheGfxObj(DoorFrameGfxObjId, doorFrameGfx!); + Assert.NotNull(cache.GetGfxObj(DoorFrameGfxObjId)); + + var engine = new PhysicsEngine { DataCache = cache }; + + // Stub landblock at world (0, 0) so TryGetLandblockContext succeeds + // for player XY in the [0, 192) range. Flat far-below terrain so it + // doesn't interact with the door collision. + var heights = new byte[81]; + var heightTable = new float[256]; + for (int i = 0; i < 256; i++) heightTable[i] = -1000f; + engine.AddLandblock( + landblockId: TestLandblockId, + terrain: new TerrainSurface(heights, heightTable), + cells: Array.Empty(), + portals: Array.Empty(), + worldOffsetX: 0f, + worldOffsetY: 0f); + + // Load door setup, build shapes via the production builder. + var setup = dats.Get(DoorSetupId); + Assert.NotNull(setup); + var shapes = ShadowShapeBuilder.FromSetup( + setup!, + entScale: 1.0f, + hasPhysicsBsp: id => cache.GetGfxObj(id)?.BSP?.Root is not null); + + _out.WriteLine($"Shapes from FromSetup: {shapes.Count} entries"); + foreach (var s in shapes) + { + _out.WriteLine($" type={s.CollisionType} gfxObj=0x{s.GfxObjId:X8} " + + $"localPos=({s.LocalPosition.X:F3},{s.LocalPosition.Y:F3},{s.LocalPosition.Z:F3}) " + + $"radius={s.Radius:F3} cylH={s.CylHeight:F3}"); + } + + // Register the door at world (12, 12, 0). That's cell (0,0) of + // landblock 0xA9B40000. + Vector3 doorWorldPos = new(12f, 12f, 0f); + Quaternion doorWorldRot = Quaternion.Identity; + const uint doorState = 0x10008u; // PhysicsState.HasPhysicsBSP | HasDefaultScript (typical) + + engine.ShadowObjects.RegisterMultiPart( + entityId: DoorEntityId, + entityWorldPos: doorWorldPos, + entityWorldRot: doorWorldRot, + shapes: shapes, + state: doorState, + flags: EntityCollisionFlags.None, + worldOffsetX: 0f, + worldOffsetY: 0f, + landblockId: TestLandblockId); + + ctx = new Context(engine, cache, setup!, shapes); + return true; + } + + /// + /// Drive a sphere from by + /// each tick until either + /// CollisionNormalValid fires or + /// elapse. Returns the outcome + final position. + /// + private (bool blocked, Vector3 finalPos, Vector3 normal, int ticks) + SweepUntilBlocked(PhysicsEngine engine, Vector3 start, Vector3 perTick, int maxTicks) + { + Vector3 pos = start; + uint cellId = TestCellId; + bool isOnGround = false; + Vector3 lastNormal = Vector3.Zero; + + for (int tick = 0; tick < maxTicks; tick++) + { + Vector3 target = pos + perTick; + var result = engine.ResolveWithTransition( + pos, target, cellId, + SphereRadius, SphereHeight, + StepUpHeight, StepDownHeight, + isOnGround, + body: null, + moverFlags: ObjectInfoState.IsPlayer | ObjectInfoState.EdgeSlide, + movingEntityId: 0); + + if (result.CollisionNormalValid) + return (true, result.Position, result.CollisionNormal, tick); + + pos = result.Position; + cellId = result.CellId; + isOnGround = result.IsOnGround; + lastNormal = result.CollisionNormal; + } + + return (false, pos, lastNormal, maxTicks); + } +}