fix(cliffs): sample the fields at the game's lattice - Nauvis is now EXACT (#18) - #70
Merged
Merged
Conversation
…EXACT (#18) The port read `cliff_elevation` and `cliffiness` at `(i*4, j*4 + 0.5)`, adding the cliff prototype's `grid_offset {0, 0.5}` to the SAMPLE position. That offset is a CENTRE offset. The game samples the bare `(i*4, j*4)`. Two independent sources: - `CliffGenerator::crossingsForChunk` (arm64 `0x10160c9cc`) reads `grid_size` at `[proto+0xb60]`/`[0xb68]` and NEVER `grid_offset` at `[0xb70]`/`[0xb78]`; its sample origin is `chunkPos << 5` converted to float, step `grid_size`. It computes no other coordinate. - `base/prototypes/entity/entity-util.lua:305` says it outright: "cliffs are auto-placed with centers at (0, 0.5) offset from the grid". Nauvis goes 0.943 -> **1.0000 recall, 1.0000 precision, ratio 1.000** at both seeds - exact agreement with `find_entities_filtered`. Vulcanus recall 0.792/0.870/0.803 -> 0.806/0.938/0.853; its over-placement is unchanged and stays open on #18. Why it hid for two months: the error moves NO placed cliff, because cell centres are derived from their own constants. So `x mod 4 == 2`, `y mod 4 == 2.5`, the preview agreement, and PR #57's substitution of the game's own field values (zero cells moved) all passed - that fixture had itself been captured at the port's assumed site. A substitution test can falsify a value; it cannot falsify the site it was sampled at. Fixture handling: the Vulcanus corner-fields oracle is re-captured at the game's lattice; the previous capture is kept as `-legacy-y0.5`, since it is valid ground truth for the site it names and is what lets a spec show the blind spot. Both carry provenance. Test changes are re-measurements, not accommodations: - `cliffPlacement.spec.ts` now pins Nauvis at EQUALITY, not >= 0.85. - `cliffResidual.spec.ts`'s "wrong cells sit on band boundaries" block asserted the residual exists; there are no wrong cells left, so it now asserts the set is empty. - `cliffSmoothing.spec.ts`'s plane gains a `.37` offset: at `j*4` the old constant put corner elevations exactly ON band edges (measured: all 7 disagreeing cells at distance 0, bilerp giving 7.1e-15), so it tested float rounding rather than smoothing. - `vulcanusCliffs.spec.ts`'s independent blend now indexes corners instead of deriving positions from centres. - `vulcanusOreCliffSeparation.spec.ts`: full-ore false positives 47 -> 46. The game still places zero; that finding is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BkKb3S2cGyRFFHSzuCQBgj
This was referenced Jul 30, 2026
wormeyman
added a commit
that referenced
this pull request
Jul 30, 2026
…#71) Reverse-engineering only - no behaviour change. These are the tables the game's cliff collision rejection needs, landed on their own so the RE is reviewable before the rejection itself is wired in. The finding they came from: `EntityMapGenerationTask::tryToAddCliff` (`0x101625038`) does not just record a cell. It switches on the `CliffOrientation`, loads that orientation's `collision_bounding_box` from `proto + 0x5c0 + id*0x48`, and calls `EntityMapGenerationTask::wouldCollide` (`0x101625468`) with the prototype's collision mask. On a hit the cliff is never added. This port runs no such check. `wouldCollide` floors the box to tiles with `(box + position) >> 8` and scans the inclusive tile rectangle against a 96x96 per-tile mask grid. It reads tiles only - `tryToAddCliff` never writes that grid - so the rejection is order-independent. On Vulcanus the only tiles carrying a layer the cliff mask holds (`water_tile`) are `lava` and `lava-hot`. Measured against `oracle-vulcanus-cliff-entities.seed123456`, applying the rule with these boxes moves region `[1500,1500]` from ratio 1.203 / precision 0.779 to ratio 1.003 / precision 0.930, rejecting 173 false positives and 4 true ones. Regions `[0,0]` and `[-1200,800]` barely move, so this is most of one region's over-placement and not the whole residual. The control arm - the same lava field sampled 10,000 tiles away - rejects 111 TP/40 FP and 361 TP/82 FP, i.e. indiscriminately, which is what makes the real arm non-vacuous. New: - `CLIFF_CODE_TO_ORIENTATION`. The orientation was recoverable all along and simply never read: `toMaybeCliffOrientation` returns one 64-bit word whose low 32 bits are the tri-state `CLIFF_PLACING_CODES` was extracted from, and whose high 32 bits are the id. - `CLIFF_ORIENTATION_NAMES`, from `CliffOrientationName::buildMapping`'s string table. - `CLIFF_ORIENTATION_COLLISION_BOX`. `rotbb`'s 45-degree rectangle has an AABB of exactly `[x, x+size] x [y, y+size]` whatever `intersect` was, which is why the shipped helper drops that argument. `test/cliffOrientation.spec.ts` asserts what the transcription has to satisfy rather than restating it: the bijection against the separately extracted `isCliffPlaced` table, a from-the-geometry derivation of every orientation name out of its code's edge crossings, and a re-derivation of the `rotbb` boxes from the full rotated rectangle. Six planted single-value errors were each confirmed to fail it; an earlier, weaker version missed two of them. Notes corrections while here, all of which were actively misleading: - The lattice section still gave the corner as `j*4 + 0.5` - the bug #70 fixed - under a heading claiming "confirmed 100% exact", with its correction 160 lines below. - Six symbol addresses had moved by 2.1.12, and `fixImpossibleCells` was attributed to the wrong class. - Added the extract-then-decompile recipe that actually worked, including the two traps: `llvm-objdump` silently ignores both `--start-address` and `--disassemble-symbols` on the universal binary, and Ghidra 12 reports a duplicate script as a missing source bundle. Claude-Session: https://claude.ai/code/session_012cd6B2MKoJF5W2T175BDoe Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
wormeyman
added a commit
that referenced
this pull request
Aug 2, 2026
) (#83) vulcanus-multisample-NOTES proved multisample(e,dx,dy) == e(x+dx, y+dy) at 150/150 comparisons with residual exactly zero. That is correct - for LuaSurface.calculate_tile_properties, whose noise program has a 1-TILE grid. It was never checked in any other channel, and the primitive's own docs say it evaluates "in a separate noise program with a larger grid" whose "sub-grids are copied to the main program". That phrase is load-bearing. Asked through the CLIFF GENERATOR, whose grid is the 4-tile corner lattice, by routing a probe onto cliff_elevation with the rule collapsed so cliffs mark exactly where the field crosses 71: x column 70 baseline multisample(x, 0, 0) column 70 identity multisample(x, 4, 0) column 54 shifted 16 TILES, not 4 multisample(x, 0, 4) column 70 null control dx = 4 moves the field by 4 x the 4-tile grid step. So the true rule is e(x + dx*G, y + dy*G) for the CALLING program's grid G. Consequence: vulcanus_basalt_lakes_multisample's min over {0,1}x{0,1} spans 4 tiles for cliffs and 1 tile for every per-tile consumer. min is erosion, so the cliff channel's elevation is much smoother. The port used the 1-tile field for both, making the cliff elevation too rough and over-placing by ~40%. before after wrong orientation 175 = 12.5% 37 = 2.4% recall 0.806/0.938/0.853 1.000/0.973/0.965 [0,0] worst region 29.8% wrong 2.5%, recall 1.000 level sweep ratio 1.20-1.49 below 120 1.00-1.09 at EVERY level VulcanusElevation now exposes cliffElevation beside elevation. Both hang off one stack and share every sub-expression below the multisample, so the cost is a second memo table - a private DAG for cliffs was tried first and is much dearer. Do not collapse them back together: they are different fields, not a cache miss. Nothing here refutes the multisample port or the per-tile consumers. calculate_tile_properties and the tile renderer both live in the 1-tile channel where e(x+dx, y+dy) is exactly right. Specs that pinned the defect are inverted rather than deleted, because the inversions are the evidence: the s=0 smoothing arm is now exact, all four collapsed arms are exact, the level sweep matches at every level, and the corner-fields substitutions now MOVE cells - those fixtures are the tile channel, so they are the right numbers for the wrong consumer, and their agreeing for months is how the wrong channel stayed invisible. cliffResidual's Vulcanus block is retired: its mismatched population is now too small to compare. The lesson is the third form of the same trap. Not a fixture captured at the wrong SITE (#70's grid_offset), not a wrong value - a fixture captured through the wrong CHANNEL, agreeing with a port that made the same mistake. Ask which code path CONSUMES a value, not only which coordinates it is sampled at. And note that no sweep inside the port could have found this: the smoothing, band and rule sweeps all searched a family that shared the defect. Claude-Session: https://claude.ai/code/session_01GRx1CU29RKLsRuUAErEDRB Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The port sampled
cliff_elevationandcliffinessat(i*4, j*4 + 0.5), adding the cliff prototype'sgrid_offset {0, 0.5}to the sample position. That offset is a centre offset. The game samples the bare(i*4, j*4).Result
[0,0][1500,1500][-1200,800]Nauvis is exact - 282/282 and 52/52 against
find_entities_filtered, nothing missed and nothing invented. That is the ~6% residual which has been open since 2026-07-20 and had five separate causes proposed and falsified for it (fixImpossibleCells,wouldCollide, field precision, boundary proximity, the smoothing knot model).Vulcanus improves on recall in all three regions but its over-placement is unchanged - 1.18x/1.20x/0.94x - so #18 stays open, now scoped to that rather than to recall.
Evidence
Two independent sources, neither of them our own tests:
CliffGenerator::crossingsForChunk(arm640x10160c9cc) readsgrid_sizeat[proto+0xb60]/[0xb68]and nevergrid_offsetat[0xb70]/[0xb78]. Sample origin ischunkPos << 5converted to float; step isgrid_size. It computes no other coordinate - it consumesNoiseCacheregisters.base/prototypes/entity/entity-util.lua:305: "cliffs are auto-placed with centers at (0, 0.5) offset from the grid".Then confirmed by capture: 12,675 corners re-sampled at the game's lattice, seed 123456, Factorio 2.1.12 with
refs:sync --checkin sync, scored against the game's 885 realcliff-vulcanusentities. A control arm using the old fixture reproduced the shipped numbers exactly, which is what makes the comparison readable rather than a harness artefact.Why it hid for two months
The error moves no placed cliff. Cell centres are derived from their own constants, so
x mod 4 == 2andy mod 4 == 2.5held, the preview-agreement check held, and PR #57's substitution of the game's own field values moved zero cells - because that fixture had itself been captured at the port's assumed site (capture.tshardcoded the same 0.5).A substitution test can falsify a value. It cannot falsify the site it was sampled at. Both "our values are right" and "our sample site is wrong" produce zero flips.
Fixtures
The Vulcanus corner-fields oracle is re-captured at the game's lattice. The previous capture is kept as
-legacy-y0.5: it is valid ground truth for the site it names, and keeping it is what lets a future spec demonstrate the blind spot rather than just describe it. Both carry provenance entries.Test changes are re-measurements, not accommodations
cliffPlacement.spec.tsnow pins Nauvis at equality rather than>= 0.85. Placement is deterministic given the seed, and slack is how a 0.943 sat unexplained for two months.cliffResidual.spec.ts's "the wrong cells sit on band boundaries" block asserted the residual exists. There are no wrong cells left, so it now asserts the set is empty (and still partitions on the same distance function, so an empty set cannot mean a loop that never ran).cliffSmoothing.spec.ts's plane gains a.37offset. Atj*4the old constant made corner elevations land exactly on band edges - measured: all 7 disagreeing cells at distance0.000e+0, with the bilerp computing7.105e-15- so the test was measuring float rounding, not smoothing. It passed before only because the old +0.5 knocked the plane off the boundaries by accident.vulcanusCliffs.spec.ts's independent blend now takes corner indices instead of deriving positions from centres - the exact mistake being fixed.vulcanusOreCliffSeparation.spec.ts: full-ore false positives 47 -> 46, per-region split shifted. The game still places zero there, so that finding is untouched.pnpm run verifygreen: 1255 passed / 3 skipped, worker 12, container 3.