diff --git a/crates/fmw-noise/src/cliffs/vulcanus_fields.rs b/crates/fmw-noise/src/cliffs/vulcanus_fields.rs index fa790fb2..b3567e6b 100644 --- a/crates/fmw-noise/src/cliffs/vulcanus_fields.rs +++ b/crates/fmw-noise/src/cliffs/vulcanus_fields.rs @@ -33,7 +33,6 @@ use crate::poison; use crate::quick_multioctave_noise::{ octave_terms, sum_octaves, QuickMultioctaveParams, QuickOctaves, }; -use crate::tiles::vulcanus_catalog::VulcanusTile; /// `cliff_elevation_0` from `planet_map_gen.vulcanus()`'s `cliff_settings`. pub const VULCANUS_CLIFF_ELEVATION_0: f64 = 70.0; @@ -183,18 +182,11 @@ impl CliffFields for VulcanusCliffFields<'_, '_> { /// The Vulcanus tiles whose `CollisionMask` shares a layer with the cliff's, so /// a cliff whose collision box touches one is never placed. /// -/// `tile_collision_masks.lava()` sets `water_tile = true` and the cliff mask -/// holds `water_tile`; no other Vulcanus tile does. Notably -/// `volcanic-jagged-ground` - the tile the ore patches paint, which the Lua -/// itself labels "CLIFF TILE" - is `tile_collision_masks.ground()`, which the -/// cliff mask does not touch, so ore does NOT exclude cliffs through this rule. -/// That distinction is why the earlier ore-separation work correctly found no -/// exclusion here while the removal rule in -/// [`super::vulcanus_ore_rejection`] exists. -/// -/// **Measured rather than deduced.** Switching lava and lava-hot out of the -/// tile autoplace category and regenerating is what established the set, not a -/// reading of `tile_collision_masks`. +/// **Which tiles those are is not a rule of its own.** It is +/// [`VulcanusTile::is_cliff_blocking`](crate::tiles::vulcanus_catalog::VulcanusTile::is_cliff_blocking), +/// which is also where the measurement behind the set is written down. This +/// used to inline `Lava | LavaHot` and restate that measurement in its own +/// words, one of four copies (#364). /// /// It resolves the tile through the ported argmax rather than reading back a /// rendered pixel, and that is load-bearing for tiled rendering: the collision @@ -215,7 +207,7 @@ impl TileCollision for VulcanusLavaTiles<'_, '_> { fn collides(&self, x: i64, y: i64) -> bool { #[allow(clippy::cast_precision_loss)] let tile = self.stack.tile(x as f64, y as f64); - matches!(tile, VulcanusTile::Lava | VulcanusTile::LavaHot) + tile.is_cliff_blocking() } } diff --git a/crates/fmw-noise/src/fixtures.rs b/crates/fmw-noise/src/fixtures.rs index 44d6645d..f624b47b 100644 --- a/crates/fmw-noise/src/fixtures.rs +++ b/crates/fmw-noise/src/fixtures.rs @@ -3629,8 +3629,18 @@ fn classifies_every_vulcanus_lava_tile_correctly() { let biomes = base.biomes_with_host_trig(); let stack = VulcanusStack::with_host_trig(&base, &biomes); - let is_lava = |t: VulcanusTile| matches!(t, VulcanusTile::Lava | VulcanusTile::LavaHot); - let want_lava = |name: &str| name == "lava" || name == "lava-hot"; + // Both sides go through `is_cliff_blocking`, which is the whole point of + // #364: these two lines used to say the same thing in two vocabularies - one + // by enum, one by string literal - and could drift apart without either file + // changing. The name side resolves through `VulcanusTile::from_name` rather + // than matching strings, so a fixture name the catalog does not know is a + // loud failure rather than a silent `false`. + let is_lava = VulcanusTile::is_cliff_blocking; + let want_lava = |name: &str| { + VulcanusTile::from_name(name) + .unwrap_or_else(|| panic!("fixture names a tile the catalog does not place: {name}")) + .is_cliff_blocking() + }; let positions = fixture.get("positions").as_array(); let want = fixture.get("tileNames").as_array(); diff --git a/crates/fmw-noise/src/tiles/vulcanus_catalog.rs b/crates/fmw-noise/src/tiles/vulcanus_catalog.rs index 97fcce83..39fb8438 100644 --- a/crates/fmw-noise/src/tiles/vulcanus_catalog.rs +++ b/crates/fmw-noise/src/tiles/vulcanus_catalog.rs @@ -136,6 +136,111 @@ impl VulcanusTile { Self::VolcanicAshSoil => [48, 48, 43], } } + + /// The tile whose prototype name is `name`, if Vulcanus places one. + /// + /// The inverse of [`Self::name`], resolved through [`TILE_ORDER`] so the two + /// cannot disagree. It exists because the oracle fixtures record tile + /// NAMES, not enum variants, and a caller holding a name used to have no way + /// to reach the enum except by matching strings of its own (#364). + #[must_use] + pub fn from_name(name: &str) -> Option { + TILE_ORDER.into_iter().find(|tile| tile.name() == name) + } + + /// Whether a cliff whose collision box touches this tile is refused. + /// + /// `tile_collision_masks.lava()` sets `water_tile = true` and the cliff mask + /// holds `water_tile`; no other Vulcanus tile does. Notably + /// `volcanic-jagged-ground` - the tile the ore patches paint, which the Lua + /// itself labels "CLIFF TILE" - is `tile_collision_masks.ground()`, which + /// the cliff mask does not touch, so ore does NOT exclude cliffs through + /// this rule. That distinction is why the earlier ore-separation work + /// correctly found no exclusion here while the removal rule in + /// [`crate::cliffs::vulcanus_ore_rejection`] exists. + /// + /// **Measured rather than deduced.** Switching lava and lava-hot out of the + /// tile autoplace category and regenerating is what established the set, not + /// a reading of `tile_collision_masks`. A future tile that blocks cliffs + /// would be found the same way. + /// + /// **This is the only definition on the Rust side, and that is the point of + /// it (#364).** There were four: here, `cliffs::vulcanus_fields`, and twice + /// in `fixtures.rs` - one of those two by name and its neighbour by enum, so + /// the string form and the enum form could drift apart without either file + /// changing. TypeScript keeps its own single definition, + /// `VULCANUS_CLIFF_BLOCKING_TILES` in `cliffCatalog.ts`, and + /// [`cliff_blocking_names_fnv1a64`] is what stops the two sides drifting. + /// **Two other gates pick the same two tiles and MUST NOT be folded into + /// this one.** `rocks::vulcanus_placement` and `resources::vulcanus_geyser` + /// each refuse lava as well, and all three sets are equal today - but they + /// are reached by three unrelated routes in the game's data, so the equality + /// is a coincidence rather than a shared rule: + /// + /// | gate | what actually decides it | + /// | --- | --- | + /// | cliffs, here | the cliff's collision mask holds `water_tile`, and `tile_collision_masks.lava()` sets it | + /// | rocks | the four rock prototypes' `vulcanus_tiles_cold` / `vulcanus_tiles_hot` autoplace lists, whose union is every tile but these | + /// | geysers | `type = "resource"`'s default mask is `{resource = true}`, which `tile_collision_masks.lava()` also lists | + /// + /// Change the tile data on any one of those three axes and the three sets + /// come apart. A single shared predicate would then be wrong in two places + /// at once, and silently, which is worse than the duplication #364 removed. + /// Each gate keeps its own definition and its own comment saying which + /// mechanism it reads. + #[must_use] + pub fn is_cliff_blocking(self) -> bool { + matches!(self, Self::Lava | Self::LavaHot) + } +} + +/// FNV-1a 64 over every cliff-blocking tile's name, sorted, joined by `\n`. +/// +/// The cross-language half of #364. TypeScript holds the same set as +/// `VULCANUS_CLIFF_BLOCKING_TILES`, and `fmw-wasm` exports this so a spec can +/// hash the TypeScript set through the module's own `fnv1a64` and compare the +/// two in process. A change to one side that misses the other then fails a +/// test instead of sitting there. +/// +/// **Sorted, so the two sides do not also have to agree on an order.** Catalog +/// order is ground truth for the argmax's tie-break, but it is not something +/// the TypeScript set carries - it is a `Set` of two strings - so hashing in +/// catalog order would make this assertion depend on a fact it is not trying +/// to check. +/// +/// Insertion sort over a fixed buffer rather than `Vec::sort`: the WASM build +/// has no allocator, and at 19 tiles the cost is irrelevant. +#[must_use] +pub fn cliff_blocking_names_fnv1a64() -> u64 { + let mut names = [""; TILE_ORDER.len()]; + let mut count = 0; + for tile in TILE_ORDER { + if tile.is_cliff_blocking() { + names[count] = tile.name(); + count += 1; + } + } + let names = &mut names[..count]; + for i in 1..names.len() { + let mut j = i; + while j > 0 && names[j - 1] > names[j] { + names.swap(j - 1, j); + j -= 1; + } + } + + // 19 names of at most 26 bytes, plus separators, fits with room to spare. + let mut buf = [0u8; 1024]; + let mut len = 0; + for (i, name) in names.iter().enumerate() { + if i > 0 { + buf[len] = b'\n'; + len += 1; + } + buf[len..len + name.len()].copy_from_slice(name.as_bytes()); + len += name.len(); + } + crate::checksum::fnv1a64(&buf[..len]) } /// `vulcanus_rock_noise` (`planet-vulcanus-map-gen.lua` ~line 872). @@ -523,4 +628,56 @@ mod tests { names.dedup(); assert_eq!(names.len(), before, "duplicate tile name"); } + + /// `from_name` is the exact inverse of `name` for every tile, and refuses + /// anything else. + /// + /// The refusal half is the one that matters: `fixtures.rs` resolves the + /// oracle's recorded tile names through this, and a mapping that answered + /// `Some` for a name Vulcanus does not place would make that grader read a + /// typo as a real tile. + #[test] + fn from_name_inverts_name_and_refuses_anything_else() { + for tile in TILE_ORDER { + assert_eq!(VulcanusTile::from_name(tile.name()), Some(tile)); + } + assert_eq!(VulcanusTile::from_name("lava-warm"), None); + assert_eq!(VulcanusTile::from_name("water"), None); + assert_eq!(VulcanusTile::from_name(""), None); + } + + /// The cliff-blocking set is exactly `lava` and `lava-hot`, frozen. + /// + /// Frozen on purpose. The set was established by measurement - switching + /// the two out of the tile autoplace category and regenerating - so a + /// future change to it is a new measurement, and this test is what makes + /// that change deliberate rather than incidental. Update the count and the + /// names together, and say in the commit what was regenerated. + #[test] + fn exactly_two_tiles_block_a_cliff_and_they_are_the_lava_pair() { + let blocking: Vec<&str> = TILE_ORDER + .iter() + .filter(|t| t.is_cliff_blocking()) + .map(|t| t.name()) + .collect(); + assert_eq!(blocking, vec!["lava", "lava-hot"]); + } + + /// The exported hash is over the SORTED names, so the TypeScript side can + /// reproduce it without knowing catalog order. + /// + /// Computed here the long way rather than restating a magic constant: a + /// hardcoded digest would still pass if `cliff_blocking_names_fnv1a64` + /// hashed the wrong thing and someone updated the constant to match. + #[test] + fn the_exported_hash_is_fnv1a64_of_the_sorted_names_joined_by_newlines() { + let mut names: Vec<&str> = TILE_ORDER + .iter() + .filter(|t| t.is_cliff_blocking()) + .map(|t| t.name()) + .collect(); + names.sort_unstable(); + let expected = crate::checksum::fnv1a64(names.join("\n").as_bytes()); + assert_eq!(cliff_blocking_names_fnv1a64(), expected); + } } diff --git a/crates/fmw-wasm/src/lib.rs b/crates/fmw-wasm/src/lib.rs index dbe7af5d..bdfe18ff 100644 --- a/crates/fmw-wasm/src/lib.rs +++ b/crates/fmw-wasm/src/lib.rs @@ -1058,6 +1058,24 @@ pub extern "C" fn vulcanus_field_count() -> u32 { VulcanusParity::FIELD_COUNT } +/// FNV-1a 64 over the sorted names of the tiles that block a Vulcanus cliff. +/// +/// The set is written on both sides of the port - here as +/// [`fmw_noise::tiles::vulcanus_catalog::VulcanusTile::is_cliff_blocking`], in +/// TypeScript as `VULCANUS_CLIFF_BLOCKING_TILES` - and before #364 nothing +/// asserted the two agreed. A spec hashes the TypeScript set through this +/// module's own [`fnv1a64`] and compares, so the two sides cannot drift. +/// +/// Names rather than a count, because a count cannot tell `lava` from +/// `volcanic-folds`. Sorted, so the sides need not also agree on an order. +/// +/// **Returns a `u64`, so the same signed-BigInt caveat as [`fnv1a64`] applies:** +/// the caller must apply `BigInt.asUintN(64, x)`. +#[unsafe(no_mangle)] +pub extern "C" fn vulcanus_cliff_blocking_names_fnv1a64() -> u64 { + fmw_noise::tiles::vulcanus_catalog::cliff_blocking_names_fnv1a64() +} + // --------------------------------------------------------------------------- // The render boundary (#223). See `abi.rs` for the request layout and // `render.rs` for what it does. diff --git a/src/noise/cliffs/cliffCatalog.ts b/src/noise/cliffs/cliffCatalog.ts index 4b779b8d..3b1cfd56 100644 --- a/src/noise/cliffs/cliffCatalog.ts +++ b/src/noise/cliffs/cliffCatalog.ts @@ -441,7 +441,13 @@ export function cliffCollisionTileBox( * rule while this one exists. * * Lives here rather than beside the Vulcanus renderer because that renderer is - * deleted by #227 and this rule is not. The Rust carries the same pair inlined - * at `crates/fmw-noise/src/cliffs/vulcanus_fields.rs:218`. + * deleted by #227 and this rule is not. + * + * **This is the only definition on the TypeScript side, and the Rust side's only + * definition is `VulcanusTile::is_cliff_blocking` in + * `crates/fmw-noise/src/tiles/vulcanus_catalog.rs`.** The two are held together + * by `test/wasmVulcanusParity.spec.ts`, which hashes this set and compares it + * against the module's `vulcanus_cliff_blocking_names_fnv1a64()`. Before #364 + * the set was written out four times with nothing checking the four agreed. */ export const VULCANUS_CLIFF_BLOCKING_TILES: ReadonlySet = new Set(["lava", "lava-hot"]); diff --git a/src/noise/wasm/engine.wasm b/src/noise/wasm/engine.wasm index a5cf8f59..276e7e5a 100755 Binary files a/src/noise/wasm/engine.wasm and b/src/noise/wasm/engine.wasm differ diff --git a/test/wasmVulcanusParity.spec.ts b/test/wasmVulcanusParity.spec.ts index ac716158..f96a669e 100644 --- a/test/wasmVulcanusParity.spec.ts +++ b/test/wasmVulcanusParity.spec.ts @@ -11,6 +11,7 @@ import { record, } from "./tier2Frozen"; +import { VULCANUS_CLIFF_BLOCKING_TILES } from "../src/noise/cliffs/cliffCatalog"; import { distanceFromNearestPoint } from "../src/noise/distanceFromNearestPoint"; import { type EvalCtxInput, withCtxDefaults } from "../src/noise/eval/ctx"; import { makeVulcanusTemperature } from "../src/noise/expressions/vulcanusElevation"; @@ -85,7 +86,9 @@ interface EngineExports { memory: WebAssembly.Memory; scratch_ptr: () => number; scratch_len: () => number; + fnv1a64: (len: number) => bigint; vulcanus_field_count: () => number; + vulcanus_cliff_blocking_names_fnv1a64: () => bigint; checksum_vulcanus: (requestLen: number, field: number) => bigint; } @@ -496,6 +499,63 @@ describe("Rust and TypeScript agree bit for bit across the Vulcanus field graph" expect(FIELD_NAMES).toHaveLength(engine.vulcanus_field_count()); }); + /** + * The two ports agree on WHICH tiles refuse a cliff, not merely how many + * (#364). + * + * The set is written once per side - `VULCANUS_CLIFF_BLOCKING_TILES` here, + * `VulcanusTile::is_cliff_blocking` in Rust - and before #364 it was written + * four times with nothing comparing them. This is what makes a change to one + * side that misses the other loud. + * + * **Names, not a count.** A count cannot tell `lava` from `volcanic-folds`, + * and swapping one blocking tile for another is exactly the drift worth + * catching. + * + * **Hashed through the module's own `fnv1a64` rather than reimplemented + * here.** A second FNV-1a written in TypeScript would be one more thing that + * can disagree, and its disagreement would look identical to the drift this + * is trying to find. + * + * Sorted before hashing, so the two sides do not also have to agree on an + * order - catalog order is ground truth for the argmax's tie-break, but a + * `Set` of two strings does not carry it. + */ + it("agrees with the module about which tiles block a Vulcanus cliff", async () => { + const engine = await instantiate(); + const joined = [...VULCANUS_CLIFF_BLOCKING_TILES].sort().join("\n"); + const bytes = new TextEncoder().encode(joined); + expect(bytes.length).toBeLessThanOrEqual(engine.scratch_len()); + new Uint8Array(engine.memory.buffer, engine.scratch_ptr(), bytes.length).set(bytes); + + expect(u64(engine.fnv1a64(bytes.length))).toBe( + u64(engine.vulcanus_cliff_blocking_names_fnv1a64()), + ); + }); + + /** + * The control on the test above: it can actually fail. + * + * Without this, a bug that made both sides hash the empty string - or made + * `fnv1a64` ignore its length - would leave the parity assertion green while + * comparing nothing. Same argument as `verify-rust.sh`'s anti-vacuity phase, + * at the scale of one test. + */ + it("would notice a set that gained, lost or swapped a tile", async () => { + const engine = await instantiate(); + const rust = u64(engine.vulcanus_cliff_blocking_names_fnv1a64()); + const hash = (names: string[]): bigint => { + const bytes = new TextEncoder().encode(names.sort().join("\n")); + new Uint8Array(engine.memory.buffer, engine.scratch_ptr(), bytes.length).set(bytes); + return u64(engine.fnv1a64(bytes.length)); + }; + const actual = [...VULCANUS_CLIFF_BLOCKING_TILES]; + + expect(hash([...actual, "volcanic-folds"])).not.toBe(rust); + expect(hash(actual.slice(1))).not.toBe(rust); + expect(hash(["lava", "volcanic-jagged-ground"])).not.toBe(rust); + }); + it("folds 676 grid points identically for every field, at two slider settings in two windows", async () => { const engine = await instantiate(); let compared = 0;