Skip to content

feat(worldgen): wire EssencePalette into WorldGenerator interface#90

Merged
mannie-exe merged 2 commits into
devfrom
feat/worldgen-essence-palette
Mar 24, 2026
Merged

feat(worldgen): wire EssencePalette into WorldGenerator interface#90
mannie-exe merged 2 commits into
devfrom
feat/worldgen-essence-palette

Conversation

@mannie-exe
Copy link
Copy Markdown
Contributor

Summary

  • Add EssencePalette* parameter to WorldGenerator::generate() and generateToBuffer() across all 5 subclasses and 6 production call sites
  • Implement classifyEssence() in MinecraftNoiseGenerator to map terrain features (warmth, wetness, ruggedness, coastalness, sediment) plus spatial hash jitter to Vec4 OCLD values clamped to [0,1]
  • Populate palette inline during generateToBuffer() when palette is non-null via palette->quantize8()

Wave 4 of the D2+D3 combined implementation plan. The essenceIdx == materialId invariant is preserved; existing assignEssence() post-pass calls remain for backward compatibility. The invariant break is deferred to a follow-up PR.

Changes

Interface (W4-A): generate() and generateToBuffer() gain EssencePalette* palette = nullptr. Default parameter on declarations only. All subclasses (MinecraftNoise, NaturalWorld, Flat, Layered, SingleMaterial, TestWorld) updated. NaturalWorldGenerator passes palette through to its inner noise generator.

classifyEssence (W4-B): Private const method on MinecraftNoiseGenerator. Maps 6 base materials (STONE, DIRT, SAND, WATER, GRAVEL, AIR) to OCLD base vectors, modulates with ColumnSample terrain features, adds spatial hash jitter, clamps all components to [0,1].

Inline essence (W4-C): MinecraftNoiseGenerator::generateToBuffer() calls classifyEssence() + palette->quantize8() for non-air cells when palette is non-null. Air cells are skipped. Null palette preserves existing behavior.

Call site updates: VoxelSimulationSystem (3 sites) passes chunk palette. WorldSession (2 sites) and ReplayExecutor (1 site) create local EssencePalette refPalette before generateToBuffer().

Test plan

  • mise run build compiles clean
  • mise run test passes 2356 tests (2353 + 3 skipped)
  • mise run lint:changed shows only pre-existing warnings
  • 6 new tests: palette population, null safety, spatial variation, OCLD clamping, multi-material diversity, cross-generator safety
  • Verify no visual regression in-game (palette population does not affect rendering until invariant break in follow-up)

Add EssencePalette* parameter to generate() and generateToBuffer()
across the WorldGenerator hierarchy (5 subclasses, 6 production call
sites). Implement classifyEssence() in MinecraftNoiseGenerator to map
terrain features (warmth, wetness, ruggedness, coastalness, sediment)
plus spatial hash jitter to Vec4 OCLD values clamped to [0,1].

When palette is non-null, MinecraftNoiseGenerator populates essence
inline during generation via palette->quantize8(). Existing
assignEssence() post-pass calls are preserved for backward
compatibility; the essenceIdx == materialId invariant is not broken
in this change.

6 new tests: palette population, null safety, spatial variation,
OCLD clamping, multi-material essence diversity, cross-generator
safety. 2356 tests pass (2353 + 3 skipped).

Wave 4 of the D2+D3 combined implementation plan.
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Mar 24, 2026

Greptile Summary

This PR (Wave 4 of the D2+D3 plan) wires EssencePalette into the WorldGenerator interface, implements terrain-feature-driven OCLD essence classification in MinecraftNoiseGenerator, and updates all 5 subclasses and 6 production call sites. The previously flagged signed-integer overflow UB in the spatial hash is correctly resolved by casting coordinates to uint32_t before multiplying.

Key changes:

  • generate() and generateToBuffer() gain EssencePalette* palette = nullptr; generators that don't yet implement inline essence simply ignore the parameter and the paletteSize() == 0 guard at each call site falls back to the existing assignEssence() post-pass, preserving backward compatibility
  • classifyEssence() maps 6 base materials to OCLD Vec4 values, modulates with column terrain features (warmth, wetness, ruggedness, coastalness, sediment), and adds a per-cell spatial hash jitter — all components are clamped to [0, 1]
  • Six new unit tests cover palette population, null-safety, OCLD clamping, spatial variation, multi-material diversity, and cross-generator safety
  • One P2 note: quantize8() silently truncates the returned uint16_t palette index to uint8_t; the bounds assertion in ClassifyEssenceClampedToUnitRange is trivially satisfied for any palette larger than 255 entries and doesn't catch the resulting silent mismatch — an assertion inside quantize8() itself would close this gap

