Skip to content

fix: align LOD biome selection with registry scoring#673

Merged
github-actions[bot] merged 1 commit into
devfrom
bug/657-lod-biome-consistency
May 2, 2026
Merged

fix: align LOD biome selection with registry scoring#673
github-actions[bot] merged 1 commit into
devfrom
bug/657-lod-biome-consistency

Conversation

@MichaelFisher1997
Copy link
Copy Markdown
Collaborator

Summary

  • Route simplified LOD biome selection through the registry scoring path so it uses elevation, continentalness, ruggedness, and ridge mask like LOD0 where structural filters are unavailable.
  • Add selector tests for LOD-compatible consistency and non-heat/humidity terrain signals.
  • Update the BiomeSource simplified selector fixture to use a valid deep-ocean elevation.

Verification

  • nix develop --command zig fmt modules/world-worldgen/src/biome_selector.zig modules/world-worldgen/src/biome_selector_tests.zig src/worldgen_tests.zig
  • nix develop --command zig build test -- --test-filter selectBiomeSimple
  • nix develop --command zig build test

Fixes #657

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label May 2, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 2, 2026

📋 Summary

This PR addresses #657 (Worldgen Phase 2: keep LOD biome selection consistent with LOD0). It replaces the hardcoded, simplified selectBiomeSimple logic with a delegation to the full registry scoring path (selectBiome), ensuring LOD2+ terrain uses the same climate, elevation, continentalness, ruggedness, and ridge signals as LOD0. It also adds targeted tests for LOD-compatible consistency and updates an existing test fixture to use valid deep-ocean elevation parameters. All tests pass, code is properly formatted, and the implementation fully satisfies the issue requirements.

📌 Review Metadata


🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.


⚠️ High Priority Issues (Should Fix)

None identified.


💡 Medium Priority Issues (Nice to Fix)

None identified.


ℹ️ Low Priority Suggestions (Optional)

[LOW] src/worldgen_tests.zig:984expectEqual argument order inconsistency
Confidence: High
Description: The PR fixed the expectEqual argument order on line 995 (expectEqual(expected, actual)), but the adjacent assertion on line 984 (try testing.expectEqual(result1, BiomeId.desert)) still uses the reversed (actual, expected) convention.
Impact: Confusing error messages if the test ever fails; minor style inconsistency in the same test block.
Suggested Fix: Swap to try testing.expectEqual(BiomeId.desert, result1); for consistency.


📊 SOLID Principles Score

Principle Score Notes
Single Responsibility 9 Eliminates duplicated biome selection logic; LOD now delegates to the single climate-scoring source of truth
Open/Closed 9 New biomes added to the registry automatically work in LOD without modifying selectBiomeSimple
Liskov Substitution 8 Not heavily applicable (no inheritance), but function contract is preserved
Interface Segregation 8 Signature unchanged; consumers see no API breakage
Dependency Inversion 9 Now depends on registry abstraction instead of hardcoded constants
Average 8.6

🎯 Final Assessment

Overall Confidence Score: 95%

  • Code Quality: 95% (clean deletion of 43-line duplicate, correct delegation, well-documented)
  • Completeness: 95% (fulfills issue Worldgen Phase 2: keep LOD biome selection consistent with LOD0 #657 requirements, adds appropriate tests, fixes fixture)
  • Risk Level: 5% (behavioral change is intentional and verified; no memory/threading/GPU concerns)
  • Test Coverage: 90% (good targeted tests; could add more boundary cases but not required)

Merge Readiness:

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

Verdict:

MERGE

Clean, well-tested fix that eliminates duplicated logic and aligns LOD biome selection with the full registry path as intended.

{
  "reviewed_sha": "e79d96708f2a555759b7f106c3bb9f613efe14d0",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 95,
  "recommendation": "MERGE"
}

New%20session%20-%202026-05-02T22%3A23%3A01.742Z
opencode session  |  github run

@github-actions github-actions Bot merged commit f277d42 into dev May 2, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Worldgen Phase 2: keep LOD biome selection consistent with LOD0

1 participant