feat: cool ambient in wreck interiors, plus the #124 optics guard - #136
Open
N1k4G wants to merge 5 commits into
Open
feat: cool ambient in wreck interiors, plus the #124 optics guard#136N1k4G wants to merge 5 commits into
N1k4G wants to merge 5 commits into
Conversation
Closes the wreck half of #124. The cave was done in #135; this is the same fix in the wreck's equivalent surface, which turned out to be somewhere else entirely. Per-pixel chroma, torch on, gloom settled, 1280x800: wreck-engine-room 14.8 -> 21.5 (+45%) luma 58.3 -> 58.5 wreck-cargo-hold 19.4 -> 27.9 (+44%) luma 64.4 -> 64.7 wreck-crew-deck 22.8 -> 31.8 (+39%) luma 66.2 -> 66.5 wreck-vehicle-deck 26.6 -> 35.4 (+33%) luma 61.4 -> 62.0 cave-upper-tunnel 19.9 -> 19.9 unchanged cave-restriction 20.5 -> 20.5 unchanged cave-cathedral 24.2 -> 24.2 unchanged wreck-exterior-bow 109.6 -> 109.8 noise reef-open-water 68.4 -> 68.3 noise Mean luminance moves by at most 0.6, which is what #124 asked for: this is not a brightening exercise, and a torch-lit overhead should still read as dark. The Pixi visual guard reports 0% changed pixels, confirming the change is confined to the legacy client. The interesting part is where the wreck's gloom lives. #124's last comment predicted the lever would be localised high-chroma detail on the interior panels, since the "tint the steel" family was already ruled out by measurement. Both readings assumed the steel was what the measurements were seeing. It was not. drawWreckHullSkin ends with a radial gradient whose stated job is feathering the rim of the torch's near-field circle. But a radial gradient holds its last stop past its outer radius, and this one fills the entire hull silhouette, so past about 53 px from the diver it paints rgba(28,33,38) at alpha 0.9 over essentially the whole interior frame. Chroma 10, near-opaque, everywhere: that is what the interior measurements were measuring, and it is the wreck's exact analogue of the cave gloom overlay #135 recoloured. drawSiltAndTorch returns early for wrecks, which is why the cave fix could not reach it and why #124 came away half closed. Recoloured to a cool blue-teal at matched luminance (31.2 against 32.3), so the interior stays exactly as dark while the warm torch from #135 and the rusting steel finally have a temperature to separate from. The localised-detail lead was built and reverted, with the finding recorded at the call site next to the earlier steel-tint one. A per-plate pass of rust blooms, scale flakes and seam tears, scaled by the zone's #56 accumulation, moved frame chroma by less than 0.5 even with every alpha at 1.0, and moved a box around the torch-lit area by 34.8 -> 34.7, 30.2 -> 29.7, 26.0 -> 26.5 — noise in both directions. The gloom is why: only a tenth of any backdrop surface detail survives it, and inside the bubble the interior structures cover the backdrop anyway. Surface detail painted there cannot reach the frame in either region, so that whole family is now ruled out by measurement rather than left as an open lead. Cooling the per-zone atmosphere tints in sites.js was also tried, and does work on the metric — the warm skew of those tints tracks the flatness monotonically, engine room [1.12,0.90,0.78] being both the warmest and the flattest. Not taken: it pushed the interior toward generic blue water and away from flooded steel, and those per-zone values are deliberately authored character. Left alone in favour of the single-surface fix. Reference frames and thresholds are still not set. They were deliberately held until the wreck was done, which it now is, so they are unblocked — but they are a regression-guard task rather than an art one and want their own issue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes the last item on #124. The art direction was settled and both halves implemented (#135 cave, #136 wreck); this is the guard that keeps them, and it is the piece the issue asked be done LAST, deliberately, so the thresholds describe the intended look rather than the shortfall. npm run interior:check verify (CI-gated in pr.yml and deploy.yml) npm run interior:update re-record the reference FRAMES, deliberately Nine scenes: four wreck interiors, three cave interiors, two open-water controls. Each has a band on per-pixel chroma and one on mean luminance. WHY THRESHOLDS ARE CODE AND FRAMES ARE DATA --update re-records the PNGs and does not touch the thresholds. That split is the whole point of #124's sequencing warning: thresholds derived from what is currently on screen describe the current state, so recording them while the current state is the shortfall locks the shortfall in. The bands therefore live in the script with their derivation attached, and moving one is a deliberate edit visible in review rather than a side effect of a red run. HOW THE FLOORS WERE PLACED Each floor sits midway between the scene's measured flat state and its measured fixed state — the placement with the most headroom either side that still fails the regression it exists to catch: scene flat -> fixed floor catches headroom wreck-vehicle-deck 26.4 -> 35.5 31.0 +4.6 -4.5 wreck-crew-deck 22.6 -> 31.7 27.0 +4.4 -4.7 wreck-cargo-hold 19.1 -> 27.8 23.5 +4.4 -4.3 wreck-engine-room 14.3 -> 21.1 17.5 +3.2 -3.6 cave-upper-tunnel 10.1 -> 17.8 14.0 +3.9 -3.8 cave-restriction 12.2 -> 19.4 15.5 +3.3 -3.9 cave-cathedral 16.6 -> 23.6 20.0 +3.4 -3.6 Both columns were measured by this script by reverting the two gloom colours and re-running it, so every floor is calibrated against a real state of this codebase rather than an estimate. Verified three ways: clean passes all nine; reverting #136 fails exactly the four wreck interiors; reverting #135 fails exactly the three cave ones. Controls hold in both. Run-to-run noise is under 0.1 across three consecutive full runs, so the whole 3.2-4.7 of headroom is there for cross-platform drift, which could not be measured — CI is linux and there is no linux node here. If linux sits further out than that, the first run says so with the number, and the fix is to widen the one band that moved. TWO CONTAMINANTS THE CROP HAD TO BE MOVED OFF #124's baseline came with a caveat that its crop still caught fixed chrome, and a note to tighten it before deriving thresholds. Doing that turned up two, and both were found by measurement rather than by eye, which is the part worth keeping: The toasts are painted onto the CANVAS, not the DOM, at y fractions 0.18, 0.30 and 0.36 — amber text, the highest-chroma pixels in frame, with the hint line refilling from a queue on a timer. They were worth 0.6-0.9 chroma and 6 SD. Suppressed at source, and the run now asserts they are silent; clearing them from one evaluate and measuring from the next left room for a frame in between that re-armed one, which is why settle, silence and read are now a single evaluation. The dive computer looks like DOM and is not: it is on the canvas at x >= 880, and a window running to 0.80 had its corner inside, worth 1.79% of the measured pixels. The window is 0.48 wide instead. Rather than trust a crop that looked right once, assertNoChrome now proves it every run: two scenes sharing no scenery (wreck-engine-room, reef-open-water) cannot legitimately agree pixel for pixel, so anything byte-identical across both is chrome. Measured 0.03%, against a 0.5% budget and the 1.79% the gauge corner cost. That is what stops this crop rotting the next time the HUD moves — #125 alone reflowed the result screens and grew the touch targets. Moving the window below the toasts instead was tried and rejected: at that offset it slides off the interior onto the seabed, taking the engine room's contrast from SD 17.1 to 7.9. WHY THE FRAMES ARE NOT BYTE-COMPARED #133 established Playwright frames are deterministic per platform but not across them. There is no linux set here, so the frames are committed as review artefacts — cut from the same canvas pixels the statistics read, so the picture and the numbers beside it cannot disagree, and sized to the sample window, which also keeps nine frames from outweighing the rest of the repo's history. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…124) The thresholds were derived on win32 and CI runs linux, and there was no linux node here to check the difference — so the headroom was sized for a drift that could only be reasoned about. The first CI run measured it. wreck-vehicle-deck 35.5 -> 35.5 cave-upper-tunnel 17.8 -> 17.7 wreck-crew-deck 31.7 -> 31.8 cave-restriction 19.4 -> 19.3 wreck-cargo-hold 27.8 -> 27.8 cave-cathedral 23.6 -> 23.6 wreck-engine-room 21.1 -> 21.6 wreck-exterior-bow 113.1 -> 114.3 reef-open-water 54.2 -> 54.1 At most 0.5 on any interior and 1.2 on a control, against 3.2-4.7 of headroom. This confirms the design choice rather than just permitting it. #133 found Playwright frames differ across platforms by a max channel delta of 230 across 10.6% of pixels, which is why it keeps a reference set per platform and why this guard enforces statistics instead. Aggregating over ~63k samples averages those per-pixel rasteriser differences away: the numbers travel even though the frames they came from do not. The practical consequence is worth stating where the bands are. The headroom is no longer mostly reserved for an unmeasured platform gap — it is slack for future art changes that are meant to happen, so a band that starts failing is far more likely to be a real change than an environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three review findings on the optics guard, the first of which made one of its assertions worthless. THE OVERHEAD ASSERTION WAS SELF-FULFILLING The harness set `inOverhead = true` for every interior scene and then asserted `inOverhead === true`. That is not a check; it is the harness reading back its own input, and it could never fail. The failure it was supposed to catch is the quiet one. In play, inOverhead is recomputed every tick from world geometry — `overheadAt(diverX, depth)`. If the wreck or cave geometry moves, or one of these hard-coded sample positions stops being inside the room it is named after, forcing the flag would keep the guard green while measuring a view the player could never be in. The thresholds would then be defending a scene that does not exist. Now the diver's position is re-pinned each frame and the game's own updateOverheadState() derives inOverhead exactly as it does in play. The assertion checks both the geometry predicate and the flag the simulation settled on, for interiors AND controls — asserting wreck-exterior-bow is NOT in an overhead is a real check on that control, which previously had none. Only the two animation ramps are still pinned, and they settle to whatever the simulation currently believes rather than to a value chosen here. Those are easing state, not world state: pinning them skips a transition, it does not assert anything about where the diver is. Verified against the game as it stands: all nine positions are genuinely correct — the seven interiors really are inside overheads, both controls really are not. Verified the assertion can now fail, in both directions: moving wreck-engine-room outside the hull (x=200) and mislabelling reef-open-water as an interior each abort with the real geometry and rested position. UPDATE MODE COULD DESTROY THE REFERENCES IT EXISTS TO PROTECT The chrome probe needs two scenes before it can say anything, so it ran after the sweep — but update mode wrote each PNG as it went. A HUD change that moved chrome into the sample window could therefore overwrite all nine references with contaminated captures and only then fail the probe, destroying them in the course of discovering they could not be trusted. Captures are now buffered and nothing reaches disk until the window is validated. Verified by widening CROP back over the dive computer: the probe aborts at 1.81% and every reference is byte-identical afterwards. UPDATE MODE IGNORED LUMA BREACHES The warning promised to flag a frame recorded outside "the thresholds" but checked only chroma, so a reference could be recorded at any brightness in silence — precisely the "this is not a brightening exercise" requirement the luma bands exist to hold. It now calls breachesFor, the same function the check path uses, so there is one definition of acceptable, and summarises how many frames were recorded out of band. Verified by tightening a luma band alone. RE-DERIVED THRESHOLDS Removing the forced state shifted some absolutes — most by under 0.1, the engine room by 1.4 and the exterior bow by 3.1 — so flat and fixed were both re-measured on the corrected harness and two floors recentred on their new midpoints (engine room 17.5 -> 18.5, cave upper tunnel 14.0 -> 13.5), plus the bow's luma band. Run-to-run noise is unchanged at under 0.1 over three runs. scene flat -> fixed floor catches headroom wreck-vehicle-deck 26.4 -> 35.5 31.0 +4.6 -4.5 wreck-crew-deck 22.5 -> 31.7 27.0 +4.5 -4.7 wreck-cargo-hold 19.1 -> 27.9 23.5 +4.4 -4.4 wreck-engine-room 15.0 -> 22.5 18.5 +3.5 -4.0 cave-upper-tunnel 9.8 -> 17.5 13.5 +3.7 -4.0 cave-restriction 12.0 -> 19.2 15.5 +3.5 -3.7 cave-cathedral 16.7 -> 23.6 20.0 +3.3 -3.6 Reverting #136 still fails exactly the four wreck interiors and reverting #135 exactly the three cave ones, with the controls holding in both. Frames re-recorded on the corrected harness. The recorded cross-platform comparison predates this change, so its paired figures no longer match what the script prints. The conclusion is about how far win32 and linux sit apart on identical code and carries over unchanged; the note now says so rather than quoting numbers that would not reproduce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit removed the win32/linux comparison table because the
harness fix had shifted the absolutes it was measured on, leaving only the
conclusion. CI has now re-run it on the corrected harness, and the result is
better than what it replaced:
wreck-vehicle-deck 35.5 / 35.5 cave-upper-tunnel 17.5 / 17.5
wreck-crew-deck 31.7 / 31.7 cave-restriction 19.2 / 19.2
wreck-cargo-hold 27.9 / 27.9 cave-cathedral 23.6 / 23.6
wreck-engine-room 22.5 / 22.5 wreck-exterior-bow 116.2 / 116.1
reef-open-water 53.9 / 53.9
Every interior agrees exactly and the controls to within 0.1, on renderers
whose individual pixels differ by a max channel delta of 230 across 10.6% of
them (#133). Aggregating over ~63k samples averages the rasteriser differences
away completely, which is the assumption this guard is built on.
The earlier measurement of up to 0.5 drift on interiors and 1.2 on a control
was not the platforms. It was this script letting the diver drift slightly
between pinning position and reading pixels; re-pinning every frame — done for
the overhead-assertion fix, not for this — removed it. Recorded at the bands,
because it inverts the natural assumption: when these numbers move, suspect the
harness before the environment.
Co-Authored-By: Claude Opus 5 <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 #124 — both the wreck half of the art direction and the reference frames and thresholds the issue asked be done last.
1. The wreck reads cool now (
src/renderer.js)The cave was fixed in #135. This is the same fix applied to the wreck's equivalent surface, which turned out to be somewhere else entirely.
Mean luminance moves by at most 0.6 — deliberately. #124 asked that this not be a brightening exercise, and a torch-lit overhead should still read as dark.
npm run pixi:visual-checkreports 0% changed pixels, so the change is confined to the legacy client.Where the wreck's gloom actually was. #124's last comment predicted the lever would be localised detail on the interior panels, the "tint the steel" family having already been ruled out. Both readings assumed the steel was what the measurements were seeing. It was not.
drawWreckHullSkinends with a radial gradient documented as feathering the rim of the torch's near-field circle. A radial gradient holds its last stop past its outer radius, and this one fills the whole hull silhouette — so past ~53 px from the diver it paintsrgba(28,33,38)at alpha 0.9 over essentially the entire interior frame. Chroma 10, near-opaque, everywhere. That is the wreck's exact analogue of the cave gloom overlay, anddrawSiltAndTorchreturning early for wrecks is why #135 could not reach it. Recoloured to a cool blue-teal at matched luminance (31.2 vs 32.3).Two leads closed with evidence rather than left open, both recorded at the call site next to the earlier steel-tint finding:
2. Reference frames and thresholds (
scripts/interior-optics-check.mjs)Nine scenes — four wreck interiors, three cave interiors, two open-water controls — each with a band on per-pixel chroma and one on mean luminance.
Thresholds are code; frames are data.
--updatere-records the PNGs and never touches a threshold. That split is the point of #124's sequencing warning: thresholds derived from what is currently on screen describe the current state, so recording them while that state is the shortfall locks the shortfall in.Floors sit midway between each scene's measured flat and fixed states — most headroom either side that still catches the regression:
Both columns were measured by this script, by reverting the two gloom colours and re-running it — every floor is calibrated against a real state of this codebase, not an estimate.
Verified three ways: clean passes all nine; reverting #136 fails exactly the four wreck interiors; reverting #135 fails exactly the three cave ones. The controls hold in both cases.
Two contaminants the crop had to be moved off. #124 flagged that its baseline crop still caught fixed chrome. Tightening it turned up two, both found by measurement rather than by eye:
Rather than trust a crop that looked right once,
assertNoChromeproves it every run: two scenes sharing no scenery cannot legitimately agree pixel for pixel, so anything byte-identical across both is chrome. Measures 0.03% against a 0.5% budget. That is what stops the crop rotting next time the HUD moves — #125 alone reflowed the result screens.Moving the window below the toasts instead was tried and rejected: it slides off the interior onto the seabed, taking the engine room's contrast from SD 17.1 to 7.9.
Frames are review artefacts, not a pixel diff. #133 established Playwright frames are deterministic per platform but not across them, and there is no linux set here. They are cut from the same canvas pixels the statistics read, so picture and numbers cannot disagree, and sized to the sample window.
Cross-platform drift: measured, not assumed
This started as a stated unknown — thresholds derived on win32, CI on linux, no linux node here to check. CI answered it, on the corrected harness:
Every interior agrees exactly and the controls to within 0.1 — on renderers whose individual pixels differ by a max channel delta of 230 across 10.6% of them (#133). Aggregating over ~63k samples averages the rasteriser differences away completely, which is the assumption this guard is built on.
An earlier run measured up to 0.5 drift on interiors and 1.2 on a control. That turned out not to be the platforms: it was the harness letting the diver drift slightly between pinning position and reading pixels, and re-pinning every frame — done for the overhead-assertion fix, not for this — removed it. Worth knowing, because it inverts the natural assumption: when these numbers move, suspect the harness before the environment.
So the headroom is not reserved for an environment gap at all. It is slack for art changes that are meant to happen, which means a band that starts failing is overwhelmingly likely to be a real change.
Review round 2 (932910a)
Three findings, all valid; the first had made an assertion worthless.
The overhead assertion was self-fulfilling. The harness set
inOverhead = truefor interior scenes and then asserted it was true — reading back its own input. The failure that mattered was the quiet one: if geometry moves, or a hard-coded sample position leaves the room it is named after, forcing the flag keeps the guard green while measuring a view no player could reach. Now the position is re-pinned each frame and the game's ownupdateOverheadState()derivesinOverhead = overheadAt(diverX, depth)as it does in play; the assertion checks both the geometry predicate and the settled flag, for interiors and controls — assertingwreck-exterior-bowis not in an overhead is a real check that control never had. Only the two animation ramps stay pinned, and they settle to whatever the simulation believes: easing state, not world state.Verified all nine positions are genuinely correct against the current geometry, and that the assertion can now fail in both directions — moving
wreck-engine-roomoutside the hull, and mislabellingreef-open-wateras an interior, each abort with the real geometry and rested position.Update mode could destroy the references it exists to protect. The chrome probe can only run after two scenes, but update wrote each PNG as it went — so a HUD change could overwrite all nine references with contaminated captures and only then fail the probe. Captures are now buffered; nothing reaches disk until the window is validated. Verified by widening
CROPback over the dive computer: aborts at 1.81%, every reference byte-identical afterwards.Update mode ignored luma breaches. It promised to flag frames outside "the thresholds" but checked chroma only, so a reference could be recorded at any brightness in silence. It now calls
breachesFor— one definition of acceptable for both paths — and summarises how many frames were recorded out of band. Verified by tightening a luma band alone.Thresholds re-derived. Removing the forced state shifted some absolutes (most under 0.1; engine room 1.4, exterior bow 3.1), so flat and fixed were re-measured on the corrected harness and two floors recentred on their new midpoints. Noise unchanged at under 0.1 over three runs. Both regressions still isolate correctly; frames re-recorded.
Verification
npm run lint,typecheck,sites:check,test:unit(133),test:parity(28),test:e2e(36),pixi:visual-check,interior:check(9 + chrome probe) — all pass.🤖 Generated with Claude Code