From 5f2b54597914e8d7542dc6d7b356d2f585f1fce2 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 20 May 2026 11:42:13 +0200 Subject: [PATCH] fix(physics): skip mesh-AABB-fallback cylinder for landblock stabs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ISSUES #83 Phase A1. Landblock stabs (entity.Id 0xC0XXYY00+n per LandblockLoader.cs:55) were being registered with TWO collision shadows: the correct per-part BSP at `entity.Id*256 + partIdx`, AND a redundant mesh-AABB-fallback cylinder at `entity.Id`. The fallback clamped to 1.5m radius, centered at the building's mesh origin, producing user-reported "thin air" collisions inside cottages and within 2m of building exteriors. The fallback was originally designed for canopy-only-BSP procedural scenery (0x80XXYY00+n) — trees whose BSP covers the canopy but not the trunk. Landblock stabs have full BSP coverage and don't need it. Probe evidence (launch-thinair capture): - 0xC0A9B479 cylinder fallback (Holtburg cottage): 104 hits in a short capture session, all inside the cottage main room (cell=0xA9B4013F), ~2m from the building's mesh origin. - 0xA9B47900 BSP (the actual cottage walls): 52 legitimate hits. Fix: one new bool _isLandblockStab + one clause in the existing mesh-AABB-fallback gate. Spec: docs/superpowers/specs/2026-05-21-cylinder-fallback-dedup-design.md. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-21-cylinder-fallback-dedup.md | 194 ++++++++++++++++++ ...26-05-21-cylinder-fallback-dedup-design.md | 157 ++++++++++++++ src/AcDream.App/Rendering/GameWindow.cs | 12 ++ 3 files changed, 363 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-21-cylinder-fallback-dedup.md create mode 100644 docs/superpowers/specs/2026-05-21-cylinder-fallback-dedup-design.md diff --git a/docs/superpowers/plans/2026-05-21-cylinder-fallback-dedup.md b/docs/superpowers/plans/2026-05-21-cylinder-fallback-dedup.md new file mode 100644 index 0000000..0171f10 --- /dev/null +++ b/docs/superpowers/plans/2026-05-21-cylinder-fallback-dedup.md @@ -0,0 +1,194 @@ +# Cylinder fallback dedupe (Phase A1) — implementation plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Skip the mesh-AABB-fallback cylinder registration for +landblock stabs (`entity.Id & 0xFF000000u == 0xC0000000u`). Eliminates +"thin air" collisions inside cottages and around building exteriors. + +**Architecture:** One-line bool definition near existing +`_isOutdoorMesh` declaration + one additional clause in the existing +`if` gate at the fallback's entry. No new files, no math changes, no +new tests beyond visual verification. + +**Tech Stack:** C# .NET 10. Single source-file change. + +**Spec:** [`docs/superpowers/specs/2026-05-21-cylinder-fallback-dedup-design.md`](../specs/2026-05-21-cylinder-fallback-dedup-design.md) + +--- + +## File Structure + +| File | Action | Responsibility | +|---|---|---| +| `src/AcDream.App/Rendering/GameWindow.cs` | Modify | Add `_isLandblockStab` flag + AND it into the mesh-AABB-fallback gate. ~3 lines added. | + +No new tests. The fix is verified by a live capture (no [entity-source] +mesh-aabb-fallback emissions for 0xC0xxxxxx entities) + visual +verification (no thin-air collisions inside cottages). + +--- + +## Task 1: Gate mesh-AABB-fallback on "not a landblock stab" + +**Files:** +- Modify: `src/AcDream.App/Rendering/GameWindow.cs` + - **Insertion site 1:** immediately after the existing `_isOutdoorMesh` / `_isScenery` declarations at lines 5826-5829. + - **Insertion site 2:** add a clause to the existing `if` at line 6050-6052 (the gate around the mesh-AABB-fallback path). + +- [ ] **Step 1: Add `_isLandblockStab` flag near `_isOutdoorMesh`** + +Find this code at lines 5825-5830 in `GameWindow.cs`: + +```csharp + uint _srcPrefix = entity.SourceGfxObjOrSetupId & 0xFF000000u; + bool _isOutdoorMesh = ((entity.Id & 0x80000000u) != 0) // scenery + || ((entity.Id < 0x40000000u) // stab + && (_srcPrefix == 0x01000000u || _srcPrefix == 0x02000000u)); + bool _isScenery = _isOutdoorMesh; + if (_isScenery) scTried++; +``` + +Insert a new line immediately after `bool _isScenery = _isOutdoorMesh;` +(but before the `if (_isScenery) scTried++;`), so the block becomes: + +```csharp + uint _srcPrefix = entity.SourceGfxObjOrSetupId & 0xFF000000u; + bool _isOutdoorMesh = ((entity.Id & 0x80000000u) != 0) // scenery + || ((entity.Id < 0x40000000u) // stab + && (_srcPrefix == 0x01000000u || _srcPrefix == 0x02000000u)); + bool _isScenery = _isOutdoorMesh; + // ISSUES #83 / Phase A1 (2026-05-21): landblock stabs + // (LandBlockInfo.Objects + Buildings) use entity.Id with the + // 0xC0XXYY00+n layout per LandblockLoader.cs:55. Their BSP + // collision covers the whole structure; the mesh-AABB-fallback + // path below is for canopy-only-BSP procedural scenery + // (0x80XXYY00+n) and produces a redundant 1.5m-clamped + // invisible disc at the stab's mesh origin — the user-reported + // "thin air" collision inside cottages. Gate the fallback to + // exclude stabs. Spec: + // docs/superpowers/specs/2026-05-21-cylinder-fallback-dedup-design.md. + bool _isLandblockStab = (entity.Id & 0xFF000000u) == 0xC0000000u; + if (_isScenery) scTried++; +``` + +- [ ] **Step 2: AND `!_isLandblockStab` into the existing fallback gate** + +Find this code at lines 6050-6052: + +```csharp + if (!isPhantomSetup + && (_isOutdoorMesh || (entityBsp == 0 && entityCyl == 0)) + && entity.MeshRefs.Count > 0) +``` + +Modify it to: + +```csharp + if (!isPhantomSetup + && !_isLandblockStab + && (_isOutdoorMesh || (entityBsp == 0 && entityCyl == 0)) + && entity.MeshRefs.Count > 0) +``` + +(One new line, `&& !_isLandblockStab`, inserted as the second clause.) + +- [ ] **Step 3: Build the project** + +Run: `dotnet build` + +Expected: PASS — no compile errors. No warnings other than the +pre-existing baseline. + +- [ ] **Step 4: Run the full Core test suite** + +Run: `dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj` + +Expected: PASS — same 8 pre-existing failures as baseline, no new +failures, no new passes. + +- [ ] **Step 5: Commit** + +```bash +git add src/AcDream.App/Rendering/GameWindow.cs +git commit -m "$(cat <<'EOF' +fix(physics): skip mesh-AABB-fallback cylinder for landblock stabs + +ISSUES #83 Phase A1. Landblock stabs (entity.Id 0xC0XXYY00+n per +LandblockLoader.cs:55) were being registered with TWO collision +shadows: the correct per-part BSP at `entity.Id*256 + partIdx`, AND a +redundant mesh-AABB-fallback cylinder at `entity.Id`. The fallback +clamped to 1.5m radius, centered at the building's mesh origin, +producing user-reported "thin air" collisions inside cottages and +within 2m of building exteriors. + +The fallback was originally designed for canopy-only-BSP procedural +scenery (0x80XXYY00+n) — trees whose BSP covers the canopy but not +the trunk. Landblock stabs have full BSP coverage and don't need it. + +Probe evidence (launch-thinair capture): +- 0xC0A9B479 cylinder fallback (Holtburg cottage): 104 hits in a + short capture session, all inside the cottage main room + (cell=0xA9B4013F), ~2m from the building's mesh origin. +- 0xA9B47900 BSP (the actual cottage walls): 52 legitimate hits. + +Fix: one new bool _isLandblockStab + one clause in the existing +mesh-AABB-fallback gate. + +Spec: docs/superpowers/specs/2026-05-21-cylinder-fallback-dedup-design.md. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Verification (manual, post-commit) + +After the commit lands, re-run the thin-air capture: + +```powershell +$env:ACDREAM_DAT_DIR = "$env:USERPROFILE\Documents\Asheron's Call" +$env:ACDREAM_LIVE = "1" +$env:ACDREAM_TEST_HOST = "127.0.0.1" +$env:ACDREAM_TEST_PORT = "9000" +$env:ACDREAM_TEST_USER = "testaccount" +$env:ACDREAM_TEST_PASS = "testpassword" +$env:ACDREAM_DEVTOOLS = "1" +$env:ACDREAM_PROBE_BUILDING = "1" +$env:ACDREAM_PROBE_RESOLVE = "1" +dotnet build -c Debug +dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Debug 2>&1 | + Tee-Object -FilePath "launch-a1-verify.log" +``` + +User walks inside a Holtburg cottage and around outside. Expected: + +1. **Visual:** can walk freely inside the cottage room — no invisible + wall in the middle of the room. +2. **Log:** convert to UTF-8 and check: + +```powershell +Get-Content launch-a1-verify.log -Encoding Unicode | + Out-File launch-a1-verify.utf8.log -Encoding utf8 +Get-Content launch-a1-verify.utf8.log | + Where-Object { $_ -match '\[entity-source\].*note=mesh-aabb-fallback.*entityId=0xC0' } | + Measure-Object | Select-Object -ExpandProperty Count +``` + +Expected: **0** (no 0xC0 entities now register a mesh-AABB-fallback). + +Also expected: no `[resolve]` hits attribute to `0xC0XXYY79`-style +cylinder IDs. + +--- + +## Self-review checklist + +- [x] **Spec coverage**: spec components 1+2 (the bool + the gate clause) both implemented in Task 1 steps 1+2. +- [x] **No placeholders**: every step has the exact line numbers and code blocks. +- [x] **Type consistency**: `_isLandblockStab` is `bool`, referenced in the gate as `!_isLandblockStab`. Consistent. +- [x] **Atomic commit**: one commit, ~3 lines added, single file. +- [x] **Acceptance**: visual verification (no thin-air collision) + grep verification (no 0xC0 fallback registrations). +- [x] **No new tests**: justified — the fix is in `GameWindow.cs` (app layer), which doesn't have natural unit tests, and the behavior is verified by live capture per the spec. diff --git a/docs/superpowers/specs/2026-05-21-cylinder-fallback-dedup-design.md b/docs/superpowers/specs/2026-05-21-cylinder-fallback-dedup-design.md new file mode 100644 index 0000000..b3958db --- /dev/null +++ b/docs/superpowers/specs/2026-05-21-cylinder-fallback-dedup-design.md @@ -0,0 +1,157 @@ +# Cylinder fallback dedupe for landblock stabs (Phase A1) + +**Date:** 2026-05-21 +**Status:** Spec — awaiting user approval before plan-writing. +**Phase:** ISSUES #83 fix sequence — Phase A1 of A1→A2→A3→A4. +**Author:** Claude Opus 4.7. + +## Summary + +Landblock stabs (entity.Id `0xC0XXYY00+n` — buildings + structures +from `LandBlockInfo.Objects` and `LandBlockInfo.Buildings`) are +registered with TWO collision shadows in +`GameWindow.RegisterShadowsForLandblock`: + +1. **BSP shadow per part** (`id = entity.Id * 256 + partIdx`, truncates + to e.g. `0xA9B47900` for the Holtburg cottage) — the correct + wall-by-wall collision. +2. **Mesh-AABB-fallback cylinder** (`id = entity.Id` = `0xC0A9B479`) + — a 1.5 m-clamped invisible disc at the mesh origin. **This is the + bug**: it blocks the player in "thin air" inside cottages and near + building exteriors. + +The fallback path was originally designed to cover canopy-only-BSP +trees (`0x80XXYY00+n` procedural scenery) where the visible trunk +isn't covered by the BSP. For landblock stabs the BSP covers the +whole structure; the fallback is redundant and incorrect. + +Fix: gate the mesh-AABB-fallback registration path on "NOT a landblock +stab." One extra clause in the existing `if` condition; ~5 lines of +code; no math changes. + +## Evidence + +From the 2026-05-21 thin-air-collision capture +(`launch-thinair.utf8.log`): + +``` +[entity-source] id=0xA9B47900 entityId=0xC0A9B479 type=BSP note=partIdx=0 hasPhys=true +[entity-source] id=0xC0A9B479 entityId=0xC0A9B479 type=Cylinder note=mesh-aabb-fallback +``` + +Same `entityId`, same `gfxObj=0x01000A2B`, same landblock +`0xA9B4FFFF`. The 1.5 m cylinder is the user-reported "thin air" +collider. + +Hit attribution counts (this capture): + +| obj id | type | hits | role | +|---|---|---:|---| +| `0xC0A9B479` | Cylinder (fallback) | **104** | invisible disc — the bug | +| `0xA9B47900` | BSP (correct walls) | 52 | real cottage walls | +| `0xB5001600` | BSP (interior static) | 97 | inn fireplace / furniture | + +The 104 fallback hits happen inside cell `0xA9B4013F` (cottage main +room) at ~2 m from the building's mesh origin +`entOrigin_lb=(130.5, 11.5, 94.0)`. + +## Root cause + +`src/AcDream.App/Rendering/GameWindow.cs:5826-5828`: + +```csharp +bool _isOutdoorMesh = ((entity.Id & 0x80000000u) != 0) // scenery + || ((entity.Id < 0x40000000u) // stab + && (_srcPrefix == 0x01000000u || _srcPrefix == 0x02000000u)); +``` + +`(entity.Id & 0x80000000u) != 0` matches BOTH `0x80...` (procedural +scenery) AND `0xC0...` (landblock stabs). The mesh-AABB-fallback gate +at line 6050-6052: + +```csharp +if (!isPhantomSetup + && (_isOutdoorMesh || (entityBsp == 0 && entityCyl == 0)) + && entity.MeshRefs.Count > 0) +``` + +triggers the fallback for both classes. For stabs (which have full BSP +coverage) this is wrong. + +## Fix + +Add `_isLandblockStab` flag near the existing `_isOutdoorMesh` +definition, and AND it into the fallback gate: + +```csharp +bool _isLandblockStab = (entity.Id & 0xFF000000u) == 0xC0000000u; +``` + +Then in the gate: + +```csharp +if (!isPhantomSetup + && !_isLandblockStab // ← NEW: stabs have BSP; fallback is redundant + && (_isOutdoorMesh || (entityBsp == 0 && entityCyl == 0)) + && entity.MeshRefs.Count > 0) +``` + +That's the entire code change. ~3 lines added. + +## Acceptance criteria + +1. `dotnet build` green. +2. Re-launch with `ACDREAM_PROBE_BUILDING=1 ACDREAM_DEVTOOLS=1`. Open + the log and grep `[entity-source].*note=mesh-aabb-fallback`. For + every match, the `entityId` must NOT start with `0xC0`. +3. **Visual verification (the acceptance test):** walk inside a + Holtburg cottage; the player should be able to walk anywhere in + the room without being blocked by invisible walls in "thin air." + Walking near the outside walls of buildings should not show + invisible barriers 2 m before the visible wall. +4. The procedural scenery cylinder coverage stays — walking up to a + tree trunk should still stop at the trunk (not pass through), + confirming we didn't regress the original purpose of the fallback. + +## Out of scope + +- **PHSP inversion fix** (Phase A2) — separate spec. +- **Synthesis removal** (Phase A3) — separate spec; needs A1+A2 first. +- **Multi-cell iteration** (Phase A4) — separate spec. +- **Distinguish CScenery vs CBuildingObj at retail level** — retail + has two different classes; we use one path here. Refactoring to + match retail's class layout is post-M2 polish. +- **`0xB5001600` interior static collisions** (97 hits) — these + attribute to a 0x40-prefix interior entity (probably inn fireplace + or furniture). Whether they're legitimate or bugged is a separate + investigation. Out of this phase's scope. + +## Risks + +- **R1: Trees with canopy-only BSP regress** (player walks through + trunk). The fallback was added for this case. *Mitigation:* + unaffected — trees are `0x80xxxxxx` scenery, not `0xC0xxxxxx` + stabs. The fix is precisely scoped to stabs. Verified by the + entity-ID layout in `src/AcDream.Core/World/LandblockLoader.cs:55`. + +- **R2: Some buildings might have INCOMPLETE BSP** (e.g., a roof + with no floor BSP). After fix, those buildings lose the fallback + cylinder coverage. *Mitigation:* none in this phase. If a user + reports a building with no collision after this fix, we add per- + building inspection. Retail design is "BSP only for stabs"; this + is the correct path. + +- **R3: The 0xB5xx interior statics (entity.Id 0x40xxxxxx) may have + the same doubled-collision bug** (BSP + cylinder fallback). They + don't have the 0xC0 prefix so our fix wouldn't apply. *Mitigation:* + out of scope. Investigated as a follow-up if visual regression + surfaces. + +## References + +- Capture log: `launch-thinair.utf8.log` (2026-05-21, uncommitted). +- Entity ID layout: [`src/AcDream.Core/World/LandblockLoader.cs:40-55`](src/AcDream.Core/World/LandblockLoader.cs:55). +- Affected code: [`src/AcDream.App/Rendering/GameWindow.cs:5791-6206`](src/AcDream.App/Rendering/GameWindow.cs:5791) (the per-entity registration loop in `RegisterShadowsForLandblock`). +- Prior phase: spec + [`2026-05-21-indoor-walk-miss-probe-design.md`](2026-05-21-indoor-walk-miss-probe-design.md). +- Findings: [`docs/research/2026-05-21-walk-miss-capture-findings.md`](../../research/2026-05-21-walk-miss-capture-findings.md). diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 302a124..37e5087 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -5827,6 +5827,17 @@ public sealed class GameWindow : IDisposable || ((entity.Id < 0x40000000u) // stab && (_srcPrefix == 0x01000000u || _srcPrefix == 0x02000000u)); bool _isScenery = _isOutdoorMesh; + // ISSUES #83 / Phase A1 (2026-05-21): landblock stabs + // (LandBlockInfo.Objects + Buildings) use entity.Id with the + // 0xC0XXYY00+n layout per LandblockLoader.cs:55. Their BSP + // collision covers the whole structure; the mesh-AABB-fallback + // path below is for canopy-only-BSP procedural scenery + // (0x80XXYY00+n) and produces a redundant 1.5m-clamped + // invisible disc at the stab's mesh origin — the user-reported + // "thin air" collision inside cottages. Gate the fallback to + // exclude stabs. Spec: + // docs/superpowers/specs/2026-05-21-cylinder-fallback-dedup-design.md. + bool _isLandblockStab = (entity.Id & 0xFF000000u) == 0xC0000000u; if (_isScenery) scTried++; // Register EACH physics-enabled part so multi-part Setups // (buildings, trees) have all their collision geometry registered. @@ -6048,6 +6059,7 @@ public sealed class GameWindow : IDisposable // L-fix3: skip entirely when the Setup is phantom — retail // decorative meshes have no collision data on purpose. if (!isPhantomSetup + && !_isLandblockStab && (_isOutdoorMesh || (entityBsp == 0 && entityCyl == 0)) && entity.MeshRefs.Count > 0) {