feat: cool ambient and warmer torch in cave interiors - #135
Merged
Conversation
Partial progress on #124. The cave is done and measured; the wreck is not. Two changes, both in the legacy renderer: The cave gloom overlay was rgba(2,5,9) at alpha 0.80-0.93 — so close to black that it erased hue rather than tinting it. Now a deep blue-teal, rgba(3,11,21), which keeps the darkness while leaving the ambient with a temperature for the torch to read against. The torch glow was already warm-to-cool but too faint to separate anything. Strengthened, and its falloff goes amber to teal rather than amber to grey-blue. Measured per-pixel chroma, torch on, gloom settled: cave-cathedral 16.7 -> 23.4 (+40%) luma 49.9 -> 53.1 cave-upper-tunnel 12.8 -> 20.2 (+58%) luma 55.8 -> 59.1 cave-restriction 12.4 -> 19.4 (+56%) luma 59.4 -> 62.4 reef-open-water 122.5 -> 122.5 unchanged wreck-exterior-bow 95.6 -> 95.6 unchanged Luminance rises about 3 of ~55, which is at the edge of "roughly stable". A measurement correction matters more than the change itself. #124 states its targets in mean channel spread, and that metric cannot see the look it asks for: warm highlights against cool ambient average toward neutral across a frame, so a scene can gain exactly the requested separation while its frame-mean spread falls. Per-pixel chroma — mean of max(R,G,B) - min(R,G,B) — measures how colourful each pixel is regardless of what the frame averages to, and is what the numbers above use. That correction is also how a wrong change got caught. Rusting the wreck's steel backdrop at matched luminance looked right by reasoning and measured worse on both metrics: the scene is blue-dominant from the depth tint, so warming a large surface pulls it toward neutral rather than away. Reverted, with the finding recorded at the call site so the next attempt does not repeat it. The wreck therefore still measures near-neutral, and its lever is elsewhere: drawSiltAndTorch returns early for wrecks, so it never had the ambient overlay this change fixes. No reference frames or thresholds yet, deliberately. Deriving them from a half-finished state would bake in the shortfall, which is the sequencing #124 explicitly warns against. The Pixi visual guard reports 0% changed pixels, confirming this is confined to the legacy client. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
added
the
release:minor
N1k4G
commented
Aug 24, 2026
N1k4G
left a comment
N1k4G
left a comment
Owner
Author
There was a problem hiding this comment.
Reviewed 18dc90 — no blocking findings.\n\nI checked the full render-path diff and paired before/after captures at the cathedral, upper tunnel, restriction, torch-off cave, and wreck vehicle deck.\n\nVisually, the cave change reads in the intended direction: the ambient is modestly cooler without becoming brighter or washed out, while the torch cone remains locally warmer and more distinct. Torch-off stays appropriately dark but readable. The shared glow change is visible in the wreck only as a small local warmth increase; I did not see an exterior or interior regression.\n\nLocal verification:\n- lint: pass\n- unit: 133/133\n- parity: 28/28\n- build/typecheck: pass\n- Pixi visual guard: 0% changed pixels across its three existing scenes\n\nThe remaining wreck treatment and legacy reference frames/thresholds are still open work under #124, consistent with this PR's stated partial scope.
N1k4G
added a commit
that referenced
this pull request
Aug 25, 2026
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>
N1k4G
added a commit
that referenced
this pull request
Aug 25, 2026
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>
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
1 participant
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.
Partial progress on #124 — the cave is done and measured, the wreck is not. Deliberately scoped that way: they have different causes in different subsystems, and holding a working cave improvement for the wreck helps nobody.
The change
Two edits, both in the legacy renderer:
rgba(2,5,9)at alpha 0.80–0.93 — close enough to black that it erased hue rather than tinting it. Now a deep blue-tealrgba(3,11,21), which keeps the darkness while leaving the ambient with a temperature for the torch to read against.Measured
Per-pixel chroma, torch on, gloom fully settled:
Open water is provably untouched, and the Pixi visual guard reports 0% changed pixels, confirming this is confined to the legacy client.
Luminance rises about 3 of ~55. Chroma and luminance both come from the same overlay so they trade against each other —
(6,20,34)gave +8,(4,14,25)+5,(3,11,21)+3, each step costing chroma. Accepted at +3.A measurement correction, which matters more than the change
#124 states its targets in mean channel spread, and that metric cannot see the look it asks for. Warm highlights against cool ambient average toward neutral across a frame, so a scene can gain exactly the requested separation while its frame-mean spread falls.
Per-pixel chroma — mean of
max(R,G,B) − min(R,G,B)— measures how colourful each pixel is regardless of what the frame averages to. Every number above uses it. Noted on #124 so thresholds are not later derived from the wrong measure.A wrong change, caught and recorded
I rusted the wreck's steel backdrop at matched luminance — correct by the stated direction, wrong in practice. It measured worse on both metrics (vehicle deck 26.1 → 25.1, engine room 14.1 → 13.1): the scene is blue-dominant from the depth tint, so warming a large surface pulls it toward neutral rather than away.
Reverted, with the finding recorded at the call site so the next attempt does not repeat it. It also rules out the whole "tint the steel" family for the wreck.
What is not here
drawSiltAndTorchreturns early for wrecks, so it never had the ambient overlay this fixes. The likely lever is localised high-chroma detail rather than large-surface tinting — interior panels already get rust streaks gated on an accumulation value, and raising their intensity is the same mechanism aimed correctly. Untested hypothesis.Verification
typecheck,lint,sites:check,build, 133 unit, 28 parity, 36 e2e, Pixi guard at 0% changed pixels.Before/after captures of both cave scenes were reviewed separately. The numbers say the change does what was asked; whether it reads right is a judgement I cannot make.