Confidence Score: 4/5

  • Safe to merge; the one remaining non-blocking concern is a latent silent-truncation in quantize8() that is unlikely to trigger with current parameters but lacks an assertion.
  • The PR is well-structured and the previous critical UB (signed overflow in the spatial hash) is correctly resolved. All 6 call sites are updated consistently and the paletteSize() == 0 backward-compatibility guard is logically sound. The only open item is the quantize8() silent truncation: the uint8_t VoxelCell field limits the palette to 255 reachable entries, but neither quantize8() nor the generateToBuffer() loop asserts this. In practice, with epsilon=0.01 and per-chunk usage, the palette stays well under 255 entries; the risk is real but low. A debug assertion inside quantize8() would make this production-safe without blocking the merge.
  • src/recurse/world/MinecraftNoiseGenerator.cc and tests/unit/world/MinecraftNoiseGeneratorTest.cc warrant a second look around the quantize8 truncation and the ClassifyEssenceClampedToUnitRange bounds assertion.

Important Files Changed

Filename Overview
src/recurse/world/MinecraftNoiseGenerator.cc Core Wave 4 implementation: adds classifyEssence() and inline palette population in generateToBuffer(). The previous signed-integer-overflow UB in the spatial hash is correctly fixed by casting to uint32_t before multiplying. The AIR early-return in classifyEssence() is dead code (the outer loop already continues on AIR) but harmless. One latent concern: quantize8() silently truncates uint16_t indices to uint8_t; if the palette ever exceeds 255 entries mid-generation, previously written cells can silently reference the wrong essence value.
include/recurse/world/WorldGenerator.hh Interface updated cleanly: EssencePalette forward-declared, generate() and generateToBuffer() gain EssencePalette* palette = nullptr. Default parameter on the base-class declaration only (correct); subclass definitions omit the default (also correct).
src/recurse/persistence/WorldSession.cc Both encode and async-load paths updated: refPalette created before generateToBuffer(), assignEssence fallback guarded by paletteSize() == 0. Logic is correct for new generators. For delta-encoded saves produced before this PR (where essenceIdx was materialId), the reference buffer now carries classifyEssence-based indices instead, which could shift the delta for existing saves — acknowledged by the author as an invariant to be broken in a follow-up.
src/recurse/persistence/ReplayExecutor.cc Same pattern as WorldSession: refPalette created before generateToBuffer(), assignEssence fallback guarded by paletteSize() == 0. Change is mechanical and correct.
src/recurse/systems/VoxelSimulationSystem.cc All three call sites (generateInitialWorld, generateChunk, generateChunksBatch) updated to pass the chunk palette and guard assignEssence with paletteSize() == 0. Pattern is consistent and correct.
tests/unit/world/MinecraftNoiseGeneratorTest.cc Six new tests provide good coverage: null-palette backward compatibility, palette population, OCLD clamping, spatial variation, multi-material diversity, and cross-generator safety. One test (ClassifyEssenceClampedToUnitRange) has a bounds assertion that is trivially satisfied when palette.paletteSize() > 255, providing less protection than the comment implies. The EssenceVariesSpatiallyForSameMaterial test shares one palette across two chunks, which is fine but could mask index-truncation issues if the combined entries exceed 255.
tests/unit/world/GeneratorTest.cc Added FlatGeneratorAcceptsPaletteWithoutCrash test; verifies that generators which ignore the palette parameter still produce correct material data and don't crash.

Sequence Diagram

sequenceDiagram
    participant Caller as VoxelSimulationSystem / WorldSession / ReplayExecutor
    participant WG as WorldGenerator (interface)
    participant MNG as MinecraftNoiseGenerator
    participant EP as EssencePalette
    participant AE as assignEssence (fallback)

    Caller->>WG: generateToBuffer(buffer, cx, cy, cz, &palette)
    WG->>MNG: generateToBuffer(buffer, cx, cy, cz, palette)
    loop for each non-AIR voxel
        MNG->>MNG: classifyMaterial(column, wy)
        alt palette != nullptr
            MNG->>MNG: classifyEssence(column, materialId, wx, wy, wz)
            MNG->>EP: quantize8(essenceVec4)
            EP-->>MNG: uint8_t index
            MNG->>MNG: cell.essenceIdx = index
        end
        MNG->>MNG: buffer[idx] = cell
    end
    MNG-->>Caller: (buffer populated)
    alt palette->paletteSize() == 0 (generator ignored palette)
        Caller->>AE: assignEssence(buffer, cx, cy, cz, materials, palette, 42)
        AE-->>Caller: (essenceIdx = materialId fallback)
    end
Loading

Reviews (2): Last reviewed commit: "fix(worldgen): address PR #90 review fin..." | Re-trigger Greptile

Comment thread src/recurse/world/MinecraftNoiseGenerator.cc Outdated
Comment thread src/recurse/systems/VoxelSimulationSystem.cc
- Cast spatial hash operands to uint32_t before multiplying to avoid
  signed-integer overflow UB in classifyEssence (R90-1)
- Guard assignEssence() calls with paletteSize() == 0 so generators
  that populate the palette inline (MinecraftNoiseGenerator) skip the
  post-pass that would read corrupted essenceIdx values (R90-2)
- Simplify tautological assertion in EssenceVariesSpatially test to
  EXPECT_NE on the two sets directly (R90-3)
Comment thread src/recurse/persistence/ReplayExecutor.cc
@mannie-exe mannie-exe merged commit 664d8e6 into dev Mar 24, 2026
6 of 7 checks passed
@mannie-exe mannie-exe deleted the feat/worldgen-essence-palette branch March 24, 2026 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant