feat: generalize biome decoration rules

[MichaelFisher1997]

Summary

• Replaces hardcoded vegetation density fields with per-biome DecorationRule lists.
• Updates the standard decoration provider to evaluate biome-defined block rules with surface, height, variant, and vegetation multiplier checks.
• Migrates existing biome vegetation densities into explicit rules and refreshes deterministic worldgen fingerprints.

Verification

• nix develop --command zig fmt src/worldgen_tests.zig modules/world-worldgen/src/biome_registry.zig modules/world-worldgen/src/decoration_registry.zig modules/world-worldgen/src/decoration_types.zig
• nix develop --command zig build test
• nix develop --command zig build -Doptimize=ReleaseFast

Notes

• nix develop --command zig build run -Dskip-present -Dauto-world=normal -Dstartup-diagnostic-seconds=5 timed out after 60s in this environment after shader validation output, with no crash/error lines emitted.

Closes #624

[github-actions]

📋 Summary

This PR implements #624 ([Phase 1B] Worldgen: Generalize biome decoration profiles). It replaces the hardcoded (and largely unused) vegetation density fields in VegetationProfile with a flexible per-biome DecorationRule list. The standard decoration provider now evaluates biome-defined block rules with surface, height, variant, and vegetation multiplier checks. Existing biome configurations are migrated to explicit rules, and deterministic worldgen fingerprints are updated accordingly.

The PR fully satisfies the issue requirements:

• Biome definitions can express multiple decoration rules with block type, chance, placement constraints, and height/variant filters.
• The API is suitable for future biome expansion.
• nix develop --command zig build test passes.

📌 Review Metadata

• Reviewed Commit SHA: 28c8e54943228cc92dabaf3949ea38d44d76a0d3
• Reviewed PR: feat: generalize biome decoration rules #635
• Linked Issues: Closes [Phase 1B] Worldgen: Generalize biome decoration profiles #624

This is a well-scoped, data-driven refactor that improves extensibility without touching rendering, threading, or memory management. Code quality is good, tests pass, and formatting is clean.

🔴 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/decoration_registry.zig:189 - Missing Y bounds check before setBlock in biome decoration placement
Confidence: Medium
Description: The new biome rule placement code computes place_y = surface_y + 1 and passes it directly to chunk.setBlock() without verifying it is within chunk Y bounds (0..CHUNK_SIZE_Y). When surface_y = 255, place_y = 256 exceeds the chunk height. setBlock calls getIndex which uses std.debug.assert(y < CHUNK_SIZE_Y) — this will panic in debug/test builds and produce undefined behavior in ReleaseFast.
Impact: Rare but possible crash/UB when decorating columns at the world height limit.
Suggested Fix: Add a bounds guard before the setBlock call (and import CHUNK_SIZE_Y):

const place_y = surface_y + 1;
if (place_y < 0 or place_y >= world_core.CHUNK_SIZE_Y) continue;
if (chunk.getBlockSafe(@intCast(local_x), place_y, @intCast(local_z)) != .air) continue;
chunk.setBlock(local_x, @intCast(place_y), local_z, rule.block);

ℹ️ Low Priority Suggestions (Optional)

None identified.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	9	DecorationRule holds only placement data; provider handles evaluation logic.
Open/Closed	9	New decoration types and biomes can be added without modifying provider code.
Liskov Substitution	8	DecorationRule conforms to the same placement patterns as SimpleDecoration.
Interface Segregation	9	Focused structs (DecorationRule, VegetationProfile) with minimal, relevant fields.
Dependency Inversion	8	Biome registry depends on types, not provider; provider uses vtable abstraction.
Average	8.6

🎯 Final Assessment

Overall Confidence Score: 85%

Confidence Breakdown:

• Code Quality: 90% (clean, idiomatic Zig; well-structured)
• Completeness: 90% (fully addresses [Phase 1B] Worldgen: Generalize biome decoration profiles #624 requirements)
• Risk Level: 75% (worldgen changes always carry some determinism risk, but fingerprints are updated)
• Test Coverage: 80% (unit tests and shader validation pass; no new tests added for the rule evaluation logic)

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 well-designed and ready to merge after addressing the medium-priority Y bounds check in the new decoration placement path.

{
  "reviewed_sha": "28c8e54943228cc92dabaf3949ea38d44d76a0d3",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 1,
  "overall_confidence_score": 85,
  "recommendation": "MERGE WITH FIXES"
}

opencode session  |  github run

[github-actions]

📋 Summary

This PR implements #624 ([Phase 1B] Worldgen: Generalize biome decoration profiles). It replaces hardcoded vegetation density fields in VegetationProfile with a flexible per-biome DecorationRule list. The standard decoration provider evaluates biome-defined block rules with surface, height, variant, and vegetation multiplier checks. Existing biome configurations are migrated to explicit rules, and deterministic worldgen fingerprints are updated accordingly.

The PR fully satisfies the linked issue requirements. It is a well-scoped, data-driven refactor that improves extensibility without touching rendering, threading, or memory management.

📌 Review Metadata

• Reviewed Commit SHA: 02fdcaf044f5a4b95a34a436cac9c22186e28e9a
• Reviewed PR: feat: generalize biome decoration rules #635
• Linked Issues: Closes [Phase 1B] Worldgen: Generalize biome decoration profiles #624

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

✅ [FIXED] Previous issue: Missing Y bounds check before setBlock in biome decoration placement. The second commit (02fdcaf044f5a4b95a34a436cac9c22186e28e9a) correctly added bounds guards for both the static decoration path and the new biome-defined decoration rules path.

None identified.

ℹ️ Low Priority Suggestions (Optional)

None identified.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	9	DecorationRule holds only placement data; provider handles evaluation logic.
Open/Closed	9	New decoration types and biomes can be added without modifying provider code.
Liskov Substitution	8	DecorationRule conforms to the same placement patterns as SimpleDecoration.
Interface Segregation	9	Focused structs (DecorationRule, VegetationProfile) with minimal, relevant fields.
Dependency Inversion	8	Biome registry depends on types, not provider; provider uses vtable abstraction.
Average	8.6

🎯 Final Assessment

Overall Confidence Score: 90%

Confidence Breakdown:

• Code Quality: 90% (clean, idiomatic Zig; well-structured)
• Completeness: 95% (fully addresses [Phase 1B] Worldgen: Generalize biome decoration profiles #624 requirements)
• Risk Level: 85% (worldgen changes carry determinism risk, but fingerprints are updated and bounds checks are now in place)
• Test Coverage: 90% (unit tests and shader validation pass; fingerprints updated for deterministic verification)

Merge Readiness:

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

Verdict:

MERGE

The previous medium-priority Y bounds issue has been resolved. The PR is well-designed, focused, and ready to merge.

{
  "reviewed_sha": "02fdcaf044f5a4b95a34a436cac9c22186e28e9a",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 90,
  "recommendation": "MERGE"
}

opencode session  |  github run