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 <noreply@anthropic.com>
9.0 KiB
Codex task 001 — make tile-skipping observable in the running game
Owner: Codex (Model Luna, xHigh) · Orchestrator: Claude · Branch: experimental
Status: specified, not started
Why this exists
Jahni built the world, looked at it, and said: "I don't know if it dropped any meshing? but it looks alright by the eye."
He's right to be unsure — there is no way to answer that question from inside the game. The
plugin has zero stat counters (grep INC_DWORD_STAT → nothing). Tile-skipping is the largest
perf item in the whole plan and it is currently unobservable in production; it has only ever been
measured in an automation harness, on 40 sampled tiles.
And a visual check cannot answer it: skipped correctly and skipped nothing render identically.
This codebase has paid repeatedly for exactly that confusion — see the "coverage is a number, not a
boolean" lessons in OPSTACK-HANDOFF.md.
The real prize: with no strate opted in, cave-archetype skips must read 0. After ticking
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 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.
bool bTrivialEmpty = false;
if (!bSheetTile && !bWantCapture && Generator && Mesher && Mesher->IsoLevel == 0.0f)
{
TRACE_CPUPROFILER_EVENT_SCOPE(VoxelForge_ClassifyTile);
bTrivialEmpty = (Generator->ClassifyTile(OriginVoxels, Step, Cells) != EVoxelTileClass::Mixed);
}
FVoxelMeshData MeshData;
if (!bTrivialEmpty)
{
TRACE_CPUPROFILER_EVENT_SCOPE(VoxelForge_GenerateMesh);
MeshData = bSheetTile ? Mesher->GenerateSheetMesh(...) : Mesher->GenerateMesh(...);
}
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:
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 resolvesAllSolid. 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
-
A stat group. New header
Source/VoxelForge/Public/VoxelStats.h:DECLARE_STATS_GROUP(TEXT("VoxelForge"), STATGROUP_VoxelForge, STATCAT_Advanced);plusDECLARE_DWORD_COUNTER_STAT_EXTERNfor each counter below.DEFINE_STATfor each goes in one.cpp— put them in a newSource/VoxelForge/Private/VoxelStats.cpp. -
Six per-frame counters (
DWORD_COUNTER, sostat VoxelForgeshows a rate, not a total):counter site incremented when TilesClassifiedA the classifier gate was entered (the ifabove ranClassifyTile)TilesSkippedAllSolidA verdict was AllSolidTilesSkippedAllAirA verdict was AllAirTilesMeshedA GenerateMesh/GenerateSheetMeshactually ranTilesOpStackSolidB the bAnyCaveblock returnedAllSolidTilesOpStackAirB the bAnyCaveblock returnedAllAirSplitting 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 ≤ TilesSkippedAllSolidalways, and the difference is the pre-existing bedrock/surface skipping.At site B, increment on the
returnline only — not before theif (bCanSolid == bCanAir) return Mixed;guard, which is where the block bails out with no verdict. -
To get the verdict you need it as a value, not a bool. Changing
bTrivialEmpty = (Classify(...) != Mixed)into a storedEVoxelTileClass Verdict = Classify(...)followed bybTrivialEmpty = (Verdict != Mixed)is fine and expected.
⚠️ Invariants — a violation here is not a bug, it is a hole
- DO NOT change
bTrivialEmpty's value or the control flow. That bool decides whether a tile gets geometry and collision. A wrong value is invisible until a player falls through the floor. Refactor the expression, never the condition. - DO NOT touch the gate
!bSheetTile && !bWantCapture && Generator && Mesher && Mesher->IsoLevel == 0.0f. Every clause is load-bearing and documented in the comment above it — sheet tiles have no marching cubes, capture tiles need the grid even when uniform, and the verdicts assume the MC iso is exactly zero. - Thread safety: this runs on WORKERS.
GenerateTileResultis called from the async ChunkGen task and the synchronous carve path. Use theINC_DWORD_STATfamily, which is per-thread-packet safe. A plainstatic int32counter, even++on anint32, is a data race — do not. - Zero cost when stats are compiled out. The
INC_DWORD_STATmacros already vanish whenSTATS == 0. Do not wrap them in anifthat survives, and do not compute anything solely to feed a counter outside the macro. - No new includes in a public header beyond
Stats/Stats.h; the plugin follows IWYU and the include debt was cleared deliberately (AUDIT §C9work). - At site B, do not touch
ClassifyTile's control flow either — and do not add an earlyreturn. That function is a chain of conservative guards that all fail toMixed; everyreturnin it is load-bearing. Add the counter to the existingreturnexpression's statement, nothing else.ClassifyTileisconstand runs on the same workers as site A, so the sameINC_DWORD_STAT-not-static int32rule applies.
Acceptance
- Editor,
stat VoxelForgeon screen, fly around: the numbers move. TilesClassified == TilesSkippedAllSolid + TilesSkippedAllAir + TilesMeshedfor tiles that entered the gate. (Tiles that fail the gate are meshed without being classified, soTilesMeshedis legitimately larger than the classified total — say so in a comment rather than "fixing" it.)TilesOpStackSolid ≤ TilesSkippedAllSolidandTilesOpStackAir ≤ 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
TunnelNetworkstrate:TilesSkippedAllSolidis 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;TilesOpStackSolidandTilesOpStackAirare 0. This is the baseline, and it must be observed before the next step even though it is guaranteed by the flag gate.
- Tick
bUseOperatorStackon oneTunnelNetworkstrate, fly the same route:TilesOpStackSolidbecomes non-zero. ← this is the deliverable, and it is the first production-side evidence T1.d has ever had.
Notes for the reviewer (Claude)
- Check the verdict refactor byte-for-byte against the original condition.
!= Mixedis the whole contract. - Check the counters are
DWORD_COUNTER(per-frame) and notDWORD_ACCUMULATOR. - Confirm no counter is incremented outside the gate in a way that double-counts the carve path,
which calls
GenerateTileResultsynchronously from the game thread. - Site B: confirm the increment sits after the
bCanSolid == bCanAirbail-out, and that the two counters followbCanSolidthe same way the returned enum does — a swapped pair reads as a plausible result and proves the wrong thing. - The automation tests call
ClassifyTiledirectly; they will move the site-B counters. Harmless, but do not let a test-only path become the only thing that moves them.