Narrow the incoming coordinate to f32 (#309, #191) - #312
Merged
Conversation
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
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>
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.
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
xandyare already f32 when it reads them, and theinput_scalemultiply 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_basisis a multioctave read at Fulgora's distorted coordinatewx = ox + wobble_x * wobble_mask, computed in f64 and therefore off the f32 grid at 55 of that fixture's 101 positions. ScoringsumOctaveswith and without its incoming narrowing: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
basisNoiseExprnarrowsxandybefore theinput_scalemultiply, 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_xis 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.variablePersistenceMultioctaveNoisenarrowsy, in both ports. It narrowedxonly as a side effect of thef32(x + offset_x)add;yhad 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.quickMultioctaveNoiseneeded 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 atquickMultioctaveNoise.ts:192-193. Read the code, not the issue.basisNoiseitself 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 gridnow asserts 0 of 74 diverging where it froze 32, and revertingprimitives.tsreproduces[ '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
fmw-noise.VulcanusParitysits besideVulcanusStack, soelevation_fieldsandtemperature- which no render path reads - are private again instead ofpubon a library type solely for a test. Pure code motion, checked as such: tier 2 and tier 3 pass unchanged. It movesengine.wasmby 142 bytes from inlining, which is a reminder that a wasm diff is not by itself evidence of a behaviour change.> 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 verifygreen: 2143 passed, 4 skipped,verify:rustincluded.🤖 Generated with Claude Code
https://claude.ai/code/session_01AA9K55yddm5D7DPuSJareA