From 6bd5589d53077abada0840152f6240b7698198d8 Mon Sep 17 00:00:00 2001 From: Fr0zka Date: Sun, 16 Aug 2026 15:40:23 +0200 Subject: [PATCH] docs(codex-001): the lumped AllSolid counter cannot prove T1.d -- add the op-stack site Reviewing the spec against ClassifyTile reversed its acceptance bar. The function reaches a non-Mixed verdict two independent ways: the hand-written path (a bedrock gap sets bCanAir=false, VoxelGenerator.cpp ~2835, and the tile resolves AllSolid) and the operator-stack path (the bAnyCave block). The first fires with NO strate opted in, so the spec's baseline -- "TilesSkippedAllSolid stays 0 underground" -- was never going to hold, and the before/after would have been unreadable. Adds site B at the bAnyCave exit (~3009) with TilesOpStackSolid / TilesOpStackAir. Those are zero BY CONSTRUCTION without an opted-in strate, since the branch returns Mixed at the UsesOperatorStackForChunk gate -- a stronger baseline than the one it replaces. Deliverable is now: tick the box, TilesOpStackSolid goes non-zero. Co-Authored-By: Claude Opus 5 --- CODEX-TASK-001-tile-skip-stats.md | 83 +++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 14 deletions(-) diff --git a/CODEX-TASK-001-tile-skip-stats.md b/CODEX-TASK-001-tile-skip-stats.md index c007c45..ed28016 100644 --- a/CODEX-TASK-001-tile-skip-stats.md +++ b/CODEX-TASK-001-tile-skip-stats.md @@ -23,7 +23,9 @@ boolean" lessons in `OPSTACK-HANDOFF.md`. `bUseOperatorStack` on one `TunnelNetwork` strate and flying underground, they must become non-zero. That is the **production-side proof of T1.d**, which does not exist today. -## The site — do not go looking, it is one place +## The sites — there are TWO, and the second one is the one that answers the question + +### Site A — the skip itself `Source/VoxelForge/Private/VoxelWorld.cpp`, in **`AVoxelWorld::GenerateTileResult`** (~line 1501). Trust the symbol, not the line number. @@ -44,6 +46,36 @@ if (!bTrivialEmpty) } ``` +### Site B — where the OPERATOR STACK's verdict is produced + +`Source/VoxelForge/Private/VoxelGenerator.cpp`, in **`UVoxelGenerator::ClassifyTile`**, at the **exit +of the `if (bAnyCave)` block** (~line 3009) — the last two lines of that block: + +```cpp +if (bCanSolid == bCanAir) { return EVoxelTileClass::Mixed; } +return bCanSolid ? EVoxelTileClass::AllSolid : EVoxelTileClass::AllAir; +``` + +**Why site A alone cannot answer the question — this is the correction that makes the task +meaningful.** `ClassifyTile` has *two* independent ways to reach a non-`Mixed` verdict: + +- the **hand-written** path, which predates all of this work: a chunk in a **bedrock gap** sets + `bCanAir = false` (VoxelGenerator.cpp ~2835) and, absent a passage or the origin spine, the tile + resolves **`AllSolid`**. Likewise the SurfaceWorld column scan. This fires with **no strate opted + in at all**; +- the **operator-stack** path, the `if (bAnyCave)` block, which is the only thing T1.d added. + +So `TilesSkippedAllSolid` at site A **will already be non-zero underground before any strate is +ticked** — the bedrock between strates guarantees it. A single lumped counter would make the +before/after unreadable, and that is exactly the "when a zero has several possible causes, give each +one its own number" lesson this project already paid for. + +Site B's counters have the opposite property, and it is a strong one: `ClassifyTile` returns `Mixed` +outright at the cave branch when `UsesOperatorStackForChunk(CC)` is false (~line 2809, and again per +chunk of the box at ~2914). **With no strate opted in, the site-B counters are zero by +construction, not merely by observation** — so a non-zero reading after ticking the box cannot come +from anywhere else. + ## What to build 1. **A stat group.** New header `Source/VoxelForge/Public/VoxelStats.h`: @@ -51,17 +83,25 @@ if (!bTrivialEmpty) `DECLARE_DWORD_COUNTER_STAT_EXTERN` for each counter below. `DEFINE_STAT` for each goes in **one** `.cpp` — put them in a new `Source/VoxelForge/Private/VoxelStats.cpp`. -2. **Four per-frame counters** (`DWORD_COUNTER`, so `stat VoxelForge` shows a rate, not a total): +2. **Six per-frame counters** (`DWORD_COUNTER`, so `stat VoxelForge` shows a rate, not a total): - | counter | incremented when | - |---|---| - | `TilesClassified` | the classifier gate was entered (the `if` above ran `ClassifyTile`) | - | `TilesSkippedAllSolid` | verdict was `AllSolid` | - | `TilesSkippedAllAir` | verdict was `AllAir` | - | `TilesMeshed` | `GenerateMesh` / `GenerateSheetMesh` actually ran | + | counter | site | incremented when | + |---|---|---| + | `TilesClassified` | A | the classifier gate was entered (the `if` above ran `ClassifyTile`) | + | `TilesSkippedAllSolid` | A | verdict was `AllSolid` | + | `TilesSkippedAllAir` | A | verdict was `AllAir` | + | `TilesMeshed` | A | `GenerateMesh` / `GenerateSheetMesh` actually ran | + | `TilesOpStackSolid` | B | the `bAnyCave` block returned `AllSolid` | + | `TilesOpStackAir` | B | the `bAnyCave` block returned `AllAir` | - Splitting solid from air is the point, not decoration: **cave archetypes prove `AllSolid`**, so - that counter is the one that answers "did the op-stack work do anything in the real game". + Splitting solid from air is the point, not decoration: **cave archetypes prove `AllSolid`**. + Splitting site B from site A is the whole deliverable — see "Why site A alone cannot answer the + question" above. `TilesOpStackSolid ≤ TilesSkippedAllSolid` always, and the difference is the + pre-existing bedrock/surface skipping. + + At site B, increment on the `return` line only — **not** before the + `if (bCanSolid == bCanAir) return Mixed;` guard, which is where the block bails out with no + verdict. 3. To get the verdict you need it as a value, not a bool. Changing `bTrivialEmpty = (Classify(...) != Mixed)` into a stored `EVoxelTileClass Verdict = Classify(...)` @@ -84,6 +124,11 @@ if (!bTrivialEmpty) feed a counter outside the macro. 5. **No new includes in a public header beyond `Stats/Stats.h`**; the plugin follows IWYU and the include debt was cleared deliberately (`AUDIT §C9` work). +6. **At site B, do not touch `ClassifyTile`'s control flow either — and do not add an early + `return`.** That function is a chain of conservative guards that all **fail to `Mixed`**; every + `return` in it is load-bearing. Add the counter to the existing `return` expression's statement, + nothing else. `ClassifyTile` is `const` and runs on the same workers as site A, so the same + `INC_DWORD_STAT`-not-`static int32` rule applies. ## Acceptance @@ -91,11 +136,16 @@ if (!bTrivialEmpty) - **`TilesClassified == TilesSkippedAllSolid + TilesSkippedAllAir + TilesMeshed`** for tiles that entered the gate. (Tiles that fail the gate are meshed without being classified, so `TilesMeshed` is legitimately larger than the classified total — say so in a comment rather than "fixing" it.) -- With **no strate opted in**: flying underground through a `TunnelNetwork` strate, - `TilesSkippedAllSolid` stays **0**. That is the baseline and it must be observed *before* the - next step, or the next step proves nothing. +- **`TilesOpStackSolid ≤ TilesSkippedAllSolid`** and **`TilesOpStackAir ≤ TilesSkippedAllAir`**, + always. A violation means site B is counting a verdict that site A did not act on. +- With **no strate opted in**, flying underground through a `TunnelNetwork` strate: + - `TilesSkippedAllSolid` is expected to be **non-zero** — that is the pre-existing bedrock/surface + skipping, not a bug, and it is why the lumped counter cannot be the deliverable; + - `TilesOpStackSolid` and `TilesOpStackAir` are **0**. This is the baseline, and it must be + *observed* before the next step even though it is guaranteed by the flag gate. - Tick `bUseOperatorStack` on **one** `TunnelNetwork` strate, fly the same route: - `TilesSkippedAllSolid` becomes **non-zero**. ← this is the deliverable. + **`TilesOpStackSolid` becomes non-zero.** ← this is the deliverable, and it is the first + production-side evidence T1.d has ever had. ## Notes for the reviewer (Claude) @@ -104,3 +154,8 @@ if (!bTrivialEmpty) - Check the counters are `DWORD_COUNTER` (per-frame) and not `DWORD_ACCUMULATOR`. - Confirm no counter is incremented outside the gate in a way that double-counts the carve path, which calls `GenerateTileResult` synchronously from the game thread. +- Site B: confirm the increment sits **after** the `bCanSolid == bCanAir` bail-out, and that the two + counters follow `bCanSolid` the same way the returned enum does — a swapped pair reads as a + plausible result and proves the wrong thing. +- The automation tests call `ClassifyTile` directly; they will move the site-B counters. Harmless, + but do not let a test-only path become the only thing that moves them.