VoxelForge/REVIEW_FINDINGS.md

VoxelForge — Review Findings (2026-06-23)

Local-LLM cross-file QUALITY review (Qwen3.6-27B @128K, codemap-guardrailed) + Claude verification. Scope: performance, redundancy, dead code, over-complexity (not correctness bugs). Verdict: codebase is healthy — no perf regressions. Items below are improvements, by appetite.

Reconciled against fable-idea.md + the perf-pass history (2026-06-23). The big density/ streaming perf wins are already SHIPPED and are not listed here (T1.a surface cache, T1.b grid normals, T1.c LOD0 collision, CullTiles spiral fix, region foliage, passage shortlist). Everything below is net-new (redundancy / dead code / complexity — a different axis), except the two notes flagged inline. Before acting on a PERF item, glance at fable-idea.md Part I.

Legend: ✅ verified against code · ◻️ checklist box.

2026-07-04 full-codebase pass (Fable 5) — applied everything marked [x] below, plus NEW items found reviewing the un-built two-grid deco + density-volume work: • Deco launch stall fix — a pending cell already in flight from a previous build was dropped, permanently stalling its region build (blank region until strate change). Now deferred + retried. • Capture gating — level-0 tiles outside the shadow window no longer pay the mesher capture (quantize + 32 KB queue payload) nor pollute CaptureCache (IsTileCaptureUseful, both ends). • Per-tick material pushes change-detected — UpdateTerrainMaterialParams (10 vectors + 3 textures × every terrain MID) and the orb MPC writes now no-op on idle frames; static FNames. • Landmark cave march bounded — FindLandmarkColumn now honours ColDepth (same bedrock early-stop as the deco march); it ran the full strate band synchronously on the game thread. • UploadDirtyTextures now calls EnsureTextures — GPU upload toggled ON at runtime works. Deliberately NOT done: the big behavior-preserving splits below (un-built tree; compile risk).

2026-07-04 perf pass 2 (per-voxel hot path, Fable 5) — ✅ BUILT & WORKING (ticked 2026-07-27). All bit-identical (same hashes/math, hoisted per chunk/cell): • DiffLayer snapshot API (HasAnyMods/GetModsVersion/GetChunkModsSnapshot/static EvaluateMods + ModsVersion atomic) — GetDensityAt snapshots a chunk's mods once per (chunk, version) via a thread_local 64-slot cache: ~27 lock ops per tile task instead of ~86k once any carve exists. • Room shape pre-bake — FCachedRoom::ShapeType/ShapeA/ShapeB/ShapeR baked in BuildChunkCache; EvaluateSDFCached signature changed (RoomShapeVariety removed, 8 call sites updated). • Strate-index memo in GetDensityWithParams, keyed (chunkZ, GetLayoutVersion()) — new inline getter on UVoxelStrateManager. • Per-cell lattice bakes (thread_local, keyed cell+seed+params): slab columns, maze open edges, vertical shafts + cross-connectors, floating-island constants, disturbance chasms/bridges/ridges. • Worm N2 short-circuit — skip the 2nd Perlin when N1 ≥ WormThreshold (lossless). Regression fixed same day: pass 1's skip-if-identical cache on the orb MPC writes broke the mini-sun lighting (MPC world instances reset behind the writer) — REVERTED, LastOrbMPC removed; never de-duplicate writes to externally resettable state. The MID-side change detection stays (MIDs own their values).

2026-07-04 batch 3 (Fable 5) — Jahni green-lit multiple changes per build + visual deltas ("nothing is set in stone"). ✅ BUILT & WORKING together with pass 2 (ticked 2026-07-27): • Terracing gradient Z-only (6→2 SDF samples — see the ticked item above). • Lerp X-macro + BakeRoomFeature dedupe (both ticked above, bit-identical). • T2.b / T2.c / T2.d and F2 shipped in the same batch — tracked in fable-idea.md (Tier 2 status, F2) and ARCHITECTURE §8.10, not repeated here.

Performance

