Add connected cave biomes and lighting

[MichaelFisher1997]

Summary

• replace short isolated cave worms with Minecraft/Luanti-inspired spaghetti and cheese noise plus a deterministic, unbounded multi-depth tunnel graph
• add seam-safe entrances, caverns, aquifers, cave dressing, lush/deep-dark cave biomes, geodes, crystals, emissive lighting, and generated cave textures
• make cave lighting robust across missing/known neighbors, propagate cave ambience to gameplay, and add comprehensive deterministic cave regressions

Cave topology

• canonical trunks continue indefinitely through 48-block cells
• cross-links, mandatory vertical connectors, graph-attached chambers, and mega-caves create connected changes of scale
• default fixture measures 6.9% cave air and a 32,128-block component spanning an 80x92x80 test volume
• entrances join canonical graph nodes instead of ending blindly in stone

Verification

• redot --editor --headless --path . --quit
• redot --headless --path . --script res://tools/worldgen_cave_verify.gd
• redot --headless --path . --script res://tools/worldgen_verify.gd
• redot --headless --path . --script res://tools/worldgen_tree_verify.gd
• redot --headless --path . --script res://tools/worldgen_lod_verify.gd
• redot --headless --path . --script res://tools/stream_full_verify.gd
• redot --headless --path . res://tools/explosives_verify.tscn
• redot --headless --path . --script res://tools/worldgen_benchmark.gd

Pinned benchmark: ~58 ms full generation, ~16 ms compact LOD.

[github-actions]

All analysis complete. Here is my review:

---

📋 Summary

No linked issues referenced in the PR body (no "Closes #N" / "Fixes #N").

📌 Review Metadata

• Reviewed Commit SHA: c140d995851383103c19041197df0894d197cf87
• Reviewed PR: Add connected cave biomes and lighting #16

This PR replaces the short worm caves with a hybrid spaghetti/cheese noise field plus a deterministic unbounded 48-block tunnel graph, and layers on cave biomes (lush/deep-dark), dressing (dripstone, moss, sculk, pools), geodes with emissive crystals, 9 new generated blocks/textures, robust 3x3-chunk light-volume handling (sealed missing neighbors, unbounded BFS queues, real attenuation for water/leaf side seeds), and cave ambience grading wired into DayNightCycle/HUD. The work is well-structured, matches the ROADMAP items it checks off, includes a purpose-built deterministic verifier added to CI, and respects the worker-thread immutability rules (noise objects are frozen after _init; all carving is value-only on chunk-owned arrays; halos of 64/11 blocks are provably sufficient for stamp reproducibility across chunk borders). No previous automated reviews existed, so there is nothing to re-verify or acknowledge as fixed.

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified. Specifically verified as safe: REBUILD_OPPOSITE_BITS (voxel_world.gd:25) maps correctly to the opposite-direction bits of DIRS_8 order; CLIMATE_BIOMES (terrain_sampler.gd:117) excludes the appended cave biome ids 19/20 so surface classification is unchanged; block ids 61-69 stay under 256; _carve_noise_caves/_stamp_small_pool/_paint_cave_floor_patch never write outside the chunk-owned array; the is_cold_biome/is_wetland_biome clamp-to-match fix prevents cave ids from being misread as FROZEN_OCEAN; BFS writes are guarded so negative light levels can never be assigned to PackedByteArray.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

**[MEDIUM]** `world/voxel_world.gd:744-754` - Explosives path skips the new full-ring light invalidation
**Confidence:** High
**Description:** `_touch_chunk` (voxel_world.gd:861-869) now correctly invalidates all eight sampled neighbors because baked light travels up to 14 cells across borders — but `carve_sphere` still uses `_collect_edge_rebuilds`, which only queues a neighbor when a removed column sits on the chunk's x/z edge. A TNT blast (radius 5) contained in a chunk interior changes the light volume that adjacent chunks baked from this chunk's pre-blast snapshot (the cavity is ≥3 cells from the border, well inside the 14-cell light reach), yet those neighbors are never requeued — their existing mask bits for this chunk also block `_remesh_on_commit_neighbors` from fixing them on re-commit.
**Impact:** Persistent stale/too-dark baked lighting around interior TNT craters near chunk borders until an unrelated edit or stream event triggers a rebuild; also contradicts the PR's own stated invariant ("edits/commits invalidate all eight sampled neighbors").
**Suggested Fix:** In `carve_sphere`, for each entry in `changed_chunks`, add the `VoxelDefs.DIRS_8` ring to `rebuild_chunks` (mirroring `_touch_chunk`), e.g. iterate `direction in VoxelDefs.DIRS_8: rebuild_chunks[chunk_position + direction] = true`. `explosives_verify.tscn` should gain a light-invalidation assertion.

