diff --git a/src/services/engine/engine.ts b/src/services/engine/engine.ts index 0113d3ac..e045ded1 100644 --- a/src/services/engine/engine.ts +++ b/src/services/engine/engine.ts @@ -146,6 +146,7 @@ import { createSpaceMouseSubsystem } from './spaceMouseSubsystem'; import { createClickResolver } from './clickHandler'; import { attachEngineInputs } from './inputBindings'; import { renderFrame } from './renderFrame'; +import { buildSettersFromTable, type SettingsTableKey } from './settingsTable'; /** * Start the WebGPU engine on `canvas`. @@ -1781,105 +1782,28 @@ export function createEngine(canvas: HTMLCanvasElement, cb: EngineCallbacks): En // ── Settings panel setters ───────────────────────────────────────────── // - // Each setter mutates the corresponding `state.*` field and fires the - // optional callback so subscribed React state stays in sync. The new - // value takes effect on the very next rendered frame. - - setPointSize(sizePx) { - state.settings.pointSizePx = sizePx; - cb.onPointSizeChange?.(sizePx); - state.subsystems.scheduler.requestRender(); - }, - - setBrightness(value) { - state.settings.brightness = value; - cb.onBrightnessChange?.(value); - state.subsystems.scheduler.requestRender(); - }, - - setAutoRotate(enabled) { - state.settings.autoRotate = enabled; - cb.onAutoRotateChange?.(enabled); - // Wake the loop — if previously idle, the new autoRotate=true - // keeps it ticking via the still-animating predicate; if - // toggling off, this single render lets the next frame body - // observe `autoRotate=false` and let the loop sleep. - state.subsystems.scheduler.requestRender(); - }, - - setGalaxyTexturesEnabled(enabled) { - // The per-frame loop reads `state.settings.galaxyTexturesEnabled` - // directly, so the toggle takes effect on the very next rendered - // frame — no extra signalling needed. We still echo via the - // optional callback so any subscribed React state mirrors the - // engine truth (same pattern as the other settings setters above). - state.settings.galaxyTexturesEnabled = enabled; - cb.onGalaxyTexturesEnabledChange?.(enabled); - state.subsystems.scheduler.requestRender(); - }, - - setMilkyWayEnabled(enabled) { - // Mirror of `setGalaxyTexturesEnabled`: mutate the per-frame - // setting bag in place (the render-on-demand scheduler will - // notice the next tick) and fire the echo callback so React's - // SettingsPanel state stays in sync with the engine truth. - state.settings.milkyWayEnabled = enabled; - cb.onMilkyWayEnabledChange?.(enabled); - state.subsystems.scheduler.requestRender(); - }, - - setFilamentsEnabled(enabled) { - // Toggle the cosmic-web filament-skeleton overlay. Mirrors the - // `setMilkyWayEnabled` setter shape (mutate the settings bag, - // request render) but DOES NOT fire an echo callback — App.tsx - // owns the boolean state for this toggle and updates it - // optimistically alongside calling this setter (see App.tsx's - // `onFilamentsChange`), so an engine echo would be redundant. - // The asymmetry vs. galaxyTextures/milkyWay is deliberate: the - // older toggles pre-date that pattern and would need a full - // App.tsx rewire to switch, which isn't this task's scope. - state.settings.filamentsEnabled = enabled; - state.subsystems.scheduler.requestRender(); - }, - - setFilamentIntensity(value) { - // Filament overlay intensity scale, [0, 1]. Same App-owns-state - // pattern as setFilamentsEnabled — no echo callback, optimistic - // update on the React side, engine just mutates + requests render. - // The shader reads the value via the per-frame uniform. - state.settings.filamentIntensity = Math.max(0, Math.min(1, value)); - state.subsystems.scheduler.requestRender(); - }, - - setHighlightFallback(enabled) { - // Tints fallback-orientation rows magenta (see fragment shader). - // Read by the per-frame draw call, so flipping it takes effect on - // the very next rendered frame. - state.settings.highlightFallback = enabled; - cb.onHighlightFallbackChange?.(enabled); - state.subsystems.scheduler.requestRender(); - }, - - setRealOnlyMode(enabled) { - // `discard`s fragments belonging to fallback rows so the user sees - // only galaxies with measured (b/a, PA). Same per-frame uniform - // path as the highlight toggle. - state.settings.realOnlyMode = enabled; - cb.onRealOnlyModeChange?.(enabled); - state.subsystems.scheduler.requestRender(); - }, - - setDepthFadeEnabled(enabled) { - // Toggles the per-galaxy camera-distance alpha fade — when on, - // the fragment shader multiplies alpha by - // `1 / (1 + (camDist / 1000Mpc)²)` so galaxies far behind the - // origin contribute less, breaking up the depth-column saturation - // at the centre of the catalog. Same per-frame uniform path as - // the other UI booleans. - state.settings.depthFadeEnabled = enabled; - cb.onDepthFadeEnabledChange?.(enabled); - state.subsystems.scheduler.requestRender(); - }, + // The thirteen "boring" setters (`setPointSize`, `setBrightness`, … + // `setExposure`, `setToneMapCurve`) all share the same body shape: + // mutate one field on `state.settings.*` (or `state.bias.*`), fire + // an optional echo callback, request a render. Rather than spell + // them out one-by-one, we build them from a declarative descriptor + // table in `./settingsTable.ts` and spread the result into the + // public-handle literal. See that module's docstring for the + // why-a-table / why-bespoke-stays-inline rationale. + // + // Bespoke setters that DO NOT fit the table — `setBiasMode` (async + // worker bake), `setTier` (per-source slot reload), `setLodMode` + // (couples to camera distance), `setSourceVisible` (mask math + + // implicit LOD-mode switch), `setSpaceMouseSensitivity` (subsystem + // forward) — keep their hand-rolled bodies below. + // `satisfies` here is the safety net the settingsTable docstring + // advertises: if the builder's return shape ever drifts away from + // `Pick` (e.g. a renamed key, or + // a value type that's not assignable due to contravariance), tsc + // catches it at this spread site rather than at distant callers. + ...(buildSettersFromTable(state, cb, () => + state.subsystems.scheduler.requestRender(), + ) satisfies Pick), setBiasMode(mode) { // Forwarded into the per-frame uniform on the next draw. The @@ -1916,49 +1840,6 @@ export function createEngine(canvas: HTMLCanvasElement, cb: EngineCallbacks): En state.subsystems.scheduler.requestRender(); }, - setAbsMagLimit(absMag) { - // Threshold used by `BiasMode.VolumeLimited`. Galaxies with absolute - // magnitude *fainter* than this (M > absMag, since fainter = larger - // M) are discarded in the vertex stage. Seeded at engine init from - // the closure default (-19, the SDSS spec sample limit); subsequent - // calls overwrite that. - state.bias.absMagLimit = absMag; - cb.onAbsMagLimitChange?.(absMag); - state.subsystems.scheduler.requestRender(); - }, - - setExposure(value) { - // Clamp into a sane range so a runaway slider or a debug - // console call (e.g. `setExposure(1e9)`) can't blow out the - // float buffer or, on the lower end, multiply the HDR signal - // by zero and produce a black frame the user can't recover - // from. 0.05 keeps a faint signal visible; 16 is well past - // any realistic peak (~5-10 in the densest cluster cores). - state.settings.exposure = Math.max(0.05, Math.min(16, value)); - // Echo the *clamped* value back to the UI so the slider's - // displayed number agrees with what the shader actually uses. - // Mirrors the setToneMapCurve / setBiasMode pattern: always - // fire (even on no-op identical values) so the first call - // seeds React state correctly without a separate code path. - cb.onExposureChange?.(state.settings.exposure); - state.subsystems.scheduler.requestRender(); - }, - - setToneMapCurve(curve) { - // Forwarded into the per-frame uniform on the next draw. The - // shader branches on the integer value (0=Linear, 1=Reinhard, - // 2=Asinh, 3=Gamma2, 4=Aces) so flipping this from devtools or - // the SettingsPanel takes effect on the next rendered frame - // without any pipeline rebuild. - // - // Always fire the echo callback — even when `curve === state.settings.toneMapCurve` - // — so the UI seeds correctly on first call (mirrors the - // setBiasMode pattern). - state.settings.toneMapCurve = curve; - cb.onToneMapCurveChange?.(curve); - state.subsystems.scheduler.requestRender(); - }, - resetCamera() { // `state.cam` may be null if the engine is destroyed or the cloud // hasn't loaded yet. We keep `state.initialCamSnapshot` declared in the diff --git a/src/services/engine/settingsTable.ts b/src/services/engine/settingsTable.ts new file mode 100644 index 00000000..49e018a7 --- /dev/null +++ b/src/services/engine/settingsTable.ts @@ -0,0 +1,307 @@ +/** + * settingsTable — declarative table-driven builder for the engine's + * "boring" public-handle setters. + * + * ### Why a table? + * + * Thirteen of the setters on `EngineHandle` (`setPointSize`, + * `setBrightness`, `setExposure`, …) all share the same three-step shape: + * + * 1. mutate one field in `state.settings.*` (or `state.bias.*`), + * 2. fire an optional echo callback so subscribed React state mirrors + * the engine truth, + * 3. call `requestRender()` to wake the render-on-demand scheduler. + * + * Spelled out one-by-one in `engine.ts`'s public-handle object literal, + * those thirteen setters consumed ~180 lines of nearly-identical code + * with the only variation being the path tuple, the callback name, and + * occasionally a clamp. The repetition is hard to scan ("did we + * remember to call requestRender in *all* of them?") and easy to + * silently regress when a new setting gets added without one of the + * three steps. + * + * Reifying the shape as a descriptor table — name, state path, optional + * callback key, optional clamp — and emitting the setter functions from + * a single builder collapses the surface to one tested helper plus a + * handful of lines per descriptor. Auditing "every setting wakes the + * scheduler" is now a one-line read of the builder. + * + * ### Why bespoke setters stay inline + * + * Five setters do NOT slot into the table: + * + * - `setBiasMode` — kicks an async per-galaxy bake on the renderer + * and chains a follow-up `requestRender` to the resolve handler. + * The descriptor's `state[path] = v; cb?.(v); requestRender()` + * shape can't express that. + * - `setTier` — orchestrates per-source asset-slot reloads via + * `cloudLoader.reloadSource`, with abort-controller plumbing. + * - `setLodMode` — flips the auto-LOD predicate AND fires an echo + * that observers (App.tsx) react to by re-driving source masks. + * - `setSourceVisible` — implicitly switches LOD mode to manual and + * touches the visible-source mask, not just one boolean. + * - `setSpaceMouseSensitivity` — forwards into the SpaceMouse + * subsystem rather than mutating engine state directly. + * + * Each does work that goes beyond "mutate + echo + render". Trying to + * express them through the table would either bloat the descriptor + * (subsystem refs, async hooks, follow-up actions) until the table is + * really a switch statement in disguise, or split their logic across + * the descriptor and a custom path until neither half is readable. + * Bespoke stays bespoke; the table only owns the simple cases. + * + * ### Why nested `path` tuples instead of a flat key + * + * Twelve of the thirteen setters write to `state.settings.X`; the + * thirteenth (`setAbsMagLimit`) writes to `state.bias.absMagLimit`. + * A flat `key: 'settings.brightness'` shape would force the builder + * to parse strings at runtime; a typed nested tuple + * (`['settings', 'brightness']`) lets the descriptor still read like + * a path while leaving runtime traversal as two indexed reads. The + * tuple shape also leaves the door open for a future setter that + * touches a third sub-bag (e.g. `picking.*`) without changing the + * descriptor type — just add another tuple entry. + * + * ### Type-narrowness tradeoff + * + * The builder returns `Record void>` + * because preserving per-method narrow types + * (`setPointSize: (n: number) => void`, `setAutoRotate: + * (b: boolean) => void`, …) would require thirteen conditional + * branches in the return type. Production callers go through + * `EngineHandle`'s declared signatures, so the narrowness loss inside + * the builder is invisible at the API edge. We assert the spread is + * compatible with the relevant slice of `EngineHandle` via a + * `satisfies` clause in `engine.ts`. + */ + +import type { EngineCallbacks, EngineHandle, EngineState } from '../../@types'; + +/** + * The thirteen names this table owns. Frozen in tests so a future + * accidental drift (boring setter promoted to bespoke, or vice versa) + * fails loudly rather than silently. + */ +export type SettingsTableKey = + | 'setPointSize' + | 'setBrightness' + | 'setAutoRotate' + | 'setGalaxyTexturesEnabled' + | 'setMilkyWayEnabled' + | 'setFilamentsEnabled' + | 'setFilamentIntensity' + | 'setHighlightFallback' + | 'setRealOnlyMode' + | 'setDepthFadeEnabled' + | 'setAbsMagLimit' + | 'setExposure' + | 'setToneMapCurve'; + +/** + * Path into `EngineState`. Two-element tuple: a sub-bag key followed + * by a leaf field. Always indexes into `state.settings` or + * `state.bias` for the current thirteen — but the type leaves room for + * other sub-bags to join. + */ +type SettingsPath = + | readonly ['settings', keyof EngineState['settings']] + | readonly ['bias', keyof EngineState['bias']]; + +/** + * One row of the descriptor table. + * + * - `name` is the EngineHandle method to emit. + * - `path` is the two-step state path the value lands in. + * - `callback` (optional) is the EngineCallbacks key to fire after + * mutation. Omit when no echo is wired (App.tsx owns the + * boolean optimistically — see `setFilamentsEnabled`). + * - `clamp` (optional) wraps the incoming value before it hits + * state AND the callback echo. Returns the post-clamp number. + * Used by `setExposure` and `setFilamentIntensity`. + */ +type SettingsDescriptor = { + name: SettingsTableKey; + path: SettingsPath; + callback?: keyof EngineCallbacks; + clamp?: (value: number) => number; +}; + +/** + * The actual table. Adding a row here automatically extends the + * builder output — no manual wiring in `engine.ts` beyond the existing + * spread. Removing a row from here without re-implementing the setter + * inline will fail typecheck wherever `EngineHandle.setX` is required. + */ +export const SETTINGS_TABLE: readonly SettingsDescriptor[] = [ + { + name: 'setPointSize', + path: ['settings', 'pointSizePx'], + callback: 'onPointSizeChange', + }, + { + name: 'setBrightness', + path: ['settings', 'brightness'], + callback: 'onBrightnessChange', + }, + { + name: 'setAutoRotate', + path: ['settings', 'autoRotate'], + callback: 'onAutoRotateChange', + }, + { + name: 'setGalaxyTexturesEnabled', + path: ['settings', 'galaxyTexturesEnabled'], + callback: 'onGalaxyTexturesEnabledChange', + }, + { + name: 'setMilkyWayEnabled', + path: ['settings', 'milkyWayEnabled'], + callback: 'onMilkyWayEnabledChange', + }, + { + // App.tsx owns this boolean optimistically; no echo callback wired. + // Asymmetry vs. galaxyTextures/milkyWay is deliberate — see the + // long comment in the original `setFilamentsEnabled`. + name: 'setFilamentsEnabled', + path: ['settings', 'filamentsEnabled'], + }, + { + // Filament-overlay intensity scale; clamps to [0, 1] same as the + // hand-rolled setter did. No callback for the same App-owns-state + // reason as `setFilamentsEnabled`. + name: 'setFilamentIntensity', + path: ['settings', 'filamentIntensity'], + clamp: (v) => Math.max(0, Math.min(1, v)), + }, + { + name: 'setHighlightFallback', + path: ['settings', 'highlightFallback'], + callback: 'onHighlightFallbackChange', + }, + { + name: 'setRealOnlyMode', + path: ['settings', 'realOnlyMode'], + callback: 'onRealOnlyModeChange', + }, + { + name: 'setDepthFadeEnabled', + path: ['settings', 'depthFadeEnabled'], + callback: 'onDepthFadeEnabledChange', + }, + { + // Note the path: `state.bias.absMagLimit`, not settings. The only + // current row that doesn't live under `state.settings`. + name: 'setAbsMagLimit', + path: ['bias', 'absMagLimit'], + callback: 'onAbsMagLimitChange', + }, + { + // Clamps to [0.05, 16] before mutation/echo — a runaway slider or + // devtools `setExposure(1e9)` must NOT blow out the float buffer + // (upper) or zero-multiply the HDR signal into a black frame + // (lower). The echo fires the *clamped* value so React's slider + // displays what the shader actually used. + name: 'setExposure', + path: ['settings', 'exposure'], + callback: 'onExposureChange', + clamp: (v) => Math.max(0.05, Math.min(16, v)), + }, + { + name: 'setToneMapCurve', + path: ['settings', 'toneMapCurve'], + callback: 'onToneMapCurveChange', + }, +]; + +/** + * Apply a value to `state` at the given two-step path. Kept as a + * standalone helper rather than inlined in the builder so the + * unsafe-but-bounded cast lives in one place — every other consumer + * of the table calls this. + * + * The `as never` cast is needed because the union over `SettingsPath` + * means TypeScript can't statically prove that `value` matches the + * leaf type at the chosen path; the descriptor table is the runtime + * guarantor instead. See the module-level note on type narrowness. + */ +function setByPath( + state: EngineState, + path: SettingsPath, + value: unknown, +): void { + const [bag, leaf] = path; + // The two branches are structurally identical but split so the + // `bag` discriminant narrows correctly inside each — saves one + // additional cast on the bag lookup. + if (bag === 'settings') { + (state.settings as Record)[leaf as string] = value; + } else { + (state.bias as Record)[leaf as string] = value; + } +} + +/** + * Build the thirteen setters from the descriptor table. Returns a + * record keyed by setter name; the consumer (`engine.ts`'s public + * handle) spreads it into the handle literal. + * + * Each emitted setter: + * 1. clamps the incoming value (if a clamp is declared); + * 2. writes the (possibly clamped) value into `state` at `path`; + * 3. fires `cb[descriptor.callback]?.(post-clamp value)` if the + * descriptor declares a callback; + * 4. calls `requestRender()` so the next frame picks up the change. + * + * The return type is widened to `(value: unknown) => void` per + * descriptor; the EngineHandle public-API surface is the place where + * the narrow per-method types live. See the module-level note on the + * type-narrowness tradeoff for why we don't try to preserve those + * here. + */ +export function buildSettersFromTable( + state: EngineState, + cb: EngineCallbacks, + requestRender: () => void, +): Record void> { + const out = {} as Record void>; + + for (const descriptor of SETTINGS_TABLE) { + const { name, path, callback, clamp } = descriptor; + + out[name] = (value: unknown) => { + // Clamps only ever apply to numeric fields; descriptors that + // declare a clamp are by definition number-typed. The cast + // here mirrors the runtime guarantee. + const next = + clamp !== undefined ? clamp(value as number) : value; + + setByPath(state, path, next); + + if (callback !== undefined) { + // Optional-chaining mirrors the hand-rolled setters' shape: + // a missing callback is silently skipped, never throws. + // Indexing through `unknown` because `EngineCallbacks` keys + // each carry their own narrow signature; the descriptor + // table is the runtime guarantor that `next` matches. + const fn = cb[callback] as ((v: unknown) => void) | undefined; + fn?.(next); + } + + requestRender(); + }; + } + + return out; +} + +/** + * Compile-time check that every setter we emit corresponds to a real + * key on `EngineHandle`. Removing or renaming an EngineHandle setter + * without updating the table will trip this assertion. + * + * (Runtime cost: zero — `satisfies` is erased.) + */ +const _enginehandleKeyCheck: SettingsTableKey extends keyof EngineHandle + ? true + : false = true; +void _enginehandleKeyCheck; diff --git a/tests/services/engine/settingsTable.test.ts b/tests/services/engine/settingsTable.test.ts new file mode 100644 index 00000000..a289d3eb --- /dev/null +++ b/tests/services/engine/settingsTable.test.ts @@ -0,0 +1,186 @@ +/** + * settingsTable — unit tests for the table-driven settings setter builder. + * + * The 13 "boring" public-handle setters on `EngineHandle` + * (`setPointSize`, `setBrightness`, …) all share the same shape: + * + * 1. Mutate one field on `state.settings.*` (or `state.bias.*`). + * 2. Optionally fire an echo callback with the (post-clamp) value. + * 3. Call `requestRender()` so the next frame picks up the change. + * + * `buildSettersFromTable` reifies that shape as a declarative descriptor + * table and emits the 13 setters from one builder. These tests exercise + * the contract directly without spinning up the full engine, which keeps + * the suite cheap and pinpoints regressions to the table itself when + * something breaks. + * + * What we cover here: + * - happy-path: mutation + callback + render-request all fire in the + * right order with the right value; + * - clamps: when a descriptor declares a clamp, the *clamped* value + * lands in state AND in the echo callback (mirrors the engine's + * `setExposure` shape); + * - missing-callback tolerance: descriptors without a `callback` key + * (or with the slot left undefined on the EngineCallbacks bag) skip + * the echo silently — same optional-chaining shape every other + * setter uses. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import { + buildSettersFromTable, + SETTINGS_TABLE, +} from '../../../src/services/engine/settingsTable'; +import type { EngineCallbacks, EngineState } from '../../../src/@types'; +import { BiasMode } from '../../../src/data/biasMode'; +import { ToneMapCurve } from '../../../src/data/toneMapCurve'; + +/** + * Build a deeply-mutable test fixture for the engine state slices the + * table touches. We keep the rest of `EngineState` as `{} as never` + * style stubs because the builder only ever follows `path` tuples that + * end inside `state.settings` or `state.bias`. + */ +function makeState(): Pick { + return { + settings: { + pointSizePx: 2.5, + brightness: 1.0, + autoRotate: false, + galaxyTexturesEnabled: true, + milkyWayEnabled: true, + filamentsEnabled: false, + filamentIntensity: 0.5, + highlightFallback: true, + realOnlyMode: false, + depthFadeEnabled: true, + exposure: 1.0, + toneMapCurve: ToneMapCurve.Reinhard, + }, + bias: { + mode: BiasMode.None, + absMagLimit: -19, + apparentMagLimit: 0, + schechterMStar: 0, + schechterAlpha: 0, + }, + }; +} + +describe('settingsTable', () => { + describe('SETTINGS_TABLE', () => { + it('declares the 13 table-candidate setters', () => { + // The plan freezes this list at 13 — bespoke setters + // (`setBiasMode`, `setTier`, `setLodMode`, `setSourceVisible`, + // `setSpaceMouseSensitivity`) MUST stay inline in engine.ts. + // If this number drifts, either a new boring setter snuck in + // (good — extend the table) or a bespoke one was accidentally + // tabled (bad — bespoke logic gets dropped silently). + const names = SETTINGS_TABLE.map((d) => d.name).sort(); + expect(names).toEqual( + [ + 'setAbsMagLimit', + 'setAutoRotate', + 'setBrightness', + 'setDepthFadeEnabled', + 'setExposure', + 'setFilamentIntensity', + 'setFilamentsEnabled', + 'setGalaxyTexturesEnabled', + 'setHighlightFallback', + 'setMilkyWayEnabled', + 'setPointSize', + 'setRealOnlyMode', + 'setToneMapCurve', + ].sort(), + ); + }); + }); + + describe('buildSettersFromTable', () => { + it('mutates state, fires the echo callback, and requests a render', () => { + const state = makeState(); + const cb: Partial = { + onPointSizeChange: vi.fn(), + onBrightnessChange: vi.fn(), + }; + const requestRender = vi.fn(); + + const setters = buildSettersFromTable( + state as EngineState, + cb as EngineCallbacks, + requestRender, + ); + + setters.setPointSize(4.2); + expect(state.settings.pointSizePx).toBe(4.2); + expect(cb.onPointSizeChange).toHaveBeenCalledExactlyOnceWith(4.2); + expect(requestRender).toHaveBeenCalledOnce(); + + setters.setBrightness(2.0); + expect(state.settings.brightness).toBe(2.0); + expect(cb.onBrightnessChange).toHaveBeenCalledExactlyOnceWith(2.0); + expect(requestRender).toHaveBeenCalledTimes(2); + }); + + it('applies clamps before mutation and callback echo', () => { + // setExposure clamps to [0.05, 16] and echoes the clamped value; + // setFilamentIntensity clamps to [0, 1] but has no callback. + const state = makeState(); + const cb: Partial = { + onExposureChange: vi.fn(), + }; + const requestRender = vi.fn(); + + const setters = buildSettersFromTable( + state as EngineState, + cb as EngineCallbacks, + requestRender, + ); + + // Above the cap. + setters.setExposure(1e9); + expect(state.settings.exposure).toBe(16); + expect(cb.onExposureChange).toHaveBeenLastCalledWith(16); + + // Below the floor. + setters.setExposure(-1); + expect(state.settings.exposure).toBe(0.05); + expect(cb.onExposureChange).toHaveBeenLastCalledWith(0.05); + + // Filament intensity clamps without an echo callback. + setters.setFilamentIntensity(2); + expect(state.settings.filamentIntensity).toBe(1); + setters.setFilamentIntensity(-3); + expect(state.settings.filamentIntensity).toBe(0); + }); + + it('tolerates a missing echo callback (optional-chaining contract)', () => { + // setFilamentsEnabled has no `callback` key in its descriptor + // (App.tsx owns that boolean optimistically). Even more broadly: + // any descriptor whose declared callback is left `undefined` on + // the EngineCallbacks bag must not throw — same shape as the + // hand-rolled setters' `cb.onXChange?.(v)`. + const state = makeState(); + const cb: Partial = {}; // every callback undefined + const requestRender = vi.fn(); + + const setters = buildSettersFromTable( + state as EngineState, + cb as EngineCallbacks, + requestRender, + ); + + expect(() => setters.setFilamentsEnabled(true)).not.toThrow(); + expect(state.settings.filamentsEnabled).toBe(true); + expect(requestRender).toHaveBeenCalledOnce(); + + // Same tolerance for setters whose callback is "declared" via the + // descriptor but happens to be undefined on the cb bag. + expect(() => setters.setBrightness(0.7)).not.toThrow(); + expect(state.settings.brightness).toBe(0.7); + expect(requestRender).toHaveBeenCalledTimes(2); + }); + }); +});