• VoxelGenerator.cpp (terrain-op gradient) — compute once per voxel and reuse. RE-VERIFIED 2026-07-04: misdiagnosed. The 6 EvaluateSDFCached calls in the terracing block are six DISTINCT sample positions (±1 on each axis) for one central-difference gradient — computed once per voxel already, gated to terrace-enabled rooms near surfaces. Nothing is redundantly re-evaluated. Optional lossy halving: APPLIED 2026-07-04 (Jahni approved visual deltas). SDFs are ≈unit-gradient, so Horizontality = clamp(|SDF(Z+1)−SDF(Z−1)|/2) — the 4 X/Y samples are gone (6→2 SDF evals per terrace-voxel); terraces fade slightly differently on SmoothMin'd slopes.
• VoxelContentManager.cpp (BuildCellSpawns) — slope-gate cosines now precomputed per entry (CosMaxSlope/CosMinSlope arrays), bit-identical gating. (2026-07-04)
• VoxelContentManager.cpp (LaunchDecoTasks) — head-index drain + one RemoveAt(0, Head) compaction (was O(N) per pop). Bonus fix: a pending cell still in flight from a previous build is now DEFERRED, not dropped (dropping stalled its region build forever → permanently blank region). (2026-07-04)
• VoxelStrateManager.cpp — linear BoundRadius stored on FVoxelPassage; shortlist no longer Sqrt's per passage. (2026-07-04)

Redundancy (factor out)

• VoxelCaveMorphology.cpp — Pit/Chimney/Column baking loops → shared BakeRoomFeature hash-placement skeleton (gate/XY/radius chain) + per-type Emit lambda. Bit-identical (same salts, same hash order). (2026-07-04, batch 3 — Jahni green-lit batching without intermediate builds)
• VoxelStrateTypes.h — FStrateGenerationParams::Lerp now expands the VF_STRATE_PARAM_FIELDS X-macro (all 74 fields verified present — no drift had happened yet). New struct fields MUST be added to that list. Bit-identical. (2026-07-04, batch 3)
• VoxelWorld.cpp brush methods — Carve/Fill sphere now funnel through ApplyModification like Box/Capsule already did (one diff-layer + remesh dispatch). (2026-07-04)
• VoxelContentManager.cpp — duplicated drain/reset loops → DrainDecoResults() + ResetGridBuildState(FDecoGrid&). (2026-07-04)
• EVoxelPassageType vs EVoxelPassageStyle — ✅ CLOSED 2026-09-04 as WONTFIX, enacting the 2026-08-16 judgement below, which recommended closure and was then left open by oversight. No code change; the two enums stay. Two overlapping passage-shape enums, both in active use (11 refs). Consider consolidating to one. (judgment call, not dead) JUDGED 2026-08-16 — LEAVE THEM. Recommendation: close this item rather than act on it. They read as duplicates from the index and are not: EVoxelPassageType (VoxelStrateTypes.h ~42) is the global inter-strate bore shape the layout generator picks (SlopedTunnel / VerticalShaft / helix…); EVoxelPassageStyle (~1741) is per-strate descent styling on FStratePassageConfig (Straight / Worm / Spiral / Cascading). Different owners, different value sets, different lifetimes. And both are UMETA-tagged, i.e. serialised into Jahni's authored strate assets — merging them silently rewrites saved content. That is a content-risk change bought for cosmetic tidiness, which is the wrong trade at any time and especially before the content lock.

Dead code