**[MEDIUM]** `world/voxel_world.gd:847-869` - Per-cell 3x3-ring invalidation amplifies water-settling and edit churn ~9x
**Confidence:** Medium
**Description:** Every `_water_place` calls `_touch_chunk`, which now queues the chunk plus all eight loaded neighbors. The 4 Hz tick places up to 1024 cells; while water settles, each tick with any placement re-dirties up to ~9-16 distinct chunks, each rebuild being a full regeneration+mesh (~58 ms gen per the pinned benchmark). A single block break similarly costs 9 chunk jobs instead of 1, and `_chunk_edit_version` bumps repeatedly discard in-flight jobs. The scope is correct (any voxel in a 16-wide chunk is within 14 blocks of some border), but the batching is not.
**Impact:** Worker-pool saturation and stalled chunk streaming while water settles or during rapid building; visible on lower-core machines.
**Suggested Fix:** Batch like `carve_sphere` already does: have `_water_tick` collect touched chunk positions during the tick and invalidate each chunk's 3x3 ring once per tick (deduped), instead of per `_water_place` call. Optionally debounce neighbor-ring invalidation for same-tick repeated edits.

ℹ️ Low Priority Suggestions (Optional)

**[LOW]** `assets/placeholders/caves/` - Missing provenance README
**Confidence:** High
**Description:** `assets/placeholders/underwater/README.md` and `assets/placeholders/explosives/README.md` both document that the textures are project-generated placeholders and how to regenerate them; the new `caves/` directory (also generated by `tools/gen_underwater_textures.gd`) has no such note.
**Impact:** Minor convention/licensing-clarity deviation only (no third-party art involved).
**Suggested Fix:** Add a short README stating the textures are generated project placeholders regenerated via `redot --headless --path . --script res://tools/gen_underwater_textures.gd`.

