Skip to content

Narrow the incoming coordinate to f32 (#309, #191) - #312

Merged
wormeyman merged 2 commits into
mainfrom
fix/309-narrow-incoming-coordinate
Aug 25, 2026
Merged

Narrow the incoming coordinate to f32 (#309, #191)#312
wormeyman merged 2 commits into
mainfrom
fix/309-narrow-incoming-coordinate

Conversation

@wormeyman

Copy link
Copy Markdown
Collaborator

Closes #309. Closes two thirds of #191 and updates it with what is left.

The noise machine holds its coordinate values at f32, so an op's x and y are already f32 when it reads them, and the input_scale multiply is an f32 operation on both operands.

Settled by measurement - from a fixture we already had

The plan was a far-field capture at |x| >= 65536, where the 1/256 MapPosition grid stops being a subset of the f32 grid. It would have worked and it was unnecessary.

A derived coordinate leaves the f32 grid right next to the origin. fulgora_basis is a multioctave read at Fulgora's distorted coordinate wx = ox + wobble_x * wobble_mask, computed in f64 and therefore off the f32 grid at 55 of that fixture's 101 positions. Scoring sumOctaves with and without its incoming narrowing:

incoming coordinate exact worst residual
narrowed (what ships) 101/101 exactly 0
un-narrowed (planted) 81/101 7.0333e-6

Twenty positions discriminate. That is also the check #191 asked for in its own words - "a caller that passes a derived coordinate" - which Fulgora has satisfied since it landed, unnoticed.

What changed

  • basisNoiseExpr narrows x and y before the input_scale multiply, which is what the Rust already did. basisNoiseExpr narrows the coordinate product at a different point than the Rust port #309 was the two ports disagreeing about this. offset_x is 0 at all six call sites, so whether the game narrows before or after that add is unreachable, and no third variant is invented for it.
  • variablePersistenceMultioctaveNoise narrows y, in both ports. It narrowed x only as a side effect of the f32(x + offset_x) add; y had no add and so was silently multiplied in f64. Both ports carried the identical asymmetry, which is exactly why tier 2 could not see it - they agreed with each other while both disagreed with the game.
  • quickMultioctaveNoise needed nothing. Narrow incoming coordinates to f32 in quickMultioctave, variablePersistence and basisNoise, once a derived-coordinate caller can prove it #191's text is stale: it already narrows both coordinates at quickMultioctaveNoise.ts:192-193. Read the code, not the issue.
  • basisNoise itself is deliberately unchanged. Its remaining direct callers (nauvisShared.ts:133, startingPatches.ts:185, regularPatches.ts:164) are unported Nauvis chains where the whole expression is un-narrowed, not just the coordinate. Planting the internal narrowing leaves all 26 of their specs passing, so no committed fixture discriminates it. That belongs to Port phase 6: Nauvis and the five overlays #226, scored layer by layer under the greedy-accept rule.

The pin is inverted, not deleted

the two ports agree off the f32 grid now asserts 0 of 74 diverging where it froze 32, and reverting primitives.ts reproduces [ 'hairlineCracks', ...(31) ] - planted, not assumed.

Its anti-vacuity is not optional: "nothing diverges" is exactly what a sweep evaluating nothing would report, so the test also asserts the off-grid window's folds differ from the on-grid window's on all 74 fields.

Also: the three tier-2 review points from #310

  • The field selector moved into fmw-noise. VulcanusParity sits beside VulcanusStack, so elevation_fields and temperature - which no render path reads - are private again instead of pub on a library type solely for a test. Pure code motion, checked as such: tier 2 and tier 3 pass unchanged. It moves engine.wasm by 142 bytes from inlining, which is a reminder that a wasm diff is not by itself evidence of a behaviour change.
  • The slider anti-vacuity guard is frozen at its measured 50 of 74 instead of > 40, matching every other count in that file. A floor of 40 would sit through nine fields silently dropping their slider dependency.
  • verify:rust's cost is recorded as a range, 1m44s to 2m48s - Build Vulcanus tier 2 and pin the divergence it found (#225) #310 measured 1m44s on CI against the 2m48s in CLAUDE.md, on unchanged Rust. Same spread the test shards show, so widen the figure rather than replace it.

Two lessons recorded in CLAUDE.md

The second is bigger than the bug: fold the fixtures you already have before capturing more, and "no fixture can grade this" is a claim about the fixtures you looked at. It was true of every Vulcanus fixture and false of a Fulgora one.

pnpm run verify green: 2143 passed, 4 skipped, verify:rust included.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AA9K55yddm5D7DPuSJareA

wormeyman and others added 2 commits August 24, 2026 16:26
The noise machine holds its coordinate values at f32, so an op's x and y are
already f32 when it reads them and the input_scale multiply is an f32
operation on both operands.

Settled by measurement, and by a fixture already committed rather than the
far-field capture that was planned. A DERIVED coordinate leaves the f32 grid
right next to the origin: fulgora_basis is a multioctave read at
wx = ox + wobble_x * wobble_mask, computed in f64 and off the f32 grid at 55
of that fixture's 101 positions. Scoring sumOctaves with and without its
incoming narrowing:

  narrowed    (shipped)   101/101, worst residual exactly 0
  un-narrowed (planted)    81/101, worst 7.0333e-6

Twenty positions discriminate. That is also the check #191 asked for in its
own words - "a caller that passes a derived coordinate" - which Fulgora has
satisfied since it landed.

What changed:

- basisNoiseExpr narrows x and y before the input_scale multiply, which is
  what the Rust already did. #309 was the two ports disagreeing about this;
  they now agree. offset_x is 0 at all six call sites, so whether the game
  narrows before or after that add is unreachable and no third variant is
  invented for it.
- variablePersistenceMultioctaveNoise narrows y, in BOTH ports. It narrowed x
  only as a side effect of the f32(x + offset_x) add; y had no add and so was
  silently multiplied in f64. Both ports had the identical asymmetry, which is
  why tier 2 could not see it - they agreed with each other while both
  disagreed with the game.
- quickMultioctaveNoise needed nothing. #191's issue text is stale: it already
  narrows both coordinates at quickMultioctaveNoise.ts:192-193.
- basisNoise itself is deliberately unchanged. Its remaining direct callers
  (nauvisShared.ts:133, startingPatches.ts:185, regularPatches.ts:164) are
  unported Nauvis chains where the whole expression is un-narrowed, not just
  the coordinate. Planting the internal narrowing leaves all 26 of their specs
  passing, so no committed fixture discriminates it. That belongs to #226,
  scored layer by layer under the greedy-accept rule.

The tier-2 pin is INVERTED rather than deleted. "the two ports agree off the
f32 grid" now asserts 0 of 74 diverging where it froze 32, and reverting
primitives.ts reproduces [ 'hairlineCracks', ...(31) ] - planted, not assumed.
Its anti-vacuity is not optional: "nothing diverges" is exactly what a sweep
evaluating nothing would report, so it also asserts the off-grid window's
folds differ from the on-grid window's on all 74 fields.

Two lessons recorded in CLAUDE.md, the second bigger than the bug: fold the
fixtures you already have before capturing more, and "no fixture can grade
this" is a claim about the fixtures you looked at. It was true of every
Vulcanus fixture and false of a Fulgora one.

pnpm run verify green: 2143 passed, 4 skipped, verify:rust included.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AA9K55yddm5D7DPuSJareA
All three were flagged when Vulcanus tier 2 landed and left for a follow-up.

Move the field selector into fmw-noise. VulcanusParity now sits beside
VulcanusStack in expressions/vulcanus_stack.rs, and the wasm export builds the
stack through the render helpers and calls parity.field(field, x, y). The
reason is ownership of test-only API: the selector needs elevation_fields and
temperature, which no render path reads, and reaching them from another crate
meant two pub methods on a library type existing solely for a test - and a pub
method cannot be #[cfg(test)]-gated because the wasm crate calls it at build
time. Both are private again. VULCANUS_FIELD_COUNT moved with it as
VulcanusParity::FIELD_COUNT, so the count and the match it bounds cannot drift.

This is pure code motion and was checked as such rather than assumed: tier 2
(74 fields, two sliders, two windows) and tier 3 (byte-identical renders) both
pass unchanged, 28 tests. It does move engine.wasm by 142 bytes, because the
selector inlines differently once it is in the same crate as the layers it
reads - a reminder that a wasm diff is not by itself evidence of a behaviour
change. tile_index moved verbatim, NAN fallback included; writing -1.0 there
would have been a behaviour change smuggled into a code move.

Freeze the slider anti-vacuity guard at its measured 50 of 74 instead of
leaving it as "> 40". The argument for a floor was that the count is a property
of which fields read a slider rather than a result - but that is equally true
of the window guard's 74 and the tile guard's 19, and both are frozen. The
freeze is strictly stronger: a floor of 40 would sit through nine fields
silently dropping their slider dependency.

Record verify:rust's cost as a RANGE, 1m44s to 2m48s. #310 measured 1m44s on CI
against the 2m48s recorded in CLAUDE.md, on code whose Rust half did not change
between the two runs. That is the same spread the test shards show, so the
honest response is to widen the figure rather than replace it with whichever
run was seen last.

pnpm run verify green: 2143 passed, 4 skipped, verify:rust included.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AA9K55yddm5D7DPuSJareA
@wormeyman
wormeyman merged commit f87b70b into main Aug 25, 2026
9 checks passed
@wormeyman
wormeyman deleted the fix/309-narrow-incoming-coordinate branch August 25, 2026 00:36
wormeyman added a commit that referenced this pull request Aug 25, 2026
#312 merged first, so this brings its #309/#191 narrowing and the tier-2
selector move onto this branch. No conflicts - the two touch different sections
of CLAUDE.md and no shared code.

The re-run this needs anyway is a free chance to correct one number. #312's CI
measured verify:rust at 2m49s, one second ABOVE the "1m44s to 2m48s" that same
change had just recorded, which is that paragraph's own warning arriving
immediately. Three runs now read 1m44s / 2m48s / 2m49s on equivalent Rust, so
the figure is rounded to "roughly 1m45s to 2m50s" - endpoints stated to the
second invite exactly the chasing the note exists to prevent.

pnpm run verify green: 2154 passed, 4 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AA9K55yddm5D7DPuSJareA
wormeyman added a commit that referenced this pull request Aug 25, 2026
* Accept Factorio 2.1.15's exchange format (2.1.15.2)

Steam auto-updated the binary to 2.1.15 (build 87284) this morning and the
app began rejecting every map-exchange string from the shipping game. Third
time, same mechanism as 2.1.12 and 2.1.14.

The payload did NOT move. Three independent readings say so, none of them
"it looked the same":

- base/prototypes/map-settings.lua is absent from the 2.1.14 -> 2.1.15 diff
  entirely. map-settings.example.json DID change and is a red herring: it was
  catching up to the 2.1.14 default change it had missed.
- All five re-captured cases inflate to exactly the byte counts their 2.1.14
  counterparts do, controls-off's odd 750 included.
- The game's own parse of the new default string is identical to the 2.1.14
  parse across all 186 leaf fields.

So 2.1.15.2 reuses the 2.1.14 tail layout rather than getting one of its own,
and the schema constants are renamed for the FIELD they carry rather than for
a version, with the selector reading a list of tags.

Both plants were run: dropping 2.1.15.2 from SUPPORTED_VERSIONS fails six
tests with "unsupported exchange format 2.1.15.2", and giving 2.1.15 the
pre-2.1.14 tail fails five with "payload truncated: read of 8 bytes at
offset 706".

Capturing a version is now a script rather than a recipe in a comment
(scripts/probes/exchange-format/capture.ts, five cases in ~10s through
factorio-oracle). It derives each case's settings from the previous fixture
via the game's own parser, so "the cases mirror setting-for-setting" is a
mechanism instead of a claim, and re-running it reproduces both fixtures
byte-identically.

The one trap it records, measured rather than reasoned: feeding a whole
parse_map_exchange_string result back as --map-gen-settings inflates every
case from 711 to 1387 bytes, because the parse fills in all 28 autoplace
controls and the exchange string writes every control supplied explicitly.
All five cases then come out the same length, which destroys the only reason
there are five - they exist to vary the layout. Feeding back only the delta
against the default case reproduces the previous fixture's own sizes.

Also worth carrying: `factorio --version` prints `Map output version: X.Y.Z-W`
and W is the exchange tag's fourth part. Confirmed against a binary whose tag
was already known (2.1.14 prints 2.1.14-1), so it is a control rather than a
pattern match. That part is not monotonic across releases, so it cannot be
guessed from the patch number.

pnpm run verify green: 2149 passed, 4 skipped, 240 files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AA9K55yddm5D7DPuSJareA

* Accept Factorio 2.1.16's exchange format (2.1.16.0)

Wube shipped 2.1.16 the same day as 2.1.15, and this machine's Steam tracks the
experimental branch, so the binary moved twice in one day and import broke
twice. Fourth occurrence overall.

Another tag-only move. Read the same three ways as 2.1.15:

- base/prototypes/map-settings.lua is absent from the 2.1.15 -> 2.1.16 diff.
  The whole data diff is info.json version bumps and the changelog.
- All five cases re-captured at 2.1.16 inflate to exactly the byte counts their
  2.1.15 counterparts do (711/711/750/711/711).
- The game's own parse of the new default string is identical to the 2.1.15
  parse across all 186 leaf fields.

So 2.1.16.0 reuses the 2.1.14 tail layout too.

Note the tag: the fourth part FELL from 2 to 0. Across the supported list it now
reads 3, 2, 1, 2, 0 - it does not increase and does not track the patch number,
so it can only be read off the binary, never inferred. That is what the
`Map output version: X.Y.Z-W` line is for.

The capture needed no edit to chain 2.1.14 -> 2.1.15 -> 2.1.16: the script
derives its source from the newest committed strings fixture older than the
target, so the mirror follows automatically.

The spec's three near-identical version blocks became one describe.each over a
LAYOUT_HEIRS table, each entry mirrored against the version before it. The table
is deliberately NOT derived from SUPPORTED_VERSIONS - that would make the spec
agree with the codec by construction, and the tag is one of the things being
asserted.

Both plants were run again for 2.1.16: dropping the tag fails six tests with
"unsupported exchange format 2.1.16.0", and giving it the pre-2.1.14 tail fails
five with "payload truncated: read of 8 bytes at offset 706".

pnpm run verify green: 2154 passed, 4 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AA9K55yddm5D7DPuSJareA

* Merge main and widen the verify:rust range to three runs

#312 merged first, so this brings its #309/#191 narrowing and the tier-2
selector move onto this branch. No conflicts - the two touch different sections
of CLAUDE.md and no shared code.

The re-run this needs anyway is a free chance to correct one number. #312's CI
measured verify:rust at 2m49s, one second ABOVE the "1m44s to 2m48s" that same
change had just recorded, which is that paragraph's own warning arriving
immediately. Three runs now read 1m44s / 2m48s / 2m49s on equivalent Rust, so
the figure is rounded to "roughly 1m45s to 2m50s" - endpoints stated to the
second invite exactly the chasing the note exists to prevent.

pnpm run verify green: 2154 passed, 4 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AA9K55yddm5D7DPuSJareA

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

basisNoiseExpr narrows the coordinate product at a different point than the Rust port

1 participant