feat: add aquatic biome decorations

[MichaelFisher1997]

Summary

• Adds aquatic blocks for seagrass, kelp, seaweed, coral blocks, and coral fans with cutout/solid registry metadata.
• Adds warm ocean and tropical biome definitions, colors, selection hooks, and vegetation density fields.
• Extends decoration placement with water-depth constraints so underwater plants/coral only replace valid water above the seafloor.

Verification

• nix develop --command zig fmt ...
• nix develop --command zig build test
• nix develop --command zig build -Doptimize=ReleaseFast
• nix develop --command zig build run -Dskip-present -Dauto-world=normal -Dstartup-diagnostic-seconds=5 timed out at 90s without crash diagnostics
• nix develop --command zig build run -Dskip-present -Dsmoke-test timed out at 60s without crash diagnostics

Closes #626

[github-actions]

📋 Summary

Linked Issue: #626 - "Aquatic: Add underwater vegetation and coral foundation"

• Issue scope includes: seagrass, tall_seagrass, kelp, coral blocks, coral fans, placement/render support, warm ocean consumption
• The PR adds seagrass, kelp, seaweed, coral_block, coral_fan with registry entries, render shapes, and water-depth constrained placement
• Warm ocean and tropical biomes are added and consume the decoration system
• tall_seagrass is explicitly mentioned in issue [Phase 2B] Aquatic: Add underwater vegetation and coral foundation #626 but not implemented in this PR

The PR extends the block registry, biome registry, and decoration system to support aquatic vegetation and warm ocean biomes. Changes are well-scoped, tests are added, and the existing decoration API is reused rather than replaced. The main concern is the reported timeout during smoke-test/integration runs, which may indicate a performance regression from the per-column water depth scanning.

---

📌 Review Metadata

• Reviewed Commit SHA: 9fed6a4cc9cfb37caab51b9bba27d4eb429a22ec
• Reviewed PR: feat: add aquatic biome decorations #638
• Linked Issue: [Phase 2B] Aquatic: Add underwater vegetation and coral foundation #626 (partially addressed - missing tall_seagrass)

---

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

---

⚠️ High Priority Issues (Should Fix)

None identified.

---

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] modules/world-worldgen/src/biome_decorator.zig:104-113 - Per-column water depth scanning may cause performance regression
Confidence: Medium
Description: columnWaterDepth() performs an upward linear scan from surface_y + 1 for every decorated column. In ocean/deep_ocean biomes, this scans the full water column (potentially 50-60+ blocks per column × 256 columns per chunk). The PR author reports both -Dsmoke-test and -Dstartup-diagnostic-seconds=5 timed out without crash diagnostics, suggesting world generation may now exceed timeout thresholds.
Impact: Automated smoke tests and CI may fail due to timeouts. Player world generation may feel slower, especially in ocean-heavy seeds.
Suggested Fix: Cache water depth per-column during the surface generation pass, or compute it lazily and memoize in a small per-chunk array. Alternatively, early-out for biomes that don't require aquatic decorations.

[MEDIUM] modules/world-core/src/block.zig - Missing tall_seagrass block from issue scope
Confidence: High
Description: Issue #626 explicitly lists tall_seagrass as a candidate block. The PR adds seagrass (.cross shape, single block) but not a two-block-tall variant analogous to tall_grass (.tall_cross). Since the issue scope specifically calls this out, its absence means the issue is not fully resolved.
Impact: Issue #626 cannot be fully closed without tall seagrass support.
Suggested Fix: Add a tall_seagrass block type with .tall_cross render shape, or update issue #626 to explicitly descope tall seagrass into a follow-up issue.

---

ℹ️ Low Priority Suggestions (Optional)

[LOW] modules/world-worldgen/src/biome_registry.zig - Tropical biome BIOME_POINTS continentalness mismatch
Confidence: Medium
Description: The BIOME_POINTS entry for tropical specifies .max_continental = 0.42, but the full BiomeDefinition uses .max = 0.48, and biome_selector.zig uses < 0.48. This inconsistency could cause confusion about which range is authoritative.
Impact: Minor - may lead to unexpected biome distribution at the edges.
Suggested Fix: Align the BIOME_POINTS entry with the selector threshold (either both 0.42 or both 0.48).

[LOW] modules/world-meshing/src/biome_colors.zig vs modules/world-worldgen/src/world_map.zig - Warm ocean color values differ slightly
Confidence: Low
Description: Warm ocean water is {0.08, 0.50, 0.82} in biome_colors.zig but {0.08, 0.48, 0.72} in world_map.zig. These represent different color spaces but may produce inconsistent visual results between chunk meshing and map rendering.
Impact: Visual inconsistency between in-world water tint and map view.
Suggested Fix: Verify and align the warm ocean water RGB values across all color provider modules.

