jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(simulation): address PR #161 review (VoxelColumn skeleton)

Browse Source 
Hoshe REQUEST_CHANGES + Tyre APPROVE:

- Cache key omitted world_seed → a cache reused across worlds could return
  another seed's column. CacheKey is now (world_seed, body_id_hash, voxel_pos).
  (Hoshe #1)
- Sub-chunk voxel seed reused SeedDomain::ChunkContext (tier-collision risk vs
  the region meander seed). Add SeedDomain::Voxel = 9 (append-only, pin test
  updated) and use it. (Hoshe #2, Tyre #1)
- get_or_derive forced callers to pre-compute body_id_hash while fnv1a_64 is
  pub(crate). Drop the param; compute the hash internally. (Tyre #2)
- Fix two inaccurate comments (micro-relief range [-4,+3] not ±2; scatter mask
  is 4-bit [0,15] not bits [4:6]). (Hoshe #3/#4)
- Strengthen the LRU eviction test: add a test-only contains_voxel() helper and
  assert WHICH entry was evicted (refreshed pos survives, LRU pos evicted). (Hoshe #5)
- LRU O(capacity)-scan perf flagged on T-1031 (Tyre #3, deferred to the budget harness).

cargo test passes, clippy -D warnings clean, fmt clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
jpmschweitzer
2026-06-08 14:48:07 +02:00
co-authored by Claude Opus 4.8
parent cd8bfec64b
commit 841e222192
2 changed files with 66 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/atlas/voxel.rs
+60 -26
View File
@@ -307,9 +307,11 @@ pub fn derive_voxel_column(
let voxel_pos: VoxelPos = (voxel_x, voxel_y);
// ── 2. Sub-chunk seed (for features with wavelength < 64 m) ───────────
// Keyed on integer voxel address after warp — deterministic (D-010).
// Keyed on integer voxel address after warp — deterministic (D-010). Uses the
// dedicated SeedDomain::Voxel (not ChunkContext) so the per-voxel stream can
// never collide with the region-scale meander seed (D-224 domain separation).
let sub_chunk_seed = SeedChain::for_body(world_seed, body_id)
.derive(SeedDomain::ChunkContext, voxel_pos_to_id(voxel_pos))
.derive(SeedDomain::Voxel, voxel_pos_to_id(voxel_pos))
.seed();
// ── 3. Family dispatch (D-239 §5) ─────────────────────────────────────
@@ -430,9 +432,10 @@ fn generate_alluvial_plain(
// ── Base elevation ──────────────────────────────────────────────────────
// Convert region `elev_q` (0–100) to metres. We scale 0–100 → 0–50 m here
// (walking-skeleton fidelity: a rough body-relative elevation).
// Sub-chunk micro-relief: small integer scatter ±2 m from the seed.
// Sub-chunk micro-relief: small integer scatter in [-4, +3] m from the seed
// (3 low bits, minus 4). The final elevation is clamped to >= 0 below.
let base_elev_m = region.elev_q / 2;
let micro_relief = (sub_chunk_seed & 0x7) as i32 - 4; // [-4, +3] but we clamp
let micro_relief = (sub_chunk_seed & 0x7) as i32 - 4; // 3 bits → [-4, +3]
let elevation_m = (base_elev_m + micro_relief).max(0);
// ── Channel (meander, D-239 §10) ───────────────────────────────────────
@@ -559,7 +562,8 @@ fn compute_channel_state(
///
/// All integer arithmetic (D-010).
fn scatter_vegetation(base: Vegetation, sub_chunk_seed: u64) -> Vegetation {
// Use bits [4:6] of the seed for scatter — independent from the channel
// Use bits [4:7] of the seed (4-bit mask, range [0, 15]) for scatter —
// independent from the channel
// computation which uses bits [0:3].
let scatter_bits = ((sub_chunk_seed >> 4) & 0xF) as u8;
@@ -621,7 +625,9 @@ fn voxel_pos_to_id(pos: VoxelPos) -> u64 {
///
/// The body_id is stored as a u64 hash (FNV-1a) to keep the key `Copy`
/// and `Ord`-able (D-010: `BTreeMap` key).
type CacheKey = (u64, VoxelPos);
/// `(world_seed, body_id_hash, post-warp voxel address)`. world_seed is part of
/// the voxel's identity — a cache must never return another world's column.
type CacheKey = (u64, u64, VoxelPos);
/// One cache entry: the derived column plus an access generation counter.
struct CacheEntry {
@@ -668,13 +674,12 @@ impl VoxelCache {
/// Cache miss: derives the column, inserts it (evicting LRU if full),
/// returns a clone.
///
/// The `body_id_hash` is the FNV-1a hash of the body identifier —
/// use [`crate::seed::fnv1a_64`] to compute it.
/// The body-id hash is computed internally via the canonical
/// [`crate::seed::fnv1a_64`] — callers pass the raw `body_id`.
pub fn get_or_derive(
&mut self,
world_seed: u64,
body_id: &str,
body_id_hash: u64,
region: &RegionProfile,
chunk: &ChunkContext,
tile_x: i32,
@@ -685,7 +690,11 @@ impl VoxelCache {
let (dx, dy) = domain_warp(world_seed, body_id, (tile_x, tile_y));
let voxel_x = (tile_x as f64 + dx) as i32;
let voxel_y = (tile_y as f64 + dy) as i32;
let key: CacheKey = (body_id_hash, (voxel_x, voxel_y));
let key: CacheKey = (
world_seed,
crate::seed::fnv1a_64(body_id),
(voxel_x, voxel_y),
);
self.gen += 1;
let current_gen = self.gen;
@@ -726,6 +735,22 @@ impl VoxelCache {
self.entries.is_empty()
}
/// Test helper: is the voxel for this `(world_seed, body_id, tile)` currently
/// cached? Recomputes the canonical post-warp key without mutating LRU state,
/// so tests can assert exactly which entry survived or was evicted.
#[cfg(test)]
pub fn contains_voxel(&self, world_seed: u64, body_id: &str, tile_x: i32, tile_y: i32) -> bool {
let (dx, dy) = domain_warp(world_seed, body_id, (tile_x, tile_y));
let voxel_x = (tile_x as f64 + dx) as i32;
let voxel_y = (tile_y as f64 + dy) as i32;
let key: CacheKey = (
world_seed,
crate::seed::fnv1a_64(body_id),
(voxel_x, voxel_y),
);
self.entries.contains_key(&key)
}
/// Evict the entry with the lowest `access_gen` (the LRU entry).
fn evict_lru(&mut self) {
if self.entries.is_empty() {
@@ -762,7 +787,6 @@ mod tests {
use crate::atlas::region_profile::{
GlaciationGrade, PrecipitationClass, TectonicClass, VegetationClass,
};
use crate::seed::fnv1a_64;
// -----------------------------------------------------------------------
// Test fixtures
@@ -927,10 +951,9 @@ mod tests {
let region = alluvial_region();
let chunk = alluvial_chunk(&region);
let body_id = "GJ1c";
let body_id_hash = fnv1a_64(body_id);
let mut cache = VoxelCache::new(64);
let cached = cache.get_or_derive(42, body_id, body_id_hash, &region, &chunk, 100, 100);
let cached = cache.get_or_derive(42, body_id, &region, &chunk, 100, 100);
let direct = derive_voxel_column(42, body_id, &region, &chunk, 100, 100);
assert_eq!(
cached, direct,
@@ -943,12 +966,11 @@ mod tests {
let region = alluvial_region();
let chunk = alluvial_chunk(&region);
let body_id = "GJ1c";
let body_id_hash = fnv1a_64(body_id);
let mut cache = VoxelCache::new(64);
let first = cache.get_or_derive(42, body_id, body_id_hash, &region, &chunk, 100, 100);
let first = cache.get_or_derive(42, body_id, &region, &chunk, 100, 100);
assert_eq!(cache.len(), 1, "first call must insert one entry");
let second = cache.get_or_derive(42, body_id, body_id_hash, &region, &chunk, 100, 100);
let second = cache.get_or_derive(42, body_id, &region, &chunk, 100, 100);
assert_eq!(
cache.len(),
1,
@@ -962,27 +984,40 @@ mod tests {
let region = alluvial_region();
let chunk = alluvial_chunk(&region);
let body_id = "GJ1c";
let body_id_hash = fnv1a_64(body_id);
let capacity = 8;
let mut cache = VoxelCache::new(capacity);
// Fill cache to capacity with distinct positions.
for i in 0..capacity as i32 {
cache.get_or_derive(42, body_id, body_id_hash, &region, &chunk, i * 64, 0);
cache.get_or_derive(42, body_id, &region, &chunk, i * 64, 0);
}
assert_eq!(cache.len(), capacity);
// Access position 0 to refresh it (not the LRU).
cache.get_or_derive(42, body_id, body_id_hash, &region, &chunk, 0, 0);
cache.get_or_derive(42, body_id, &region, &chunk, 0, 0);
// Insert a new entry — should evict the LRU (position 64, not 0).
cache.get_or_derive(42, body_id, body_id_hash, &region, &chunk, 999, 999);
// Insert a new entry — should evict the LRU (position 64, not the
// just-refreshed position 0).
cache.get_or_derive(42, body_id, &region, &chunk, 999, 999);
assert_eq!(
cache.len(),
capacity,
"cache size must stay at capacity after eviction"
);
// Verify the RIGHT entry was evicted, not just that some eviction happened.
assert!(
cache.contains_voxel(42, body_id, 0, 0),
"refreshed entry (pos 0) must survive eviction"
);
assert!(
!cache.contains_voxel(42, body_id, 64, 0),
"LRU entry (pos 64) must have been the one evicted"
);
assert!(
cache.contains_voxel(42, body_id, 999, 999),
"newly inserted entry must be present"
);
}
#[test]
@@ -991,20 +1026,19 @@ mod tests {
let region = alluvial_region();
let chunk = alluvial_chunk(&region);
let body_id = "GJ1c";
let body_id_hash = fnv1a_64(body_id);
let capacity = 2;
let mut cache = VoxelCache::new(capacity);
// Populate the cache fully.
let col0 = cache.get_or_derive(42, body_id, body_id_hash, &region, &chunk, 0, 0);
let _col1 = cache.get_or_derive(42, body_id, body_id_hash, &region, &chunk, 100, 0);
let col0 = cache.get_or_derive(42, body_id, &region, &chunk, 0, 0);
let _col1 = cache.get_or_derive(42, body_id, &region, &chunk, 100, 0);
// Insert a third entry — evicts (0,0) which is the LRU.
let _col2 = cache.get_or_derive(42, body_id, body_id_hash, &region, &chunk, 200, 0);
let _col2 = cache.get_or_derive(42, body_id, &region, &chunk, 200, 0);
// Re-derive position (0,0) — cache miss, must re-derive correctly.
let col0_again = cache.get_or_derive(42, body_id, body_id_hash, &region, &chunk, 0, 0);
let col0_again = cache.get_or_derive(42, body_id, &region, &chunk, 0, 0);
assert_eq!(
col0, col0_again,
"re-derived column after cache eviction must be identical"
server/src/seed.rs
+6
View File
@@ -100,6 +100,11 @@ pub enum SeedDomain {
/// ChunkContext derivation (64 m carrier, D-239 §1, T-1028).
/// Keyed by region-scale position id (see `atlas::chunk_context::pos_to_id`).
ChunkContext = 8,
/// Per-voxel sub-chunk derivation (1 m, D-239 §10, T-1028). Keyed by the
/// post-warp integer voxel-position id. Distinct from `ChunkContext` so the
/// voxel stream can never collide with the region-scale meander seed (D-224
/// domain separation).
Voxel = 9,
}
/// A position in the deterministic seed tree (D-224).
@@ -257,6 +262,7 @@ mod tests {
assert_eq!(SeedDomain::Npc as u64, 6);
assert_eq!(SeedDomain::DomainWarp as u64, 7);
assert_eq!(SeedDomain::ChunkContext as u64, 8);
assert_eq!(SeedDomain::Voxel as u64, 9);
}
#[test]