From 54a704230336edb5c5d289b9a396e06cf4a96d9c Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 13 Jun 2026 20:22:08 -0700 Subject: [PATCH] Resolve render settings before renderer construction (#4271) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What The client now resolves render settings (defaults + user overrides) **up front** and passes the result into the renderer, instead of the renderer constructing defaults itself and the client re-applying overrides afterward. ``` before: new GPURenderer(...) // this.settings = createRenderSettings() (defaults) view.getSettings() → deepAssign(defaults) → applyGraphicsOverrides(...) // patch after the fact after: const settings = createRenderSettings(); applyGraphicsOverrides(settings, ...); applyDarkModeOverride(settings, ...) new GPURenderer(..., settings) // built with the final values ``` ## Why - **Removes the construct-with-defaults / re-apply-overrides dance.** Every pass — including texture-baking ones like terrain that read their settings *once* at build time rather than every frame — is now built with the final values on the first try. (This is the cleanup that motivated the change, surfaced while adding a terrain color override in a separate PR.) - **Fixes a latent context-restore bug.** On WebGL context loss/restore the renderer was rebuilt via `createRenderSettings()` → fresh **defaults**, silently dropping any user overrides until the next settings change. `MapRenderer` now holds the resolved settings object and hands the same one to the recreated `GPURenderer`, so overrides survive a restore. Live setting changes still work exactly as before: `regenerateRenderSettings()` re-resolves and `deepAssign`s onto the renderer's live settings object in place (passes hold a reference, so they pick it up next frame). ## Changes - `Renderer.ts` (`GPURenderer`) — constructor takes a `settings: RenderSettings`; drops the internal `createRenderSettings()` call. - `MapRenderer.ts` — holds the resolved settings and passes it through on construction and on context-restore re-init. - `ClientGameRunner.ts` — new `resolveRenderSettings()` helper used both at construction and by `regenerateRenderSettings()`; `createWebGLView` takes the resolved settings; the now-redundant initial `regenerateRenderSettings()` call is removed. ## Testing Verified live in a headless singleplayer game: - A saved `nameScaleFactor` override is present in `getSettings()` immediately after game start, with no settings-change event fired (construction path). - A mid-game override change is reflected in the live settings (regenerate/in-place path). - The map renders correctly through spawn phase. `tsc` and ESLint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 --- src/client/ClientGameRunner.ts | 25 ++++++++++++++++++++----- src/client/render/gl/MapRenderer.ts | 5 +++++ src/client/render/gl/Renderer.ts | 8 ++++++-- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/client/ClientGameRunner.ts b/src/client/ClientGameRunner.ts index de4510085..8a67b84a4 100644 --- a/src/client/ClientGameRunner.ts +++ b/src/client/ClientGameRunner.ts @@ -74,6 +74,7 @@ import { deepAssign, MapRenderer, preloadAtlasData, + type RenderSettings, } from "./render/gl"; import { ALL_UNIT_TYPES, UnitState } from "./render/types"; import { SoundManager } from "./sound/SoundManager"; @@ -256,6 +257,7 @@ export function joinLobby( function createWebGLView( terrainMap: TerrainMapData, config: Config, + settings: RenderSettings, ): { view: MapRenderer; glCanvas: HTMLCanvasElement; @@ -311,6 +313,7 @@ function createWebGLView( terrainBytes, palette, config, + settings, captureRaf, captureCaf, ); @@ -485,9 +488,21 @@ async function createClientGame( const soundManager = new SoundManager(eventBus, userSettings); try { + // Resolve render settings (defaults + user overrides) up front so the + // renderer is built with the final values — no construct-with-defaults, + // re-apply-overrides dance, and texture-baking passes (terrain) get the + // right colors on the first build. + const resolveRenderSettings = (): RenderSettings => { + const settings = createRenderSettings(); + applyGraphicsOverrides(settings, userSettings.graphicsOverrides()); + applyDarkModeOverride(settings, userSettings.darkMode()); + return settings; + }; + const { view, glCanvas, cachedWebGLFrameCallback } = createWebGLView( gameMap, config, + resolveRenderSettings(), ); view.setShowPatterns(userSettings.territoryPatterns()); @@ -497,11 +512,10 @@ async function createClientGame( ); const graphicsListenerAbort = new AbortController(); + // Re-resolve settings and copy them onto the renderer's live object in + // place (passes hold a reference to it, so they pick the change up). const regenerateRenderSettings = (): void => { - const live = view.getSettings(); - deepAssign(live, createRenderSettings()); - applyGraphicsOverrides(live, userSettings.graphicsOverrides()); - applyDarkModeOverride(live, userSettings.darkMode()); + deepAssign(view.getSettings(), resolveRenderSettings()); }; // Re-apply render settings, then re-theme and recolor players, on a // graphics-override change (covers a theme switch such as colorblind mode). @@ -513,7 +527,8 @@ async function createClientGame( gameView.refreshPlayerColors(); webglBuilder.refreshPalette(gameView); }; - regenerateRenderSettings(); + // No initial regenerate needed — the renderer was constructed with the + // resolved settings above. globalThis.addEventListener( `${USER_SETTINGS_CHANGED_EVENT}:${GRAPHICS_KEY}`, onGraphicsChanged, diff --git a/src/client/render/gl/MapRenderer.ts b/src/client/render/gl/MapRenderer.ts index 4ecc9c31b..423b9189a 100644 --- a/src/client/render/gl/MapRenderer.ts +++ b/src/client/render/gl/MapRenderer.ts @@ -50,6 +50,10 @@ export class MapRenderer { private terrainBytes: Uint8Array, private paletteData: Float32Array, private config: Config, + // Resolved render settings (defaults + overrides). Held so the same object + // is re-used when a GPURenderer is recreated after a context restore, + // preserving any user overrides that were applied to it. + private settings: RenderSettings, private raf?: typeof requestAnimationFrame, private caf?: typeof cancelAnimationFrame, ) { @@ -78,6 +82,7 @@ export class MapRenderer { this.terrainBytes, this.paletteData, this.config, + this.settings, this.raf, this.caf, ); diff --git a/src/client/render/gl/Renderer.ts b/src/client/render/gl/Renderer.ts index a5d651228..3231204b5 100644 --- a/src/client/render/gl/Renderer.ts +++ b/src/client/render/gl/Renderer.ts @@ -57,7 +57,7 @@ import { TerritoryPass } from "./passes/TerritoryPass"; import { TrailPass } from "./passes/TrailPass"; import { UnitPass } from "./passes/UnitPass"; import { WorldTextPass } from "./passes/WorldTextPass"; -import { createRenderSettings, type RenderSettings } from "./RenderSettings"; +import type { RenderSettings } from "./RenderSettings"; import { AffiliationPalette } from "./utils/Affiliation"; import { buildTerrainRGBA, getPaletteSize } from "./utils/ColorUtils"; import { @@ -180,11 +180,15 @@ export class GPURenderer { terrainBytes: Uint8Array, paletteData: Float32Array, config: Config, + settings: RenderSettings, raf: typeof requestAnimationFrame = requestAnimationFrame.bind(window), caf: typeof cancelAnimationFrame = cancelAnimationFrame.bind(window), ) { this.canvas = canvas; - this.settings = createRenderSettings(); + // Settings are resolved (defaults + user overrides) by the caller and + // passed in, so every pass — including texture-baking ones like terrain — + // is built with the final values. Live changes mutate this object in place. + this.settings = settings; this.raf = raf; this.caf = caf;