---

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8/10	Clear separation: blocks in world-core, biomes/decoration in world-worldgen, colors in their respective consumers
Open/Closed	9/10	Extends enums, registries, and decoration context without modifying core logic
Liskov Substitution	9/10	New biomes and blocks conform to existing type patterns and registry contracts
Interface Segregation	8/10	DecorateContext grows by one field, but all call sites are updated; no bloated interfaces created
Dependency Inversion	8/10	Reuses existing RHI and worldgen abstractions; no new hard dependencies introduced
Average	8.4

---

🎯 Final Assessment

Overall Confidence Score: 82%

Confidence Breakdown:

• Code Quality: 85% (Clean extensions, consistent patterns, good test coverage)
• Completeness: 75% (Missing tall_seagrass from issue scope; otherwise complete)
• Risk Level: 80% (Low risk of crashes, but timeout reports suggest possible perf regression)
• Test Coverage: 90% (Unit tests added for blocks and biomes; fingerprint test updated)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• Tests present and passing (unit tests confirmed by author)

Verdict:

MERGE WITH FIXES

The PR is safe to merge after addressing or explicitly descoping the missing tall_seagrass and investigating the smoke-test timeout to confirm it's not a new regression.

{
  "reviewed_sha": "9fed6a4cc9cfb37caab51b9bba27d4eb429a22ec",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 2,
  "overall_confidence_score": 82,
  "recommendation": "MERGE_WITH_FIXES"
}

opencode session  |  github run

[github-actions]

📋 Summary

Linked Issue: #626 - "Aquatic: Add underwater vegetation and coral foundation"

• Issue scope includes: seagrass, tall_seagrass, kelp, coral blocks, coral fans, placement/render support, warm ocean consumption
• All issue requirements are now fully addressed (tall_seagrass was added since the previous review)
• The PR extends block registry, biome registry, and decoration system with water-depth constrained placement

📌 Review Metadata

• Reviewed Commit SHA: 401b84a06b192c7bf6f04ce90acc40e25ba184de
• Reviewed PR: feat: add aquatic biome decorations #638
• Linked Issue: [Phase 2B] Aquatic: Add underwater vegetation and coral foundation #626 (fully addressed)

This PR adds aquatic blocks (seagrass, tall_seagrass, kelp, seaweed, coral_block, coral_fan), warm ocean and tropical biomes, and water-depth-aware decoration placement. Code quality is high with proper tests and consistent patterns.

---

🔴 Critical Issues (Must Fix - Blocks Merge)

✅ All previously reported critical issues have been resolved.

None identified.

---

⚠️ High Priority Issues (Should Fix)

✅ All previously reported high priority issues have been resolved.

None identified.

---

💡 Medium Priority Issues (Nice to Fix)

✅ [FIXED] Previous issue: Per-column water depth scanning performance regression - mayPlaceAquaticDecoration() now early-outs for non-aquatic biomes.

✅ [FIXED] Previous issue: Missing tall_seagrass from issue scope - now fully implemented with registry entries, render shape, decoration rules, and tests.

None identified.

---

ℹ️ Low Priority Suggestions (Optional)

[LOW] modules/world-worldgen/src/biome_decorator.zig:90-93 - Verify smoke-test/integration test performance after early-out fix
Confidence: Medium
Description: The PR author previously reported smoke-test timeouts. While mayPlaceAquaticDecoration() now early-outs for non-aquatic biomes (addressing the likely cause), the PR description still lists timeouts in the verification section.
Impact: If timeouts persist, there may be another performance regression.
Suggested Fix: Re-run nix develop --command zig build run -Dskip-present -Dsmoke-test and nix develop --command zig build run -Dskip-present -Dauto-world=normal -Dstartup-diagnostic-seconds=5 to confirm timeouts are resolved.

---

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8/10	Clear separation: blocks in world-core, biomes/decoration in world-worldgen, colors in consumers
Open/Closed	9/10	Extends enums, registries, and decoration context without modifying core logic
Liskov Substitution	9/10	New biomes and blocks conform to existing type patterns and registry contracts
Interface Segregation	8/10	DecorateContext grows by one field with all call sites updated
Dependency Inversion	8/10	Reuses existing RHI and worldgen abstractions; no new hard dependencies
Average	8.4

---

🎯 Final Assessment

Overall Confidence Score: 88%

Confidence Breakdown:

• Code Quality: 90% (Clean extensions, consistent patterns, good test coverage)
• Completeness: 90% (Issue [Phase 2B] Aquatic: Add underwater vegetation and coral foundation #626 fully addressed with tall_seagrass added)
• Risk Level: 85% (Low risk of crashes; performance concern addressed but unverified)
• Test Coverage: 90% (Unit tests added for blocks and biomes; all tests passing)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• Tests present and passing

Verdict:

MERGE WITH FIXES

The PR is safe to merge after the author verifies smoke-test performance is acceptable with the new early-out optimization. All substantive code issues from the previous review have been resolved.

{
  "reviewed_sha": "401b84a06b192c7bf6f04ce90acc40e25ba184de",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 88,
  "recommendation": "MERGE_WITH_FIXES"
}

opencode session  |  github run