• UVoxelMarchingCubesMesher::GetDensity() — removed, along with InterpolateEdge, ComputeGradientNormal and GradientOffset (all dead since T1.b). (2026-07-04)
• Vestigial MidPoint — MidPoint/bHasMidPoint fields + the unreachable debug-draw branch removed. (2026-07-04)
• Codemap-known dead/legacy — GetLODForChunk, LODToStep, IsChunkInRange, LOD0Distance, LOD1Distance, ContentMaxLevel, VoxelChunk.h (whole file — FVoxelTileKey is the identity now) all removed 2026-07-04. MaxLODLevel / DecorationActorRadiusChunks were already replaced by the two-grid deco redesign (StreamTier / DecorationNearRadiusChunks). CORRECTION: GetStrateChunkZBounds is NOT dead — BuildDesiredTiles uses it for the strate-aware vertical clamp; struck from the dead list.
• 2026-09 cleanup (cleanup/2026-09) — the far sheet ring runtime (GenerateSheetMesh + its streaming path) and ~40 unreferenced functions/members/types removed (each proven by a search of Source/, Tools/, Config/ and build files; see that branch's refactor(dead-code) commit). Reflected (UPROPERTY/UFUNCTION/UENUM…) and enum candidates were NOT removed — see Owner decisions below.

Owner decisions (2026-09 cleanup)

Items left for the owner: unused data that assets, Blueprints, tools or tests may reference by name, and changes that would alter generated terrain.

• UVoxelSettings::bFarSheetRing / FarSheetSpanLevels, voxel.TestFarSheetRing and the selector's sheet branch — removed (owner go-ahead); the harness no longer passes the switch and the sheet_tile dump field stays for the parity compare.
• UVoxelSettings::CeilingViewMultiplier / CeilingBandChunks / DensityVolumeMaxTasks — removed (never read).
• Operator stack as the only density path (cleanup/2026-09, owner-approved). Every strate — authored, cooked-season (recipe or native) and editor composer candidate — now generates through its operator stack; a degenerate strate is air. Removed: UVoxelStrateDefinition::bUseOperatorStack, the archetype switch in GetDensityAt, GetSlabDensity / GetMazeDensity / GetSurfaceDensity / SurfaceDensityFromColumn / GetVerticalShaftDensity / GetFloatingIslandDensity, the four Cave Bail Not Op Stack * stats, the startup opt-in warning, commandlet -opstack, and the legacy-vs-stack comparisons in the tests. Terrain changes for any strate asset that did not already have the opt-in ticked.
• No-strate-manager fallback world. A world whose settings have no season, pool or fixed strates runs GetDensityAt's fallback: default FStrateGenerationParams through GetDensityWithParams with its direct-caller ("legacy") structural posts. It is the only caller of that post path outside tests/commandlet diagnostics. Decide whether an empty configuration should keep generating generic caves or refuse to generate.
• UVoxelStrateDefinition::WaterColor, Creatures, AmbientSound, Music, MusicVolume and the FStrateCreature / FStrateAmbientActor spawn fields (SpawnChancePerChunk, MaxPerStrate, SpawnWeight) — authored data with no consuming system yet (F9 audio, creatures).
• UVoxelBiomeDefinition::WaterMaterial — never read; the water plane uses the strate's WaterMaterial only.
• Plain C++ enums with no user: EVoxelFace (VoxelTypes.h), EVoxelSeasonSelectionReason (VoxelSeasonManifest.h), EVoxelOpCombine (VoxelDensityOp.h — its comment carries the two-channel sign convention; keep that text if the enum goes).
• IVoxelHeightOp::MaxDisplacement / FVoxelHeightStack::MaxTotalDisplacement — no caller; kept as the documented hook for a future heightfield ClassifyBox.

Over-complexity (behavior-preserving splits, optional)

⛔ REASSESSED 2026-08-16 — do not pick these up as filler work. They were written before the operator-stack refactor existed, and two of them are now actively counter-productive rather than merely optional. Read the reason before ticking anything here.

• ~GetDensityWithParams (600-1000 L) → DON'T. ⛔ Two independent reasons. (1) The operator stack is replacing this function archetype by archetype — splitting it produces code that gets deleted, and churns the eight equivalence tests that compare the stack against it bit for bit. (2) It is the hottest path in the plugin and carries the ARCHITECTURE §8.10 invariants (thread_local box-valid caches, two-pass MC loop, SSE noise). The one time a "behaviour-preserving" change was made here it silently deleted the overhang and only 1 sample in 20 000 crossed the isosurface — a perf change can be a correctness change. Revisit only once the switch path is retired for good.
• ~BuildChunkCache (450 L) → DON'T, same reason. FRoomGraphSource deliberately calls BuildChunkCache/EvaluateSDFCached instead of transcribing them, precisely so there is one definition. Restructuring it now forks the thing that was kept unforked on purpose.
• GenerateMesh (~250 L) → PrecalcDensityGrid/MarchCells/GenerateSkirts. ⚠️ Still genuinely optional, but the two-pass loop is an §8.10 invariant — a split must not merge the passes, and the Z-outermost pre-sample order is load-bearing for every column cache downstream (see the column-memo work of 2026-08-16). Low value, non-zero risk.
• GetGenerationParams (~180 L) → extract ApplyBoundaryTransition(...). The safest of the six — pure params math, no caches, and AUDIT §C2 already forced a close reading of both Gradient arms. If any of these is ever worth doing, it is this one.
• GeneratePassages (~150 L) → ComputePlacement/BuildControlChain/ComputeBounds
• BuildCellSpawns (~150 L) → FindSurfaceCrossings/PlaceDecorationsAtCrossings

---

Full method notes + raw per-pass output: E:\LocalLLM\reviews\QUALITY_VoxelForge.md (+ QUALITY_pass1/2/3).