docs: #100 ship + indoor-cell culling investigation handoff
Session-end documentation for the issue #100 ship and the visibility- culling investigation handoff for the next session. Three documents land together: - docs/superpowers/plans/2026-05-25-issue-100-terrain-cutout.md (the 3-task plan that drove this session'sf48c74a/a64e6f2/84e3b72— never committed by Tasks 1-2) - docs/research/2026-05-25-issue-100-terrain-cutout-handoff.md (the predecessor session's smoking-gun research that drove the #100 fix — never committed by the prior session) - docs/research/2026-05-25-issue-100-shipped-and-culling-handoff.md (THIS session's handoff: what shipped, what visual-verification surfaced, the issue family map for #78 + #95 + the new cellar- stairs finding, root-cause hypothesis, retail anchors, WB references, do-not-retry list, and pickup prompt for the next session's investigation + plan + implementation) Plus two updates to existing files: - CLAUDE.md — adds a ship paragraph for #100 to the M1.5 progress block. References the new handoff doc as the next-session pickup point. - docs/ISSUES.md #78 — broadens scope from "outdoor stabs visible through floor" to "outdoor stabs + terrain mesh visible inside EnvCells". Adds the 2026-05-25 cellar-stairs evidence (per user direction: not filed as new issue; treated as evidence reinforcing #78's hypothesis #2). Promotes hypothesis #2 to "high confidence as of 2026-05-25" and adds the retail anchor (acclient_2013_pseudo_c.txt:311397 CEnvCell::find_visible_child_cell). Acceptance criteria broadened to include the cellar-stairs case. Next session: pickup prompt at the bottom of the new handoff doc drives a /investigate → writing-plans → subagent-driven-development pass on indoor-cell visibility culling — the work that closes #78 + cellar-stairs together, and possibly #95 if the infrastructure overlaps. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik
parent
84e3b72b27
commit
4cbfbf98af
5 changed files with 1431 additions and 11 deletions
646
docs/superpowers/plans/2026-05-25-issue-100-terrain-cutout.md
Normal file
646
docs/superpowers/plans/2026-05-25-issue-100-terrain-cutout.md
Normal file
|
|
@ -0,0 +1,646 @@
|
|||
# Issue #100 — Transparent Ground Around Buildings — 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:** Replace acdream's cell-level `hiddenTerrainCells` mechanism (which produces 24m × 24m transparent rectangles around every Holtburg house) with retail's per-vertex Z nudge (`zFightTerrainAdjust = 0.00999999978`). Render terrain everywhere and let coplanar building floors win the depth test by being 1 cm higher than the rendered terrain.
|
||||
|
||||
**Architecture:** One-line change to [src/AcDream.App/Rendering/Shaders/terrain_modern.vert:139](src/AcDream.App/Rendering/Shaders/terrain_modern.vert:139) — pre-subtract 0.01 from `aPos.z` before the projection multiply, so every terrain vertex renders 1 cm below its physical Z. Physics path untouched (reads the un-nudged heightmap via [TerrainSurface](src/AcDream.Core/Physics/TerrainSurface.cs)). Then delete the `hiddenTerrainCells` / `BuildingTerrainCells` plumbing that's been threading through `LandblockMesh.Build`, `LoadedLandblock`, `LandblockLoader`, `GameWindow`, `GpuWorldState`, and `LandblockStreamer` — ~50 LOC of dead surface area once the nudge replaces it.
|
||||
|
||||
**Tech Stack:** GLSL 460 (terrain shader), C# .NET 10 (Core + App layers), xUnit tests, dotnet CLI.
|
||||
|
||||
**Retail oracle:**
|
||||
- `docs/research/named-retail/acclient_2013_pseudo_c.txt:1120769` — `float zFightTerrainAdjust = 0.00999999978`
|
||||
- `docs/research/named-retail/acclient_2013_pseudo_c.txt:702254` (address `006b6402`) — `edi_4[1] = ((float)(((long double)esi_1[2]) - ((long double)zFightTerrainAdjust)));` inside `ACRender::landPolysDraw(arg2=2)`
|
||||
|
||||
**Predecessor research:** [docs/research/2026-05-25-issue-100-terrain-cutout-handoff.md](docs/research/2026-05-25-issue-100-terrain-cutout-handoff.md). Phase 1 verification: see chat transcript (research confirmed end-to-end).
|
||||
|
||||
---
|
||||
|
||||
## Constraints
|
||||
|
||||
1. **Render-only.** The Z nudge MUST land in the shader, NOT in `LandblockMesh.Build` vertex output. Physics reads terrain Z from the same source — if we modify the mesh data, physics breaks too.
|
||||
2. **Constant value `0.01f`.** Match retail's literal `0.00999999978` to single-precision: `0.01f` is bit-identical when round-tripped. Don't use `glPolygonOffset` (slope-dependent, hardware-variable); use a constant world-Z subtract in the vertex shader.
|
||||
3. **No belt-and-suspenders.** Per [handoff §do-not-retry #5](docs/research/2026-05-25-issue-100-terrain-cutout-handoff.md), don't keep the `hiddenTerrainCells` mechanism alongside the nudge — delete it.
|
||||
4. **One commit per logical change.** Don't bundle the shader nudge with the plumbing removal — keep the bisect window honest if a regression appears.
|
||||
5. **Test-suite baseline.** Pre-flight `dotnet test` produces a baseline number for the focused suites (some pre-existing flakiness exists per the A6.P3 evening v2 follow-on note in CLAUDE.md). Each task must not increase the failure count.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
**Files modified:**
|
||||
|
||||
| File | Role | Change |
|
||||
|---|---|---|
|
||||
| `src/AcDream.App/Rendering/Shaders/terrain_modern.vert` | Terrain vertex shader | Add Z nudge before projection multiply |
|
||||
| `src/AcDream.Core/Terrain/LandblockMesh.cs` | Terrain mesh builder | Drop `hiddenTerrainCells` parameter + collapse block |
|
||||
| `src/AcDream.Core/World/LoadedLandblock.cs` | Loaded-landblock DTO | Drop `BuildingTerrainCells` field |
|
||||
| `src/AcDream.Core/World/LandblockLoader.cs` | Dat → LoadedLandblock | Drop `BuildBuildingTerrainCells` method + its call |
|
||||
| `src/AcDream.App/Rendering/GameWindow.cs` | Runtime wiring (3 sites) | Drop the field reference at each Build / ctor site |
|
||||
| `src/AcDream.App/Streaming/GpuWorldState.cs` | World-state owner (6 ctor sites) | Drop the 4th arg at each ctor site |
|
||||
| `src/AcDream.App/Streaming/LandblockStreamer.cs` | Worker-side hydration | Drop the 4th arg at the one ctor site |
|
||||
| `tests/AcDream.Core.Tests/World/LandblockLoaderTests.cs` | Loader unit tests | Delete `BuildBuildingTerrainCells_*` test |
|
||||
| `docs/ISSUES.md` | Issue tracker | Close #100 with the commit SHA |
|
||||
|
||||
**Files NOT modified (verified):**
|
||||
- `src/AcDream.Core/Physics/TerrainSurface.cs` — physics reads un-nudged Z; unaffected.
|
||||
- `src/AcDream.App/Rendering/TerrainModernRenderer.cs` — consumes `LandblockMeshData` (vertices unchanged).
|
||||
- `tests/AcDream.Core.Tests/Terrain/LandblockMeshTests.cs` — no `hiddenTerrainCells` test exists here (handoff was wrong about this surface; the `LandblockMesh.Build` test surface is for triangle/index correctness, not the cell-hide feature).
|
||||
- All `references/WorldBuilder/**` — read-reference; unchanged.
|
||||
|
||||
---
|
||||
|
||||
## Pre-flight
|
||||
|
||||
- [ ] **Step 0.1: Confirm working tree is on the expected branch**
|
||||
|
||||
```powershell
|
||||
git rev-parse --abbrev-ref HEAD
|
||||
```
|
||||
|
||||
Expected: `claude/strange-albattani-3fc83c` (or whatever branch the user is operating on; not `main`).
|
||||
|
||||
- [ ] **Step 0.2: Establish the pre-fix test baseline**
|
||||
|
||||
```powershell
|
||||
dotnet build
|
||||
```
|
||||
|
||||
Expected: `Build succeeded.` (warnings OK, no errors).
|
||||
|
||||
```powershell
|
||||
dotnet test --nologo --no-build --verbosity quiet
|
||||
```
|
||||
|
||||
Capture the total / passed / failed numbers. **Record the baseline failure count** — that's the regression sentinel for Tasks 1 and 2. Per CLAUDE.md, some pre-existing static-state-leak flakiness (8–19 failures across runs) is known; it's independent of issue #100. The plan's regression check is "failures didn't grow."
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Terrain shader Z nudge
|
||||
|
||||
**Files:**
|
||||
- Modify: [src/AcDream.App/Rendering/Shaders/terrain_modern.vert:139](src/AcDream.App/Rendering/Shaders/terrain_modern.vert:139)
|
||||
|
||||
The single substantive change. After this commit the rectangles around buildings are still there (the `hiddenTerrainCells` plumbing still collapses the terrain inside each building's outdoor cell). Task 2 removes that.
|
||||
|
||||
- [ ] **Step 1.1: Edit the terrain vertex shader**
|
||||
|
||||
In `src/AcDream.App/Rendering/Shaders/terrain_modern.vert`, replace the final two lines of `void main()` (currently lines 138–139, the blank line before `gl_Position` and the `gl_Position` write itself):
|
||||
|
||||
```glsl
|
||||
gl_Position = uProjection * uView * vec4(aPos, 1.0);
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```glsl
|
||||
// Retail zFightTerrainAdjust (acclient_2013_pseudo_c.txt:1120769 = 0.00999999978,
|
||||
// applied per terrain vertex inside ACRender::landPolysDraw at line 702254,
|
||||
// address 006b6402). Render terrain 1 cm below its physical Z so coplanar
|
||||
// building floors win the depth test. Physics path is unaffected — it reads
|
||||
// the un-nudged heightmap via TerrainSurface.SampleZ.
|
||||
// Closes issue #100; supersedes the hiddenTerrainCells cell-collapse hack.
|
||||
vec3 terrainPos = vec3(aPos.xy, aPos.z - 0.01);
|
||||
gl_Position = uProjection * uView * vec4(terrainPos, 1.0);
|
||||
```
|
||||
|
||||
- [ ] **Step 1.2: Build to verify the shader file compiles into the binary**
|
||||
|
||||
The shader is copied to `bin/Debug/net10.0/Rendering/Shaders/terrain_modern.vert` at build time (it's a `CopyToOutputDirectory` content item). The C# build doesn't statically validate GLSL, but it does confirm the file is in the right place.
|
||||
|
||||
```powershell
|
||||
dotnet build
|
||||
```
|
||||
|
||||
Expected: `Build succeeded.` (warnings OK, 0 errors).
|
||||
|
||||
- [ ] **Step 1.3: Run the focused test suites to make sure we haven't broken anything**
|
||||
|
||||
```powershell
|
||||
dotnet test --nologo --no-build --verbosity quiet
|
||||
```
|
||||
|
||||
Expected: total failure count ≤ the baseline from Step 0.2. The shader change cannot affect any non-rendering test, so this is a sanity check that nothing else regressed during the build.
|
||||
|
||||
- [ ] **Step 1.4: Commit**
|
||||
|
||||
```powershell
|
||||
git add src/AcDream.App/Rendering/Shaders/terrain_modern.vert
|
||||
git commit -m @'
|
||||
fix(render): #100 — render terrain 1 cm below physical Z (retail zFightTerrainAdjust)
|
||||
|
||||
Subtract 0.01 from every terrain vertex Z in the modern terrain vertex
|
||||
shader, matching retail's per-draw nudge applied inside
|
||||
ACRender::landPolysDraw(arg2=2). Coplanar building floors now always win
|
||||
the depth test against the rendered terrain, so the visual "ground at
|
||||
the building floor" reads as the building's floor, not as Z-fighting.
|
||||
|
||||
Constant 0.01f bit-equals retail's float literal 0.00999999978 when
|
||||
rounded to single precision.
|
||||
|
||||
Render-only — physics reads the un-nudged heightmap via
|
||||
TerrainSurface.SampleZ / SampleZFromHeightmap. The same render-vs-
|
||||
physics split is already established for EnvCell render lift
|
||||
(+0.02m at GameWindow.cs around the cell-mesh draw).
|
||||
|
||||
Retail anchors:
|
||||
docs/research/named-retail/acclient_2013_pseudo_c.txt:1120769
|
||||
docs/research/named-retail/acclient_2013_pseudo_c.txt:702254
|
||||
|
||||
Cross-ref:
|
||||
docs/research/2026-05-25-issue-100-terrain-cutout-handoff.md
|
||||
docs/superpowers/plans/2026-05-25-issue-100-terrain-cutout.md
|
||||
|
||||
Followed by Task 2 (delete the hiddenTerrainCells / BuildingTerrainCells
|
||||
plumbing). Visible result of this commit alone: building floors stop
|
||||
Z-fighting, but the 24m × 24m transparent rectangles persist until the
|
||||
plumbing is removed.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||||
'@
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Remove `hiddenTerrainCells` / `BuildingTerrainCells` plumbing
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/AcDream.Core/Terrain/LandblockMesh.cs` (drop parameter + collapse block)
|
||||
- Modify: `src/AcDream.Core/World/LoadedLandblock.cs` (drop field)
|
||||
- Modify: `src/AcDream.Core/World/LandblockLoader.cs` (drop method + call)
|
||||
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` (lines 1809, 5149, 8806)
|
||||
- Modify: `src/AcDream.App/Streaming/GpuWorldState.cs` (6 ctor sites)
|
||||
- Modify: `src/AcDream.App/Streaming/LandblockStreamer.cs` (line 231–235)
|
||||
- Modify: `tests/AcDream.Core.Tests/World/LandblockLoaderTests.cs` (delete `BuildBuildingTerrainCells_*` test)
|
||||
|
||||
Pure removal. The cell-collapse mechanism is the cause of the 24m transparent rectangles around buildings; Task 1's shader nudge replaces it. After this commit the rectangles are gone and terrain renders continuously under every building.
|
||||
|
||||
**Order matters within this task:** start at the deepest leaf (`LandblockMesh.Build`'s parameter and collapse block), then work outward through the data type (`LoadedLandblock` record), the loader, and finally the callers. Build at each step to catch shifted-line surprises. The commit at the end captures the whole removal.
|
||||
|
||||
- [ ] **Step 2.1: Remove the `hiddenTerrainCells` parameter from `LandblockMesh.Build`**
|
||||
|
||||
In `src/AcDream.Core/Terrain/LandblockMesh.cs`:
|
||||
|
||||
Delete the docs line for the parameter (currently line 43):
|
||||
|
||||
```csharp
|
||||
/// <param name="hiddenTerrainCells">Optional cell indices (cy * 8 + cx) to draw as zero-area triangles.</param>
|
||||
```
|
||||
|
||||
Change the `Build` signature (currently lines 44–51) from:
|
||||
|
||||
```csharp
|
||||
public static LandblockMeshData Build(
|
||||
LandBlock block,
|
||||
uint landblockX,
|
||||
uint landblockY,
|
||||
float[] heightTable,
|
||||
TerrainBlendingContext ctx,
|
||||
System.Collections.Generic.IDictionary<uint, SurfaceInfo> surfaceCache,
|
||||
System.Collections.Generic.IReadOnlySet<int>? hiddenTerrainCells = null)
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```csharp
|
||||
public static LandblockMeshData Build(
|
||||
LandBlock block,
|
||||
uint landblockX,
|
||||
uint landblockY,
|
||||
float[] heightTable,
|
||||
TerrainBlendingContext ctx,
|
||||
System.Collections.Generic.IDictionary<uint, SurfaceInfo> surfaceCache)
|
||||
```
|
||||
|
||||
Replace the index-build loop (currently lines 171–185, between the closing `}` of the cell loop and the `return new LandblockMeshData(...)`):
|
||||
|
||||
```csharp
|
||||
// Indices are trivial 0..383 since we don't deduplicate verts. When
|
||||
// a building owns an outdoor terrain cell, keep the fixed 384-index
|
||||
// contract but collapse its two triangles so the building/stair mesh
|
||||
// can visually own the hole.
|
||||
for (uint i = 0; i < VerticesPerLandblock; i++)
|
||||
{
|
||||
int cellIdx = (int)i / VerticesPerCell;
|
||||
if (hiddenTerrainCells is not null && hiddenTerrainCells.Contains(cellIdx))
|
||||
{
|
||||
indices[i] = (uint)(cellIdx * VerticesPerCell);
|
||||
continue;
|
||||
}
|
||||
|
||||
indices[i] = i;
|
||||
}
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```csharp
|
||||
// Indices are trivial 0..383 since we don't deduplicate verts.
|
||||
for (uint i = 0; i < VerticesPerLandblock; i++)
|
||||
indices[i] = i;
|
||||
```
|
||||
|
||||
**Don't build yet** — `LoadedLandblock.BuildingTerrainCells` references will still be present in other files; we'll align them in the next steps before the first build.
|
||||
|
||||
- [ ] **Step 2.2: Remove the `BuildingTerrainCells` field from `LoadedLandblock`**
|
||||
|
||||
In `src/AcDream.Core/World/LoadedLandblock.cs`, replace the entire file:
|
||||
|
||||
```csharp
|
||||
using DatReaderWriter.DBObjs;
|
||||
|
||||
namespace AcDream.Core.World;
|
||||
|
||||
public sealed record LoadedLandblock(
|
||||
uint LandblockId,
|
||||
LandBlock Heightmap,
|
||||
IReadOnlyList<WorldEntity> Entities);
|
||||
```
|
||||
|
||||
- [ ] **Step 2.3: Remove `BuildBuildingTerrainCells` from `LandblockLoader` and update the `Load` caller**
|
||||
|
||||
In `src/AcDream.Core/World/LandblockLoader.cs`, replace the entire body of the `Load` method (currently lines 16–31) with:
|
||||
|
||||
```csharp
|
||||
public static LoadedLandblock? Load(DatCollection dats, uint landblockId)
|
||||
{
|
||||
var block = dats.Get<LandBlock>(landblockId);
|
||||
if (block is null)
|
||||
return null;
|
||||
|
||||
var info = dats.Get<LandBlockInfo>((landblockId & 0xFFFF0000u) | 0xFFFEu);
|
||||
var entities = info is null
|
||||
? Array.Empty<WorldEntity>()
|
||||
: BuildEntitiesFromInfo(info, landblockId);
|
||||
|
||||
return new LoadedLandblock(landblockId, block, entities);
|
||||
}
|
||||
```
|
||||
|
||||
Then delete the `BuildBuildingTerrainCells` method entirely (currently lines 33–50, the `/// <summary>` block and the method body). The deleted lines look like:
|
||||
|
||||
```csharp
|
||||
/// <summary>
|
||||
/// Map LandBlockInfo.Buildings to 8x8 terrain mesh cells (cy * 8 + cx).
|
||||
/// Retail attaches each CBuildingObj to its outside landcell during
|
||||
/// CLandBlock::init_buildings; keep this signal separate from stabs so
|
||||
/// ordinary static props do not punch holes in terrain.
|
||||
/// </summary>
|
||||
public static IReadOnlySet<int> BuildBuildingTerrainCells(LandBlockInfo info)
|
||||
{
|
||||
var result = new HashSet<int>();
|
||||
foreach (var building in info.Buildings)
|
||||
{
|
||||
int cx = Math.Clamp((int)(building.Frame.Origin.X / 24f), 0, 7);
|
||||
int cy = Math.Clamp((int)(building.Frame.Origin.Y / 24f), 0, 7);
|
||||
result.Add(cy * 8 + cx);
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2.4: Update the two `LandblockMesh.Build` call sites in `GameWindow.cs`**
|
||||
|
||||
In `src/AcDream.App/Rendering/GameWindow.cs`:
|
||||
|
||||
**Site 1 (around line 1808–1809):** replace
|
||||
|
||||
```csharp
|
||||
return AcDream.Core.Terrain.LandblockMesh.Build(
|
||||
lb.Heightmap, lbX, lbY, _heightTable, _blendCtx, _surfaceCache, lb.BuildingTerrainCells);
|
||||
```
|
||||
|
||||
with
|
||||
|
||||
```csharp
|
||||
return AcDream.Core.Terrain.LandblockMesh.Build(
|
||||
lb.Heightmap, lbX, lbY, _heightTable, _blendCtx, _surfaceCache);
|
||||
```
|
||||
|
||||
**Site 2 (around line 8805–8806):** replace
|
||||
|
||||
```csharp
|
||||
return AcDream.Core.Terrain.LandblockMesh.Build(
|
||||
lb.Heightmap, lbX, lbY, _heightTable, _blendCtx, _surfaceCache, lb.BuildingTerrainCells);
|
||||
```
|
||||
|
||||
with
|
||||
|
||||
```csharp
|
||||
return AcDream.Core.Terrain.LandblockMesh.Build(
|
||||
lb.Heightmap, lbX, lbY, _heightTable, _blendCtx, _surfaceCache);
|
||||
```
|
||||
|
||||
**Site 3 (around line 5145–5149):** the `new LoadedLandblock(...)` ctor that passes `baseLoaded.BuildingTerrainCells`. Replace
|
||||
|
||||
```csharp
|
||||
return new AcDream.Core.World.LoadedLandblock(
|
||||
baseLoaded.LandblockId,
|
||||
baseLoaded.Heightmap,
|
||||
merged,
|
||||
baseLoaded.BuildingTerrainCells);
|
||||
```
|
||||
|
||||
with
|
||||
|
||||
```csharp
|
||||
return new AcDream.Core.World.LoadedLandblock(
|
||||
baseLoaded.LandblockId,
|
||||
baseLoaded.Heightmap,
|
||||
merged);
|
||||
```
|
||||
|
||||
- [ ] **Step 2.5: Update the six `LoadedLandblock` ctor sites in `GpuWorldState.cs`**
|
||||
|
||||
In `src/AcDream.App/Streaming/GpuWorldState.cs`, each of the six ctor sites currently passes a 4th argument referencing `BuildingTerrainCells`. Drop that argument at each site. The line numbers may shift as edits land — use Grep with pattern `BuildingTerrainCells` to find each site, and edit each one to drop its 4th argument and the trailing comma on the previous line.
|
||||
|
||||
For example, the site around line 176–180 changes from:
|
||||
|
||||
```csharp
|
||||
landblock = new LoadedLandblock(
|
||||
landblock.LandblockId,
|
||||
landblock.Heightmap,
|
||||
merged,
|
||||
landblock.BuildingTerrainCells);
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```csharp
|
||||
landblock = new LoadedLandblock(
|
||||
landblock.LandblockId,
|
||||
landblock.Heightmap,
|
||||
merged);
|
||||
```
|
||||
|
||||
The site around line 344 is a one-line form:
|
||||
|
||||
```csharp
|
||||
_loaded[kvp.Key] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, newList, lb.BuildingTerrainCells);
|
||||
```
|
||||
|
||||
becomes:
|
||||
|
||||
```csharp
|
||||
_loaded[kvp.Key] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, newList);
|
||||
```
|
||||
|
||||
Apply the same transform to all six sites. After this step there must be **zero** matches for `BuildingTerrainCells` in `src/AcDream.App/Streaming/GpuWorldState.cs`.
|
||||
|
||||
- [ ] **Step 2.6: Update the one `LoadedLandblock` ctor site in `LandblockStreamer.cs`**
|
||||
|
||||
In `src/AcDream.App/Streaming/LandblockStreamer.cs`, around line 231–235, replace
|
||||
|
||||
```csharp
|
||||
lb = new LoadedLandblock(
|
||||
lb.LandblockId,
|
||||
lb.Heightmap,
|
||||
System.Array.Empty<AcDream.Core.World.WorldEntity>(),
|
||||
lb.BuildingTerrainCells);
|
||||
```
|
||||
|
||||
with
|
||||
|
||||
```csharp
|
||||
lb = new LoadedLandblock(
|
||||
lb.LandblockId,
|
||||
lb.Heightmap,
|
||||
System.Array.Empty<AcDream.Core.World.WorldEntity>());
|
||||
```
|
||||
|
||||
- [ ] **Step 2.7: Delete the `BuildBuildingTerrainCells_*` test**
|
||||
|
||||
In `tests/AcDream.Core.Tests/World/LandblockLoaderTests.cs`, delete the test method `BuildBuildingTerrainCells_UsesBuildingsOnlyAndMapsToMeshCellIndex` (currently lines 120–147 — the `[Fact]` attribute, the method declaration, the body, and the trailing blank line). The deleted block looks like:
|
||||
|
||||
```csharp
|
||||
[Fact]
|
||||
public void BuildBuildingTerrainCells_UsesBuildingsOnlyAndMapsToMeshCellIndex()
|
||||
{
|
||||
var info = new LandBlockInfo
|
||||
{
|
||||
Objects =
|
||||
{
|
||||
new Stab
|
||||
{
|
||||
Id = 0x02000001u,
|
||||
Frame = new Frame { Origin = new Vector3(120, 72, 0) },
|
||||
},
|
||||
},
|
||||
Buildings =
|
||||
{
|
||||
new BuildingInfo
|
||||
{
|
||||
ModelId = 0x020000AAu,
|
||||
Frame = new Frame { Origin = new Vector3(141.5f, 7.2f, 94f) },
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
var cells = LandblockLoader.BuildBuildingTerrainCells(info);
|
||||
|
||||
Assert.Single(cells);
|
||||
Assert.Contains(5, cells); // cy=0, cx=5 => mesh index cy * 8 + cx.
|
||||
}
|
||||
```
|
||||
|
||||
Leave the rest of the test class untouched.
|
||||
|
||||
- [ ] **Step 2.8: Sweep for any remaining `BuildingTerrainCells` / `hiddenTerrainCells` / `BuildBuildingTerrainCells` references**
|
||||
|
||||
```powershell
|
||||
# Should return ONLY hits in docs/ (handoff doc, ISSUES.md historical entries, archived plans).
|
||||
# Zero hits in src/ and tests/.
|
||||
```
|
||||
|
||||
Use Grep with pattern `BuildingTerrainCells|hiddenTerrainCells|BuildBuildingTerrainCells` over the repo. Confirm `src/` and `tests/` are clean; `docs/` may still reference these names historically and that's fine.
|
||||
|
||||
- [ ] **Step 2.9: Build + test**
|
||||
|
||||
```powershell
|
||||
dotnet build
|
||||
```
|
||||
|
||||
Expected: `Build succeeded.` 0 errors. Warnings should be similar in count to the pre-flight baseline (no new warnings introduced — if a warning appears, the removal missed a site).
|
||||
|
||||
```powershell
|
||||
dotnet test --nologo --no-build --verbosity quiet
|
||||
```
|
||||
|
||||
Expected: failure count ≤ baseline minus 1 (the deleted `BuildBuildingTerrainCells_*` test was passing, so failure count stays at the baseline; total count drops by 1).
|
||||
|
||||
- [ ] **Step 2.10: Close issue #100 in `docs/ISSUES.md`**
|
||||
|
||||
In `docs/ISSUES.md`, locate the `#100 — Transparent rectangular patches around every house (terrain rendering)` block (around line 764) and update its **Status:** line from:
|
||||
|
||||
```markdown
|
||||
**Status:** OPEN
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```markdown
|
||||
**Status:** DONE
|
||||
**Closed:** 2026-05-25
|
||||
**Commits:** `<TASK_1_COMMIT_SHA>`, `<TASK_2_COMMIT_SHA>`
|
||||
```
|
||||
|
||||
Replace `<TASK_1_COMMIT_SHA>` with the first 7 chars of Task 1's commit, and `<TASK_2_COMMIT_SHA>` with the first 7 chars of this task's commit (you'll know it after Step 2.11 — re-edit ISSUES.md if needed, OR get the SHAs via `git log --oneline -5` and amend before pushing).
|
||||
|
||||
Then move the closed block to the **Recently closed** section at the bottom of `docs/ISSUES.md`, following the format used by the other DONE entries (e.g. #84, #85, #87).
|
||||
|
||||
Append a one-line resolution paragraph immediately under the **Commits:** line:
|
||||
|
||||
```markdown
|
||||
**Resolution (2026-05-25 · #100):** Replaced the cell-level
|
||||
`hiddenTerrainCells` mechanism with retail's per-vertex Z nudge
|
||||
(`zFightTerrainAdjust = 0.00999999978`) applied inside the modern
|
||||
terrain vertex shader. Render terrain everywhere; coplanar building
|
||||
floors win the depth test by being 1 cm higher than the rendered
|
||||
terrain. Physics path untouched. ~50 LOC of `BuildingTerrainCells`
|
||||
plumbing removed across LandblockMesh / LoadedLandblock /
|
||||
LandblockLoader / GameWindow / GpuWorldState / LandblockStreamer
|
||||
plus the corresponding unit test. Retail anchors:
|
||||
acclient_2013_pseudo_c.txt:1120769 + :702254.
|
||||
```
|
||||
|
||||
- [ ] **Step 2.11: Commit**
|
||||
|
||||
```powershell
|
||||
git add src/AcDream.Core/Terrain/LandblockMesh.cs `
|
||||
src/AcDream.Core/World/LoadedLandblock.cs `
|
||||
src/AcDream.Core/World/LandblockLoader.cs `
|
||||
src/AcDream.App/Rendering/GameWindow.cs `
|
||||
src/AcDream.App/Streaming/GpuWorldState.cs `
|
||||
src/AcDream.App/Streaming/LandblockStreamer.cs `
|
||||
tests/AcDream.Core.Tests/World/LandblockLoaderTests.cs `
|
||||
docs/ISSUES.md
|
||||
git commit -m @'
|
||||
refactor: #100 — remove hiddenTerrainCells / BuildingTerrainCells plumbing
|
||||
|
||||
Retired in favour of Task 1's retail-faithful terrain shader Z nudge.
|
||||
Pure removal — ~50 LOC of dead surface area across:
|
||||
|
||||
- src/AcDream.Core/Terrain/LandblockMesh.cs (drop parameter +
|
||||
cell-collapse block)
|
||||
- src/AcDream.Core/World/LoadedLandblock.cs (drop field)
|
||||
- src/AcDream.Core/World/LandblockLoader.cs (drop method + call)
|
||||
- src/AcDream.App/Rendering/GameWindow.cs (3 sites)
|
||||
- src/AcDream.App/Streaming/GpuWorldState.cs (6 ctor sites)
|
||||
- src/AcDream.App/Streaming/LandblockStreamer.cs (1 ctor site)
|
||||
- tests/AcDream.Core.Tests/World/LandblockLoaderTests.cs (drop test)
|
||||
|
||||
No retail anchor — the deleted mechanism never had one; this commit
|
||||
rolls our code back to the actual retail behaviour established in
|
||||
the prior commit's shader nudge.
|
||||
|
||||
ISSUES.md #100 moved to Recently closed.
|
||||
|
||||
Cross-ref:
|
||||
docs/research/2026-05-25-issue-100-terrain-cutout-handoff.md
|
||||
docs/superpowers/plans/2026-05-25-issue-100-terrain-cutout.md
|
||||
|
||||
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||||
'@
|
||||
```
|
||||
|
||||
After committing, fix up the SHA references in `docs/ISSUES.md` if Step 2.10 used placeholders:
|
||||
|
||||
```powershell
|
||||
git log --oneline -3
|
||||
```
|
||||
|
||||
Capture the two SHAs (Task 1 and Task 2). Edit `docs/ISSUES.md` to replace `<TASK_1_COMMIT_SHA>` and `<TASK_2_COMMIT_SHA>` with the real values, then amend:
|
||||
|
||||
```powershell
|
||||
git add docs/ISSUES.md
|
||||
git commit --amend --no-edit
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Visual verification at Holtburg
|
||||
|
||||
**This is the acceptance test.** The M1.5 milestone explicitly states visual verification is the acceptance gate. The two unit tests we have (`dotnet build` and `dotnet test`) prove the code compiles and the focused suites still pass — they don't prove the bug is gone.
|
||||
|
||||
- [ ] **Step 3.1: Launch the client against the live ACE server**
|
||||
|
||||
Per CLAUDE.md "Running the client against the live server" — graceful-close any prior session first.
|
||||
|
||||
```powershell
|
||||
$proc = Get-Process -Name AcDream.App -ErrorAction SilentlyContinue
|
||||
if ($proc) {
|
||||
$proc.CloseMainWindow() | Out-Null
|
||||
if (-not $proc.WaitForExit(5000)) { $proc | Stop-Process -Force }
|
||||
}
|
||||
Start-Sleep -Seconds 3
|
||||
|
||||
$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"
|
||||
dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Debug 2>&1 |
|
||||
Tee-Object -FilePath "issue100-verify-launch.log"
|
||||
```
|
||||
|
||||
Launch in the background; give the window ~8 seconds to reach in-world state.
|
||||
|
||||
- [ ] **Step 3.2: Inspect each acceptance scenario**
|
||||
|
||||
| Scenario | Expected outcome | Pass/Fail |
|
||||
|---|---|---|
|
||||
| Stand outside Holtburg cottage (any cottage) | Ground around the building reads as continuous cobblestone / grass — no dark rectangular patch | |
|
||||
| Walk in a circle around a cottage | Terrain stays continuous on all four sides | |
|
||||
| Approach the inn from the south | No transparent rectangle in front of the inn | |
|
||||
| Approach the inn from the north | No transparent rectangle behind the inn | |
|
||||
| Building floors at threshold height | No Z-fighting flicker between terrain and building floor | |
|
||||
| Walk inside the cottage cellar | (Regression check) Z-fight inside / floor still walkable | |
|
||||
|
||||
Each row must read PASS. If any row reads FAIL — stop, capture a screenshot, file as a follow-up, and ASK before "fixing" anything new.
|
||||
|
||||
- [ ] **Step 3.3: Close the client cleanly**
|
||||
|
||||
```powershell
|
||||
$proc = Get-Process -Name AcDream.App -ErrorAction SilentlyContinue
|
||||
if ($proc) {
|
||||
$proc.CloseMainWindow() | Out-Null
|
||||
if (-not $proc.WaitForExit(5000)) { $proc | Stop-Process -Force }
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 3.4: User sign-off**
|
||||
|
||||
This is the *user's* acceptance gate. After Step 3.2 produces all-PASS, report to the user with concrete observations from at least 3 cottages + the inn, then await the user's explicit confirmation before declaring #100 closed.
|
||||
|
||||
If the user observes any new visual regression (e.g. terrain visibly sinking into water polygons, or scenery objects appearing to float), pause and investigate — that's a sign the 0.01 nudge interacts with something we haven't anticipated. Do not retry or paper over.
|
||||
|
||||
---
|
||||
|
||||
## Self-review (post-write)
|
||||
|
||||
**Spec coverage:**
|
||||
|
||||
| Requirement | Task | Coverage |
|
||||
|---|---|---|
|
||||
| Add 1 cm Z subtract in terrain vertex shader at line 139 | Task 1 | ✔ |
|
||||
| Reference retail anchors in code comment | Task 1.1 | ✔ |
|
||||
| Delete `hiddenTerrainCells` parameter + collapse block | Task 2.1 | ✔ |
|
||||
| Delete `BuildingTerrainCells` field on `LoadedLandblock` | Task 2.2 | ✔ |
|
||||
| Delete `BuildBuildingTerrainCells` method + `Load` call | Task 2.3 | ✔ |
|
||||
| Update all `LandblockMesh.Build` call sites | Task 2.4 | ✔ (3 sites in GameWindow.cs) |
|
||||
| Update all `LoadedLandblock` ctor sites | Tasks 2.4, 2.5, 2.6 | ✔ (1 in GameWindow.cs + 6 in GpuWorldState.cs + 1 in LandblockStreamer.cs) |
|
||||
| Delete dead unit test | Task 2.7 | ✔ |
|
||||
| Sweep for stragglers | Task 2.8 | ✔ |
|
||||
| Close issue #100 in ISSUES.md | Task 2.10 | ✔ |
|
||||
| Visual verification at Holtburg cottages | Task 3.2 | ✔ |
|
||||
| Don't touch physics | Constraint 1, Task 1.1 comment | ✔ |
|
||||
| Don't use glPolygonOffset | Constraint 2 | ✔ |
|
||||
| Don't keep both mechanisms | Constraint 3 | ✔ |
|
||||
|
||||
**Placeholder scan:** Done — no "TBD", "TODO", "implement later", or "similar to Task N" references. Every step has the actual code.
|
||||
|
||||
**Type consistency:** `LandblockMesh.Build` signature appears in Tasks 2.1 + 2.4; both drop the 7th parameter consistently. `LoadedLandblock` ctor appears in Tasks 2.2 + 2.4 + 2.5 + 2.6; all use the 3-argument form. `BuildingTerrainCells` field referenced in Tasks 2.4 + 2.5 + 2.6 + 2.8; all removals consistent.
|
||||
|
||||
**Spec compliance check:** plan matches the user's session brief structure (3 tasks ≈ "expect 3–4 tasks" — the original brief's Tasks 2 + 3 are merged here because the test deletion has a compile dependency on the plumbing removal). Visual verification is the explicit acceptance test, matching the brief's "visual verification is the acceptance test" sentence. Do-not-retry items from the handoff doc are honored as Constraints + do-not-fix language in Step 3.4.
|
||||
Loading…
Add table
Add a link
Reference in a new issue