**[LOW]** `world/worldgen/voxel_populator.gd:614-626` - Cave floor patches clip at chunk borders
**Confidence:** Medium
**Description:** `_paint_cave_floor_patch` clamps its 5-wide patch to the current chunk's bounds; the adjacent chunk does not extend the patch (its own lattice anchors gate independently on `hash_value % 3`), so moss/sculk regions can end in straight edges exactly on chunk borders.
**Impact:** Visual-only seam-like edges in cave biome materials; still deterministic (no cell disagreement), so it does not violate seam-safety correctness.
**Suggested Fix:** Evaluate patch membership from world-space coordinates (e.g., derive each column's paint decision from a world-anchored hash like `_decorate_floor_patches` does) so both chunks agree on border columns.

**[LOW]** `world/worldgen/voxel_populator.gd:719-720` - Geode hollow core converts liquids to air
**Confidence:** Medium
**Description:** The geode core sets any non-bedrock block (including aquifer water and lava placed by `_fill_underground_liquids`, which runs earlier) to air, while the calcite ring replaces liquids with solid. Suspended water/lava faces at a geode/aquifer intersection never flow because generated liquid is edit-seeded only.
**Impact:** Occasionally odd static liquid walls bounding geode hollows; no crash or determinism issue.
**Suggested Fix:** In the `distance < 0.66` branch, skip cells where `_blocks`-style water/lava ids are present (leave the liquid, or encase it in calcite as the ring does).

**[LOW]** `tools/worldgen_cave_verify.gd:118-131` - Trunk-walk check is tautological
**Confidence:** High
**Description:** The "endless trunk" test re-implements the east/south step inline and asserts the walk advances, which is true by construction of the test itself; it never exercises `_carve_cave_network` topology code. The 5x5 flood-fill portion of the same test does exercise real carving, so coverage overall is still meaningful.
**Impact:** Slight overstatement of what the regression pins.
**Suggested Fix:** Either drop the inline walk or derive the expected route from `VoxelPopulator`-exposed helpers so a graph regression can actually fail the check.

**[LOW]** `world/chunk_mesher.gd:567/708` - Unbounded BFS queues raise transient per-job memory
**Confidence:** Low
**Description:** Replacing the truncating ring buffers with growing `PackedInt32Array` queues is the correct correctness fix (the old `tail < queue.size()` guards silently dropped light), but worst-case queue length is now several times the ~300k-cell volume, transiently a few MB per job across up to 8 concurrent workers.
**Impact:** Negligible on desktop-class hardware; worth remembering if mobile targets are ever added.
**Suggested Fix:** None needed now; optionally reserve a coarse initial capacity to reduce reallocation churn.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8	Cave-region classification lives as statics in BiomeCatalog, carving/dressing stages in VoxelPopulator, light transport in ChunkMesher, ambience grading in DayNightCycle/Main — each stage is a focused, ordered pass
Open/Closed	7	New blocks/geodes/regions extend via tables (BLOCK_DEFS, EMISSIVE_COLORS, _add) without touching consumers; lighting/BFS internals were opened up for editing rather than extended, which is expected for a fix
Liskov Substitution	7	NeighborSample.from_lod() and block_at() keep the LOD/full substitutability contract intact; build_light_volume() reuses the production path rather than duplicating it
Interface Segregation	7	get_cave_ambience() returns a small focused dictionary mirroring get_water_ambience(); set_cave_ambience() keeps the DayNight setter surface minimal
Dependency Inversion	8	Populator/mesher depend on injected immutable config/catalogs; the verifier drives the same production classes
Average	7.4

🎯 Final Assessment

Overall Confidence Score: 78%

Confidence Breakdown:

• Code Quality: 85% (clean stage decomposition, careful bounds guards, honest comments explaining tradeoffs)
• Completeness: 88% (implements all four ROADMAP cave items it checks off, including verifier + CI + docs; misses only the carve_sphere light-invalidation consistency)
• Risk Level: 70% (determinism and worker-safety reasoning verified sound; residual risk in rebuild-churn performance and the explosives stale-light edge)
• Verification: 65% (author lists the full verifier suite incl. the new worldgen_cave_verify.gd; the engine binary is unavailable on this runner so none were re-run here — residual risk, not a blocker)

Merge Readiness:

• All critical issues resolved (none found)
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• No unresolved worker-thread, determinism, or data-loss risk

Verdict:

MERGE WITH FIXES

The two Medium issues (explosives bypassing the new light-ring invalidation, and per-cell water-tick rebuild amplification) are contained, correctness-motivated follow-ups that should land before or shortly after merge.

{
  "reviewed_sha": "c140d995851383103c19041197df0894d197cf87",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 2,
  "overall_confidence_score": 78,
  "recommendation": "MERGE WITH FIXES"
}

opencode session  |  github run

[MichaelFisher1997]

Addressed the actionable review findings in f394143:

• explosions now invalidate the complete loaded 3x3 light-sampling ring for every changed chunk, with regression coverage
• water changes batch edit-version bumps and deduplicated ring rebuilds once per tick instead of once per cell
• added cave texture provenance/regeneration documentation
• made the trunk walk exercise the production primary-route helper

The cave and explosives verifiers and editor parse check pass locally.

[github-actions]

All checks complete. Here is my review:

---

📋 Summary

No linked issues referenced in the PR body (no "Closes #N" / "Fixes #N").

📌 Review Metadata

• Reviewed Commit SHA: f394143d4aa5b6f6e7c75d1ca4a0e4abd5a494c8
• Reviewed PR: Add connected cave biomes and lighting #16

Commit f394143 is a focused follow-up to the original cave PR: it routes explosives through the full 3x3 light-sampling invalidation ring, batches water-tick invalidation to once per tick, adds the cave-texture provenance README, and makes the trunk-walk regression exercise the production route helper. All four actionable findings from the 2026-09-14T21:45:53Z review are correctly resolved, the previously-verified safety properties (block ids 61-69, biome ids 19/20 appended after FROZEN_OCEAN, clamp-removal in is_cold_biome/is_wetland_biome, negative-light BFS guards, worker-thread immutability) are unchanged, and the new regression assertions genuinely pin the fixed behavior.

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified. ✅ All previously reported critical issues have been resolved (none were reported).

⚠️ High Priority Issues (Should Fix)

None identified. ✅ All previously reported high priority issues have been resolved (none were reported).

💡 Medium Priority Issues (Nice to Fix)

✅ [FIXED] Previous issue: explosives path skipped the new full-ring light invalidation — carve_sphere (world/voxel_world.gd:726-733) now bumps the edit version for every changed chunk and queues the complete loaded 3x3 ring via _add_loaded_light_ring() (voxel_world.gd:866-869); _collect_edge_rebuilds was removed entirely, and tools/explosives_verify.gd:41-43 asserts all eight sampled neighbors are requeued after a blast. As a bonus, the new code only queues loaded ring chunks, where the old edge-collection queued blindly.

✅ [FIXED] Previous issue: per-cell 3x3-ring invalidation amplified water-settling churn — _water_tick (voxel_world.gd:778-796) now accumulates changed_chunks through the whole tick and performs one deduplicated version-bump + ring rebuild per touched chunk per tick; _water_place (voxel_world.gd:841-852) only records into the batch dict. The deferral is safe: _water_tick runs synchronously on the main thread inside _process after _stream_tick, so no commit can interleave between data mutation and the end-of-tick version bump, and _schedule_jobs reads job.version/edits at schedule time so queued chunks always pick up fresh edits. tools/explosives_verify.gd:45-59 pins the once-per-tick invariant. (A single block break/place still costs the 9-chunk ring via _touch_chunk, but that is the by-design cost of correct 14-block light reach, deduplicated by _gen_queued/_dirty — not the reported amplification.)

ℹ️ Low Priority Suggestions (Optional)

✅ [FIXED] Previous issue: missing cave-texture provenance README — assets/placeholders/caves/README.md added, and its regeneration command matches CAVE_OUTPUT_DIR in tools/gen_underwater_textures.gd:9.

✅ [FIXED] Previous issue: tautological trunk-walk check — tools/worldgen_cave_verify.gd:74 now calls the production helper VoxelPopulator._cave_primary_offset() (extracted to voxel_populator.gd:341-343 and used by _carve_cave_network), so a route regression can now fail the check.

**[LOW]** `world/worldgen/voxel_populator.gd:619-631` - Cave floor patches clip at chunk borders (unresolved from previous review)
**Confidence:** Medium
**Description:** `_paint_cave_floor_patch` still clamps its 5-wide patch to the current chunk's bounds; the adjacent chunk's lattice anchors gate independently, so moss/sculk regions can end in straight edges exactly on chunk borders. Deterministic (no cell disagreement), so seam-safety correctness is intact.
**Impact:** Visual-only seam-like edges in cave biome materials.
**Suggested Fix:** Derive each column's paint decision from a world-anchored hash (like `_decorate_floor_patches`) so both chunks agree on border columns.

**[LOW]** `world/worldgen/voxel_populator.gd:724-725` - Geode hollow core converts liquids to air (unresolved from previous review)
**Confidence:** Medium
**Description:** Confirmed ordering: `_fill_underground_liquids` runs at populate() line 68, before `_place_geodes` at line 70, and the core branch still airifies any non-bedrock cell (including aquifer water/lava), while the calcite ring converts liquids to solid. Generated liquid is edit-seeded only, so suspended liquid faces at geode/aquifer intersections never flow.
**Impact:** Occasionally odd static liquid walls bounding geode hollows; no crash or determinism issue.
**Suggested Fix:** In the `distance < 0.66` branch, skip water/lava cells (leave the liquid or encase it in calcite as the ring does).

**[LOW]** `world/voxel_world.gd:855` - `_touch_chunk` retains a dead parameter
**Confidence:** High
**Description:** After the refactor, `_block_position` is unused (renamed with underscore). Both call sites (voxel_world.gd:629, 649) still pass it.
**Impact:** Trivial signature noise.
**Suggested Fix:** Drop the parameter and update the two call sites.

The previously noted unbounded BFS queues (world/chunk_mesher.gd) remain unchanged and were already judged acceptable with no action required.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8	The fix extracts _add_loaded_light_ring() as one shared primitive used by edits, explosives, and water ticks, replacing three divergent invalidation paths
Open/Closed	7	Ring-invalidation policy extended without touching consumers; BFS internals remain opened-for-edit rather than extended, as expected for a fix
Liskov Substitution	7	_water_place/_update_water_cell signatures grew a batch parameter without changing LOD/full substitutability or NeighborSample contracts
Interface Segregation	7	get_cave_ambience()/set_cave_ambience() remain small focused surfaces; verifiers use the same production helpers
Dependency Inversion	8	Verifier drives production classes (_cave_primary_offset, carve_sphere, _water_tick) instead of reimplementations
Average	7.4

🎯 Final Assessment

Overall Confidence Score: 84%

Confidence Breakdown:

• Code Quality: 86% (both mediums fixed cleanly with a shared helper and honest comments; minor dead-parameter noise remains)
• Completeness: 92% (all actionable review findings addressed with regression coverage; two cosmetic LOWs deferred)
• Risk Level: 80% (worker-thread safety, determinism, and edit/version invariants re-verified sound; residual risk is the unrun engine verifiers and the two visual LOWs)
• Verification: 75% (author reports the cave/explosives verifiers and editor parse check pass locally; the engine binary is unavailable on this runner so none were re-run here — residual risk, not a blocker; CI now runs worldgen_cave_verify.gd)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• No unresolved worker-thread, determinism, or data-loss risk

Verdict:

MERGE

Both previously blocking Medium issues are correctly fixed with regression coverage, and the remaining findings are cosmetic follow-ups that do not affect correctness, determinism, or thread safety.

{
  "reviewed_sha": "f394143d4aa5b6f6e7c75d1ca4a0e4abd5a494c8",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 84,
  "recommendation": "MERGE"
}

opencode session  |  github run