From c59cfdd801ffad0938adf1b99ac578ebf1d245a3 Mon Sep 17 00:00:00 2001 From: Eric J Date: Sat, 1 Aug 2026 22:01:50 -0700 Subject: [PATCH 1/2] docs(cliffs): shipping orientation is 2.4%, not 2.0% (#84) The banner kept the pre-#88 figure. Since the collision box was corrected to rotbb's rotated rectangle the rejection costs ZERO true positives, so the shipping matched set is identical to the unfiltered one - 1531 either way, 37 wrong orientations, 2.42%. Measured on both paths. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WRKSNgkidxc6daeHGJHqpt --- docs/noise/cliffs-NOTES.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/noise/cliffs-NOTES.md b/docs/noise/cliffs-NOTES.md index fbba376..ef655ac 100644 --- a/docs/noise/cliffs-NOTES.md +++ b/docs/noise/cliffs-NOTES.md @@ -5,9 +5,14 @@ > | | recall | precision | wrong orientation | > | --- | --- | --- | --- | > | **Nauvis** | 1.0000 | 1.0000 | **0 / 334** | -> | **Vulcanus**, as shipped | **0.9758** | **0.9727** | 2.0% | +> | **Vulcanus**, as shipped | **0.9758** | **0.9727** | **37 / 1531 = 2.4%** | > | **Vulcanus**, no lava rejection | 0.9758 | 0.8719 | 37 / 1531 = 2.4% | > +> The two rows now share a recall and an orientation count because, since the +> collision box was corrected to `rotbb`'s rotated rectangle (#88), the rejection +> costs **zero** true positives - it only removes false ones. A `2.0%` here was +> the pre-#88 figure and was stale within hours. +> > **Read the shipping row.** The renderer applies `tryToAddCliff`'s lava-collision > rejection and the second row does not; leaving it off is what produced the > "precision 0.872 / 187-cell excess" figure #84 opened with. The rejection drops From 8e5bed20ba4dc5211c77e6e70b59cdea42d06d83 Mon Sep 17 00:00:00 2001 From: Eric J Date: Sat, 1 Aug 2026 22:22:38 -0700 Subject: [PATCH 2/2] fix(cliffs): the collision box is the RAW rectangle - #88 was wrong (#84) Disassembly, prompted by flagging #88's fix as the thing I was least confident about. It was right to flag: #88 scored best on every metric and was wrong about the mechanism. The engine does NOT collide against rotbb's rectangle rotated 45 degrees. It uses the RAW stored rectangle and discards the orientation tag entirely: 1. EntityMapGenerationTask::tryToAddCliff (0x101625038) loads the orientation's box from proto + 0x5c0 + id*0x48 (20 bytes: four int32 edges at +4, the orientation word at +0x14) and calls wouldCollide with Direction = 0 - literally `mov x4, #0x0`. 2. wouldCollide (0x101625468) forwards box and direction to BoundingBox::BoundingBox(BoundingBox const&, Direction) (0x101c04380), then floors with (box + position) >> 8 over an inclusive tile rect. 3. That constructor zeroes the destination, writes sentinel 0x80010000 into the destination's orientation word, and dispatches through a jump table whose entry 0 is 0 (read at 0x102d01400) - the identity arm, which copies left_top/right_bottom verbatim. The source orientation is never read; the rotate arm is reachable only for a non-zero Direction. Corroborated by the API mirror: BoundingBox is documented as {MapPosition, MapPosition} or {..., RealOrientation} with orientation OPTIONAL, and OrientedCliffPrototype::collision_bounding_box is a plain BoundingBox. | box | false rej | recall | precision | evidence | | ------------------------------ | --------- | ------ | --------- | ---------- | | AABB (until #88) | 13 | 0.9675 | 0.9743 | assumption | | 45-degree oriented rect (#88) | 0 | 0.9758 | 0.9727 | fit only | | raw stored rect (this) | 6 | 0.9720 | 0.9713 | disasm | The middle row is the trap. It shrank the box past the engine's, and the excess shrinkage absorbed a DIFFERENT defect: 4 of the 6 cliffs the correct box still rejects are cells where our orientation disagrees with the game's, so we load the wrong box entirely. Those 4 belong to the standing orientation residual and should stay visible. A model that scores perfectly by hiding a second bug is worse than one that leaves it exposed. Edges are quantised to 1/256 - MapPosition is 8-bit fixed point, so x_dist's sqrt(2) cannot reach the engine at full precision. test/cliffOrientation.spec.ts asserted the OPPOSITE of this until now, on the stated reasoning that the AABB "is what the engine ends up scanning". That block is rewritten: the shipped box is the raw rectangle, it DEPENDS on intersect (the old file asserted independence), and its area is at most half the AABB's - though it is not contained in it, since a small intersect pushes hx past size/2. An attempt to assert containment on every axis failed for exactly that reason. cliffBoxCoversTile and the separating-axis narrow phase are deleted; the broad phase alone is now the whole rule, which is what the engine does. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WRKSNgkidxc6daeHGJHqpt --- docs/noise/cliffs-NOTES.md | 12 +- docs/noise/vulcanus-cliffs-NOTES.md | 83 +++++++++++-- src/noise/cliffs/cliffCatalog.ts | 170 ++++++++------------------- src/noise/cliffs/cliffPlacement.ts | 18 +-- test/cliffOrientation.spec.ts | 70 ++++++++--- test/cliffOrientedBox.spec.ts | 113 ------------------ test/vulcanusCliffEntities.spec.ts | 50 +++++--- test/vulcanusElevationLevels.spec.ts | 19 ++- 8 files changed, 240 insertions(+), 295 deletions(-) delete mode 100644 test/cliffOrientedBox.spec.ts diff --git a/docs/noise/cliffs-NOTES.md b/docs/noise/cliffs-NOTES.md index ef655ac..65590b3 100644 --- a/docs/noise/cliffs-NOTES.md +++ b/docs/noise/cliffs-NOTES.md @@ -5,13 +5,15 @@ > | | recall | precision | wrong orientation | > | --- | --- | --- | --- | > | **Nauvis** | 1.0000 | 1.0000 | **0 / 334** | -> | **Vulcanus**, as shipped | **0.9758** | **0.9727** | **37 / 1531 = 2.4%** | +> | **Vulcanus**, as shipped | **0.9720** | **0.9713** | ~2.4% | > | **Vulcanus**, no lava rejection | 0.9758 | 0.8719 | 37 / 1531 = 2.4% | > -> The two rows now share a recall and an orientation count because, since the -> collision box was corrected to `rotbb`'s rotated rectangle (#88), the rejection -> costs **zero** true positives - it only removes false ones. A `2.0%` here was -> the pre-#88 figure and was stale within hours. +> The shipped row went 0.9675 -> 0.9758 -> **0.9720** in one day as the collision +> box was corrected twice. The middle value came from a 45-degree oriented-box +> model (#88) that scored best and was **wrong**; disassembly showed the engine +> discards the box's orientation tag entirely. Do not "restore" the better +> number - see `## The collision box, settled by disassembly` in +> `vulcanus-cliffs-NOTES.md`. > > **Read the shipping row.** The renderer applies `tryToAddCliff`'s lava-collision > rejection and the second row does not; leaving it off is what produced the diff --git a/docs/noise/vulcanus-cliffs-NOTES.md b/docs/noise/vulcanus-cliffs-NOTES.md index d3c8c39..4899377 100644 --- a/docs/noise/vulcanus-cliffs-NOTES.md +++ b/docs/noise/vulcanus-cliffs-NOTES.md @@ -2,15 +2,14 @@ > ## STATUS, 2026-08-01: issue #18 is CLOSED; remainder tracked in #84 > -> **As shipped**: recall **0.9758**, precision **0.9727**, ratio 1.003. Region -> `[0,0]` reproduces the game's cliff set entirely (recall **1.0000**). Before +> **As shipped**: recall **0.9720**, precision **0.9713**, ratio 1.001. Before > #18's fix: recall 0.806 / 0.938 / 0.853 and 12.5% wrong orientations. > > Two rules do that work and both were found late: `tryToAddCliff`'s -> lava-collision rejection (185 false positives across the three oracle regions, -> precision 0.8719 -> 0.9743) and the fact that its box is a **rotated** -> rectangle rather than its bounding box (recovers 13 real cliffs, recall -> 0.9675 -> 0.9758). See the last two sections. +> lava-collision rejection (185 false positives across the three oracle regions) +> and the shape of its box, which is the **raw stored rectangle** - the engine +> discards the `1/8` orientation tag. See the last three sections, and note the +> LAST one corrects the one before it. > > **Root cause: `multisample`'s offsets are in the calling noise program's GRID > UNITS, not tiles**, so `vulcanus_basalt_lakes_multisample`'s `min` is a 4-tile @@ -22,7 +21,8 @@ > **Every accuracy table below this banner is the PRE-FIX state**, kept because the > reasoning is the useful part. Do not quote one as current. The exceptions are > the last two sections, which are post-fix and are where this banner's numbers -> come from. Note the SECOND of them corrects the first - read both. +> come from. Each of the last three corrects the one before it - read all three, +> in order, or you will act on a superseded number. Factorio 2.1.12 (build 87038, mac-arm64). Ported 2026-07-26. Companion to `cliffs-NOTES.md`, which holds the reverse-engineering of the placement rule @@ -1135,3 +1135,72 @@ loosening that trades precision for recall. enrichment), and in every case the game's is a smaller `-to-none` variant of ours. A wrong orientation means the wrong box, so the two defects compound. Those 6 belong to the standing orientation residual, which is unchanged. + +## The collision box, settled by disassembly (2026-08-02) + +The section above is **wrong about the mechanism** and its numbers are +superseded. It concluded the engine collides against `rotbb`'s rectangle rotated +45 degrees. It does not. The engine uses the **raw stored rectangle**, and +discards the orientation tag. + +### What the binary does + +Three steps, all in the 2.1.12 arm64 slice: + +1. `EntityMapGenerationTask::tryToAddCliff` (`0x101625038`) switches on the + orientation, loads that entry's box from `proto + 0x5c0 + id*0x48` (20 bytes: + four `int32` edges at `+4`, the orientation word at `+0x14`), and calls + `wouldCollide` with **`Direction = 0`** - literally `mov x4, #0x0`. +2. `EntityMapGenerationTask::wouldCollide` (`0x101625468`) forwards box and + direction to `BoundingBox::BoundingBox(BoundingBox const&, Direction)` + (`0x101c04380`), then floors the result with `(box + position) >> 8` and scans + the inclusive tile rectangle against a 96x96 mask grid. +3. That constructor zeroes the destination, writes the sentinel `0x80010000` into + the destination's orientation word, and dispatches on the direction through a + jump table whose **entry 0 is 0** (read at `0x102d01400`) - the identity arm, + which copies `left_top`/`right_bottom` verbatim and returns. The source box's + own orientation is never read. The rotation arm below it, which calls + `Vector2::rotate(Direction)`, is reachable only for a non-zero + `Direction`. + +Corroborated by the API mirror rather than by disassembly alone: `BoundingBox` is +documented as `{MapPosition, MapPosition}` **or** `{MapPosition, MapPosition, +RealOrientation}` with `orientation` optional, and +`OrientedCliffPrototype::collision_bounding_box` is a plain `BoundingBox`. +Nothing in the docs says collision honours the orientation, and the binary says +it does not. + +### Three shapes, and the best-scoring one is wrong + +| box | false rejections | recall | precision | evidence | +| --- | --- | --- | --- | --- | +| AABB `[x, x+size] x [y, y+size]` | 13 | 0.9675 | 0.9743 | none - an assumption | +| 45-degree oriented rect (#88) | **0** | **0.9758** | 0.9727 | empirical fit only | +| **raw stored rect (current)** | 6 | 0.9720 | 0.9713 | **disassembly + API docs** | + +**#88 scored best on every metric and was wrong.** It shrank the box past what +the engine uses, and the excess shrinkage also absorbed a *different* defect: 4 +of the 6 cliffs the correct box still rejects are cells where our orientation +disagrees with the game's, so we load the wrong box entirely. Those 4 belong to +the standing orientation residual and should stay visible. + +Note the raw rectangle is not simply "smaller". `hx + hy` is fixed at +`size/2*sqrt2`, so its area is at most half the AABB's - but with a small +`intersect` it sticks out PAST the AABB in x while collapsing in y. A first +attempt to assert containment on every axis failed for that reason. + +Edges are quantised to 1/256 because `MapPosition` is 8-bit fixed point, so +`x_dist`'s `sqrt(2)` cannot reach the engine at full precision. + +### The lesson, which is the same one twice in two days + +**A correction that scores better than the truth is still wrong, and it is +dangerous precisely because it scores better.** #86 over-reported a collapsing +gap because the AABB box was over-rejecting; #88 then hit 13/13 by over-shrinking +and hid four orientation bugs. Both times the flattering number came from a +too-strong correction. When a fix lands on a metric perfectly, treat that as a +prompt to find the independent evidence, not as the evidence. + +The route that worked here was: stop tuning shapes against the metric, and go +read what the engine does. The binary is unstripped and the whole chain took +three `lldb` calls. diff --git a/src/noise/cliffs/cliffCatalog.ts b/src/noise/cliffs/cliffCatalog.ts index cb684fe..e2a85d8 100644 --- a/src/noise/cliffs/cliffCatalog.ts +++ b/src/noise/cliffs/cliffCatalog.ts @@ -263,38 +263,65 @@ export const CLIFF_CODE_TO_ORIENTATION: Readonly> = { export type CliffCollisionBox = readonly [number, number, number, number]; /** - * `rotbb(x, y, size, intersect)`'s axis-aligned bounding box + * `rotbb(x, y, size, intersect)` as the ENGINE reads it back * (`base/prototypes/entity/entity-util.lua:9`). * * `rotbb` builds a rectangle centred at `(x + size/2, y + size/2)` with * half-extents `((1 - intersect/size) * d, (intersect/size) * d)` where - * `d = size/2 * sqrt(2)`, and tags it with an orientation of **1/8** - a 45 - * degree rotation. Rotating those half-extents by 45 gives an AABB half-extent - * of `(hx + hy) * cos(45) = (d) * cos(45) = size/2` on BOTH axes, whatever - * `intersect` was. So the AABB is exactly the square `[x, x+size] x [y, y+size]` - * and `intersect` only decides how the diagonal is split inside it. + * `d = size/2 * sqrt(2)`, and tags it with an orientation of **1/8**. * - * **The AABB is the BROAD phase only - `intersect` is load-bearing after all.** - * A note here used to say `intersect` could be dropped because it does not move - * the AABB. That is true of the AABB and false of the collision: `rotbb` tags - * the box with orientation `1/8` and the engine collides against the ROTATED - * rectangle, whose corners the AABB overruns. `intersect` decides how far the - * diagonal is split, hence which corners are empty. See - * {@link CLIFF_ORIENTATION_ROTBB} and {@link cliffBoxCoversTile}; measured in - * `test/cliffOrientedBox.spec.ts`. + * **The 1/8 tag is DISCARDED for collision, so this returns the raw rectangle.** + * Established by disassembly 2026-08-02, three steps deep: + * + * 1. `EntityMapGenerationTask::tryToAddCliff` (`0x101625038`) loads the + * orientation's box from `proto + 0x5c0 + id*0x48`, copies 20 bytes (four + * `int32` edges at `+4` plus the orientation word at `+0x14`), and calls + * `wouldCollide` with **`Direction = 0`** (`mov x4, #0x0`). + * 2. `EntityMapGenerationTask::wouldCollide` (`0x101625468`) forwards that box + * and direction to `BoundingBox::BoundingBox(BoundingBox const&, Direction)` + * (`0x101c04380`). + * 3. That constructor zeroes the destination, writes the sentinel `0x80010000` + * into the destination's orientation word, and dispatches on the direction + * through a jump table whose **entry 0 is 0** - the identity arm, which + * copies `left_top`/`right_bottom` verbatim and returns. The source box's own + * orientation is never read; the rotation arm below it is reached only for a + * non-zero `Direction`. + * + * So the collision rectangle is the stored rectangle, axis-aligned, and the + * tile scan floors it with `(box + position) >> 8` over an inclusive rect. + * + * **Two shapes were shipped here before this and both were wrong.** The AABB + * `[x, x+size] x [y, y+size]` (until #88) is too big at the corners; a 45-degree + * separating-axis test (#88) is too SMALL, and scored better than the truth + * because it also absorbed the unrelated orientation residual. See + * `test/cliffCollisionBox.spec.ts`. + * + * Edges are quantised to 1/256 because `MapPosition` is 8-bit fixed point, so + * `x_dist`'s `sqrt(2)` cannot survive into the engine at full precision. */ -function rotbbBox(x: number, y: number, size: number): CliffCollisionBox { - return [x, y, x + size, y + size]; +function rotbbBox(x: number, y: number, size: number, intersect: number): CliffCollisionBox { + const dist = (size / 2) * SQRT2; + const yRatio = intersect / size; + const xDist = (1 - yRatio) * dist; + const yDist = yRatio * dist; + const cx = x + size / 2; + const cy = y + size / 2; + const q = (v: number): number => Math.round(v * 256) / 256; + return [q(cx - xDist), q(cy - yDist), q(cx + xDist), q(cy + yDist)]; } +/** The four straight orientations, written as plain boxes in the Lua. */ +const CLIFF_STRAIGHT_COLLISION_BOX: readonly CliffCollisionBox[] = [ + [-2.0, -1.5, 2.0, 1.5], // 0 west-to-east + [-1.0, -2.0, 1.0, 2.0], // 1 north-to-south + [-2.0, -0.5, 2.0, 0.5], // 2 east-to-west + [-1.0, -2.0, 1.0, 2.0], // 3 south-to-north +]; + /** * `rotbb(x, y, size, intersect)`'s four arguments per orientation id, verbatim * from `create_cliff_data_specification` (`entity-util.lua:85`), or `null` for - * the four straight orientations, whose boxes are written out as plain - * axis-aligned rectangles with no orientation tag. - * - * This exists because {@link CLIFF_ORIENTATION_COLLISION_BOX} is only the - * bounding box. The engine's collision uses the rotated rectangle itself. + * the four straight orientations. Verified identical to the Lua, in order. */ export const CLIFF_ORIENTATION_ROTBB: readonly ( | readonly [number, number, number, number] @@ -324,81 +351,6 @@ export const CLIFF_ORIENTATION_ROTBB: readonly ( const SQRT2 = 1.4142135623730951; -/** - * Does the tile `[tx, tx+1] x [ty, ty+1]` overlap the collision shape of a - * cliff of orientation `id` centred at `(centerX, centerY)`? - * - * For the four straight orientations the shape IS the axis-aligned box, so any - * tile the broad phase enumerated overlaps it and this returns `true`. For the - * sixteen `rotbb` orientations the shape is that rectangle rotated 45 degrees - * clockwise (Factorio orientation `1/8`, and `+y` is south), which the AABB - * overruns at all four corners - a separating-axis test over the two world axes - * and the rectangle's own two decides it. - * - * **Why this is not gold-plating.** Using the AABB drops real cliffs: across the - * three Vulcanus oracle regions the game placed 13 cliffs whose AABB contains - * lava and whose rotated box does not, and it kept every one. Narrowing to the - * oriented rectangle clears **13 of 13** while retaining 182 of the 185 - * rejections that were removing genuine false positives - so it is not a - * loosening that trades precision for recall, it is the correct shape. - */ -export function cliffBoxCoversTile( - id: number, - centerX: number, - centerY: number, - tx: number, - ty: number, -): boolean { - const spec = CLIFF_ORIENTATION_ROTBB[id]; - if (spec === undefined || spec === null) return true; - const [bx, by, size, intersect] = spec; - const dist = (size / 2) * SQRT2; - const yRatio = intersect / size; - const xDist = (1 - yRatio) * dist; - const yDist = yRatio * dist; - const cx = centerX + bx + size / 2; - const cy = centerY + by + size / 2; - // cos 45 = sin 45; clockwise in screen coords (x east, y south). - const k = Math.SQRT1_2; - const corners: readonly (readonly [number, number])[] = [ - [-xDist, -yDist], - [xDist, -yDist], - [xDist, yDist], - [-xDist, yDist], - ].map(([u, v]) => [cx + (u - v) * k, cy + (u + v) * k] as const); - const square: readonly (readonly [number, number])[] = [ - [tx, ty], - [tx + 1, ty], - [tx + 1, ty + 1], - [tx, ty + 1], - ]; - const axes: readonly (readonly [number, number])[] = [ - [1, 0], - [0, 1], - [k, k], - [-k, k], - ]; - for (const [ax, ay] of axes) { - let aMin = Infinity; - let aMax = -Infinity; - let bMin = Infinity; - let bMax = -Infinity; - for (const [px, py] of corners) { - const d = px * ax + py * ay; - if (d < aMin) aMin = d; - if (d > aMax) aMax = d; - } - for (const [px, py] of square) { - const d = px * ax + py * ay; - if (d < bMin) bMin = d; - if (d > bMax) bMax = d; - } - // Touching is not overlapping: a tile the rectangle only grazes is free. - if (aMax <= bMin || bMax <= aMin) return false; - } - return true; -} - /** * `CliffOrientation` id -> the orientation's `collision_bounding_box`, at * `scale = 1.0` (both `cliff` and `cliff-vulcanus`), relative to the cliff's @@ -427,28 +379,10 @@ export function cliffBoxCoversTile( * 0.779) to 888 predicted (ratio **1.003**, precision **0.930**), rejecting 173 * false positives and only 4 true ones. See issue #18. */ -export const CLIFF_ORIENTATION_COLLISION_BOX: readonly CliffCollisionBox[] = [ - [-2.0, -1.5, 2.0, 1.5], // 0 west-to-east - [-1.0, -2.0, 1.0, 2.0], // 1 north-to-south - [-2.0, -0.5, 2.0, 0.5], // 2 east-to-west - [-1.0, -2.0, 1.0, 2.0], // 3 south-to-north - rotbbBox(-3.5, -3, 4.5), // 4 west-to-north - rotbbBox(-1, -3, 4.5), // 5 north-to-east - rotbbBox(-1, -0.5, 3.5), // 6 east-to-south - rotbbBox(-2.5, -0.5, 3.5), // 7 south-to-west - rotbbBox(-3.5, -1.5, 4.5), // 8 west-to-south - rotbbBox(-2.5, -3, 3.5), // 9 north-to-west - rotbbBox(-1, -3, 3.5), // 10 east-to-north - rotbbBox(-1, -1.5, 4.5), // 11 south-to-east - rotbbBox(-3, -1.5, 3), // 12 west-to-none - rotbbBox(0, -1.5, 3), // 13 none-to-east - rotbbBox(0, -0.5, 2.5), // 14 east-to-none - rotbbBox(-2.5, -0.5, 2.51), // 15 none-to-west - rotbbBox(-1, -2.5, 3), // 16 north-to-none - rotbbBox(-1, -0.5, 3), // 17 none-to-south - rotbbBox(-2, -0.5, 3), // 18 south-to-none - rotbbBox(-2, -2.5, 3), // 19 none-to-north -]; +export const CLIFF_ORIENTATION_COLLISION_BOX: readonly CliffCollisionBox[] = + CLIFF_ORIENTATION_ROTBB.map((spec, id) => + spec === null ? CLIFF_STRAIGHT_COLLISION_BOX[id] : rotbbBox(...spec), + ); /** * The `CliffOrientation` id a cell code places, or `undefined` when the code diff --git a/src/noise/cliffs/cliffPlacement.ts b/src/noise/cliffs/cliffPlacement.ts index fd9c875..5087c90 100644 --- a/src/noise/cliffs/cliffPlacement.ts +++ b/src/noise/cliffs/cliffPlacement.ts @@ -13,9 +13,7 @@ import { CLIFF_CELL_CENTER_X, CLIFF_CELL_CENTER_Y, CLIFF_GRID_SIZE, - cliffBoxCoversTile, cliffCollisionTileBox, - cliffOrientationForCode, getModifiedElevationInterval, isCliffPlaced, } from "./cliffCatalog"; @@ -321,24 +319,18 @@ export function makeCliffPlacementFromFields( * the orientation's collision box and drop the cell if any tile in it collides. * With no `tileCollides` supplied this is a constant `false` and costs nothing. * - * **Two phases, because sixteen of the twenty boxes are rotated.** - * `cliffCollisionTileBox` is the BROAD phase - the axis-aligned tile rectangle - * `wouldCollide` derives with `(box + position) >> 8`. For the four straight - * orientations that is the whole shape. For the sixteen `rotbb` ones the real - * shape is that rectangle turned 45 degrees, so `cliffBoxCoversTile` runs a - * narrow phase and discards the AABB's four empty corners. Skipping it drops - * 13 real Vulcanus cliffs whose corners happen to overhang lava. + * The box is `cliffCollisionTileBox` and nothing narrows it: `wouldCollide` + * floors the stored rectangle with `(box + position) >> 8` and scans the + * inclusive tile rect, with the box's own `1/8` orientation tag discarded. + * See `rotbbBox` in `cliffCatalog.ts` for the disassembly that establishes it. */ const rejected = (code: number, x: number, y: number): boolean => { if (tileCollides === undefined) return false; const box = cliffCollisionTileBox(code, x, y); // `undefined` only for a code that places nothing, which cannot reach here. if (box === undefined) return false; - const id = cliffOrientationForCode(code); - if (id === undefined) return false; for (let tx = box.left; tx <= box.right; tx++) - for (let ty = box.top; ty <= box.bottom; ty++) - if (tileCollides(tx, ty) && cliffBoxCoversTile(id, x, y, tx, ty)) return true; + for (let ty = box.top; ty <= box.bottom; ty++) if (tileCollides(tx, ty)) return true; return false; }; diff --git a/test/cliffOrientation.spec.ts b/test/cliffOrientation.spec.ts index 49badb7..eeb5f2b 100644 --- a/test/cliffOrientation.spec.ts +++ b/test/cliffOrientation.spec.ts @@ -154,13 +154,19 @@ describe("cliff orientation collision boxes", () => { }); /** - * Re-derives the 16 `rotbb` boxes the long way, from the Lua source's actual - * rectangle plus its 1/8-turn orientation, and checks the result against the - * shipped square. `cliffCatalog.ts` ships the closed form (`[x, x+size] x - * [y, y+size]`) because it is what the engine ends up scanning; this is the - * working that justifies dropping the `intersect` argument. + * Re-derives the 16 `rotbb` boxes the long way from the Lua source, and + * checks the shipped table against the RAW rectangle - not against the AABB + * of its 1/8 turn. + * + * **This block asserted the opposite until 2026-08-02**, on the stated + * reasoning that the AABB "is what the engine ends up scanning". Disassembly + * says otherwise: `tryToAddCliff` calls `wouldCollide` with `Direction = 0`, + * and `BoundingBox(BoundingBox const&, Direction)` takes its identity arm, + * copying `left_top`/`right_bottom` verbatim and overwriting the destination's + * orientation with a sentinel. The `1/8` tag never reaches the tile scan. See + * `rotbbBox` in `cliffCatalog.ts`. */ - describe("the rotbb boxes are the AABB of the 45-degree rectangle", () => { + describe("the rotbb boxes are the RAW rectangle, orientation discarded", () => { /** `rotbb` verbatim (base/prototypes/entity/entity-util.lua:9). */ const rotbb = ( x: number, @@ -214,25 +220,57 @@ describe("cliff orientation collision boxes", () => { [19, [-2, -2.5, 3, 2]], ]; + /** The raw rectangle, quantised to MapPosition's 1/256 as the engine stores it. */ + const rawRect = ( + x: number, + y: number, + size: number, + intersect: number, + ): [number, number, number, number] => { + const { cx, cy, hx, hy } = rotbb(x, y, size, intersect); + const q = (v: number): number => Math.round(v * 256) / 256; + return [q(cx - hx), q(cy - hy), q(cx + hx), q(cy + hy)]; + }; + it("reproduces every shipped rotbb box", () => { expect(CALLS).toHaveLength(16); for (const [id, [x, y, size, intersect]] of CALLS) { const want = CLIFF_ORIENTATION_COLLISION_BOX[id]; - const got = rotatedAabb(x, y, size, intersect); + const got = rawRect(x, y, size, intersect); for (let i = 0; i < 4; i++) expect(got[i]).toBeCloseTo(want[i], 9); } }); - it("is independent of `intersect`, which is why the shipped form omits it", () => { - // The claim that lets `rotbbBox` take three arguments. Sweeping the fourth - // must not move the AABB. - for (const [id, [x, y, size]] of CALLS) { - for (const intersect of [0.25, 1, size / 2, size - 0.25]) { - const got = rotatedAabb(x, y, size, intersect); - const want = CLIFF_ORIENTATION_COLLISION_BOX[id]; - for (let i = 0; i < 4; i++) expect(got[i]).toBeCloseTo(want[i], 9); - } + it("DEPENDS on `intersect`, which is why the shipped form now takes it", () => { + // The inverse of what this file asserted before 2026-08-02. `intersect` + // does not move the AABB - that much was true - but it moves the raw + // rectangle, which is the thing the engine scans. Sweeping it must change + // the box for every orientation, or `rotbbBox` is ignoring an argument. + for (const [, [x, y, size, intersect]] of CALLS) { + const base = rawRect(x, y, size, intersect); + const other = rawRect(x, y, size, intersect === size / 2 ? size / 4 : size / 2); + expect(base).not.toEqual(other); + } + }); + + it("covers at most HALF the AABB's area, but is not inside it", () => { + // Non-vacuity for the change itself: if the raw rectangle equalled the + // AABB, none of this would have mattered. It is much smaller in area - + // `hx + hy` is fixed at `size/2*sqrt2`, so `4*hx*hy <= size^2/2` - yet it + // is NOT contained in the AABB. With a small `intersect`, `hx` approaches + // `0.707*size` and the unrotated rectangle sticks out PAST the AABB in x + // while collapsing in y. An earlier version of this test asserted + // containment on every axis and was wrong for exactly that reason. + let stickOut = 0; + for (const [, [x, y, size, intersect]] of CALLS) { + const raw = rawRect(x, y, size, intersect); + const aabb = rotatedAabb(x, y, size, intersect); + const areaRaw = (raw[2] - raw[0]) * (raw[3] - raw[1]); + const areaAabb = (aabb[2] - aabb[0]) * (aabb[3] - aabb[1]); + expect(areaRaw).toBeLessThan(areaAabb * 0.51); + if (raw[0] < aabb[0] - 1e-9 || raw[2] > aabb[2] + 1e-9) stickOut++; } + expect(stickOut).toBeGreaterThan(0); }); it("differs from the UNROTATED rectangle, so the check above is not trivial", () => { diff --git a/test/cliffOrientedBox.spec.ts b/test/cliffOrientedBox.spec.ts deleted file mode 100644 index 9ef3d03..0000000 --- a/test/cliffOrientedBox.spec.ts +++ /dev/null @@ -1,113 +0,0 @@ -import { describe, expect, it } from "vite-plus/test"; - -import { - CLIFF_ORIENTATION_COLLISION_BOX, - CLIFF_ORIENTATION_NAMES, - CLIFF_ORIENTATION_ROTBB, - cliffBoxCoversTile, -} from "../src/noise/cliffs/cliffCatalog"; - -/** - * **`rotbb` boxes are ROTATED, and the port used their bounding box** (issue - * #84, the lava-perimeter thread). - * - * `rotbb(x, y, size, intersect)` (`base/prototypes/entity/entity-util.lua:9`) - * returns `{{cx - x_dist, cy - y_dist}, {cx + x_dist, cy + y_dist}, 1/8}` - a - * rectangle **plus an orientation of 1/8**, i.e. 45 degrees. Sixteen of the - * twenty cliff orientations are built with it; only the four straight ones are - * written as plain axis-aligned rectangles. - * - * `CLIFF_ORIENTATION_COLLISION_BOX` holds the axis-aligned BOUNDING box, which - * is the right broad phase - `wouldCollide` derives its tile rectangle from a - * fixed-point floor, and that is what `cliffCollisionTileBox` reproduces. But - * the collision itself is against the rotated rectangle, and the AABB overruns - * it at all four corners. Using the AABB is therefore strictly too eager, and - * only ever in the corners. - * - * **How it was found, because the route matters more than the fix.** The - * negative-space oracle said 13 real Vulcanus cliffs had lava inside their box, - * and the standing explanation - written into `vulcanusCliffEntities.spec.ts` - * and the notes - was that our lava mask was "off by about one tile SOMEWHERE". - * A dense 994-position capture at exactly those boundaries - * (`oracle-vulcanus-lava-boundary.seed123456.json`) found **zero** lava - * mismatches, 35/35 correct at the accusing tiles. The mask was innocent; the - * shape was wrong. Re-testing every hit against the rotated rectangle clears - * **13 of 13** while keeping 182 of the 185 rejections that remove genuine false - * positives. - */ -describe("cliff collision boxes are rotated, not axis-aligned", () => { - it("has a rotbb spec for the 16 diagonal orientations and none for the 4 straight", () => { - expect(CLIFF_ORIENTATION_ROTBB.length).toBe(CLIFF_ORIENTATION_COLLISION_BOX.length); - const straight = CLIFF_ORIENTATION_ROTBB.filter((s) => s === null).length; - expect(straight).toBe(4); - // The four axis-aligned ones are exactly the straight walls. - for (let id = 0; id < 4; id++) expect(CLIFF_ORIENTATION_ROTBB[id]).toBeNull(); - for (const name of [0, 1, 2, 3].map((i) => CLIFF_ORIENTATION_NAMES[i])) - expect(name).toMatch(/^(west-to-east|north-to-south|east-to-west|south-to-north)$/); - }); - - /** - * The AABB is derived from the same `(x, y, size)` the rotbb spec carries, so - * the two cannot drift apart silently. This is the invariant that makes the - * broad phase a genuine superset of the narrow one. - */ - it("each rotbb's bounding box is the square the AABB table already holds", () => { - for (const [id, spec] of CLIFF_ORIENTATION_ROTBB.entries()) { - if (spec === null) continue; - const [x, y, size] = spec; - expect(CLIFF_ORIENTATION_COLLISION_BOX[id]).toEqual([x, y, x + size, y + size]); - } - }); - - /** - * The point of the whole change: for a rotated box the AABB's corner tiles are - * NOT covered, and its edge-midpoint tiles are. A test that only checked the - * centre would pass for the AABB too. - */ - it("excludes the AABB's corners and keeps the box's own centre", () => { - let cornersExcluded = 0; - let centresIncluded = 0; - let rotated = 0; - for (const [id, spec] of CLIFF_ORIENTATION_ROTBB.entries()) { - if (spec === null) continue; - rotated++; - const [x, y, size] = spec; - // Corner tiles of the AABB, in cell-centre-relative coordinates. - for (const [cx, cy] of [ - [x, y], - [x + size - 1, y], - [x, y + size - 1], - [x + size - 1, y + size - 1], - ]) { - if (!cliffBoxCoversTile(id, 0, 0, Math.floor(cx), Math.floor(cy))) cornersExcluded++; - } - // The tile containing the rectangle's own centre is inside it under any - // rotation, so it must survive. Edge midpoints were tried here first and - // are the WRONG probe: `Math.floor` of an AABB edge can name a tile that - // is only partly inside the AABB at all (ids 7, 9, 17, 18), so a miss - // there says nothing about the narrow phase. - if (cliffBoxCoversTile(id, 0, 0, Math.floor(x + size / 2), Math.floor(y + size / 2))) - centresIncluded++; - } - // 16 rotated orientations x 4 corners = 64 candidate corners, of which - // **26 are excluded** (measured 2026-08-01). Not a majority: how much of its - // AABB a rotated rectangle fills depends on `intersect`, and the corner tile - // is a whole 1x1 square sitting inside the corner rather than the corner - // point itself. The bound is the measured value with headroom, not a round - // number - what it has to catch is the narrow phase becoming a no-op, which - // would give 0. - expect(cornersExcluded).toBeGreaterThan(20); - // And the narrow phase must not have eaten the box: every rotated box keeps - // its own centre. Without this, `cliffBoxCoversTile` could return `false` - // everywhere and the corner assertion above would still pass. - expect(rotated).toBe(16); - expect(centresIncluded).toBe(16); - }); - - /** The four straight orientations are unrotated, so nothing is ever carved off. */ - it("never narrows the four axis-aligned boxes", () => { - for (let id = 0; id < 4; id++) - for (let tx = -4; tx <= 4; tx++) - for (let ty = -4; ty <= 4; ty++) expect(cliffBoxCoversTile(id, 0, 0, tx, ty)).toBe(true); - }); -}); diff --git a/test/vulcanusCliffEntities.spec.ts b/test/vulcanusCliffEntities.spec.ts index 6b14169..116c81f 100644 --- a/test/vulcanusCliffEntities.spec.ts +++ b/test/vulcanusCliffEntities.spec.ts @@ -114,19 +114,34 @@ describe("Vulcanus cliff placement vs find_entities", () => { // That fix is measured in `test/multisampleGrid.spec.ts`; these are its // end-to-end numbers on the path the renderer actually runs. // - // **Updated 2026-08-01 again, after the collision box was narrowed from - // `rotbb`'s AABB to the rotated rectangle it actually is** - // (`cliffBoxCoversTile`). That recovered 13 real cliffs at the cost of 3 - // false positives. + // **Updated 2026-08-02, after DISASSEMBLING the collision test.** The box + // is the RAW stored rectangle: `tryToAddCliff` calls `wouldCollide` with + // `Direction = 0`, and `BoundingBox(BoundingBox const&, Direction)` takes + // its identity arm, copying `left_top`/`right_bottom` and discarding the + // `1/8` orientation tag. See `rotbbBox` in `cliffCatalog.ts`. // // | region | game | ours | recall | precision | ratio | // | --- | --- | --- | --- | --- | --- | - // | 0 `[0,0]` | 283 | 285 | **1.0000** | 0.9930 | 1.007 | - // | 1 `[1500,1500]` | 885 | 901 | 0.9729 | 0.9556 | 1.018 | - // | 2 `[-1200,800]` | 401 | 388 | 0.9651 | 0.9974 | 0.968 | - // | **total** | **1569** | **1574** | **0.9758** | **0.9727** | 1.003 | - // - // Region 0 reproduces the game's cliff set ENTIRELY - recall 1.0000. + // | 0 `[0,0]` | 283 | 283 | 0.9929 | 0.9929 | 1.000 | + // | 1 `[1500,1500]` | 885 | 900 | 0.9695 | 0.9533 | 1.017 | + // | 2 `[-1200,800]` | 401 | 387 | 0.9626 | 0.9974 | 0.965 | + // | **total** | **1569** | **1570** | **0.9720** | **0.9713** | 1.001 | + // + // **These are WORSE than the numbers this comment carried for one day, and + // they are the right ones.** Three box models have shipped here: + // + // | box | false rejections | recall | precision | evidence | + // | --- | --- | --- | --- | --- | + // | AABB of the rotated rect | 13 | 0.9675 | 0.9743 | none | + // | 45-degree oriented rect (#88) | 0 | 0.9758 | 0.9727 | empirical fit | + // | raw stored rect (current) | 6 | 0.9720 | 0.9713 | **disassembly** | + // + // #88's middle row scored best on every metric and was wrong. It shrank + // the box past what the engine uses, which ALSO absorbed the unrelated + // orientation residual - 4 of the 6 cliffs the correct box still rejects + // are cells where our orientation disagrees with the game's, so we load + // the wrong box entirely. A model that scores perfectly by hiding a second + // defect is worse than one that leaves it visible. // // **The lava rejection is what closes the over-placement, and #84 item 1 // asked how much.** The answer is nearly all of it. Without it the same @@ -175,17 +190,16 @@ describe("Vulcanus cliff placement vs find_entities", () => { // indiscriminate, ratio collapsing to 0.65 / 0.70. The real arm rejects // almost only false positives. // - // **Still not Nauvis-grade, and what is left is now pure over-placement.** - // `test/cliffPlacement.spec.ts` measures Nauvis at 1.0000 recall AND - // precision. Here 38 of the game's 1569 are missing and 43 of our 1574 are - // spurious - and none of the 38 is a collision-rejection loss any more, - // which is what the box fix bought. The residual is not one-directional - // (region 2 UNDER-places, regions 0 and 1 over-place), which is why the - // ratio is guarded on both sides below. + // **Still not Nauvis-grade.** `test/cliffPlacement.spec.ts` measures Nauvis + // at 1.0000 recall AND precision. Here 44 of the game's 1569 are missing + // and 45 of our 1570 are spurious; 6 of the 44 are collision rejections, + // 4 of those traceable to the orientation residual. The remainder is not + // one-directional (region 2 UNDER-places, region 1 over-places), which is + // why the ratio is guarded on both sides below. // // Guards sit just outside the measured values in the direction that would // signal a regression, and open in the direction of improvement. - expect(recall).toBeGreaterThan(0.96); + expect(recall).toBeGreaterThan(0.95); expect(precision).toBeGreaterThan(0.94); expect(predicted.size / actual.size).toBeLessThan(1.05); expect(predicted.size / actual.size).toBeGreaterThan(0.95); diff --git a/test/vulcanusElevationLevels.spec.ts b/test/vulcanusElevationLevels.spec.ts index 68bc4ca..04eabde 100644 --- a/test/vulcanusElevationLevels.spec.ts +++ b/test/vulcanusElevationLevels.spec.ts @@ -220,10 +220,19 @@ describe("Vulcanus elevation, inverted through a cliff_elevation_0 sweep", () => // with it. Guarded as an upper bound only - it may shrink to zero or invert. expect(low - high).toBeLessThan(0.03); - // **Recall, which is what the box fix bought.** Every level reproduces the - // game's whole cliff set; level 20 misses 1 of 658. Before `cliffBoxCoversTile` - // this ran 0.951 at level 20. Asserted per level rather than in aggregate so - // a regime-shaped regression cannot average itself away. - for (const w of withRejection) expect(w.both / w.game).toBeGreaterThan(0.99); + // **Recall, which is what the box shape buys.** Asserted per level rather + // than in aggregate so a regime-shaped regression cannot average itself + // away. The three box models measured on this sweep, worst level: + // + // | box | worst per-level recall | + // | --- | --- | + // | AABB of the rotated rect (until #88) | 0.951 | + // | 45-degree oriented rect (#88, WRONG) | 0.999 | + // | raw stored rect (disasm, current) | 0.977 | + // + // The middle row scored best and was wrong - it shrank the box past what + // the engine uses and so also absorbed the unrelated orientation residual. + // Guarding at the truth, not at the flattering number. + for (const w of withRejection) expect(w.both / w.game).toBeGreaterThan(0.97); }, 120000); });