From 8e873da7b476c8bcfcbdb7f881baaaa8a7688731 Mon Sep 17 00:00:00 2001 From: Alexander Rulkens Date: Mon, 8 Jun 2026 00:30:33 +0200 Subject: [PATCH 1/7] docs(tour): pre-tour decomplection spec + cinematic-core seed plan Adds the design spec for the two behaviour-preserving refactors that land before the guided tour: a camera-driver authority (single per-frame write-site, precedence as data) and a settings snapshot/restore seam (geography-aware readVisibility + applyVisibility with an animate flag). Both came out of an entanglement-radar pass over the camera path and engine.ts and are new findings, not in the documented backlog. Also rewrites the splash Part-2 plan from a tween-delegation stub into a cinematic-core engine seed whose camera core is the shape the full cinematic tour (docs/tour/) extends additively, and adds a dependency note pointing at the decomplection spec. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-05-20-splash-screen-02-stub-tour.md | 341 +++++++++++++----- ...026-06-08-pre-tour-decomplection-design.md | 195 ++++++++++ 2 files changed, 444 insertions(+), 92 deletions(-) create mode 100644 docs/superpowers/specs/2026-06-08-pre-tour-decomplection-design.md diff --git a/docs/superpowers/plans/2026-05-20-splash-screen-02-stub-tour.md b/docs/superpowers/plans/2026-05-20-splash-screen-02-stub-tour.md index 6d9ca80d..0e8265f8 100644 --- a/docs/superpowers/plans/2026-05-20-splash-screen-02-stub-tour.md +++ b/docs/superpowers/plans/2026-05-20-splash-screen-02-stub-tour.md @@ -1,12 +1,52 @@ -# Splash Screen — Tour Implementation Plan (Part 2 of 2) +# Tour Engine Seed (cinematic-core) — Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. Follows [`plan-style.md`](../conventions/plan-style.md): contract code (types, test names, signatures) is included; implementation bodies are not — read the cited code and write the body from the tests. -> **Companion plan:** `2026-05-20-splash-screen-01-core.md` — the splash dialog + AboutPill + useSplash hook + WebGPU gate. **Plan 1 has landed:** the `Splash` component, `useSplash` (`dismissExplore` / `dismissTour` / `reopen`), and the `` prop all exist; `onTour` is currently wired to `splash.dismissTour` (the Tour button just dismisses). This plan replaces that no-op with a real camera tour. +> **Companion plan:** `2026-05-20-splash-screen-01-core.md` — the splash dialog + AboutPill + useSplash hook + WebGPU gate. **Plan 1 has landed:** the `Splash` component, `useSplash` (`dismissExplore` / `dismissTour` / `reopen`), and the `` prop all exist; `onTour` is currently wired to `splash.dismissTour` (the Tour button just dismisses). This plan replaces that no-op with a real, frame-driven camera tour. -> **Architecture pivot (2026-06-04).** Originally this plan shipped a deliberately-throwaway stub: a React-driven async `sleep` chain in `App.tsx`. We changed direction — **the tour runner now lives engine-side as a `tourSubsystem` + an `engine.tour` sub-handle**, frame-driven exactly like the camera tween, so the polished cinematic tour (`../specs/2026-05-07-tour-animation-design.md`) *extends this seed* rather than throwing it away. This plan ships a minimal-but-real tour: chained focus + per-beat dwell + generic side-effects, no rotation slerp and no captions yet (those are the cinematic extensions). It resolves the cinematic spec's decision 4 ("tour-engine API shape"). +> **The cinematic target.** `docs/tour/` is the full guided-tour design — a narrated ~2½-min powers-of-ten journey (`goal.md`, `script.md`, `cinematography.md`, `graphic-design.md`, and the eleven `stages/NN-*.md` front-matter files). **This plan ships the SEED that the cinematic tour purely extends.** The seed's camera core is built in the *same shape* the cinematic uses, running a trivial subset — so the cinematic is additive, with zero rework or cruft. (The `stages/*.facts.md` files are trivia and out of scope.) -**Goal:** Wire a six-beat Powers-of-Ten camera tour (Milky Way → Local Group → Virgo Cluster → Boötes Void → Coma Supercluster → wide view) to the Tour button, driven by an engine subsystem and exposed as `engine.tour`, with cancel-on-input, UI-hide coordination, and pre-tour settings restoration. +> **DEPENDS ON the pre-tour decomplection** (`../specs/2026-06-08-pre-tour-decomplection-design.md`), which lands first. That spec reconciles two tasks here: (1) the tour does **not** "write the camera pose after the other mutators" — it registers a `tour` `CameraDriver` (priority 80) in the new camera-driver registry, and `applyCameraPose` becomes that driver's `apply`; (2) `TourActions.snapshot`/`restore`/`applyEffect` build on the seam's `readVisibility` / `applyVisibility({ animate })` rather than hand-coding the four settings storage shapes. Apply those two reconciliations when picking this plan up; the rest of the plan stands. + +--- + +## The architecture pivot (read this first) + +An earlier draft of this plan delegated camera control to one-shot tweens: `actions.focus()` → `commitFocus` / `focusOnMilkyWay` / `selectFamous` → `tweenToStructure` / `tweenToGalaxy`. **The cinematic design requires the opposite**, and `cinematography.md` is explicit about it: + +> "**Driver consequence:** `log-dolly` + `pass-through-spline` + `dwell-drift` mean the tour subsystem must **own the camera per-frame** — own a global tour clock and evaluate the spline + dwell-orbit into the camera — not fire one-shot tweens through `tweenManager`." + +One-shot tweens are the wrong substrate for three independent reasons: + +1. **Scale is logarithmic.** Framing distances span ~0.05 → ~6,000 Mpc (5 orders of magnitude). The camera must interpolate `logDist = ln(distance)`, not raw distance — `tweenToGalaxy` / `tweenToStructure` lerp raw distance. (`goal.md` "Hard constraints"; `cinematography.md` "The one hard constraint".) +2. **Dwell is never frozen.** Every stop carries a slow drift (`cinematography.md` "Dwell is never frozen"). A one-shot tween settles and lets the render-on-demand loop go idle — dwell-drift literally cannot run on that substrate. +3. **Per-frame ownership is the only thing a Catmull-Rom spline + arc-length reparam can extend.** A bag of sequential tweens has no global clock to evaluate a spline against. + +Building the tween-delegation version means ~40 % of its core (the focus-delegation adapter + wall-clock-between-tweens sequencing) gets thrown away when the cinematic lands, and risks a two-camera-mode entanglement (tour-via-tweens vs cinematic-via-pose-writes). So **we reshape the seed's camera core now to be the cinematic's core**, running the trivial subset. + +### What the seed ships + +- The tour subsystem **owns the camera every frame** while active: it writes a `CameraPose` into `state.cam` directly (NOT via `tweenManager`). +- A **single linear-in-log segment per beat**: `logDist` lerps linearly between the previous keyframe's `ln(distanceMpc)` and the current beat's, `target` lerps as a `Vec3`, yaw/pitch held constant. +- **Per-beat framing distance** (`distanceMpc`) and **per-beat travel duration** (`travelMs`) — the keyframe model, not a global tween constant. +- **Generic per-beat effects** (instant boolean toggles), snapshotted at start and restored on end/stop. +- The **real eleven-stage beat table** authored from `docs/tour/stages/`. +- Cancel-on-input, UI-hide coordination, render-on-demand participation. + +### The additive extension points (cinematic, NOT in the seed) + +Each is purely additive on top of what the seed ships — a new field, a new interpolation strategy, or a new subsystem — never a reshape: + +- **Catmull-Rom spline** through N keyframes with **arc-length reparam** — replaces the seed's straight log-lerp between two keyframes. (`TourBeat` keyframes already feed it.) +- **Pass-through waypoints** (`dwell_s: 0`) as spline control points that bend the path at constant speed — the seed collapses these to a settle on the stage's primary focus. +- **Dwell-orbit / dwell-drift** — a tiny orbit evaluated during the dwell instead of holding a fixed pose. +- **Azimuth / elevation** per keyframe (approach angle) — the seed carries yaw/pitch constant. +- **Captions** (`caption?: TourCaption` on `TourBeat`) + a `tourCaptionSubsystem` rendering the stage title + narration — the seed renders no text. +- **Ramped effects** (`ramp_s` / `rampMs` on `TourEffect`) tweening an effect's intensity over a leg — the seed's effects are instant. +- **Look-offset** (look-ahead) and **per-segment easing** parameters. +- **Flow-field toggle** — already expressible as a `TourEffect` variant when the layer lands. + +None of these require changing a type the seed ships — they extend it. --- @@ -14,44 +54,53 @@ - `type` aliases only, never `interface`. - **One exported type per file** under `src/@types` — never co-locate two. -- `readonly` fields + `Vec2`/`Vec3` aliases (not raw tuples); prefer immutability. -- Multi-paragraph didactic comments at module headers; comments timeless (no dates/PR refs in code). +- `readonly` fields + `Vec2` / `Vec3` aliases (not raw tuples); prefer immutability. +- Multi-paragraph didactic comments at module headers; comments timeless (no dates / PR refs in code). - No barrel exports; deep imports. - Tests under `tests/` mirroring `src/`. - Dev server is left running. +- **Re-verify every cited line number against the live tree before editing** — engine.ts and runFrame.ts churn and the numbers below will have drifted. --- ## Architecture -The tour is an **engine subsystem** (`tourSubsystem`) that owns beat sequencing, plus a thin **`engine.tour` sub-handle** (`start` / `stop` / `isActive`). It is frame-driven, mirroring the existing `tweenManager` (`src/services/engine/camera/tweenManager.ts`) and `clusterFocus` (imperative methods + per-frame `update` + `isAwake`) subsystems. +The tour is an **engine subsystem** (`tourSubsystem`) owning beat sequencing **and the per-frame camera evaluator**, plus a thin **`engine.tour` sub-handle** (`start` / `stop` / `isActive`). It is frame-driven, mirroring the factory shape of `tweenManager` (`src/services/engine/camera/tweenManager.ts:52` — a closure-over-mutable-state factory returning an imperative + `isActive` object) and the per-frame `update(…, nowMs)` cadence of `structureFocusSubsystem` (`src/services/engine/subsystems/structureFocusSubsystem.ts:56`). -**Why engine-side, not React.** The render-on-demand loop only keeps ticking while something animates (`stillAnimating` predicate at `src/services/engine/frame/runFrame.ts:502-509`). A tour dwelling between beats has no in-flight tween, so a React `sleep` driver could not keep frames flowing — the loop would sleep and the dwell timer would stall. By living in the subsystem registry and adding `|| state.subsystems.tour.isActive()` to that predicate, the tour participates in the loop the same way an in-flight tween or autoRotate does: frames keep coming through each dwell, the subsystem advances on a wall-clock comparison, and each beat's focus tween renders normally. +**Why per-frame camera ownership, not tweens.** See the pivot section above. The single load-bearing consequence: the subsystem holds a global tour clock and, every frame, evaluates the current segment into a `CameraPose` it writes to the camera via the `TourActions` port. While the tour is active its pose write is **authoritative** — it must run *after* the other camera mutators in the frame (tweens / spaceMouse / autoRotate) and must NOT itself start a `tweenManager` tween. -**Per-beat dispatch via a `TourActions` port.** The subsystem does not import engine internals directly. It calls an injected `TourActions` object — `focus(focus)`, `applyEffect(effect)`, `snapshot(effects)`, `requestRender()`. The engine builds this adapter closing over its internal `state` (routing `focus` to the same `commitFocus` / `focusOnMilkyWay` / `selectFamous` paths the camera/selection handles already use — see `engine.ts:790-804`, `engine.ts:760-784`, `helpers/commitFocus.ts:25`). Tests inject a fake `TourActions` that records calls and a controllable clock (`advance(nowMs)`), so the sequencing core is unit-tested with no real timers and no GPU. +**Render-on-demand.** The loop only keeps ticking while something animates (`stillAnimating` predicate, `runFrame.ts:493-501`). A tour dwelling between beats has no in-flight tween, so without participation the loop would sleep and the dwell clock would stall. Adding `|| state.subsystems.tour.isActive()` to that predicate makes the tour keep frames flowing through every beat and dwell — the same way an in-flight tween or autoRotate does. -**Completion.** The engine has no completion-promise idiom for tweens (they're polled via `isActive()`), but `fades.fadeTo()` returns `Promise` (`@types/animation/FadeRegistry.d.ts:52`). Following that precedent, `engine.tour.start(beats)` returns `Promise` that resolves when the tour ends — naturally (last dwell elapsed) or via `stop()`. App.tsx clears `tourActive` in the resolution. +**Per-beat dispatch via a `TourActions` port.** The subsystem does not import engine internals. It calls an injected `TourActions` object. The engine builds the real adapter closing over its internal `state`; tests inject a fake that records calls and is driven by an explicit `advance(nowMs)` clock — so the sequencing + interpolation core is unit-tested with no real timers and no GPU. -**Restoration.** On `start`, the subsystem asks the adapter to `snapshot` the union of every setting any beat's `effects` touch, and replays that snapshot on end/stop — generalizing the original plan's filament try/finally to the whole effect set. The stub's beats only touch `filaments`, so in practice this restores the user's pre-tour filaments setting; the mechanism is already general for the cinematic palette. +**Completion.** The engine has no completion-promise idiom for tweens (polled via `isActive()`), but `fades.fadeTo()` returns `Promise`. Following that precedent, `engine.tour.start(beats)` returns `Promise` that resolves when the tour ends — naturally (last dwell elapsed) or via `stop()`. App.tsx clears `tourActive` in the resolution. -### Data structure +**Restoration.** On `start`, the subsystem asks the adapter to `snapshot` the union of every setting any beat's `effects` touch, and replays that snapshot on end / stop — restoring the user's pre-tour state for the whole effect set. -These types are the contract the cinematic tour also consumes — include them exactly. One exported type per file. +### Data contracts + +These types are the contract the cinematic tour also consumes. One exported type per file. Verify every import path against the live tree before writing — they are cited from the current tree below but will be re-confirmed by the implementer. ```ts // src/@types/engine/tour/TourFocus.d.ts +import type { Vec3 } from '../../math/Vec3'; + /** * Symbolic camera target for a tour beat. A beat table is static data * authored at build time, but GalaxyInfo is built at runtime and - * StructureRecords come from the structure store — so a beat references - * its target by name and the runner resolves it, matching how the engine - * already resolves selectFamous(id) and POI lookups. + * StructureRecords come from the structure store — so a beat references its + * target by name and the runner resolves it to a world position, matching how + * the engine already resolves selectFamous(id) and structure-store lookups. + * + * The `point` variant carries a literal world position for stages that frame + * no single catalog object (the deep field, the edge — stages 06 / 08 / 09). */ export type TourFocus = | { readonly kind: 'milkyWay' } | { readonly kind: 'home' } | { readonly kind: 'famous'; readonly id: string } - | { readonly kind: 'structure'; readonly structureId: string }; + | { readonly kind: 'structure'; readonly structureId: string } + | { readonly kind: 'point'; readonly position: Vec3 }; ``` ```ts @@ -61,10 +110,14 @@ import type { VolumeFieldId } from '../../data/VolumeFieldId'; import type { PoiCategory } from '../data/PoiCategory'; /** - * A per-beat side-effect, applied on beat entry. A generic delta union - * (not a hardcoded `filamentsOn?: boolean`) so the cinematic tour can add - * volume / source / label beats with no change to the beat shape. Each - * variant maps 1:1 to an existing engine-handle setter. + * A per-beat side-effect, applied instantly on beat entry. A generic delta + * union (not a hardcoded `filamentsOn?: boolean`) so the cinematic tour can add + * volume / source / label beats with no change to the beat shape. Each variant + * maps 1:1 to an existing engine-handle setter. + * + * Effects are instant booleans here. The cinematic tour adds an optional + * `rampMs` field additively, to tween an effect's intensity over a leg + * (cinematography.md "Effects can animate"); the seed always toggles instantly. */ export type TourEffect = | { readonly kind: 'filaments'; readonly enabled: boolean } @@ -81,18 +134,48 @@ import type { TourFocus } from './TourFocus'; import type { TourEffect } from './TourEffect'; /** - * One beat of a guided tour. `dwellMs` is the pause AFTER the focus tween - * settles (per-beat, so the cinematic tour can weight legs). `effects` - * are applied on entry. `caption` is the seam for the future - * tourCaptionSubsystem (cinematic decision 2b); the stub leaves it unset - * and nothing renders it yet. + * One beat of a guided tour — a keyframe in the cinematic camera model. + * + * `distanceMpc` is the per-beat FRAMING distance (camera→focus, world units ≈ + * Mpc); the seed interpolates `ln(distanceMpc)` linearly toward it, the + * cinematic splines it in log space. `travelMs` is the per-beat travel + * duration (NOT a global constant — the cinematic weights legs); `dwellMs` is + * the hold AFTER travel settles. `effects` apply instantly on beat entry. + * + * The cinematic tour adds, ADDITIVELY: `caption?: TourCaption` (the editorial + * title + narration layer), per-keyframe `azimuth` / `elevation` (approach + * angle), and pass-through-waypoint fields (a zero-dwell control point that + * bends the spline). None of those reshape this type. */ export type TourBeat = { readonly id: string; readonly focus: TourFocus; + readonly distanceMpc: number; + readonly travelMs: number; readonly dwellMs: number; readonly effects?: readonly TourEffect[]; - readonly caption?: string; +}; +``` + +```ts +// src/@types/engine/tour/CameraPose.d.ts +import type { Vec3 } from '../../math/Vec3'; + +/** + * The per-frame orbit-camera pose the tour writes through TourActions. + * Exactly the four mutable orbit fields (`target` / `distance` / `yaw` / + * `pitch`); the immutable framing fields (`fovYRad` / `near` / `far`) stay + * owned by `state.cam` and are never written by the tour. + * + * Intentionally narrower than `InitialCam` (src/@types/camera/InitialCam.d.ts), + * which carries the framing fields too — see the Task-1 note for why a fresh + * type rather than reusing it. + */ +export type CameraPose = { + readonly target: Vec3; + readonly distance: number; + readonly yaw: number; + readonly pitch: number; }; ``` @@ -100,15 +183,26 @@ export type TourBeat = { // src/@types/engine/tour/TourActions.d.ts import type { TourFocus } from './TourFocus'; import type { TourEffect } from './TourEffect'; +import type { CameraPose } from './CameraPose'; +import type { Vec3 } from '../../math/Vec3'; /** - * The port the tourSubsystem calls to affect the world. The engine wires - * the real adapter (closing over internal state); tests inject a fake that - * records calls. `snapshot` reads the current value of every setting the - * given effects touch and returns a thunk that restores them. + * The port the tourSubsystem calls to affect the world. The engine wires the + * real adapter (closing over internal state); tests inject a fake that records + * calls. + * + * `resolveFocus` turns a symbolic target into a world position WITHOUT moving + * the camera or changing selection (returns null for an unresolvable id). + * `applyCameraPose` is how the tour OWNS the camera — it writes `state.cam` + * every frame. `markFocused` sets selection/focus state for label + ring + * emphasis ONLY (no camera move) — fired once on settle. `snapshot` reads the + * current value of every setting the given effects touch and returns a thunk + * that restores them. */ export type TourActions = { - focus(focus: TourFocus): void; + resolveFocus(focus: TourFocus): Vec3 | null; + applyCameraPose(pose: CameraPose): void; + markFocused(focus: TourFocus): void; applyEffect(effect: TourEffect): void; snapshot(effects: readonly TourEffect[]): () => void; requestRender(): void; @@ -120,9 +214,10 @@ export type TourActions = { import type { TourBeat } from '../tour/TourBeat'; /** - * Engine subsystem owning tour sequencing. Frame-driven: `advance(nowMs)` - * is called once per frame and issues the next beat when the current beat's - * (tween + dwell) budget has elapsed. `start` resolves when the tour ends. + * Engine subsystem owning tour sequencing AND the per-frame camera evaluator. + * Frame-driven: `advance(nowMs)` is called once per frame while active and + * writes the camera pose for the current segment. `start` resolves when the + * tour ends (naturally or via `stop`). */ export type TourSubsystem = { start(beats: readonly TourBeat[]): Promise; @@ -148,22 +243,32 @@ export type EngineTourHandle = { }; ``` -(Confirm the exact import paths for `SourceType`, `VolumeFieldId`, `PoiCategory` against the current tree before writing — cite, don't guess. `SourceType`/`VolumeFieldId` live under `src/@types/data/`; `PoiCategory` under `src/@types/engine/data/`.) +### The beat table -### Itinerary (the beat table) +`src/data/tourBeats.ts` — `TOUR_BEATS: readonly TourBeat[]` authored from the eleven cinematic stages (`docs/tour/stages/00..10`). For each stage, map the front-matter: `focus` → `TourFocus`, `distance_mpc` → `distanceMpc`, `travel_s * 1000` → `travelMs`, `dwell_s * 1000` → `dwellMs`, `effects` → `TourEffect[]`. -`src/data/tourBeats.ts` — six beats, ~50 s wall time (each ≈ `FOCUS_TWEEN_MS` + `STUB_TOUR_DWELL_MS`). Items marked *(auto)* render with no tour-side work thanks to shipped subsystems. +| # | id | focus | distanceMpc | travelMs | dwellMs | effects | +|---|----|-------|-------------|----------|---------|---------| +| 00 | `opening-title` | `{ kind: 'milkyWay' }` | 0.05 | 0 | 8000 | — | +| 01 | `you-are-here` | `{ kind: 'milkyWay' }` | 0.05 | 3000 | 7000 | — | +| 02 | `nearest-neighbour` | `{ kind: 'famous', id: 'm31' }` | 0.8 | 7000 | 7000 | — | +| 03 | `our-neighbourhood` | `{ kind: 'structure', structureId: 'group-sculptor-group' }` | 4 | 9000 | 5000 | `markerCategory group on`, `labelCategory group on` | +| 04 | `nearest-cluster` | `{ kind: 'structure', structureId: 'cluster-virgo-m87' }` | 16 | 7000 | 7000 | — | +| 05 | `cosmic-web` | `{ kind: 'structure', structureId: 'supercluster-coma-sc' }` | 90 | 9000 | 9000 | `filaments on` (+ `volume` mcpm on — see note) | +| 06 | `cosmic-flows` | `{ kind: 'point', position: [0,0,0] }` | 80 | 5000 | 9000 | — (flow layer toggle deferred — see note) | +| 07 | `emptiness` | `{ kind: 'structure', structureId: 'void-bootes-void' }` | 150 | 6000 | 5000 | — | +| 08 | `deep-field` | `{ kind: 'point', position: [0,0,0] }` | 2000 | 8000 | 4000 | `source milliquas visible` (see note) | +| 09 | `the-edge` | `{ kind: 'point', position: [0,0,0] }` | 6000 | 9000 | 8000 | — | +| 10 | `home-again` | `{ kind: 'milkyWay' }` | 0.05 | 8000 | 5000 | — | -| # | id | focus | effects | notes | -|---|----|-------|---------|-------| -| 1 | `milky-way` | `{ kind: 'milkyWay' }` | — | MW impostor *(auto, on by default)* + "You are here" *(auto, ≤2 Mpc)*. | -| 2 | `local-group` | `{ kind: 'famous', id: 'm31' }` | — | Andromeda. Famous name label *(auto)*. | -| 3 | `virgo` | `{ kind: 'structure', structureId: 'cluster-virgo-m87' }` | — | Name label *(auto)*. | -| 4 | `bootes-void` | `{ kind: 'structure', structureId: 'void-bootes-void' }` | — | Mid-sequence; next beat re-populates the frame. Name label *(auto)* softens the "is it broken?" read. | -| 5 | `coma` | `{ kind: 'structure', structureId: 'supercluster-coma-sc' }` | `[{ kind: 'filaments', enabled: true }]` | Cosmic web. Name label *(auto)*. | -| 6 | `wide-view` | `{ kind: 'home' }` | — | Climax. | +**Verified IDs** (live tree): famous `m31` exists (`data/famous_galaxies.seed.json:813`); structure ids are `${category}-${seed.id}` (`buildStaticAnchorStructures.ts:103`), so `group-sculptor-group`, `cluster-virgo-m87`, `supercluster-coma-sc`, `void-bootes-void` all resolve against `data/structure_anchors.seed.json`. The stage front-matter slugs already carry the `category-` prefix, matching the store's id rule. -The void sits mid-sequence (not last) so the camera never *ends* on an empty region — the cheap, robust mitigation; the auto-rendered "Boötes Void" name label is a bonus on top. Filaments are turned on for beat 5 and restored to the user's pre-tour setting on tour end via the snapshot mechanism (beat 5 is the only beat touching `filaments`, so the snapshot captures + restores exactly that). The stub deliberately leaves the volume / milliquas / horizon / groups palette to the cinematic tour. +**Seed-vs-cinematic notes** (author these as comments in `tourBeats.ts`): + +- **Pass-through waypoints collapse.** Stage 03's director notes route the path *through* `group-m81-group` and `group-cen-a-group` as `dwell_s: 0` pass-throughs before settling on `group-sculptor-group`. The seed has no spline, so it collapses to a single settle on the primary focus (`group-sculptor-group`). The cinematic restores the two pass-throughs as Catmull-Rom control points. +- **`point` focuses use the front-matter coords.** Stages 06 / 08 / 09 declare `focus: point:0,0,0` (placeholder origin — the cinematic will retarget 06 to the local flow basin). Use the literal coords as the `point` variant's `position`. +- **Effect mapping is best-effort against shipped handles.** Stage 05 declares "mcpm volume fade-in + filaments fade-in"; the seed toggles them instantly (`filaments on`, and `volume` mcpm on IF the implementer confirms the field id from `VolumeFieldId` and the master-enable interplay — otherwise ship filaments-only and leave a TODO comment). Stage 06's flow-field toggle is **omitted from the seed** (the CF4++ flow layer's `TourEffect` variant is a cinematic add — see the extension list); leave a comment. Stage 08's "milliquas emphasized" maps to a `source` visible toggle if a milliquas `SourceType` exists; otherwise omit with a comment. +- **The void sits mid-sequence (07), never last** — the camera never *ends* on an empty region. Stage 10 (`home-again`) is the climax-return to the Milky Way. --- @@ -173,68 +278,109 @@ The void sits mid-sequence (not last) so the camera never *ends* on an empty reg - `src/@types/engine/tour/TourFocus.d.ts` - `src/@types/engine/tour/TourEffect.d.ts` - `src/@types/engine/tour/TourBeat.d.ts` +- `src/@types/engine/tour/CameraPose.d.ts` - `src/@types/engine/tour/TourActions.d.ts` - `src/@types/engine/subsystems/TourSubsystem.d.ts` - `src/@types/engine/handles/EngineTourHandle.d.ts` -- [ ] Write the six type files exactly as specified in the Data-structure section above. Verify the imported type names + paths (`SourceType`, `VolumeFieldId`, `PoiCategory`) against the current tree first. -- [ ] Add `tour: EngineTourHandle` to the `EngineHandle` type (`src/@types/engine/EngineHandle.d.ts`) and `tour: TourSubsystem` to the subsystem-registry type (find it via the existing `tweens` / `labelDirector` / `clusterFocus` fields — likely `src/@types/engine/EngineSubsystems.d.ts` or similar; cite the real file). +- [ ] Write the seven type files exactly as specified in the Data-contracts section. Verify the imported type names + paths against the live tree first: `Vec3` is `src/@types/math/Vec3` (confirmed), `SourceType` is `src/@types/data/SourceType` (confirmed `export type SourceType`), `VolumeFieldId` is `src/@types/data/VolumeFieldId` (confirmed), `PoiCategory` is `src/@types/engine/data/PoiCategory` (confirmed). +- [ ] **`CameraPose` — reuse-check first.** Before creating `CameraPose.d.ts`, look at `src/@types/camera/InitialCam.d.ts` and the camera-snapshot helpers (`src/services/engine/camera/cameraSnapshot.ts`). `InitialCam` has `target/distance/yaw/pitch` PLUS `fovYRad/near/far` — it is NOT a clean match (the tour must never write the framing fields). Create the narrower `CameraPose` as specified, and add a one-line note in its docblock pointing at `InitialCam` and why it is intentionally narrower. (Document the decision either way: if you find an exact-shape existing type, reuse it instead.) +- [ ] Add `tour: EngineTourHandle` to the `EngineHandle` type — `src/@types/engine/EngineHandle.d.ts` (the sub-handle cluster, ~lines 46-59; add the import alongside the other `./handles/*` imports). +- [ ] Add `tour: TourSubsystem` to the subsystem-registry type — `src/@types/engine/handles/EngineSubsystemHandles.d.ts`. Note the `_EnforceDestroyable` mapped type at the bottom (~lines 141-145) requires every field satisfy `Destroyable`; `TourSubsystem` has `destroy()`, so it passes. Add the field as **non-nullable** (eager construction — no GPU dep) alongside `structureFocus` / `tweens`. - [ ] `npm run typecheck` → PASS (types compile; nothing consumes them yet). - [ ] Commit. -## Task 2: `tourSubsystem` — sequencing core +## Task 2: `tourSubsystem` — sequencing + per-frame camera evaluator (the heart) **Files:** - Create: `src/services/engine/subsystems/tourSubsystem.ts` — `createTourSubsystem(actions: TourActions): TourSubsystem` - Test: `tests/services/engine/subsystems/tourSubsystem.test.ts` -**Shape to mirror:** `tweenManager.ts:52-92` (closure-over-mutable-state factory returning the imperative+`isActive` object) and the `clusterFocus` per-frame `update(…, nowMs)` cadence. The subsystem holds `beats`, `index`, `beatStartMs`, an `active` flag, the completion `resolve`, and the `restore` thunk. `advance(nowMs)` advances to the next beat when `nowMs - beatStartMs >= FOCUS_TWEEN_MS + beat.dwellMs` (import `FOCUS_TWEEN_MS` from `src/services/engine/camera/focusTweenDuration.ts`); issuing a beat = `actions.applyEffect(...)` for each effect then `actions.focus(beat.focus)` then stamp `beatStartMs`. On running off the end, restore + resolve + `active=false`. - -**Behaviour contract — write these tests (fake `TourActions` recording calls; drive `advance` with explicit `nowMs`):** - -- [ ] `start issues the first beat immediately` — after `start(beats)`, `actions.focus` called once with `beats[0].focus`; `isActive()` is `true`. -- [ ] `applies a beat's effects before its focus` — a beat with `effects: [{kind:'filaments',enabled:true}]` records the `applyEffect` call ordered before the `focus` call. -- [ ] `advances to the next beat only after FOCUS_TWEEN_MS + dwellMs` — `advance(start + FOCUS_TWEEN_MS + dwell - 1)` issues nothing new; `advance(start + FOCUS_TWEEN_MS + dwell)` issues `beats[1].focus`. -- [ ] `plays all beats in order` — stepping the clock through every beat issues each `focus` once, in table order. -- [ ] `resolves the start() promise after the last beat's dwell` — the promise from `start` resolves once the final beat's budget elapses; `isActive()` becomes `false`. -- [ ] `stop() ends the tour and resolves the promise` — `stop()` mid-tour flips `isActive()` to `false` and resolves the pending `start` promise; subsequent `advance` is a no-op. -- [ ] `restores snapshot on natural completion` — `actions.snapshot` is called once at `start` with the union of beat effects; its returned restore thunk is invoked exactly once on completion. -- [ ] `restores snapshot on stop()` — restore thunk invoked on `stop()` too (not double-invoked if completion races). +**Shape to mirror:** `tweenManager.ts:52` (closure-over-mutable-state factory returning the imperative + `isActive` object). The subsystem holds `beats`, segment `index`, `segmentStartMs`, an `active` flag, the completion `resolve`, the `restore` thunk, the **previous-keyframe pose** (the start pose for beat 0; the prior beat's resolved keyframe thereafter), and a `settled` flag for the current segment. + +**Behaviour contract** (specify as contract + tests, NOT a full body): + +- `start(beats)`: stamp segment 0's `segmentStartMs`; apply beat 0's effects at segment entry (before its travel); call `actions.snapshot(union-of-all-beat-effects)` ONCE and store the restore thunk; capture the **previous keyframe for beat 0** (see the decision below); return a `Promise` resolved on end. Set `active = true`. +- `advance(nowMs)` runs every frame while active and **drives the camera**. For the current segment: + - `p = clamp((nowMs - segmentStartMs) / travelMs, 0, 1)` (a `travelMs: 0` beat — stage 00 — yields `p = 1` immediately: snap to the keyframe and dwell). + - Interpolate `logDist` **linearly** between `ln(prevDistanceMpc)` and `ln(beat.distanceMpc)`; `distance = Math.exp(that)`. (This is the load-bearing log-scale fact — assert the midpoint is the geometric mean, not the arithmetic mean.) + - Interpolate `target` by `Vec3` lerp between the previous resolved focus position and the current beat's resolved focus position (via `actions.resolveFocus`). + - Carry `yaw` / `pitch` **constant** (seed keeps orientation fixed; the cinematic adds azimuth/elevation + dwell-orbit). + - Call `actions.applyCameraPose(pose)` each frame. + - When `p` first reaches 1, call `actions.markFocused(beat.focus)` ONCE (the settle), set `settled`; then hold the settled pose during the dwell. + - Advance to the next segment when `nowMs - segmentStartMs >= travelMs + dwellMs`: stamp the new `segmentStartMs`, set the new previous-keyframe to the just-finished beat's resolved keyframe, apply the next beat's effects at entry, clear `settled`. + - Running off the end → invoke the restore thunk, resolve the promise, `active = false`. +- **Previous-keyframe-for-beat-0 decision (document the contract you pick):** the cleaner contract is to accept the camera's current pose at `start` via the port — i.e. resolve beat 0's *own* focus as both the from- and to-target so beat 0 with `travelMs: 0` simply settles on its keyframe (stage 00 is "held open" and frames the Milky Way). Pick this unless you find a reason the start pose must be read from the live camera; if you read the live camera, add a `resolveCurrentPose(): CameraPose` to the port and document it. **Prefer the simpler "from = beat 0's own keyframe" contract** — it needs no new port method and matches stage 00's "begins here, no travel" intent. +- `stop()`: invoke the restore thunk **exactly once** (guard so a completion racing `stop` doesn't double-invoke), resolve the pending promise, `active = false`. Subsequent `advance` is a no-op. +- `destroy()` calls `stop()`. + +**Tests** (fake `TourActions` recording calls; explicit `nowMs` clock; no GPU; `resolveFocus` returns deterministic fixture positions): + +- [ ] `start applies beat-0 effects then begins travel` — beat 0's `applyEffect` calls recorded at start, ordered before the first `applyCameraPose`; `isActive()` is `true`. +- [ ] `applyCameraPose called each advance while active` — N advances → N pose writes. +- [ ] `logDist interpolation is geometric at p=0/0.5/1` — for a beat from 1 Mpc → 100 Mpc, the pose distance at `p=0` is 1, at `p=1` is 100, and at `p=0.5` is `exp((ln1+ln100)/2) = 10` (the geometric mean), NOT 50.5 (arithmetic). +- [ ] `target Vec3 lerps between previous and current resolved focus` — assert the midpoint component-wise. +- [ ] `effects of a segment apply at segment entry, ordered before that segment's travel` — entering segment 1 records segment 1's effects before its first pose write. +- [ ] `markFocused fires once on settle` — exactly one `markFocused(beat.focus)` per beat, at the frame `p` first reaches 1, never during travel, never repeated during dwell. +- [ ] `advances to the next beat only after travelMs + dwellMs` — `advance(segmentStart + travelMs + dwellMs - 1)` does not advance; `advance(... + 0)` does. +- [ ] `plays all beats in order` — stepping the clock through every beat settles each focus once, in table order. +- [ ] `resolves the start() promise after the last beat's dwell` — the promise resolves once the final beat's `travelMs + dwellMs` elapses; `isActive()` becomes `false`. +- [ ] `stop() ends + resolves + restores once` — `stop()` mid-tour flips `isActive()` to `false`, resolves the pending promise, invokes the restore thunk exactly once. +- [ ] `restore on natural completion exactly once` — restore thunk invoked once on running off the end (and never double-invoked if a later `stop` follows). - [ ] `advance before start is a no-op` and `advance after completion is a no-op`. +- [ ] `point-focus beats resolve via resolveFocus` — a `{ kind: 'point', position }` beat drives the target from `resolveFocus`'s returned position (proves the seed actually exercises the point variant via stages 06/08/09). +- [ ] `travelMs:0 beat snaps to its keyframe and dwells` — a beat with `travelMs: 0` writes its keyframe pose on the first advance and holds through the dwell (stage 00). -- [ ] Implement `createTourSubsystem` against those tests (no body in this plan — read `tweenManager.ts` for the factory shape). `destroy()` calls `stop()`. +- [ ] Implement `createTourSubsystem` against those tests (no body in this plan — read `tweenManager.ts` for the factory shape). Use `Vec3` lerp from the project's math utils (cite the helper you find; do not open-code a tuple). - [ ] `npx vitest run tests/services/engine/subsystems/tourSubsystem.test.ts` → PASS. - [ ] Commit. -## Task 3: Engine wiring — actions adapter, subsystem registration, handle, frame tick + RoD gate +## Task 3: Engine wiring — TourActions adapter, registration, handle, frame tick + RoD gate **Files (modify):** -- `src/services/engine/engine.ts` — build the `TourActions` adapter, create + register the subsystem, add the `tour` sub-handle. +- `src/services/engine/engine.ts` — build the `TourActions` adapter, construct + register the subsystem, add the `tour` sub-handle, add teardown. - `src/services/engine/frame/runFrame.ts` — tick the subsystem + extend the reschedule gate. -- [ ] **TourActions adapter** (engine.ts, near the other internal helpers). `focus` dispatches on `TourFocus.kind`: - - `'milkyWay'` → the existing Milky-Way focus path (`engine.ts:760-784`). - - `'home'` → the home-focus path behind `camera.focusOnHome` (`engine.ts`, the `focusOnHome` impl). - - `'famous'` → the `selectFamous(id)` inline path (`engine.ts:790-804`). - - `'structure'` → `state.data.structures.byId(id)` (the structure store's by-id getter — `src/@types/engine/data/StructureStore.d.ts`, returns `StructureRecord | null`; the static anchors come from `buildStaticAnchorStructures()`), then `commitFocus`/`commitPoiFocus` (`helpers/commitPoiFocus.ts`). Missing id (`byId` → `null`) → skip silently (a renamed slug is a catalog bug, not a tour crash). - `applyEffect` dispatches each `TourEffect.kind` to the matching setter already in the handle literal (filaments `engine.ts:1241`, milkyWay `:1227`, sources `:1211`, volumes `:1294`, labels `:1258`/`:1273`). `snapshot(effects)` reads current values from `state.settings.*` for the touched keys and returns a restore thunk that re-applies them through the same setters. `requestRender` → `state.subsystems.scheduler.requestRender()`. -- [ ] **Register** `state.subsystems.tour = createTourSubsystem(tourActions)` alongside the other subsystem constructions; add it to teardown (`destroy`) with the others. -- [ ] **Sub-handle:** add `tour: { start: (beats) => state.subsystems.tour.start(beats), stop: () => state.subsystems.tour.stop(), isActive: () => state.subsystems.tour.isActive() }` to the `const handle: EngineHandle` literal (~`engine.ts:1184`). -- [ ] **Frame tick:** call `state.subsystems.tour.advance(nowMs)` once per frame in `runFrame.ts`, near the other subsystem ticks (`tweens.advance` at `:139`, `labelDirector.runFrame` at `:231`, `clusterFocus.update` at `:262`). Use the same `nowMs`/`performance.now()` the neighbours use. -- [ ] **Reschedule gate:** add `|| state.subsystems.tour.isActive()` to the `stillAnimating` predicate (`runFrame.ts:502-509`). +Cite these locations (verified against the live tree; **re-verify — they drift**): + +- [ ] **TourActions adapter** (engine.ts, build near the focus helpers — `focusOnHome` ~840-853, `focusOnMilkyWay` ~855-879, `selectFamous` ~885-904): + - `resolveFocus(focus)` → `Vec3 | null`: + - `milkyWay` → `MILKY_WAY_CENTER_WORLD` (imported at engine.ts:132 from `../../data/galacticCenter`). + - `home` → `state.initialCamSnapshot?.target` (null until bootstrap framing computed — return null then). + - `famous` → look up the famous-meta position the way `selectFamous` does: `state.data.galaxies.famousMeta.findIndex(m => m.id === id)` then read the position (confirm whether the world position is on the meta entry or must come from the famous cloud via `buildGalaxyInfo` — `selectFamous` at ~885-904 shows the lookup; reuse the same path, returning the world `Vec3`). + - `structure` → `state.data.structures.byId(id)?.worldPos` (StructureStore.byId returns `StructureRecord | null` — `src/@types/engine/data/StructureStore.d.ts:32`; `StructureRecord.worldPos` is a `Vec3` — `StructureRecord.d.ts`). Missing id → `null`. + - `point` → the literal `position`. + - A `null` from any branch → the subsystem skips the target (don't crash). + - `applyCameraPose(pose)` → write `state.cam`'s four orbit fields and re-derive the basis. **Reuse the existing snap path**: this is exactly what `snapToCameraSnapshot` (`cameraSnapshot.ts:84`) does (copy `target/distance/yaw/pitch`, call `updatePosition(cam)`, requestRender) — call it (or factor a `CameraPose`-shaped variant) rather than open-coding the field copy. Cam-null → no-op (it absorbs the guard). + - `markFocused(focus)` → set selection/focus state for label + ring emphasis **without moving the camera**. The existing commit helpers braid selection + tween: `commitStructureFocus` (`helpers/commitStructureFocus.ts:21`) does `selection.setSelected(...)` + `selection.setFocused(...)` THEN `tweenToStructure`; `commitGalaxyFocus` (`helpers/commitGalaxyFocus.ts:40`) does the same THEN `tweenToGalaxy`. The adapter must do **only the `setSelected` + `setFocused` writes**, omitting the tween — call `state.subsystems.selection.setSelected/setFocused` directly (for structure: `{ kind: 'structure', id }`; for famous: build the `GalaxyInfo` as `selectFamous` does and pass `{ kind: 'galaxy', source, localIdx }` + info). For `milkyWay` / `home` / `point` (no catalog object), `setFocused(null)` (matching `focusOnMilkyWay`'s "not a catalog object → drop the focus slot", engine.ts ~868). Document this split in the adapter's comment. + - `applyEffect(effect)` → dispatch each `TourEffect.kind` to the matching handle setter on `state` (the same setters the public handle wraps): `filaments` → the filaments enable path (handle literal ~1346-1360 / `boringSetters.setFilamentsEnabled` + fade); `milkyWay` → ~1332-1345; `source` → `setSourceVisible` (~943); `volume` → the volumes enable path (`volumes.setEnabled` wiring); `labelCategory` → `labels.setCategoryLabelVisible`; `markerCategory` → `labels.setCategoryMarkerVisible`. Prefer calling the same internal functions the handle delegates to. + - `snapshot(effects)` → read the current value for each touched key from `state.settings.*` (or the source draw mask for `source` effects) and return a thunk that re-applies them via the same setters as `applyEffect`. Read only the keys the passed effects touch. + - `requestRender()` → `state.subsystems.scheduler.requestRender()`. +- [ ] **Construct + register.** Build `tour: createTourSubsystem(tourActions)` **eagerly** in the subsystem literal alongside `structureFocus` (~636) — it has no GPU dependency. (The adapter closes over `state`, so construct it where `state` is in scope; if construction order needs the adapter first, mint the adapter just above the literal.) +- [ ] **Teardown.** Add `state.subsystems.tour.destroy();` to the destroy walk near the other eager subsystems (~1214, beside `structureFocus.destroy()`). +- [ ] **Sub-handle.** Add a `tour` sub-handle to the `const handle: EngineHandle` literal (~1293) delegating to the subsystem: `start: (beats) => state.subsystems.tour.start(beats)`, `stop: () => state.subsystems.tour.stop()`, `isActive: () => state.subsystems.tour.isActive()`. +- [ ] **Frame tick (ORDERING IS LOAD-BEARING).** In `runFrame.ts`, call `state.subsystems.tour.advance(nowMs)` **after** the other camera mutators — tweens (`runFrame.ts:138-140`), spaceMouse (`:151-153`), autoRotate (`:167-170`) — and before `deriveFrameContext` (`:179`). While the tour is active its pose write must be the authoritative camera state for the frame. Use the `nowMs` parameter the function already receives (the tour must NOT start a `tweenManager` tween — it owns the camera directly). +- [ ] **Reschedule gate.** Add `|| state.subsystems.tour.isActive()` to the `stillAnimating` predicate (`runFrame.ts:493-501`). - [ ] `npm run typecheck && npm test` → PASS. - [ ] Commit. -## Task 4: Beat table + App.tsx wiring (start / cancel / UI hide) +## Task 4: Beat table + App.tsx wiring (start / cancel / UI-hide) **Files:** -- Create: `src/data/tourBeats.ts` — `export const STUB_TOUR_DWELL_MS = 2_500;` and `export const TOUR_BEATS: readonly TourBeat[]` per the Itinerary table. +- Create: `src/data/tourBeats.ts` — `export const TOUR_BEATS: readonly TourBeat[]` per the beat table above. +- Create: `tests/data/tourBeats.test.ts`. - Modify: `src/components/App/App.tsx`. -- [ ] Author `TOUR_BEATS` exactly as the Itinerary table specifies (six beats; beat 5 carries the single `filaments` effect). Add a test `tests/data/tourBeats.test.ts`: `TOUR_BEATS has six beats ending on home` and `only the Coma beat toggles filaments` (guards the void-not-last + single-effect invariants). -- [ ] App.tsx: replace `onTour={splash.dismissTour}` with a `startTour` callback that: dismisses the splash (`splash.dismissTour()`), sets a `tourActive` state `true`, calls `engine.tour.start(TOUR_BEATS)`, and clears `tourActive` in the promise's `.finally`. Idempotent if `tourActive` already true. (Cite the current `useSplash` block + `` JSX — `grep -n "onTour\|useSplash\|Splash" src/components/App/App.tsx`.) -- [ ] Cancel-on-input: a `useEffect` armed only while `tourActive` adds capture-phase `pointerdown` / `keydown` / `wheel` / `touchstart` window listeners that call `engine.tour.stop()`; cleanup removes them. (`wheel`/`touchstart` passive.) The `start` promise resolves on stop, so the `.finally` clears `tourActive` and restores the UI — no token plumbing needed. -- [ ] Force `uiHidden` while active: add `tourActive` to the `uiStack` hidden condition (the line Plan 1 left as `(uiHidden || splash.splashVisible)`). +- [ ] Author `TOUR_BEATS` from the eleven stages per the beat table + the seed-vs-cinematic notes (collapse stage 03's pass-throughs; use `point` coords for 06/08/09; instant effects only; omit the flow toggle; map milliquas/mcpm only if the source/field ids confirm). Keep `src/data/` pure (no `services/` imports — it's identity data). +- [ ] Test `tests/data/tourBeats.test.ts`: + - `has 11 beats in stage order` — `TOUR_BEATS.length === 11` and `ids` equal the ordered stage slugs. + - `ends on milkyWay (home-again)` — the last beat's focus kind is `milkyWay`. + - `distance ladder spans ~0.05 → 6000 Mpc` — first/min ≈ 0.05, max ≈ 6000; the outbound legs (00→09) are monotonic-non-decreasing in `distanceMpc`. + - `void is mid-sequence, never last` — the `void-bootes-void` beat is not the final beat. + - `effects only where stages declare them` — only the beats that declare effects (e.g. stage 03 group toggles, stage 05 filaments) carry a non-empty `effects` array. +- [ ] App.tsx: replace `onTour={splash.dismissTour}` (currently App.tsx ~427; `useSplash.dismissTour` at `src/hooks/useSplash.ts:180-183`) with a `startTour` callback that: calls `splash.dismissTour()`, sets a new `tourActive` state to `true`, calls `handleRef.current?.tour.start(TOUR_BEATS)`, and clears `tourActive` in the promise's `.finally`. Idempotent if `tourActive` already true. (Add a `const [tourActive, setTourActive] = useState(false)` near the other UI state — `uiHidden` is at App.tsx ~150.) +- [ ] Cancel-on-input: a `useEffect` armed only while `tourActive` adds **capture-phase** `pointerdown` / `keydown` / `wheel` / `touchstart` window listeners that call `handleRef.current?.tour.stop()`; cleanup removes them. (`wheel` / `touchstart` registered `passive: true`.) The `start` promise resolves on stop, so the `.finally` clears `tourActive` and restores the UI — no token plumbing needed. +- [ ] Force the HUD hidden while active: add `tourActive` to the `uiStack` hidden condition — currently `(uiHidden || splash.splashVisible) && appStyles.uiStackHidden` at App.tsx ~215 → `(uiHidden || splash.splashVisible || tourActive)`. - [ ] `npm run typecheck && npm test` → PASS. - [ ] Commit. @@ -244,20 +390,31 @@ The void sits mid-sequence (not last) so the camera never *ends* on an empty reg - [ ] `npm run typecheck && npm test && npm run build` → all green. - [ ] Manual smoke (ask the user) in the live dev server: - 1. Tour button triggers the six-beat tour (camera flies the itinerary), not just a dismiss. - 2. UI chrome (left stack, top bar, status bar) is hidden while the tour plays; returns on end. - 3. Any click / scroll / key during the tour stops it cleanly at the current beat and restores the UI. - 4. Filaments are on for the Coma beat and restored to the pre-tour setting at end (toggle them on *before* starting to confirm restore-to-on as well as restore-to-off). - 5. Beats 1/2/3/4/5 show their auto-rendered visuals/labels (MW impostor + "You are here", M31 name, cluster/void/supercluster names). - 6. The render-on-demand loop keeps ticking through dwells (the tour doesn't stall between beats) and goes idle again after it ends. + 1. Tour button flies the **log-scale ladder** — uniform decades/sec, NOT linear jumps (the local universe doesn't blow past in the first frames of a long leg). + 2. The tour **owns the camera each beat** — the camera moves under tour control, not via a focus-tween snap. + 3. Dwell **holds** at each settled beat (no idle stall — the loop keeps ticking through dwells and goes idle again after the tour ends). + 4. Effects toggle on their beats and **restore** to the pre-tour state on end (toggle filaments on *before* starting to confirm restore-to-on as well as restore-to-off). + 5. Any click / scroll / key during the tour **stops it cleanly** at the current beat, restores the UI chrome, and the camera is left usable. + 6. **Point-focus stages work** — the deep field (08) and the edge (09) frame the origin and pull back to the horizon shell; stage 06 reframes the basin. 7. Clicking About mid-tour reopens the splash AND stops the tour (the click hits the cancel listener). -- [ ] Confirm the cinematic spec is still present and now references this as its decision-4 resolution: `ls docs/superpowers/specs/ | grep tour` → `2026-05-07-tour-animation-design.md`. +- [ ] Confirm the cinematic design is still the additive target: `ls docs/tour/stages/` shows the eleven stage files; this seed's `TOUR_BEATS` reads from their front-matter. --- +## Decisions baked in + +- **Per-frame camera ownership, not tweens.** The subsystem writes a `CameraPose` to the camera every frame while active (via `applyCameraPose` → `snapToCameraSnapshot`-style write), and runs *after* the other camera mutators so its pose is authoritative. The tour never starts a `tweenManager` tween. This is the single shape the cinematic extends. +- **Seed = a single linear-in-log segment per beat.** `logDist` lerps linearly between two keyframes (geometric interpolation of distance); the cinematic replaces this with a Catmull-Rom spline through N keyframes with arc-length reparam — additive, same keyframe inputs. +- **The real eleven-stage beat table.** Authored from `docs/tour/stages/00..10`, not a six-beat stub — so the cinematic only enriches each beat (captions, azimuth/elevation, pass-throughs), never re-authors the itinerary. +- **Captions / rampMs / azimuth-elevation / pass-through waypoints / dwell-orbit are OMITTED from the seed** and added additively by the cinematic. None reshape a type the seed ships (`caption?` and `rampMs?` are new optional fields; azimuth/elevation are new optional keyframe fields; pass-throughs are zero-dwell control points the spline reads). +- **`point` focus is exercised by the seed** (stages 06 / 08 / 09) — not reserved for later, so the variant is real and tested from day one. +- **Reserve nothing speculative.** No empty caption/ramp slots, no two-camera-mode switch, no unused fields. Every type the seed ships is fully consumed by the seed; the cinematic adds new fields/subsystems when it needs them. +- **`CameraPose` is its own narrow type** (four orbit fields), distinct from `InitialCam` (which also carries the immutable framing fields the tour must never write). +- **`markFocused` is the un-braided half of the commit helpers** — selection/focus writes only, no camera tween — so emphasis (labels + rings) and camera motion stay separable, the way the cinematic needs them. + ## Self-review notes -- **Seed, not stub.** `engine.tour` + `tourSubsystem` + `TourBeat`/`TourEffect`/`TourFocus` are the foundation the cinematic tour extends: richer beats (groups/volumes/milliquas/horizon via existing `TourEffect` variants), per-leg `dwellMs` (already per-beat), `caption` (the wired seam for the future caption producer), and — the one genuinely new build — rotation-toward-target slerp in the focus tween (cinematic decision 1). None of those require reshaping what this plan ships. -- **Plan-style compliance:** type contracts + test names/assertions included; no implementation bodies; integration points cited by `file:line`. Verify the cited line numbers against the live tree on pickup — they will have drifted. -- **Convention checks:** one type per `@types` file (six files in Task 1); `readonly` throughout; no barrels; tests mirror `src/`. -- **Removed from the original plan:** `TourCancelToken` + `createTourCancelToken` (the engine now owns lifecycle via `stop()` + the completion promise), and the React `sleep`-chain runner (replaced by the frame-driven subsystem). +- **Seed, not stub.** `engine.tour` + `tourSubsystem` + the `Tour*` / `CameraPose` types are the foundation the cinematic tour *extends*, not throws away. The camera evaluator is the cinematic's evaluator running a trivial interpolation; swapping the log-lerp for a spline and adding the omitted optional fields is the whole cinematic camera delta. +- **Plan-style compliance:** type contracts + test names/assertions included; no implementation bodies; integration points cited by `file:line`. Re-verify the cited line numbers against the live tree on pickup — engine.ts and runFrame.ts churn. +- **Convention checks:** one type per `@types` file (seven files in Task 1); `readonly` throughout; `Vec3` aliases not raw tuples; no barrels; tests mirror `src/`. +- **Corrected paths from the prior draft:** `useSplash` lives at `src/hooks/useSplash.ts` (not under `components/Splash/`); the focus-mode subsystem is `structureFocus` (not `clusterFocus`); the frame body is `src/services/engine/frame/runFrame.ts` (`stillAnimating` at ~493-501, camera mutators at ~138-170); the handle literal is at engine.ts ~1293; eager subsystem construction ~636, teardown ~1214. diff --git a/docs/superpowers/specs/2026-06-08-pre-tour-decomplection-design.md b/docs/superpowers/specs/2026-06-08-pre-tour-decomplection-design.md new file mode 100644 index 00000000..fc144cc9 --- /dev/null +++ b/docs/superpowers/specs/2026-06-08-pre-tour-decomplection-design.md @@ -0,0 +1,195 @@ +# Pre-Tour Decomplection — Camera Authority + Settings Seam (design) + +> **Status:** approved design, awaiting implementation plans. +> **Why this exists:** the guided-tour feature (`docs/tour/`, and the engine-seed +> plan `docs/superpowers/plans/2026-05-20-splash-screen-02-stub-tour.md`) adds a +> subsystem that must own the camera every frame and toggle/restore visual layers. +> An `entanglement-radar` pass (2026-06-08) over the camera path and `engine.ts` +> found two knots the tour would otherwise fight and amplify. We decomplect them +> **first**, so the tour lands as a clean citizen instead of a fourth +> implicit-ordering dependency. Neither knot is in the documented backlog +> (`simplicity.md` "Known entanglements") — both are new findings. + +## Scope + +Two independent, behaviour-preserving refactors, executed in order: + +1. **Camera-driver authority** — a single per-frame authority for who writes + `state.cam`, replacing implicit call-order + ad-hoc guards. +2. **Settings snapshot/restore seam** — a single geography-aware read of the + toggleable visual layers, plus a write-path with an `animate` flag. + +Then the tour plan is reconciled to build on both. + +**Out of scope (deferred hygiene, do not scope-creep):** mask→registry migration +for source visibility; moving `sources.tier` into `settings`; generalising the +camera tween's hardcoded channels (fov/near/far); a subsystem auto-teardown +registry; splitting the focus helpers' selection/move/distance braid (the tour +sidesteps it via its own port and does not need it changed). + +--- + +## 1. Camera-driver authority + +### The knot today + +`runFrame.ts:138-170` mutates `state.cam` through three drivers in a fixed call +order — `tweens.advance` → `spaceMouse.applyToCamera` → `autoRotate` yaw — with +correctness resting on the **call order itself** plus ad-hoc guards +(`autoRotate` runs only `!tweens.isActive()`; `spaceMouse` cancels the tween then +writes). "Who controls the camera this frame" is implicit. Adding the tour as a +fourth driver by "running it after the others so its write wins" would add a +fourth implicit-ordering dependency — the braid we are removing. + +### The un-braided shape + +Precedence becomes **data**, and there is **one** camera-write site. + +```ts +// src/@types/engine/camera/CameraDriver.d.ts (one type per file) +import type { OrbitCamera } from '...'; +export type CameraDriver = { + readonly id: string; // 'input' | 'tour' | 'tween' | 'autoRotate' + readonly priority: number; // input 100 > tour 80 > tween 60 > autoRotate 20 + isActive(nowMs: number): boolean; // wants to write this frame + apply(cam: OrbitCamera, nowMs: number): void; +}; +``` + +A small registry/resolver (new module under `src/services/engine/camera/`, +e.g. `cameraDrivers.ts`). Once per frame, replacing `runFrame.ts:138-170`: + +```ts +const active = drivers.filter(d => d.isActive(nowMs)).sort(byPriorityDesc); +active[0]?.apply(state.cam, nowMs); +``` + +The existing pieces become thin driver wrappers: + +| Driver | Wraps | `isActive` | Notes | +|---|---|---|---| +| `input` (100) | `spaceMouse.applyToCamera` | `spaceMouse.hasAxes()` | | +| `tour` (80) | the tour subsystem's per-frame evaluator | `tour.isActive()` | added by the tour plan | +| `tween` (60) | `tweenManager.advance` | `tweenManager.isActive()` | | +| `autoRotate` (20) | the yaw increment | `settings.camera.autoRotate` | the `!tweens.isActive()` guard **disappears** — it is just lower priority | + +### Preserved behaviour (not changed by this refactor) + +- **Input interrupts animation.** Input — spaceMouse puck *or* mouse-drag via + `OrbitControls` — cancels any active `tween` and `tour`, exactly as today's + `cancelTween` callback does (now also stopping the tour). Precedence only + resolves the same-frame race; cancellation is the real interrupt mechanism. + `OrbitControls` stays an event-driven writer between frames, not a registered + per-frame driver; it cancels active higher drivers on input. +- **Idle is static.** With no driver active and `autoRotate` off, no `apply` + runs and the camera holds. + +### Bonus decomplection — RoD gate + +The render-on-demand `stillAnimating` predicate (`runFrame.ts:493-501`) currently +hand-lists camera animation terms (`tweens.isActive()`, `spaceMouse.hasAxes()`, +`autoRotate`). Its camera terms collapse to `drivers.some(d => d.isActive(nowMs))` +— the registry owns "is the camera animating." The non-camera terms +(`texturedDisks.hasInFlightWork()`, `fades.isAnyAnimating()`, +`flowFieldRenderer.isAnimating()`, `structureFocus.isAwake()`) stay as-is. + +### Testing + +- The resolver is pure: with fake drivers of known priority + `isActive`, it + selects the single highest-priority active driver and calls its `apply`; with + none active it writes nothing. +- Each wrapper's `isActive`/`apply` is unit-tested against its underlying piece. +- A regression test pins "tween active + autoRotate on → tween wins, autoRotate + does not nudge yaw" (the behaviour the old `!tweens.isActive()` guard encoded). + +--- + +## 2. Settings snapshot/restore seam + +### The knot today + +The toggleable visual layers live in **four** storage shapes: +`settings.filaments.enabled` / `settings.milkyWay.enabled` (booleans), +`sources.drawMask`/`pickMask` (bitmasks), `settings.volumes.{masterEnabled,fields}`, +and `settings.labelCategoryVisibility` / `markerCategoryVisibility` (dicts). A +programmatic caller (the tour's `snapshot`/`restore`/`applyEffect`) must know all +four. Separately, **fade orchestration is welded into the setters** +(`setSourceVisibleImpl` is async and *awaits* fades), so an instant restore via +the public setters becomes a sequential async chain. + +### The un-braided shape + +```ts +// src/@types/engine/settings/VisibilitySnapshot.d.ts (plain data, one type/file) +export type VisibilitySnapshot = { + readonly sourceDrawMask: number; + readonly filamentsEnabled: boolean; + readonly milkyWayEnabled: boolean; + readonly volumesMasterEnabled: boolean; + readonly volumeFieldEnabled: Readonly>; + readonly labelCategoryVisibility: Readonly>; + readonly markerCategoryVisibility: Readonly>; +}; +``` + +- `readVisibility(state): VisibilitySnapshot` — the **single** geography-aware + reader. Callers stop hand-coding where each toggle lives. +- `applyVisibility(state, patch, { animate }): void` — the **single** write-path. + The one new capability is the `animate` flag: `animate: true` routes through the + existing fade orchestration; `animate: false` sets the value instantly. Where + clean, the existing handle visibility setters route through this same path so we + do not introduce a second write path; otherwise they remain and we accept narrow + overlap (noted, not hidden). + +### Testing + +- `readVisibility` returns the live values from each storage shape. +- `applyVisibility(read → mutate → apply)` round-trips: reading a snapshot, + changing layers, then applying the snapshot restores every layer. +- `applyVisibility(..., { animate: false })` writes synchronously (no pending + fades); `{ animate: true }` drives the fade registry. + +--- + +## 3. How the tour plugs into both + +Reconciles the rewritten engine-seed plan +(`docs/superpowers/plans/2026-05-20-splash-screen-02-stub-tour.md`, Task 3): + +- **Camera:** instead of "tour writes pose *after* the other mutators," the tour + registers a `tour` `CameraDriver` (priority 80). `TourActions.applyCameraPose` + becomes that driver's `apply`, writing `state.cam` via `snapToCameraSnapshot`. + The resolver handles ordering; no after-the-others hack. +- **Effects/restore:** `TourActions.snapshot` = `readVisibility` over the touched + layers; `restore` = `applyVisibility(snapshot)`; `applyEffect` = + `applyVisibility(single-layer patch, { animate })` (`animate: false` for the + instant-toggle seed; `animate: true` is the seam the cinematic ramped effects + use). + +These are edits to the tour plan's Task 1/3, applied when that plan is picked up +after the two refactors land. + +--- + +## 4. Execution order + +1. **Camera-driver authority** — new `CameraDriver` type + `cameraDrivers` + resolver/registry + wrapper drivers; rewrite `runFrame.ts:138-170`; collapse + the RoD camera terms. Behaviour-preserving; tests first. +2. **Settings seam** — `VisibilitySnapshot` type + `readVisibility` + + `applyVisibility({ animate })`; route the existing setters through it where + clean. Behaviour-preserving; tests first. +3. **Tour engine seed** — the existing plan, reconciled per §3. + +Each refactor reproduces current behaviour as tests before changing structure, so +"green before and after" is the gate. + +## Decisions baked in + +- Single-writer arbitration (one driver writes per frame); **no** cooperative + blending of multiple drivers. Input cancels tour/tween rather than co-animating. +- Precedence is data (`priority`); the registry also answers "is the camera + animating" for the RoD gate. +- Settings seam is minimal: a geography-aware read + a write-path with an + `animate` flag. **No** mask→registry migration, **no** `tier`-into-settings. +- One spec, three sequential plans/efforts; each behaviour-preserving and tested. From 897d962218eaf98fb2b19ca014b361279d0ff2d9 Mon Sep 17 00:00:00 2001 From: Alexander Rulkens Date: Mon, 8 Jun 2026 00:41:33 +0200 Subject: [PATCH 2/7] docs(tour): implementation plans for the two pre-tour decomplections Camera-driver authority: CameraDriver type + pure runCameraDrivers resolver + wrapper drivers on RunFrameDeps, replacing the implicit call-order camera-mutation block and folding the RoD camera terms into the registry. Behaviour-preserving with regression tests pinning tween-wins-over-autoRotate and idle-holds. Settings visibility seam: VisibilitySnapshot plain-data type + readVisibility + applyVisibility({animate}) single write-path, with the existing clean setters delegated through it and the async setSourceVisibleImpl deliberately left as the toggle path. Both plans are contract-only per plan-style and execute via subagent-driven-development. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-06-08-camera-driver-authority.md | 410 +++++++++++++++ .../2026-06-08-settings-visibility-seam.md | 497 ++++++++++++++++++ 2 files changed, 907 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-08-camera-driver-authority.md create mode 100644 docs/superpowers/plans/2026-06-08-settings-visibility-seam.md diff --git a/docs/superpowers/plans/2026-06-08-camera-driver-authority.md b/docs/superpowers/plans/2026-06-08-camera-driver-authority.md new file mode 100644 index 00000000..0478d55c --- /dev/null +++ b/docs/superpowers/plans/2026-06-08-camera-driver-authority.md @@ -0,0 +1,410 @@ +# Camera-driver authority — implementation plan + +> **REQUIRED SUB-SKILL:** execute this plan with `superpowers:subagent-driven-development` +> (fresh implementer subagent per task + spec + quality reviews). Each implementer +> EDITS FILES ONLY; the **main thread** runs `npm test` / `npm run typecheck` and +> commits. Dispatch implementers with `run_in_background: true`. + +## Goal + +Establish a single per-frame camera-control authority. Replace the implicit +call-order + ad-hoc guards in `runFrame.ts`'s per-frame camera-mutation block +(`tweens.advance` → `spaceMouse.applyToCamera` → guarded auto-rotate) with a +**driver registry + one resolver** that writes `state.cam` exactly once. Precedence +becomes data (`priority`), and the resolver also answers "is the camera animating?" +for the render-on-demand gate. + +This is **behaviour-preserving**. The `tour` driver (priority 80) is added LATER +by the tour plan — this plan stops at three drivers (`input` 100, `tween` 60, +`autoRotate` 20). Every task reproduces current behaviour as a test FIRST, then +refactors green. + +Source of truth: `docs/superpowers/specs/2026-06-08-pre-tour-decomplection-design.md` +section "1. Camera-driver authority". + +## Architecture + +The un-braided shape (from the spec): + +``` +runFrame.ts (one camera-write site) + └─ runCameraDrivers(drivers, state.cam, nowMs) ← pure resolver + picks the single highest-priority active driver, calls its apply() + +drivers: readonly CameraDriver[] (built in startLoop, carried on RunFrameDeps) + ├─ input (100) wraps spaceMouse: isActive=hasAxes, apply=applyToCamera + ├─ tween (60) wraps tweenManager: isActive=isActive, apply=advance + └─ autoRotate (20) isActive=settings.camera.autoRotate, apply=yaw increment + (tour (80) added later by the tour plan — NOT here) +``` + +Cancellation is unchanged and is the real interrupt mechanism: the SpaceMouse +subsystem still calls its `cancelTween` callback when the puck deflects +(`spaceMouseSubsystem.ts:204`); mouse-drag still cancels via its existing path. +The registry only resolves the **same-frame race** between active drivers. The +auto-rotate `!tweens.isActive()` guard is **deleted** — auto-rotate is just lower +priority. + +The RoD `stillAnimating` predicate's three camera terms +(`settings.camera.autoRotate`, `tweens.isActive()`, `spaceMouse.hasAxes()`) +collapse to `drivers.some(d => d.isActive(nowMs))`. The non-camera terms stay. + +### Skymap conventions reminder (these override defaults) + +- **`type` aliases, never `interface`.** One exported type per file under + `src/@types`. Filename = export name. +- **Readonly + values.** `readonly` fields; pure functions; no mirror state. +- **`Vec3` alias, never raw tuples** (not needed for the new types here, but the + rule stands if you touch camera math). +- **Didactic comments.** Multi-paragraph module headers explaining _why_ and + _what the alternative was_ — match the style of `tweenManager.ts` / + `spaceMouseSubsystem.ts`. Comments are timeless: no dates, no PR refs, no + "pre-X" history. +- **No `git add -A`/`.`** — stage specific paths. Branch + PR; squash-merge. +- **Re-verify every cited `file:line`** before editing — line numbers drift. +- **Behaviour-preserving:** "green before and after" is the gate. Write the + current-behaviour test FIRST, watch it pass against the OLD code, THEN refactor + and watch it stay green. (For the resolver's own brand-new unit tests, the + red→green order is the usual TDD: test fails because the module doesn't exist, + then passes.) + +### Verified current-tree facts (re-verify line numbers; they drift) + +- Per-frame camera mutation block: `src/services/engine/frame/runFrame.ts` + ~138-170 — `state.subsystems.tweens.advance(state.cam, performance.now())` + (~138-140); `state.subsystems.spaceMouse.applyToCamera(state.cam, performance.now())` + (~151-153); auto-rotate yaw `if (state.settings.camera.autoRotate && state.cam && !state.subsystems.tweens.isActive()) { state.cam.yaw += 0.000873; updatePosition(state.cam); }` + (~167-170). +- RoD `stillAnimating`: `runFrame.ts` ~493-501 — disjunction. Camera terms: + `state.settings.camera.autoRotate`, `state.subsystems.tweens.isActive()`, + `state.subsystems.spaceMouse.hasAxes()`. Non-camera terms (KEEP): + `(ready && state.subsystems.texturedDisks.hasInFlightWork())`, + `state.subsystems.fades.isAnyAnimating(nowMs)`, + `state.subsystems.structureFocus.isAwake(nowMs)`, + `state.gpu.flowFieldRenderer?.isAnimating(state.settings.flow) === true`. +- `TweenManager`: `src/@types/camera/TweenManager.d.ts`; factory + `src/services/engine/camera/tweenManager.ts:52`. Methods: `start`, `cancel`, + `isActive(): boolean`, `advance(cam, nowMs): boolean`, `destroy`. +- `advanceCameraTween` (the lerp): `src/services/camera/cameraTween.ts:69` — calls + `updatePosition` internally; does NOT tween fov/near/far. +- SpaceMouse subsystem: `src/services/engine/subsystems/spaceMouseSubsystem.ts`; + type `src/@types/engine/subsystems/SpaceMouseSubsystem.d.ts`. Methods include + `hasAxes(): boolean` (`:187`) and `applyToCamera(cam, nowMs): void` (`:190`). + `applyToCamera` calls `cancelTween()` (`:204`) when axes are non-zero — that is + the input-interrupts-animation mechanism. KEEP IT. +- OrbitControls: `src/services/camera/orbitControls.ts` — mutates `state.cam` on + pointer events OUTSIDE the frame loop; reports via `OrbitControlsOptions.onCameraChange` + (`src/@types/camera/OrbitControlsOptions.d.ts:53`). It is NOT a per-frame driver + and is NOT touched by this plan. (Its tween-cancel-on-grab wiring lives in + engine.ts's input wiring — leave it as-is.) +- `OrbitCamera`: `src/@types/camera/OrbitCamera.d.ts`. Import path from the new + driver type: `'../../../camera/OrbitCamera'` (see Task 1 for the exact relative + depth from `src/@types/engine/camera/`). +- `updatePosition`: `src/services/camera/orbitCamera.ts` (imported in `runFrame.ts:40` + as `import { updatePosition } from '../../camera/orbitCamera'`). +- `state.settings.camera.autoRotate`: `boolean` + (`src/@types/settings/EngineSettingsState.d.ts:86-87`). +- `EngineSubsystemHandles`: `src/@types/engine/handles/EngineSubsystemHandles.d.ts` + — has `tweens: TweenManager`, `spaceMouse: SpaceMouseSubsystem`. +- `RunFrameDeps`: `src/@types/engine/frame/RunFrameDeps.d.ts`; assembled in + `src/services/engine/phases/startLoop.ts:115-131`; consumed in `runFrame.ts:64`. +- Engine subsystem construction: `src/services/engine/engine.ts:569` (`tweens`), + `:575` (`spaceMouse`). The driver list's home is decided in Task 3 (built in + `startLoop`, carried on `RunFrameDeps` — see that task's rationale). +- No existing `CameraDriver` / `cameraDrivers` / `drivers` symbol in `src/` + (verified). This is greenfield for the type + resolver. + +--- + +## Task 1: `CameraDriver` type + +**Files:** +- Create `src/@types/engine/camera/CameraDriver.d.ts` + +**Contract:** + +```ts +import type { OrbitCamera } from '../../../camera/OrbitCamera'; + +export type CameraDriver = { + readonly id: string; // 'input' | 'tween' | 'autoRotate' (tour added later) + readonly priority: number; // input 100 > tour 80 > tween 60 > autoRotate 20 + isActive(nowMs: number): boolean; // wants to write state.cam this frame + apply(cam: OrbitCamera, nowMs: number): void; +}; +``` + +- [ ] Verify the `OrbitCamera` import resolves: from + `src/@types/engine/camera/CameraDriver.d.ts`, the path to + `src/@types/camera/OrbitCamera.d.ts` is `'../../../camera/OrbitCamera'` + (`engine/camera/` → up 3 → `@types/` → `camera/OrbitCamera`). Confirm against + the live tree before committing. +- [ ] Write the type exactly as above, with a didactic module header explaining: + the type is the seam that makes camera precedence **data** instead of call + order; `id` is for debugging/identity, `priority` orders the resolver, `isActive` + answers both "should I write this frame" and (collectively) "is the camera + animating" for the RoD gate; `apply` is the single mutation each driver performs. +- [ ] Main thread: `npm run typecheck` passes (no consumer yet — this just proves + the import path). +- [ ] Commit. + +--- + +## Task 2: `runCameraDrivers` resolver (pure) + +**Files:** +- Create `src/services/engine/camera/cameraDrivers.ts` +- Create `tests/services/engine/camera/cameraDrivers.test.ts` + +**Signature:** +`runCameraDrivers(drivers: readonly CameraDriver[], cam: OrbitCamera, nowMs: number): void` + +**Behaviour:** filter `drivers` to those with `isActive(nowMs) === true`, select the +single highest-`priority` one, call its `apply(cam, nowMs)`. If none active, write +nothing (no `apply` call). Pure over the driver list — no captured state, no I/O. +(`OrbitCamera` is only forwarded to `apply`; the resolver never reads or mutates +`cam` itself, so the tests can pass a trivial cam stub.) + +**Tests** (TDD red→green; fake drivers, `cam` is a throwaway stub): + +- [ ] `runCameraDrivers calls the single highest-priority active driver` — two + active drivers (priority 60 + 20); assert the priority-60 driver's `apply` was + called once and the priority-20 driver's `apply` was NOT called. +- [ ] `runCameraDrivers ignores inactive higher-priority drivers` — priority-100 + driver `isActive=false`, priority-20 driver `isActive=true`; assert the + priority-20 `apply` is called (an inactive higher driver does not block a lower + active one). +- [ ] `runCameraDrivers writes nothing when no driver is active` — all drivers + `isActive=false`; assert no `apply` is called on any driver. +- [ ] `runCameraDrivers forwards cam and nowMs to apply` — assert the active + driver's `apply` received the exact `cam` reference and `nowMs` value passed in. +- [ ] Run the suite (`npm test -- cameraDrivers`) — fails (module absent). +- [ ] Implement `runCameraDrivers` with a didactic header: this is the ONE + camera-write site; single-writer arbitration (no cooperative blending — the spec + bakes that in); precedence is data. Note it does not sort-mutate the input array + (treat `drivers` as readonly). +- [ ] Main thread: `npm test -- cameraDrivers` → all pass; `npm run typecheck`. +- [ ] Commit. + +--- + +## Task 3: Wrapper drivers + driver list in `startLoop`, carried on `RunFrameDeps` + +**Decision — where the list lives:** build the `drivers` array in +`startLoop.ts` (where `state.subsystems` and `state.settings` are all in scope) +and carry it on `RunFrameDeps` as `readonly drivers: readonly CameraDriver[]`. +Rationale: `RunFrameDeps` already exists to carry per-frame closure captures +assembled once at loop start (`startLoop.ts:115`); the drivers are exactly that. +Putting them on `EngineState` would widen the state contract for one consumer +(runFrame) and invite mirror-state. The wrapper drivers close over +`state.subsystems.{spaceMouse,tweens}` and `state` (for `settings.camera.autoRotate`), +so the closures stay live as settings change. + +**Files:** +- Modify `src/@types/engine/frame/RunFrameDeps.d.ts` (add `drivers` field) +- Modify `src/services/engine/phases/startLoop.ts` (build the three wrapper drivers + + the `drivers` array; add to `frameDeps`) +- Create `tests/services/engine/camera/cameraDriverWrappers.test.ts` (wrapper + `isActive`/`apply` mapping) + +**Contract — `RunFrameDeps` addition:** +```ts +import type { CameraDriver } from '../camera/CameraDriver'; +// ... +/** + * Camera-control drivers, highest priority first is NOT assumed — the + * resolver sorts by `priority`. Built once at loop start; the resolver + * (`runCameraDrivers`) picks the single active winner each frame and is + * also the source of truth for "is the camera animating" (RoD gate). + */ +readonly drivers: readonly CameraDriver[]; +``` + +**Wrapper drivers** (constructed inline in `startLoop`, closing over `state`): +- `input` — `{ id: 'input', priority: 100, isActive: () => state.subsystems.spaceMouse.hasAxes(), apply: (cam, nowMs) => state.subsystems.spaceMouse.applyToCamera(cam, nowMs) }` +- `tween` — `{ id: 'tween', priority: 60, isActive: () => state.subsystems.tweens.isActive(), apply: (cam, nowMs) => { state.subsystems.tweens.advance(cam, nowMs); } }` +- `autoRotate` — `{ id: 'autoRotate', priority: 20, isActive: () => state.settings.camera.autoRotate, apply: (cam) => { cam.yaw += 0.000873; updatePosition(cam); } }` + +(The implementer writes the bodies; the above pins the contract — `isActive` +predicates, the yaw delta `0.000873`, and that `autoRotate.apply` calls +`updatePosition`. Import `updatePosition` from `../../camera/orbitCamera` in +`startLoop.ts` — verify the relative path from `src/services/engine/phases/`.) + +> **Note on `nowMs` vs `performance.now()`:** today `tweens.advance` and +> `spaceMouse.applyToCamera` are called with `performance.now()` inline +> (`runFrame.ts:139,152`), while the auto-rotate block reads no time. After this +> refactor the resolver is called with `runFrame`'s `nowMs` parameter (the value +> `startLoop` passes as `performance.now()` at the call site, +> `startLoop.ts:143`). These are the same wall-clock source, so behaviour is +> preserved. Use the `nowMs` the resolver receives — do NOT re-read +> `performance.now()` inside the wrappers. + +**Tests** (wrapper mapping — each wrapper's predicate maps to its underlying piece): + +- [ ] `input driver isActive reflects spaceMouse.hasAxes` — fake spaceMouse with + toggleable `hasAxes`; assert the driver's `isActive` returns the same boolean + both ways. +- [ ] `input driver apply forwards to spaceMouse.applyToCamera` — assert the fake's + `applyToCamera` was called with the cam + nowMs. +- [ ] `tween driver isActive reflects tweenManager.isActive` — fake tween manager; + assert both booleans. +- [ ] `tween driver apply forwards to tweenManager.advance` — assert `advance` + called with cam + nowMs. +- [ ] `autoRotate driver isActive reflects settings.camera.autoRotate` — flip the + setting; assert the boolean tracks it. +- [ ] `autoRotate driver apply increments yaw and updates position` — assert + `cam.yaw` increased by `0.000873` and `updatePosition` ran (spy or assert + `position` recompute via a real-ish cam stub). + +> **How to test the wrappers in isolation:** the wrappers are currently inline in +> `startLoop`. To unit-test them without booting the engine, extract a small pure +> builder `buildCameraDrivers(state): readonly CameraDriver[]` co-located in +> `cameraDrivers.ts` (the module from Task 2), taking the slices it needs +> (`state.subsystems.spaceMouse`, `state.subsystems.tweens`, and a +> `() => state.settings.camera.autoRotate` getter — or just `state`). `startLoop` +> then calls `buildCameraDrivers(state)`. This keeps the wrapper logic pure and +> testable and keeps `startLoop` a thin assembler. Decide the exact argument shape +> when you read the live `EngineState`; prefer passing `state` and reading the +> slices inside, matching how other subsystems close over `state`. + +- [ ] Run the wrapper suite — fails (builder absent). +- [ ] Implement `buildCameraDrivers` in `cameraDrivers.ts`; wire `startLoop` to + call it and put the result on `frameDeps.drivers`; add the `drivers` field to + `RunFrameDeps.d.ts`. +- [ ] Main thread: `npm test -- cameraDriver` (resolver + wrappers); `npm run typecheck`. +- [ ] Commit. + +--- + +## Task 4: Rewrite `runFrame`'s camera block + RoD gate (the behaviour-preserving swap) + +**Files:** +- Modify `src/services/engine/frame/runFrame.ts` +- Modify `tests/services/engine/frame/runFrame.test.ts` (add regression tests) + +**The two edits:** + +1. Replace `runFrame.ts` ~138-170 (the three `if (state.cam) { … }` blocks for + tween-advance, spaceMouse-apply, and the guarded auto-rotate) with a single + guarded resolver call: + ```ts + if (state.cam) { + runCameraDrivers(deps.drivers, state.cam, nowMs); + } + ``` + - The `!state.subsystems.tweens.isActive()` guard on auto-rotate is **DELETED** + — auto-rotate's lower priority now encodes "tween wins". Carry over the + didactic comment, rewritten to explain the driver-precedence model (cite the + spec). Remove the now-stale per-block comments about call order. + - Import `runCameraDrivers` from `../camera/cameraDrivers` (verify relative path + from `src/services/engine/frame/`). + +2. Replace the **camera terms only** of the `stillAnimating` disjunction + (`runFrame.ts` ~493-501) — `state.settings.camera.autoRotate`, + `state.subsystems.tweens.isActive()`, `state.subsystems.spaceMouse.hasAxes()` + — with `deps.drivers.some((d) => d.isActive(nowMs))`. KEEP the non-camera terms + exactly (`texturedDisks.hasInFlightWork()` with its `ready &&` guard, + `fades.isAnyAnimating(nowMs)`, `structureFocus.isAwake(nowMs)`, + `flowFieldRenderer?.isAnimating(...)`). Update the predicate-breakdown comment. + +**Regression tests** (these encode CURRENT behaviour — write them so they describe +what the existing tree does, then confirm green after the swap). The existing +`runFrame.test.ts` builds a minimal `EngineState` fixture and a `RunFrameDeps` +fixture (`makeState()` at ~line 47); extend those — the new `drivers` field must be +added to the deps fixture (use the real `buildCameraDrivers(state)` so the fixture +exercises the production wiring, or hand-built fake drivers where that's simpler). + +- [ ] `runFrame: tween active + autoRotate on → tween wins, autoRotate does not nudge yaw` + — fixture: `state.settings.camera.autoRotate = true`, tween manager `isActive` + returns true and its `advance` sets cam to a known pose; spaceMouse `hasAxes` + false. Run `runFrame`. Assert: `tween.advance` was called; `cam.yaw` equals the + tween's pose (NOT pose + 0.000873) — i.e. auto-rotate did NOT add its delta on + top. (This is the behaviour the deleted `!tweens.isActive()` guard encoded.) +- [ ] `runFrame: idle (no driver active, autoRotate off) → camera holds` — all + driver predicates false; run `runFrame`; assert `cam.yaw` (and target/distance/ + pitch) are unchanged. +- [ ] `runFrame: autoRotate on, nothing else active → yaw advances by 0.000873` + — only `autoRotate.isActive` true; assert `cam.yaw` increased by exactly the + delta and `updatePosition` ran. (Confirms the lower-priority driver still fires + when it is the sole active one.) +- [ ] `runFrame: RoD stays awake iff a camera driver is active` — two sub-cases: + with one driver active (e.g. autoRotate on) assert `scheduler.requestRender` was + called; with no driver active AND all non-camera terms false assert it was NOT + called. (Mirrors the old camera terms; mock `reevaluateDemand` as the existing + file already does.) + +> The existing `runFrame.test.ts` mocks `reevaluateDemand` (`:29`). Keep that mock. +> The fixture short-circuits the GPU body via the renderer-null guard +> (`deriveFrameContext` → early return), so the camera block + the RoD tail are +> exercisable without a GPU. Confirm the camera block runs BEFORE the +> `deriveFrameContext` early-return (it does today, `runFrame.ts:138` is above +> `:179`) so these tests reach it. The RoD tail runs only if the body does NOT hit +> an earlier `return` — verify the fixture reaches line ~493 (it must have +> `state.cam` non-null and pass the catalog/hover gates or hit the +> `deriveFrameContext` early-return path which itself `requestRender`s; design the +> RoD assertions around the path the fixture actually takes — read the body +> top-to-bottom against your fixture before asserting). + +- [ ] Run `npm test -- runFrame` with the NEW tests against the OLD camera block + first if feasible (to confirm they encode current behaviour); then apply the swap + and confirm still green. If running against old code first is impractical given + the resolver dependency, write the tests and the swap together but keep each + assertion tied to a documented current-behaviour fact above. +- [ ] Main thread: `npm test -- runFrame`; full `npm test`; `npm run typecheck`. +- [ ] Commit. + +--- + +## Task 5: Entanglement-radar pass + dead-code sweep + +**Files:** (review only; small edits if found) +- `src/services/engine/frame/runFrame.ts` +- `src/services/engine/phases/startLoop.ts` +- `src/services/engine/camera/cameraDrivers.ts` + +- [ ] Run the `entanglement-radar` skill over the diff. Confirm: precedence is now + data (no `if (source/order)` chain), one camera-write site, the RoD camera terms + collapsed to the registry, no mirror of the driver list on `EngineState`. +- [ ] Grep for any remaining direct `state.subsystems.tweens.advance` / + `spaceMouse.applyToCamera` / `cam.yaw += 0.000873` outside the driver wrappers — + there should be none in `runFrame.ts`. (Use Grep tool, not bash grep.) +- [ ] Confirm the SpaceMouse `cancelTween` mechanism (`spaceMouseSubsystem.ts:204`) + and the OrbitControls/engine.ts mouse-drag tween-cancel wiring are UNTOUCHED — + cancellation is the interrupt mechanism; the registry only resolves the + same-frame race. The spec is explicit about this; do not "simplify" it away. +- [ ] Confirm no fov/near/far behaviour changed (the tween still does not touch + them; auto-rotate touches only yaw). +- [ ] Main thread: full `npm test`; `npm run typecheck`. All green. +- [ ] Commit any cleanup. + +--- + +## Self-review notes + +- **Behaviour preserved?** The three drivers reproduce the exact prior effects: + spaceMouse-when-deflected (cancels tween, applies axes), tween-when-active + (advance + auto-clear), auto-rotate-when-on. The only behaviour the OLD code had + that's now expressed differently is "auto-rotate is suppressed while a tween is + active" — previously a `!tweens.isActive()` guard, now `priority` ordering + (tween 60 > autoRotate 20). Task 4's first regression test pins this. +- **Single-writer, no blending.** The resolver calls exactly one `apply`. The spec + bakes in "no cooperative blending"; if you find yourself wanting two `apply`s in + one frame, STOP — that's a spec change, escalate. +- **Cancellation untouched.** The registry does NOT replace the cancel-on-input + interrupt. Input still cancels tween via the existing `cancelTween` callback; + precedence only resolves the same-frame race. Verify in Task 5. +- **`nowMs` source.** All drivers now receive `runFrame`'s `nowMs` (= the + `performance.now()` passed at `startLoop.ts:143`), where previously the two + per-frame mutators each read `performance.now()` inline. Same clock — preserved. +- **Driver list home.** On `RunFrameDeps` (built in `startLoop`), NOT on + `EngineState` — avoids widening the state contract / mirror state. If a later + consumer outside `runFrame` needs the list, revisit then; do not pre-emptively + promote it. +- **Tour is out of scope.** No `tour` driver here. The priority gap (60 < 80 < 100) + is left deliberately so the tour plan slots its driver in without renumbering. +- **Line numbers drift.** Every `file:line` in this plan was verified against the + tree on 2026-06-08; re-verify before each edit. +- **Conventions.** One type per `@types` file; `type` not `interface`; readonly + driver fields; didactic timeless headers; staged paths only; branch + PR + + squash-merge. npm/typecheck/commits are the MAIN thread's job during + subagent-driven execution. diff --git a/docs/superpowers/plans/2026-06-08-settings-visibility-seam.md b/docs/superpowers/plans/2026-06-08-settings-visibility-seam.md new file mode 100644 index 00000000..90e24f07 --- /dev/null +++ b/docs/superpowers/plans/2026-06-08-settings-visibility-seam.md @@ -0,0 +1,497 @@ +# Settings snapshot/restore seam — implementation plan + +> **REQUIRED SUB-SKILL:** execute this plan via `superpowers:subagent-driven-development` +> (one fresh implementer subagent per task, plus spec + quality reviews). The +> main thread runs `npm test` / `npm run typecheck` and makes commits; implementers +> **edit files only** (they cannot run npm/npx — see project memory). + +## Goal + +Introduce a single seam for **reading** and **restoring** the toggleable visual +layers, so a programmatic caller (the upcoming guided tour) does not have to +hand-code the four scattered settings storage shapes, and can set values +**instantly** (no pending fades) OR **faded**. + +This is the second of the two pre-tour decomplection refactors from the approved +spec `docs/superpowers/specs/2026-06-08-pre-tour-decomplection-design.md` +(**section 2, "Settings snapshot/restore seam"** is the source of truth — read it +first). It is **behaviour-preserving**: every observable behaviour today is +reproduced as a test *before* any structure changes, and the suite stays green +before and after. + +The new public surface is exactly two functions plus one plain-data type: + +- `readVisibility(state): VisibilitySnapshot` — the single geography-aware reader. +- `applyVisibility(state, patch, { animate }): void` — the single write-path; + `animate:false` writes synchronously, `animate:true` routes through the + existing fade orchestration. + +## Architecture + +### The knot today (spec §2) + +The toggleable visual layers live in **four** storage shapes. Verified against +the current tree (re-verify line numbers — they drift): + +| Layer | Storage shape | Read / write today | +|---|---|---| +| Surveys (SDSS/2MRS/GLADE/Famous/Synthetic) | `state.sources.drawMask` / `state.sources.pickMask` — 32-bit bitmasks (`src/utils/sourceMask.ts`: `maskHas`/`maskWith`/`maskWithout`, `ALL_VISIBLE_MASK`) | `setSourceVisibleImpl` (`engine.ts` ~195–239, **async**: flips pickMask immediately, awaits fade-out, conditionally writes drawMask, awaits fade-in). Handle `sources.setVisible`. | +| Filaments | `state.settings.filaments.enabled` (boolean) | `boringSetters.setFilamentsEnabled` then `fades.fadeTo({kind:'filaments'}, …)`. Handle `filaments.setEnabled` (`engine.ts` ~1346–1360). | +| Milky Way | `state.settings.milkyWay.enabled` (boolean) | `boringSetters.setMilkyWayEnabled` then `fades.fadeTo({kind:'overlay', id:'milkyWay'}, …)`. Handle `milkyWay.setEnabled` (`engine.ts` ~1332–1345). | +| Volumes — master | `state.settings.volumes.masterEnabled` (boolean) | `setVolumesEnabled` (`engine.ts` ~1000–1015) → settings + `fadeTo({kind:'volumesMaster'}, …)`. Handle `volumes.setMasterEnabled`. | +| Volumes — per-field | `state.settings.volumes.fields[fieldId].enabled` (`Partial>`) | `setVolumeFieldEnabled` (`engine.ts` ~1085–1102) → `writeVolumeFieldSetting` + `fadeTo({kind:'scalarField', field}, …)` + lazy debug load. Handle `volumes.setEnabled`. | +| Label categories | `state.settings.labelCategoryVisibility` (`Record`) | `setCategoryLabelVisible` (`engine.ts` ~261–293). Structure categories fire `fadeTo({kind:'labelLayer', layer:'poi', category})`; `famousGalaxy` calls `galaxies.setFamousLabelsVisible` + `fadeTo({kind:'labelLayer', layer:'galaxyNames'})`. Mirrors to settings. Handle `labels.setCategoryLabelVisible`. | +| Marker categories | `state.settings.markerCategoryVisibility` (`Record`) | `setCategoryMarkerVisible` (`engine.ts` ~295–323). Structure categories fire `fadeTo({kind:'markerLayer', category})`; `famousGalaxy` fires NO fade (no ring). Mirrors to settings. Handle `labels.setCategoryMarkerVisible`. | + +Two separate complecting strands: + +1. A programmatic caller must know all four storage geographies to read/restore. +2. **Fade orchestration is welded into the setters** — `setSourceVisibleImpl` is + `async` and *awaits* fades, so an instant restore via the public setters is a + sequential async chain. + +### The un-braided shape (spec §2) + +A plain-data snapshot type + a geography-aware reader + a single write-path with +an `animate` flag. + +```ts +// src/@types/engine/settings/VisibilitySnapshot.d.ts (one type per file, plain data, readonly) +export type VisibilitySnapshot = { + readonly sourceDrawMask: number; + readonly filamentsEnabled: boolean; + readonly milkyWayEnabled: boolean; + readonly volumesMasterEnabled: boolean; + readonly volumeFieldEnabled: Readonly>; + readonly labelCategoryVisibility: Readonly>; + readonly markerCategoryVisibility: Readonly>; +}; +``` + +> **Key-set note (re-verify before writing):** the spec sketch used `string` keys. +> The live tree has narrower unions — prefer them: `VolumeFieldId` +> (`src/@types/data/VolumeFieldId.d.ts`) and `PoiCategory` +> (`src/@types/engine/data/PoiCategory.d.ts`). `volumeFieldEnabled` should use +> `Partial>` if a sparse snapshot is cleaner — +> decide against `state.settings.volumes.fields` (which is +> `Partial>`); the reader iterates the +> *present* field rows. Confirm and pick the tightest type that round-trips. + +### The instant-vs-fade asymmetry (the one subtlety to get right) + +Some layers have **no synchronous boolean** that the renderer reads — their +producers read fade opacity directly: + +- **Structure-category labels/markers**: `produceStructureLabels` / + `produceStructureMarkers` read `fades.opacityOf({kind:'labelLayer'|'markerLayer', …})` + for their alpha. There is **no** structure-store visibility flag (confirm: + `setCategoryLabelVisible`/`setCategoryMarkerVisible` only fire a fade + mirror + to settings; only the `famousGalaxy` branch writes a store boolean via + `galaxies.setFamousLabelsVisible`). So an **instant** apply of a structure + category must `fades.setImmediate(handle, value ? 1 : 0)` (jump the controller), + not `fadeTo`. `settings.*CategoryVisibility` is the mirror, still written. +- **Surveys / filaments / milkyWay / volumes** *do* have a synchronous gate + (drawMask bit / `settings.*.enabled` / `volumes.fields[id].enabled`) that the + pass-enabled checks accept as `boolean OR opacity > 0`. For an **instant** + apply, write that gate synchronously AND `setImmediate` the fade handle to the + target so a half-finished ramp doesn't drift the value back. + +So `applyVisibility`'s dispatch is, per touched field: + +- `animate: true` → write the synchronous gate (where one exists) + `fadeTo(handle, target, dur)`. +- `animate: false` → write the synchronous gate (where one exists) + `setImmediate(handle, target)`. + +This is why the seam is not "just call the existing setters": the existing setters +hardcode `fadeTo`, and `setSourceVisibleImpl` is async. `applyVisibility` is the +synchronous-capable single write-path. + +### Reader/writer homes (single-function-file convention) + +Per the one-function-one-file naming rule, create a new sub-folder +`src/services/engine/settings/` with: + +- `src/services/engine/settings/readVisibility.ts` — exports `readVisibility`. +- `src/services/engine/settings/applyVisibility.ts` — exports `applyVisibility`. + +Both take the narrow `Pick` slices they actually touch (mirror the +`setSourceVisibleImpl` / `setCategoryLabelVisible` style of accepting a `Pick` so +the tests can drive them against a partial stub without a GPU engine). The +`applyVisibility` writer reuses `maskWith`/`maskWithout` (`src/utils/sourceMask.ts`), +`writeVolumeFieldSetting` (`src/services/engine/helpers/writeVolumeFieldSetting.ts`), +and the `FADE_IN_DURATION_MS`/`FADE_OUT_DURATION_MS` constants +(`src/services/animation/fadeController.ts`). + +### Harmonise with `settingsTable.ts`, do not fight it + +`src/services/engine/wiring/settingsTable.ts` is the existing single-write-path +precedent (declarative `name/path/clamp/callback` rows → `mutate + echo + +requestRender`). `applyVisibility` is the *animation-aware* sibling: where a +layer is a pure `settings.*.enabled` boolean (filaments, milkyWay, +volumes.masterEnabled), the synchronous write should go through the same +`state.settings` leaf those table rows own (do not introduce a parallel mirror). +The seam adds the fade dispatch + the bitmask/per-field/category geographies the +table deliberately does not cover; it does not duplicate the table's boolean +write contract. + +### Delegation (avoid a second write path) + +After the seam exists, route the existing handle setters through it **where +clean**, so there is not a second write-path: + +- `milkyWay.setEnabled` / `filaments.setEnabled` / `volumes.setMasterEnabled` → + delegate to `applyVisibility(state, {: enabled}, {animate: true})`. + These are the cleanest (single boolean + single fade handle). +- `setVolumeFieldEnabled` → has an extra concern (lazy debug-volume load + + `onFieldsChanged` echo). Delegate the **enabled write + fade** to + `applyVisibility` and keep the lazy-load + echo at the call site, OR (if that + split reads worse) leave it and **NOTE the overlap explicitly** in a comment. + Implementer's judgement — do not hide the overlap either way. +- `setSourceVisibleImpl` → it is **async** (awaits fade-out before clearing + drawMask, with last-issued-wins re-read). `applyVisibility` is synchronous and + does NOT replicate that await/re-read. **Do NOT collapse `setSourceVisibleImpl` + into `applyVisibility`** — the async fade-out-then-clear ordering is + load-bearing behaviour (see `setSourceVisibleFade.test.ts` case 3). Leave + `setSourceVisibleImpl` as the survey *toggle* path; `applyVisibility` is the + *snapshot-restore* path (instant or simple fade). NOTE this overlap explicitly: + two write paths touch drawMask, justified because one is the interactive + async-await toggle and the other is the synchronous programmatic restore. +- `setCategoryLabelVisible` / `setCategoryMarkerVisible` → these carry the + famousGalaxy store write + the no-ring-for-famous special case + the echo + callbacks. Routing the *structure-category* fade through `applyVisibility` while + keeping the famousGalaxy + echo concerns at the call site is acceptable but may + read worse than leaving them. Implementer's judgement; if left, NOTE the + overlap (both write `settings.*CategoryVisibility`). + +The delegation tasks (5–8) are **behaviour-preserving**: the existing fade tests +(`setSourceVisibleFade.test.ts`, `setCategoryVisibleFade.test.ts`, +`flowFieldsHandle.test.ts`) must stay green unchanged. + +## Scope guards (NON-goals — do not scope-creep) + +- **Do NOT** migrate source visibility from the bitmask to a registry. +- **Do NOT** move `sources.tier` into `settings`. +- **Do NOT** generalise volume per-field params (intensity/contrast/palette/…) + into the snapshot — only the `enabled` axis is a *visibility* layer. +- **Do NOT** add `flow` to the snapshot unless the spec's snapshot type lists it + — it does not (`VisibilitySnapshot` has no `flowEnabled`). Flow stays out. +- Keep the change minimal: a reader + a write-path with an `animate` flag, plus + clean delegation. + +## Skymap conventions reminder (for every implementer) + +- `type` aliases, never `interface`. One type per `@types` file. `readonly` on + snapshot fields. Use `Vec2`/`Vec3` aliases if any vector appears (none expected here). +- Single-function utility files take the function's name (`readVisibility.ts` + exports `readVisibility`). +- Didactic comments: explain *why* + the alternative, multi-paragraph module + headers matching the surrounding files. No history notes (no dates/PR refs). +- Deep relative imports; no barrels. +- **Pause before implementing:** reuse existing helpers (`maskWith`/`maskWithout`, + `writeVolumeFieldSetting`, the fade constants); surface the simplest alternative + before editing. If a clean delegation is blocked by something structural, STOP + and report rather than re-braiding around it. +- **Re-verify every cited `file:line`** against the live tree before relying on it + — the engine.ts line numbers WILL have drifted. +- Run bash sequentially; use Read/Grep tools, not sed/awk/grep. +- The MAIN thread runs `npm test` / `npm run typecheck` and commits; you edit + files only. + +--- + +## Task 1 — `VisibilitySnapshot` type + +**Files:** +- Create `src/@types/engine/settings/VisibilitySnapshot.d.ts` +- Test `tests/@types/visibilitySnapshot.test.ts` (a tiny type-smoke test, mirror + an existing `tests/@types/*.test.ts` shape — e.g. `tests/@types/engineState.test.ts`) + +**Contract:** the exact shape in the Architecture section. Re-verify the key-set +note: use `VolumeFieldId` / `PoiCategory` unions (not bare `string`); decide +`Record` vs `Partial` for `volumeFieldEnabled` and document the choice in +the module header. + +- [ ] Add a type-smoke test that constructs a `VisibilitySnapshot` literal and + asserts (compile-time via `satisfies` + a trivial runtime `expect`) that all + seven fields are present and `readonly`. Name it + `VisibilitySnapshot has the seven visibility-layer fields`. +- [ ] Run fails (type does not exist). +- [ ] Create the `.d.ts` with a didactic module header explaining: this is plain + data (spec §2), one fold per visual layer, used by `readVisibility` / + `applyVisibility`; why narrow unions over `string`; why per-field is `enabled`-only. +- [ ] Run passes. +- [ ] Commit. + +## Task 2 — `readVisibility` (behaviour-preserving reader) + +**Files:** +- Create `src/services/engine/settings/readVisibility.ts` +- Test `tests/services/engine/settings/readVisibility.test.ts` + +**Signature:** `readVisibility(state: Pick): VisibilitySnapshot` +(confirm the minimal `Pick` — it reads `sources.drawMask` and the `settings.*` +shapes; it does NOT read fade opacity, since the synchronous gates are the truth +for surveys/filaments/milkyWay/volumes, and `settings.*CategoryVisibility` is the +truth for categories). + +**Behaviour:** project each storage shape into the snapshot: +- `sourceDrawMask` ← `state.sources.drawMask` +- `filamentsEnabled` ← `state.settings.filaments.enabled` +- `milkyWayEnabled` ← `state.settings.milkyWay.enabled` +- `volumesMasterEnabled` ← `state.settings.volumes.masterEnabled` +- `volumeFieldEnabled` ← map each present `state.settings.volumes.fields[id]` to its `.enabled` +- `labelCategoryVisibility` ← shallow copy of `state.settings.labelCategoryVisibility` +- `markerCategoryVisibility` ← shallow copy of `state.settings.markerCategoryVisibility` + +Return a fresh frozen-ish value (shallow copies of the records, not aliases of +live state) so a held snapshot does not mutate when state later changes. + +- [ ] Add test `readVisibility returns the live values from each of the four storage shapes`: + build a minimal state stub with a non-default `drawMask`, `filaments.enabled:false`, + `milkyWay.enabled:true`, `volumes.masterEnabled:false`, one field + `enabled:true` + one `enabled:false`, and mixed label/marker category maps; + assert every snapshot field equals the corresponding source. +- [ ] Add test `readVisibility snapshot does not alias live state` — mutate + `state.settings.labelCategoryVisibility` after reading; assert the prior + snapshot's record is unchanged. +- [ ] Run fails. +- [ ] Implement against the live `state.settings` shape; reuse no helpers beyond + `Object` copies. Didactic header: why the reader is geography-aware in one place. +- [ ] Run passes. +- [ ] Commit. + +## Task 3 — `applyVisibility({ animate: false })` (synchronous write-path) + +**Files:** +- Create `src/services/engine/settings/applyVisibility.ts` +- Test `tests/services/engine/settings/applyVisibility.test.ts` + +**Signature:** +`applyVisibility(state: Pick, patch: Partial | VisibilitySnapshot, opts: { animate: boolean }): void` +(confirm the minimal `Pick`: it writes `sources.drawMask`, `settings.*`, calls +`subsystems.fades.setImmediate`/`fadeTo` + `subsystems.scheduler.requestRender`, +and the famousGalaxy category branch reads `data.galaxies.setFamousLabelsVisible` +— mirror how `setCategoryLabelVisible` reaches the store). + +**Behaviour (animate:false):** for each **present** key in `patch`, write the +synchronous gate AND `fades.setImmediate(handle, target)`: +- `sourceDrawMask` → assign `state.sources.drawMask` (and `pickMask`? — **decide + and document**: a snapshot restore should set both so a restored-hidden survey + is also unclickable; confirm against `setSourceVisibleImpl`'s pickMask + semantics. For per-survey fade handles you must `setImmediate` each survey's + `{kind:'survey', source}` controller to the bit value, OR — simpler and + acceptable — leave survey fade controllers alone when restoring drawMask + directly, since the pass reads the bit. Pick the minimal correct option and + document the reasoning in the header.) +- `filamentsEnabled` → `state.settings.filaments.enabled = v`; `setImmediate({kind:'filaments'}, v?1:0)` +- `milkyWayEnabled` → `state.settings.milkyWay.enabled = v`; `setImmediate({kind:'overlay', id:'milkyWay'}, v?1:0)` +- `volumesMasterEnabled` → `state.settings.volumes.masterEnabled = v`; `setImmediate({kind:'volumesMaster'}, v?1:0)` +- `volumeFieldEnabled[id]` → `writeVolumeFieldSetting(..., {enabled:v})`; `setImmediate({kind:'scalarField', field:id}, v?1:0)` +- `labelCategoryVisibility[cat]` → mirror into `state.settings.labelCategoryVisibility`; + structure category → `setImmediate({kind:'labelLayer', layer:'poi', category}, v?1:0)`; + famousGalaxy → `data.galaxies.setFamousLabelsVisible(v)` + `setImmediate({kind:'labelLayer', layer:'galaxyNames'}, v?1:0)` +- `markerCategoryVisibility[cat]` → mirror into `state.settings.markerCategoryVisibility`; + structure category → `setImmediate({kind:'markerLayer', category}, v?1:0)`; + famousGalaxy → mirror only, NO fade handle (no ring — mirror `setCategoryMarkerVisible`) +- End with one `state.subsystems.scheduler.requestRender()`. + +> The fade-handle dispatch (kind per layer) mirrors the existing setters +> 1:1 — read `engine.ts` setSourceVisibleImpl / setCategoryLabelVisible / +> setCategoryMarkerVisible / setVolumesEnabled / setVolumeFieldEnabled and the +> milkyWay/filaments handle literals for the exact handle shapes. Do not invent +> new handle kinds. + +- [ ] Add test `applyVisibility({animate:false}) writes filaments/milkyWay/volumes gates synchronously`: + apply a patch toggling those three; assert `state.settings.*` written + immediately (no await), and `fades.setImmediate` called for each with the right + handle + 0/1 (NOT `fadeTo`). +- [ ] Add test `applyVisibility({animate:false}) restores a structure label/marker category via setImmediate`: + apply `{labelCategoryVisibility:{cluster:false}, markerCategoryVisibility:{cluster:false}}`; + assert settings mirrored, `setImmediate({kind:'labelLayer',layer:'poi',category:'cluster'},0)` + + `setImmediate({kind:'markerLayer',category:'cluster'},0)`, and `fadeTo` NOT called. +- [ ] Add test `applyVisibility({animate:false}) restores a survey drawMask bit`: + apply `{sourceDrawMask: }`; assert + `state.sources.drawMask` equals the patch value (and pickMask per the documented decision). +- [ ] Add test `applyVisibility({animate:false}) requests a render exactly once`. +- [ ] Run fails. +- [ ] Implement. Didactic header: the instant-vs-fade asymmetry (why structure + categories MUST go through `setImmediate`, not a synchronous boolean) and the + drawMask/pickMask decision. +- [ ] Run passes. +- [ ] Commit. + +## Task 4 — `applyVisibility({ animate: true })` (fade write-path) + round-trip + +**Files:** +- Modify `src/services/engine/settings/applyVisibility.ts` +- Modify `tests/services/engine/settings/applyVisibility.test.ts` + +**Behaviour (animate:true):** identical synchronous-gate writes as Task 3, but +each fade-handle dispatch uses `fades.fadeTo(handle, target, target ? FADE_IN_DURATION_MS : FADE_OUT_DURATION_MS)` +instead of `setImmediate`. The synchronous gate is still written (so the pass +draws through the ramp), matching the existing setters. + +- [ ] Add test `applyVisibility({animate:true}) drives the fade registry for each touched layer`: + apply a multi-layer patch; assert `fades.fadeTo` invoked for each touched + layer with the correct handle + target + FADE_IN/OUT duration, and + `setImmediate` NOT called for those layers. +- [ ] Add test `applyVisibility round-trip restores every layer`: + `const snap = readVisibility(state)`; mutate several layers via + `applyVisibility(state, {...}, {animate:false})`; then + `applyVisibility(state, snap, {animate:false})`; assert `readVisibility(state)` + deep-equals the original `snap` (every storage shape restored). This is the + core acceptance test from spec §2. +- [ ] Run fails. +- [ ] Implement the `animate` branch (factor the per-field dispatch so animate + only swaps `setImmediate` ↔ `fadeTo`). +- [ ] Run passes (+ re-run readVisibility tests stay green). +- [ ] Commit. + +## Task 5 — Delegate `milkyWay.setEnabled` + `filaments.setEnabled` + +**Files:** +- Modify `src/services/engine/engine.ts` (the two handle-literal setters, ~1332–1360) +- Test: existing `tests/services/engine/*` must stay green; add a focused + regression test only if the existing coverage doesn't already pin the fade. + +**Behaviour-preserving:** route each through +`applyVisibility(state, {Enabled: enabled}, {animate: true})`, preserving +the `requestRender` (applyVisibility already calls it — confirm no double-render +regression; if the handle previously called `requestRender` once, applyVisibility +calling it once is equivalent). + +- [ ] Confirm existing tests cover the observable behaviour (settings boolean + + `fadeTo({kind:'filaments'|...overlay milkyWay}, …)`). If not, add + `milkyWay.setEnabled still fades the overlay handle` / + `filaments.setEnabled still fades the filaments handle` regression tests + asserting the same fade + settings write as before. +- [ ] Run fails (if a new test) / establish green baseline. +- [ ] Refactor the two setters to delegate to `applyVisibility`. Keep the + didactic comment, updated to point at the seam. +- [ ] Run passes (existing + new). +- [ ] Commit. + +## Task 6 — Delegate `volumes.setMasterEnabled` + +**Files:** +- Modify `src/services/engine/engine.ts` (`setVolumesEnabled`, ~1000–1015) +- Test: existing coverage; add regression if missing. + +**Behaviour-preserving:** route through +`applyVisibility(state, {volumesMasterEnabled: enabled}, {animate: true})`. +Confirm no echo callback exists today (the comment says React owns it +optimistically) — applyVisibility must NOT add one. + +- [ ] Establish/confirm a green baseline test for `setVolumesEnabled` + (settings.volumes.masterEnabled + `fadeTo({kind:'volumesMaster'}, …)`). +- [ ] Refactor to delegate. +- [ ] Run passes. +- [ ] Commit. + +## Task 7 — Reconcile `setVolumeFieldEnabled` (delegate the visibility half, NOTE the overlap) + +**Files:** +- Modify `src/services/engine/engine.ts` (`setVolumeFieldEnabled`, ~1085–1102) +- Test: existing coverage; add regression if missing. + +**Behaviour-preserving.** `setVolumeFieldEnabled` does THREE things: write +`fields[id].enabled` (via `writeVolumeFieldSetting`, with an early-return when the +field row is absent), fire the scalarField fade, lazy-load DEV debug volumes when +enabling, and echo `onFieldsChanged`. The visibility half (write + fade) is what +`applyVisibility` owns. + +- [ ] Decide: delegate the **enabled write + fade** to + `applyVisibility(state, {volumeFieldEnabled:{[id]:enabled}}, {animate:true})` + and keep `maybeLazyLoadDebugVolume(fieldId)` + `onFieldsChanged` + + `requestRender` at the call site — **OR** leave `setVolumeFieldEnabled` intact + and add a comment explicitly noting the overlap with `applyVisibility`'s + per-field branch (both write `fields[id].enabled` + fire the scalarField fade). + Choose whichever reads cleaner; do not hide the overlap. +- [ ] Preserve the absent-field early-return semantics (if the field row doesn't + exist, today's setter no-ops via `writeVolumeFieldSetting` returning null; + applyVisibility must match — confirm `writeVolumeFieldSetting` is the shared + guard). +- [ ] Establish/confirm green baseline; refactor or annotate per the decision. +- [ ] Run passes. +- [ ] Commit. + +## Task 8 — Reconcile category setters (delegate structure-category fade OR NOTE overlap) + +**Files:** +- Modify `src/services/engine/engine.ts` (`setCategoryLabelVisible` ~261–293, + `setCategoryMarkerVisible` ~295–323) +- Test: `tests/services/engine/setCategoryVisibleFade.test.ts` must stay green + unchanged. + +**Behaviour-preserving.** These carry the `famousGalaxy` store write +(`galaxies.setFamousLabelsVisible`), the no-ring-for-famous special case, and the +echo callbacks (`onLabelCategoryVisibilityChange` / `onMarkerCategoryVisibilityChange`) +— which `applyVisibility` does NOT fire (it has no `cb`). + +- [ ] Decide: either (a) route the **structure-category fade + settings mirror** + through `applyVisibility(..., {animate:true})` and keep the famousGalaxy branch + + the echo callbacks at the call site, OR (b) leave both setters as-is and add a + comment noting the overlap (both they and `applyVisibility` write + `settings.*CategoryVisibility` + the same per-category fade handles). The echo + callback is the friction that may make (b) cleaner — implementer's judgement. +- [ ] Whichever path: `setCategoryVisibleFade.test.ts` passes unchanged (it pins + the exact handles, durations, settings writes, famous-store call, and the + famous-marker-no-fade case). +- [ ] Run passes. +- [ ] Commit. + +## Task 9 — Entanglement-radar pass on the diff + +**Files:** none (review only; capture findings in the PR description / backlog). + +- [ ] Run the `entanglement-radar` skill over the full diff. Confirm: the four + storage geographies now have ONE reader and ONE write-path; any remaining + second-write-path overlaps (Tasks 5–8) are explicitly NOTED in comments, not + hidden; the snapshot type is plain data; no new switch-on-discriminant that + should be a registry row. Record the result (a clean pass or named residual + knots) so the tour plan's §3 reconciliation can build on it. + +--- + +## Self-review notes + +- **Source of truth:** spec §2 of + `docs/superpowers/specs/2026-06-08-pre-tour-decomplection-design.md`. The + snapshot type, the `readVisibility` / `applyVisibility({animate})` surface, and + the testing bullets are taken from there verbatim. +- **The one subtlety:** structure-category labels/markers have **no synchronous + boolean** the renderer reads — producers read `fades.opacityOf(handle)`. So an + *instant* (`animate:false`) apply MUST use `fades.setImmediate`, not a boolean + write. This is the single thing most likely to be implemented wrong; Task 3's + tests pin it. Re-verify against `produceStructureLabels` / + `produceStructureMarkers` that they read opacity (not a store flag) before + implementing. +- **`setSourceVisibleImpl` is deliberately NOT collapsed** into `applyVisibility`: + it is async (awaits fade-out before clearing drawMask, with a last-issued-wins + opacity re-read — `setSourceVisibleFade.test.ts` case 3). `applyVisibility` is + the synchronous restore path. Two write paths touch drawMask; the overlap is + intentional and must be noted, not hidden (spec §2: "otherwise they remain and + we accept narrow overlap (noted, not hidden)"). +- **Scope guards honoured:** no mask→registry migration, no `tier`-into-settings, + no per-field intensity/contrast in the snapshot, no `flow` in the snapshot (the + spec's type omits it). +- **Behaviour-preserving gate:** Tasks 1–4 add the new seam with its own tests; + Tasks 5–8 are refactors that must keep `setSourceVisibleFade.test.ts`, + `setCategoryVisibleFade.test.ts`, and `flowFieldsHandle.test.ts` green + unchanged. "Green before and after" is the gate (spec §4). +- **Line numbers WILL drift** — every `engine.ts:NNN` citation is approximate as + of plan authoring; implementers must re-verify by reading the current file + (the setters are easy to find by name: `setSourceVisibleImpl`, + `setCategoryLabelVisible`, `setCategoryMarkerVisible`, `setVolumesEnabled`, + `setVolumeFieldEnabled`, and the `milkyWay`/`filaments` handle-literal entries). +- **Open decision left to the implementer (documented, not dodged):** whether + `applyVisibility` writes `pickMask` alongside `drawMask` on a survey restore. + Task 3 requires the implementer to pick the minimal-correct option and document + the reasoning; the spec's snapshot type only carries `sourceDrawMask`, so the + conservative reading is "restore drawMask; leave pickMask coupled or set it to + match" — resolve against `setSourceVisibleImpl`'s pickMask semantics. +- **Type-key decision:** `VolumeFieldId` / `PoiCategory` narrow unions over + `string` (spec sketch used `string`; the live tree has the unions). Confirmed + `PoiCategory = StructureCategory | 'famousGalaxy'` and `VolumeFieldId` derives + from `SOURCE_REGISTRY` volume entries. `volumeFieldEnabled` likely + `Partial>` to match the sparse + `volumes.fields` map — Task 1 confirms. From a5b25034180cc8784083691f17313a0b2b87cb70 Mon Sep 17 00:00:00 2001 From: Alexander Rulkens Date: Mon, 8 Jun 2026 00:58:42 +0200 Subject: [PATCH 3/7] =?UTF-8?q?feat(camera):=20CameraDriver=20type=20?= =?UTF-8?q?=E2=80=94=20precedence=20as=20data?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seam for the camera-driver-authority decomplection: a uniform, self-describing mover unit so a single resolver can pick the per-frame camera winner by priority instead of statement order. Type-only; the resolver, wrapper drivers, and runFrame rewrite follow in later tasks. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/@types/engine/camera/CameraDriver.d.ts | 54 ++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 src/@types/engine/camera/CameraDriver.d.ts diff --git a/src/@types/engine/camera/CameraDriver.d.ts b/src/@types/engine/camera/CameraDriver.d.ts new file mode 100644 index 00000000..e6685fe5 --- /dev/null +++ b/src/@types/engine/camera/CameraDriver.d.ts @@ -0,0 +1,54 @@ +/** + * CameraDriver — the seam that turns camera precedence into DATA. + * + * The engine has several things that all want to move the camera on a + * given frame: raw input (SpaceMouse / drag), an in-flight tween, the + * idle auto-rotate, and (added later) a guided tour. Historically the + * winner was decided by *call order* inside the per-frame body — tween + * advanced first, then input overwrote it, then a hand-written guard + * suppressed auto-rotate when anything else was active. Precedence was + * an emergent property of how the statements happened to be sequenced, + * which meant inserting a new mover (or changing who beats whom) was a + * surgical edit to control flow rather than a one-line declaration. + * + * A CameraDriver makes each mover a uniform, self-describing unit so a + * single resolver can pick the winner by comparing `priority` instead + * of relying on statement order. The set of drivers is a registry; the + * ordering between them is a number. Adding a tour, or re-ranking the + * existing movers, becomes data — not a rewrite of the frame loop. + * + * The four members, and why each exists: + * + * - `id` — a stable string identity ('input' | 'tween' | 'autoRotate', + * with 'tour' joining later). Purely for debugging and logging: it + * lets a trace say "frame written by 'tween'" without the resolver + * needing to know any concrete driver's type. + * + * - `priority` — the sole thing the resolver orders by. The current + * ranking is input 100 > tour 80 > tween 60 > autoRotate 20; the + * gap below 100 and around 80 is deliberate headroom so a future + * driver can slot between two existing ones without renumbering. + * + * - `isActive(nowMs)` — answers two questions with one predicate. + * Per-driver it means "do I want to write state.cam this frame?", + * which is how the resolver knows whether to even consider me. + * Collectively (any driver active) it is also the render-on-demand + * signal: if no driver is active and nothing else is animating, the + * frame loop can sleep. Folding both into one predicate keeps the + * "is the camera moving?" truth in a single place per driver. + * + * - `apply(cam, nowMs)` — the single mutation a driver performs. Only + * the resolver's chosen winner gets its `apply` called, so there is + * exactly one camera-write site per frame. A driver mutates `cam` + * in place (the camera is the engine's live mutable shell); it + * returns nothing because the camera *is* the output. + */ + +import type { OrbitCamera } from '../../../camera/OrbitCamera'; + +export type CameraDriver = { + readonly id: string; + readonly priority: number; + isActive(nowMs: number): boolean; + apply(cam: OrbitCamera, nowMs: number): void; +}; From 2dd6f1eeb175ac498930d51efe000060f94eda36 Mon Sep 17 00:00:00 2001 From: Alexander Rulkens Date: Mon, 8 Jun 2026 01:01:23 +0200 Subject: [PATCH 4/7] =?UTF-8?q?feat(camera):=20runCameraDrivers=20?= =?UTF-8?q?=E2=80=94=20single-writer=20arbiter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure resolver at the heart of camera authority: among active drivers the highest priority wins and ONLY that winner's apply() runs (no cooperative blending, no apply at all when none active). Max-scan over a readonly driver list — no sort-mutation, no captured state. Four unit tests cover winner selection, inactive-higher-doesn't-block, none-active, and exact cam+nowMs forwarding. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/services/engine/camera/cameraDrivers.ts | 62 +++++++++++++ .../engine/camera/cameraDrivers.test.ts | 86 +++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 src/services/engine/camera/cameraDrivers.ts create mode 100644 tests/services/engine/camera/cameraDrivers.test.ts diff --git a/src/services/engine/camera/cameraDrivers.ts b/src/services/engine/camera/cameraDrivers.ts new file mode 100644 index 00000000..df7af1f7 --- /dev/null +++ b/src/services/engine/camera/cameraDrivers.ts @@ -0,0 +1,62 @@ +/** + * cameraDrivers — the single arbiter that turns camera precedence from + * statement order into data. + * + * The engine has several movers that all want to write `state.cam` on a + * given frame: raw input, an in-flight tween, idle auto-rotate, and a + * guided tour. Previously the winner was whoever ran last in the + * per-frame body, with hand-written guards suppressing the losers. + * Precedence was emergent from control flow, so inserting a mover or + * changing who-beats-whom meant surgery on the frame loop. + * + * `runCameraDrivers` collapses that into one rule: among the drivers + * that declare themselves active this frame, the highest `priority` + * wins, and ONLY that winner's `apply` runs. Two properties fall out of + * this deliberately: + * + * - Single-writer arbitration. There is exactly one camera-write site + * per frame — the chosen winner's `apply`. The design bakes this in: + * there is NO cooperative blending of multiple drivers into one + * frame. A frame is authored by one driver or by none. Blending + * would reintroduce the "who contributed what, in what order" + * entanglement this seam exists to remove. + * + * - Precedence is data. The ordering between movers is a `priority` + * number on each driver, not a position in a sequence of statements. + * Re-ranking or slotting in a new mover is a one-line declaration. + * + * The resolver is pure over its driver list. It captures no state, does + * no I/O, and treats `drivers` as readonly — it never sort-mutates the + * caller's array (a single max-scan, not a `.sort()`). It also never + * reads or mutates `cam`; it only forwards the reference (and `nowMs`) + * to the winner's `apply`, so the camera remains entirely the winner's + * concern. That purity is what makes the heart of camera authority + * testable with a throwaway `cam` stub and a handful of fake drivers. + */ + +import type { CameraDriver } from '../../../@types/engine/camera/CameraDriver'; +import type { OrbitCamera } from '../../../@types/camera/OrbitCamera'; + +export function runCameraDrivers( + drivers: readonly CameraDriver[], + cam: OrbitCamera, + nowMs: number, +): void { + // Single max-scan rather than filter-then-sort: sorting would either + // mutate the caller's array (forbidden — `drivers` is readonly) or + // allocate a copy for no reason. We only ever need the one winner, so + // we fold the active drivers into a running best-by-priority. + let winner: CameraDriver | null = null; + for (const driver of drivers) { + if (!driver.isActive(nowMs)) continue; + if (winner === null || driver.priority > winner.priority) { + winner = driver; + } + } + + // No active driver means no camera write this frame — the resolver + // calls no `apply` at all rather than falling back to some default. + if (winner !== null) { + winner.apply(cam, nowMs); + } +} diff --git a/tests/services/engine/camera/cameraDrivers.test.ts b/tests/services/engine/camera/cameraDrivers.test.ts new file mode 100644 index 00000000..1a8b1199 --- /dev/null +++ b/tests/services/engine/camera/cameraDrivers.test.ts @@ -0,0 +1,86 @@ +/** + * cameraDrivers — unit tests for the single-writer camera arbiter. + * + * The resolver is pure over its driver list, so the tests use fake + * drivers built from `vi.fn()` spies and a throwaway `cam` stub (the + * resolver never touches `cam` — it only forwards it to the winner). + * The behaviours under test: + * + * - Among multiple active drivers, only the single highest-priority + * one's `apply` runs. + * - An inactive higher-priority driver does not block a lower active + * one — precedence is decided among the active set, not the whole + * registry. + * - When no driver is active, no `apply` runs at all. + * - The winner's `apply` receives the exact `cam` reference and the + * `nowMs` passed in. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import type { CameraDriver } from '../../../../src/@types/engine/camera/CameraDriver'; +import type { OrbitCamera } from '../../../../src/@types/camera/OrbitCamera'; +import { runCameraDrivers } from '../../../../src/services/engine/camera/cameraDrivers'; + +// The resolver never reads or mutates the camera, so a bare object cast +// to OrbitCamera is a sufficient stub. We only assert it is forwarded +// by-reference to the winner's apply. +const camStub = {} as OrbitCamera; + +function makeDriver( + id: string, + priority: number, + active: boolean, +): CameraDriver { + return { + id, + priority, + isActive: vi.fn(() => active), + apply: vi.fn(), + }; +} + +describe('runCameraDrivers', () => { + it('calls the single highest-priority active driver', () => { + const high = makeDriver('high', 60, true); + const low = makeDriver('low', 20, true); + + runCameraDrivers([low, high], camStub, 0); + + expect(high.apply).toHaveBeenCalledTimes(1); + expect(low.apply).not.toHaveBeenCalled(); + }); + + it('ignores inactive higher-priority drivers', () => { + const inactiveHigh = makeDriver('inactiveHigh', 100, false); + const activeLow = makeDriver('activeLow', 20, true); + + runCameraDrivers([inactiveHigh, activeLow], camStub, 0); + + expect(activeLow.apply).toHaveBeenCalledTimes(1); + expect(inactiveHigh.apply).not.toHaveBeenCalled(); + }); + + it('writes nothing when no driver is active', () => { + const a = makeDriver('a', 100, false); + const b = makeDriver('b', 20, false); + + runCameraDrivers([a, b], camStub, 0); + + expect(a.apply).not.toHaveBeenCalled(); + expect(b.apply).not.toHaveBeenCalled(); + }); + + it('forwards cam and nowMs to apply', () => { + const driver = makeDriver('only', 50, true); + const nowMs = 1234; + + runCameraDrivers([driver], camStub, nowMs); + + expect(driver.apply).toHaveBeenCalledTimes(1); + expect(driver.apply).toHaveBeenCalledWith(camStub, nowMs); + // The EXACT reference, not just a structural match. + const [firstArg] = (driver.apply as ReturnType).mock.calls[0] ?? []; + expect(firstArg).toBe(camStub); + }); +}); From f2426e5afdd69f44773908ea85318de508760fc7 Mon Sep 17 00:00:00 2001 From: Alexander Rulkens Date: Mon, 8 Jun 2026 01:07:57 +0200 Subject: [PATCH 5/7] feat(camera): wrapper drivers + driver list on RunFrameDeps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildCameraDrivers wraps the engine's existing movers (spaceMouse/input 100, tweens 60, auto-rotate 20) as CameraDrivers, closing over live state so toggled settings reflect next frame. The list rides on RunFrameDeps (built once in startLoop), not EngineState — it is a per-frame dependency, not engine-owned mutable state, so the state contract stays un-widened. Auto-rotate's yaw delta is pinned to 0.000873 and recomputes position. Groundwork only: runFrame does not call the resolver yet (next task). runFrame.test fixture gets drivers:[] to stay green at this boundary. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/@types/engine/frame/RunFrameDeps.d.ts | 9 ++ src/services/engine/camera/cameraDrivers.ts | 74 ++++++++++ src/services/engine/phases/startLoop.ts | 6 + .../camera/cameraDriverWrappers.test.ts | 139 ++++++++++++++++++ tests/services/engine/frame/runFrame.test.ts | 1 + 5 files changed, 229 insertions(+) create mode 100644 tests/services/engine/camera/cameraDriverWrappers.test.ts diff --git a/src/@types/engine/frame/RunFrameDeps.d.ts b/src/@types/engine/frame/RunFrameDeps.d.ts index 6af551b4..3ebe8234 100644 --- a/src/@types/engine/frame/RunFrameDeps.d.ts +++ b/src/@types/engine/frame/RunFrameDeps.d.ts @@ -17,6 +17,7 @@ import type { HorizonShellRenderer } from '../../rendering/HorizonShellRenderer' import type { FilamentRenderer } from '../../rendering/FilamentRenderer'; import type { FpsCounter } from '../subsystems/FpsCounter'; import type { GpuTimingService } from '../../gpu/timing/GpuTimingService'; +import type { CameraDriver } from '../camera/CameraDriver'; export type RunFrameDeps = { /** createEngine arg — for resize + viewport reads. */ @@ -58,4 +59,12 @@ export type RunFrameDeps = { * `renderFrame` via `RenderFrameInput.timingService`. */ timingService: GpuTimingService; + /** + * Camera-control drivers, built once at loop start. The resolver + * (`runCameraDrivers`) picks the single highest-priority active winner + * each frame and is also the source of truth for "is the camera + * animating" (render-on-demand gate). Order in this array is not + * significant — `priority` decides. + */ + readonly drivers: readonly CameraDriver[]; }; diff --git a/src/services/engine/camera/cameraDrivers.ts b/src/services/engine/camera/cameraDrivers.ts index df7af1f7..47ef9ed0 100644 --- a/src/services/engine/camera/cameraDrivers.ts +++ b/src/services/engine/camera/cameraDrivers.ts @@ -32,10 +32,26 @@ * to the winner's `apply`, so the camera remains entirely the winner's * concern. That purity is what makes the heart of camera authority * testable with a throwaway `cam` stub and a handful of fake drivers. + * + * `buildCameraDrivers` is the other half of the seam: a pure builder + * that wraps the engine's existing camera movers as `CameraDriver`s and + * hands back the list the resolver scans. Each wrapper closes over the + * live `state`, so a toggled setting or a freshly-started tween is + * reflected on the very next frame without rebuilding the list. The + * wrappers are deliberately thin — they map "is this mover active?" and + * "let this mover write the camera" onto the subsystems that already own + * that behaviour (`spaceMouse`, `tweens`) or onto the one-line + * auto-rotate increment, rather than reimplementing any of it. The list + * is built once at loop start and carried on `RunFrameDeps`, not on + * `EngineState`: it is a per-frame dependency of the frame body, not a + * piece of engine-owned mutable state, so threading it through the deps + * bag keeps the state contract from widening into mirror data. */ import type { CameraDriver } from '../../../@types/engine/camera/CameraDriver'; import type { OrbitCamera } from '../../../@types/camera/OrbitCamera'; +import type { EngineState } from '../../../@types/engine/state/EngineState'; +import { updatePosition } from '../../camera/orbitCamera'; export function runCameraDrivers( drivers: readonly CameraDriver[], @@ -60,3 +76,61 @@ export function runCameraDrivers( winner.apply(cam, nowMs); } } + +/** + * Idle auto-rotate yaw increment, in radians per frame. + * + * A constant per-frame nudge (not time-scaled) so the spin reads as a + * gentle, frame-rate-tied drift — the same value the per-frame body used + * before camera authority moved behind the driver seam. + */ +const AUTO_ROTATE_YAW_DELTA = 0.000873; + +/** + * Build the engine's camera drivers — pure over `state`, returning the + * fixed set of wrappers in no particular order (priority, not position, + * decides who wins). + * + * Each wrapper closes over the live `state`, so the list is built once + * and never needs rebuilding: settings toggles and subsystem state are + * read fresh every frame through the closures. The drivers, highest + * priority first: + * + * - `input` (100) — raw SpaceMouse axes. Beats everything: direct user + * input always wins. + * - `tween` (60) — an in-flight focus/framing tween. + * - `autoRotate` (20) — the idle drift, lowest priority so any of the + * above suppresses it. + * + * The `tour` driver (priority 80) is intentionally absent — a separate + * plan slots it in between input and tween. + */ +export function buildCameraDrivers(state: EngineState): readonly CameraDriver[] { + return [ + { + id: 'input', + priority: 100, + isActive: () => state.subsystems.spaceMouse.hasAxes(), + apply: (cam, nowMs) => state.subsystems.spaceMouse.applyToCamera(cam, nowMs), + }, + { + id: 'tween', + priority: 60, + isActive: () => state.subsystems.tweens.isActive(), + apply: (cam, nowMs) => { + // `advance` returns a finished-this-frame boolean the engine has + // never consumed; a block discards it so `apply` stays void. + state.subsystems.tweens.advance(cam, nowMs); + }, + }, + { + id: 'autoRotate', + priority: 20, + isActive: () => state.settings.camera.autoRotate, + apply: (cam) => { + cam.yaw += AUTO_ROTATE_YAW_DELTA; + updatePosition(cam); + }, + }, + ]; +} diff --git a/src/services/engine/phases/startLoop.ts b/src/services/engine/phases/startLoop.ts index bbf9db9a..1f378779 100644 --- a/src/services/engine/phases/startLoop.ts +++ b/src/services/engine/phases/startLoop.ts @@ -60,6 +60,7 @@ */ import { runFrame } from '../frame/runFrame'; +import { buildCameraDrivers } from '../camera/cameraDrivers'; import type { RunFrameDeps } from '../../../@types/engine/frame/RunFrameDeps'; import type { EngineState } from '../../../@types/engine/state/EngineState'; @@ -128,6 +129,11 @@ export async function startLoop(state: EngineState, deps: BootstrapDeps): Promis // Forward the timing service hung off `state.gpu` by initGpu. // Always non-null; `renderFrame` gates work behind `.enabled`. timingService: state.gpu.timingService, + // Wrap the engine's camera movers as drivers once, here. The + // wrappers close over the live `state`, so the list never needs + // rebuilding — toggled settings and subsystem state are read fresh + // each frame through the closures. + drivers: buildCameraDrivers(state), }; // Assign the real frame body to the forward-declared `frame` diff --git a/tests/services/engine/camera/cameraDriverWrappers.test.ts b/tests/services/engine/camera/cameraDriverWrappers.test.ts new file mode 100644 index 00000000..f59e7bd5 --- /dev/null +++ b/tests/services/engine/camera/cameraDriverWrappers.test.ts @@ -0,0 +1,139 @@ +/** + * cameraDriverWrappers — unit tests for `buildCameraDrivers`, the pure + * builder that wraps the engine's camera movers as `CameraDriver`s. + * + * The wrappers are thin mappings: `isActive` should mirror the + * underlying predicate (a subsystem method or a settings flag) and + * `apply` should forward to the underlying mutator with the same `cam` + * and `nowMs` the resolver hands it. These tests assert exactly that + * mapping against a minimal fake `state` of `vi.fn()` spies. The + * auto-rotate wrapper has no subsystem to forward to, so it is checked + * directly: a precise yaw delta and a recomputed `position`. + * + * Because every wrapper closes over the live `state`, toggling a fake's + * return value (or mutating a settings flag) between assertions exercises + * that the closures read state fresh rather than snapshotting at build + * time. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import type { CameraDriver } from '../../../../src/@types/engine/camera/CameraDriver'; +import type { EngineState } from '../../../../src/@types/engine/state/EngineState'; +import type { OrbitCamera } from '../../../../src/@types/camera/OrbitCamera'; +import { buildCameraDrivers } from '../../../../src/services/engine/camera/cameraDrivers'; + +// Must match the constant baked into the autoRotate wrapper. +const AUTO_ROTATE_YAW_DELTA = 0.000873; + +type FakeState = { + subsystems: { + spaceMouse: { hasAxes: ReturnType; applyToCamera: ReturnType }; + tweens: { isActive: ReturnType; advance: ReturnType }; + }; + settings: { camera: { autoRotate: boolean } }; +}; + +function makeFakeState(): FakeState { + return { + subsystems: { + spaceMouse: { hasAxes: vi.fn(() => false), applyToCamera: vi.fn() }, + tweens: { isActive: vi.fn(() => false), advance: vi.fn(() => false) }, + }, + settings: { camera: { autoRotate: false } }, + }; +} + +function driverById(state: EngineState, id: string): CameraDriver { + const drivers = buildCameraDrivers(state); + const driver = drivers.find((d) => d.id === id); + if (!driver) throw new Error(`no driver with id '${id}'`); + return driver; +} + +const camStub = {} as OrbitCamera; + +describe('buildCameraDrivers — input wrapper', () => { + it('isActive reflects spaceMouse.hasAxes', () => { + const fake = makeFakeState(); + const input = driverById(fake as unknown as EngineState, 'input'); + + fake.subsystems.spaceMouse.hasAxes.mockReturnValue(true); + expect(input.isActive(0)).toBe(true); + + fake.subsystems.spaceMouse.hasAxes.mockReturnValue(false); + expect(input.isActive(0)).toBe(false); + }); + + it('apply forwards cam + nowMs to spaceMouse.applyToCamera', () => { + const fake = makeFakeState(); + const input = driverById(fake as unknown as EngineState, 'input'); + + input.apply(camStub, 1234); + + expect(fake.subsystems.spaceMouse.applyToCamera).toHaveBeenCalledTimes(1); + expect(fake.subsystems.spaceMouse.applyToCamera).toHaveBeenCalledWith(camStub, 1234); + }); +}); + +describe('buildCameraDrivers — tween wrapper', () => { + it('isActive reflects tweenManager.isActive', () => { + const fake = makeFakeState(); + const tween = driverById(fake as unknown as EngineState, 'tween'); + + fake.subsystems.tweens.isActive.mockReturnValue(true); + expect(tween.isActive(0)).toBe(true); + + fake.subsystems.tweens.isActive.mockReturnValue(false); + expect(tween.isActive(0)).toBe(false); + }); + + it('apply forwards cam + nowMs to tweenManager.advance', () => { + const fake = makeFakeState(); + const tween = driverById(fake as unknown as EngineState, 'tween'); + + tween.apply(camStub, 5678); + + expect(fake.subsystems.tweens.advance).toHaveBeenCalledTimes(1); + expect(fake.subsystems.tweens.advance).toHaveBeenCalledWith(camStub, 5678); + }); +}); + +describe('buildCameraDrivers — autoRotate wrapper', () => { + it('isActive tracks settings.camera.autoRotate (read fresh through the closure)', () => { + const fake = makeFakeState(); + const autoRotate = driverById(fake as unknown as EngineState, 'autoRotate'); + + fake.settings.camera.autoRotate = true; + expect(autoRotate.isActive(0)).toBe(true); + + fake.settings.camera.autoRotate = false; + expect(autoRotate.isActive(0)).toBe(false); + }); + + it('apply increments yaw by the auto-rotate delta and recomputes position', () => { + const fake = makeFakeState(); + const autoRotate = driverById(fake as unknown as EngineState, 'autoRotate'); + + // A real-ish cam stub with the fields `updatePosition` reads/writes: + // it reads yaw/pitch/distance/target and overwrites `position`. + const cam = { + yaw: 0, + pitch: 0, + distance: 10, + target: [0, 0, 0], + position: new Float32Array([0, 0, 0]), + } as unknown as OrbitCamera; + + autoRotate.apply(cam, 0); + + expect(cam.yaw).toBeCloseTo(AUTO_ROTATE_YAW_DELTA, 12); + // updatePosition ran: with yaw≈delta, pitch 0, distance 10, the + // position is recomputed to target + distance·dir. dir.z = cos(yaw)· + // cos(pitch) ≈ 1, so z ≈ 10; a tiny yaw also puts a small +x there. + // Asserting position moved off its zeroed seed proves updatePosition + // wrote it. + expect(cam.position[2]).toBeCloseTo(10, 4); + expect(cam.position[0]).toBeGreaterThan(0); + }); +}); diff --git a/tests/services/engine/frame/runFrame.test.ts b/tests/services/engine/frame/runFrame.test.ts index d9ba0ac8..ba865bce 100644 --- a/tests/services/engine/frame/runFrame.test.ts +++ b/tests/services/engine/frame/runFrame.test.ts @@ -157,6 +157,7 @@ function makeDeps(opts: { milkyWayITimeEpochMs: 0, // Disabled stub matches production's "no `?gpuTimings`" path. timingService: createDisabledGpuTimingService(), + drivers: [], }; } From df0a603d99a3134db220d290c0cf1c2f81fe3972 Mon Sep 17 00:00:00 2001 From: Alexander Rulkens Date: Mon, 8 Jun 2026 01:16:22 +0200 Subject: [PATCH 6/7] feat(camera): runFrame writes the camera through the driver registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The behaviour-preserving swap. The three sequenced per-frame camera blocks (tween.advance → spaceMouse.applyToCamera → guarded auto-rotate) collapse to a single runCameraDrivers(deps.drivers, state.cam, nowMs) call — one camera-write site, precedence by priority not statement order. The auto-rotate !tweens.isActive() guard is deleted: tween (60) outranks auto-rotate (20), so the resolver subsumes it. Cancellation is unchanged (lives in spaceMouse.applyToCamera's cancelTween callback). The render-on-demand stillAnimating gate's three camera terms collapse to deps.drivers.some(d => d.isActive(nowMs)) — equivalent because each driver's isActive maps one-to-one to an old term (proven by the wrapper suite). Three regression tests pin tween-beats-autoRotate, idle-holds, and the exact auto-rotate yaw delta. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/services/engine/frame/runFrame.ts | 72 ++++----- tests/services/engine/frame/runFrame.test.ts | 148 +++++++++++++++++++ 2 files changed, 176 insertions(+), 44 deletions(-) diff --git a/src/services/engine/frame/runFrame.ts b/src/services/engine/frame/runFrame.ts index faf46fde..1c7fe090 100644 --- a/src/services/engine/frame/runFrame.ts +++ b/src/services/engine/frame/runFrame.ts @@ -38,6 +38,7 @@ import type { EngineState } from '../../../@types/engine/state/EngineState'; import type { RunFrameDeps } from '../../../@types/engine/frame/RunFrameDeps'; import { updatePosition } from '../../camera/orbitCamera'; +import { runCameraDrivers } from '../camera/cameraDrivers'; import { resizeCanvasToDisplay } from '../../gpu/device'; import { cssToTexPx } from '../helpers/cssToTexPx'; import { isEngineReady } from '../helpers/engineReady'; @@ -125,48 +126,30 @@ export function runFrame(state: EngineState, deps: RunFrameDeps, nowMs: number): deps.cb.camera?.onCameraChange?.(snap); } - // ── Focus / home tween ──────────────────────────────────────────── + // ── Camera drivers ──────────────────────────────────────────────── // - // `advance` mutates the camera and calls updatePosition internally, so - // by the auto-rotate block below the camera is at the eased - // intermediate. The manager auto-clears when the tween finishes, so - // later frames skip this via `isActive()`. + // One camera-write site per frame, behind the camera-driver-authority + // model. Every mover that wants to write the camera (raw input, an + // in-flight tween, idle auto-rotate) is a `CameraDriver` with a numeric + // `priority`; `runCameraDrivers` scans the list, picks the single + // highest-priority driver that declares itself active, and runs ONLY + // that one's `apply`. Precedence is therefore data (priority), not + // statement order, and there is no blending — a frame is authored by one + // driver or by none. // - // Tween / SpaceMouse / auto-rotate all run *before* the - // `deriveFrameContext` gate so a camera-only-ready frame still makes - // motion progress before we early-return for missing GPU handles. - if (state.cam) { - state.subsystems.tweens.advance(state.cam, performance.now()); - } - - // ── SpaceMouse per-frame application ────────────────────────────── + // Auto-rotate is suppressed-by-tween purely through priority: the tween + // driver outranks auto-rotate, so when a tween is active it wins and + // auto-rotate never fires. The old explicit `!tweens.isActive()` guard + // that encoded the same rule is gone — the resolver subsumes it. // - // The subsystem owns the whole "if puck deflected, apply axes - // scaled by wall-clock dt, otherwise reset the dt baseline" - // dance — including the `tweens.cancel()` precedence rule (it - // calls back into the engine via the `cancelTween` callback we - // wired up at construction). Calling unconditionally is fine: - // on a resting puck it's a single hasAnyAxis read + a null - // assignment to the dt baseline. + // This runs *before* `deriveFrameContext` so a camera-only-ready frame + // still makes motion progress before we early-return for missing GPU + // handles. Cancellation (raw input cancelling an in-flight tween) is + // unchanged and does NOT live here: the SpaceMouse subsystem fires its + // `cancelTween` callback as part of `applyToCamera`. The resolver only + // arbitrates the same-frame race for who gets to write the camera. if (state.cam) { - state.subsystems.spaceMouse.applyToCamera(state.cam, performance.now()); - } - - // ── Auto-rotate yaw ─────────────────────────────────────────────── - // - // Advance yaw by a fixed per-frame delta (~3°/sec at 60 Hz → 0.000873 - // rad/frame). Fixed delta, not wall-clock: at 120 Hz it's smoother but - // twice as fast — acceptable for a gentle ambient effect, no timer - // bookkeeping. - // - // Skipped while a tween is active: focus / focusOnHome tweens drive - // `cam.yaw` toward a target, and `tweens.advance()` already set the - // eased intermediate this frame. Adding the auto-rotate delta on top - // would overshoot the target (and drift forever after), so gating on - // `!tweens.isActive()` lets the home tween land exactly. - if (state.settings.camera.autoRotate && state.cam && !state.subsystems.tweens.isActive()) { - state.cam.yaw += 0.000873; - updatePosition(state.cam); + runCameraDrivers(deps.drivers, state.cam, nowMs); } // ── Per-frame derived snapshot ──────────────────────────────────── @@ -465,9 +448,12 @@ export function runFrame(state: EngineState, deps: RunFrameDeps, nowMs: number): // frame each. // // Predicate breakdown: - // - autoRotate: continuous yaw advancement. - // - tweens.isActive(): camera tween in progress. - // - spaceMouse.hasAxes(): puck deflected; apply per-frame velocity. + // - camera drivers active: any camera mover (raw input, an in-flight + // tween, or idle auto-rotate) declares itself active this frame, via + // the same driver registry the per-frame camera write resolves + // through. `.some(d => d.isActive(nowMs))` IS the boolean OR of + // those movers, so it tracks the resolver exactly — one place decides + // "is the camera moving" for both the write and the keep-ticking gate. // - texturedDisks.hasInFlightWork(): a thumbnail fetch is racing the // network OR a landed bitmap is in its 400 ms load-fade window. The // onResult calls requestRender(), but we keep a frame queued anyway @@ -491,9 +477,7 @@ export function runFrame(state: EngineState, deps: RunFrameDeps, nowMs: number): // fade-out in setSourceVisible / tier-swap commit would hang forever. state.subsystems.fades.tick(nowMs); const stillAnimating = - state.settings.camera.autoRotate || - state.subsystems.tweens.isActive() || - state.subsystems.spaceMouse.hasAxes() || + deps.drivers.some((d) => d.isActive(nowMs)) || (ready && state.subsystems.texturedDisks.hasInFlightWork()) || state.subsystems.fades.isAnyAnimating(nowMs) || state.subsystems.structureFocus.isAwake(nowMs) || diff --git a/tests/services/engine/frame/runFrame.test.ts b/tests/services/engine/frame/runFrame.test.ts index ba865bce..d5fc12b4 100644 --- a/tests/services/engine/frame/runFrame.test.ts +++ b/tests/services/engine/frame/runFrame.test.ts @@ -30,11 +30,23 @@ vi.mock('../../../../src/services/engine/wiring/reevaluateDemand', () => ({ reevaluateDemand: vi.fn(), })); +// resizeCanvasToDisplay reads window.devicePixelRatio; in this node test +// environment window is undefined. The cam-bearing regression fixtures +// reach the resize block (non-null cam), so stub it to a no-op false — +// resize is incidental to what these tests exercise (the camera drivers). +// runFrame imports only resizeCanvasToDisplay from this module, so a full +// replacement is safe (nothing else from gpu/device needs the real impl). +vi.mock('../../../../src/services/gpu/device', () => ({ + resizeCanvasToDisplay: () => false, +})); + import { runFrame } from '../../../../src/services/engine/frame/runFrame'; +import { buildCameraDrivers } from '../../../../src/services/engine/camera/cameraDrivers'; import { reevaluateDemand } from '../../../../src/services/engine/wiring/reevaluateDemand'; import { createDisabledGpuTimingService } from '../../../../src/services/gpu/timing/gpuTimingService'; import type { RunFrameDeps } from '../../../../src/@types/engine/frame/RunFrameDeps'; import type { EngineState } from '../../../../src/@types/engine/state/EngineState'; +import type { OrbitCamera } from '../../../../src/@types/camera/OrbitCamera'; /** * Build a minimal `EngineState`-shaped fixture that lets `runFrame` @@ -204,6 +216,142 @@ describe('runFrame — FPS wiring', () => { }); }); +/** + * Camera-driver regression fixtures. + * + * Unlike `makeState()` (whose `cam: null` bails before the camera block), + * these fixtures carry a real `OrbitCamera`-shaped `cam` so the per-frame + * camera-driver resolver — which runs BEFORE `deriveFrameContext` — is + * actually exercised. The renderer is still null, so `deriveFrameContext` + * reports not-ready and the body early-returns right after the camera + * block: exactly the slice we want to pin without standing up a GPU. + * + * `makeCamState` lets each test control the three driver predicates + * (autoRotate / tween-active / spaceMouse-axes) and inject a tween + * `advance` that mutates the camera, so we can prove which driver actually + * authored the frame. + */ +function makeCamState(opts: { + autoRotate?: boolean; + tweenActive?: boolean; + tweenAdvance?: (cam: OrbitCamera, nowMs: number) => void; + spaceMouseHasAxes?: boolean; + spaceMouseApply?: (cam: OrbitCamera, nowMs: number) => void; +}): EngineState { + const cam: OrbitCamera = { + yaw: 0, + pitch: 0, + distance: 100, + target: new Float32Array([0, 0, 0]), + position: new Float32Array([0, 0, 0]), + fovYRad: 0.8, + aspect: 1, + near: 0.01, + far: 1000, + } as unknown as OrbitCamera; + + const state = makeState(); + // Splice the cam + driver-predicate stubs onto the shared base fixture. + // The base already supplies a no-op selection stub and null gpu handles, + // which is all we need to reach the early-return after the camera block. + (state as { cam: OrbitCamera }).cam = cam; + state.settings.camera.autoRotate = opts.autoRotate ?? false; + state.subsystems.tweens = { + isActive: () => opts.tweenActive ?? false, + advance: vi.fn(opts.tweenAdvance ?? (() => {})), + } as unknown as EngineState['subsystems']['tweens']; + state.subsystems.spaceMouse = { + hasAxes: () => opts.spaceMouseHasAxes ?? false, + applyToCamera: vi.fn(opts.spaceMouseApply ?? (() => {})), + } as unknown as EngineState['subsystems']['spaceMouse']; + return state; +} + +/** + * Deps for the cam-fixture tests: real camera drivers built from the same + * `state` (so the production wiring is exercised, not a hand-rolled fake), + * and a canvas whose client/backing dims match so `resizeCanvasToDisplay` + * returns false and doesn't poke `cam.aspect`. + */ +function makeCamDeps(state: EngineState): RunFrameDeps { + const deps = makeDeps({ fpsValue: null, lastReportedFps: { current: null } }); + return { + ...deps, + canvas: { + width: 100, + height: 100, + clientWidth: 100, + clientHeight: 100, + } as unknown as HTMLCanvasElement, + drivers: buildCameraDrivers(state), + }; +} + +describe('runFrame — camera drivers (regression)', () => { + it('tween active + autoRotate on → tween wins; auto-rotate does not nudge yaw on top', () => { + // The tween driver (priority 60) outranks auto-rotate (20), so with both + // active the resolver runs ONLY the tween's apply. This pins the + // behaviour the deleted `!tweens.isActive()` guard used to encode: the + // home tween lands exactly, with no auto-rotate delta added on top. + const state = makeCamState({ + autoRotate: true, + tweenActive: true, + tweenAdvance: (cam) => { + cam.yaw = 1.5; + }, + }); + const deps = makeCamDeps(state); + + runFrame(state, deps, 1000); + + expect(state.subsystems.tweens.advance).toHaveBeenCalledOnce(); + // 1.5 exactly, NOT 1.5 + 0.000873 — auto-rotate was suppressed by priority. + expect(state.cam!.yaw).toBe(1.5); + }); + + it('idle (no driver active, autoRotate off) → camera holds', () => { + const state = makeCamState({}); + const deps = makeCamDeps(state); + + const before = { + yaw: state.cam!.yaw, + pitch: state.cam!.pitch, + distance: state.cam!.distance, + }; + + runFrame(state, deps, 1000); + + expect(state.cam!.yaw).toBe(before.yaw); + expect(state.cam!.pitch).toBe(before.pitch); + expect(state.cam!.distance).toBe(before.distance); + expect(state.subsystems.tweens.advance).not.toHaveBeenCalled(); + expect(state.subsystems.spaceMouse.applyToCamera).not.toHaveBeenCalled(); + }); + + it('autoRotate on, nothing else active → yaw advances by exactly 0.000873', () => { + const state = makeCamState({ autoRotate: true }); + const deps = makeCamDeps(state); + + const startYaw = state.cam!.yaw; + runFrame(state, deps, 1000); + + // Auto-rotate is the only active driver, so its fixed per-frame delta lands. + expect(state.cam!.yaw).toBeCloseTo(startYaw + 0.000873, 9); + // updatePosition ran off the +Z convention (yaw≈0, pitch 0): position.z ≈ distance. + expect(state.cam!.position[2]).toBeCloseTo(state.cam!.distance, 3); + }); + + // RoD camera-term collapse (`deps.drivers.some(d => d.isActive(nowMs))` + // replacing the three old autoRotate/tween/spaceMouse OR terms) is NOT + // directly asserted here: the `stillAnimating` tail is reachable only on + // the GPU-ready path, and these lightweight fixtures early-return at the + // renderer-null guard. The equivalence is underwritten elsewhere — each + // driver's `isActive` maps one-to-one onto an old term, proven by + // cameraDriverWrappers.test.ts, and `.some` over those three IS their + // boolean OR. Building a full GPU-ready fixture solely to re-prove that + // identity would be disproportionate, so we document the coverage instead. +}); + describe('runFrame — demand re-evaluation', () => { it('re-derives demand once per frame', () => { // The per-frame call is what lets every setter Just Work: a setter flips From 5a8cde3231ab8727bb59719d6a50bb1ce3b0243e Mon Sep 17 00:00:00 2001 From: Alexander Rulkens Date: Mon, 8 Jun 2026 01:20:25 +0200 Subject: [PATCH 7/7] =?UTF-8?q?docs(tour):=20tick=20camera-driver-authorit?= =?UTF-8?q?y=20plan=20=E2=80=94=20all=205=20tasks=20complete?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-06-08-camera-driver-authority.md | 95 ++++++++++--------- 1 file changed, 52 insertions(+), 43 deletions(-) diff --git a/docs/superpowers/plans/2026-06-08-camera-driver-authority.md b/docs/superpowers/plans/2026-06-08-camera-driver-authority.md index 0478d55c..2e849607 100644 --- a/docs/superpowers/plans/2026-06-08-camera-driver-authority.md +++ b/docs/superpowers/plans/2026-06-08-camera-driver-authority.md @@ -134,19 +134,19 @@ export type CameraDriver = { }; ``` -- [ ] Verify the `OrbitCamera` import resolves: from +- [x] Verify the `OrbitCamera` import resolves: from `src/@types/engine/camera/CameraDriver.d.ts`, the path to `src/@types/camera/OrbitCamera.d.ts` is `'../../../camera/OrbitCamera'` (`engine/camera/` → up 3 → `@types/` → `camera/OrbitCamera`). Confirm against the live tree before committing. -- [ ] Write the type exactly as above, with a didactic module header explaining: +- [x] Write the type exactly as above, with a didactic module header explaining: the type is the seam that makes camera precedence **data** instead of call order; `id` is for debugging/identity, `priority` orders the resolver, `isActive` answers both "should I write this frame" and (collectively) "is the camera animating" for the RoD gate; `apply` is the single mutation each driver performs. -- [ ] Main thread: `npm run typecheck` passes (no consumer yet — this just proves +- [x] Main thread: `npm run typecheck` passes (no consumer yet — this just proves the import path). -- [ ] Commit. +- [x] Commit. --- @@ -167,24 +167,24 @@ nothing (no `apply` call). Pure over the driver list — no captured state, no I **Tests** (TDD red→green; fake drivers, `cam` is a throwaway stub): -- [ ] `runCameraDrivers calls the single highest-priority active driver` — two +- [x] `runCameraDrivers calls the single highest-priority active driver` — two active drivers (priority 60 + 20); assert the priority-60 driver's `apply` was called once and the priority-20 driver's `apply` was NOT called. -- [ ] `runCameraDrivers ignores inactive higher-priority drivers` — priority-100 +- [x] `runCameraDrivers ignores inactive higher-priority drivers` — priority-100 driver `isActive=false`, priority-20 driver `isActive=true`; assert the priority-20 `apply` is called (an inactive higher driver does not block a lower active one). -- [ ] `runCameraDrivers writes nothing when no driver is active` — all drivers +- [x] `runCameraDrivers writes nothing when no driver is active` — all drivers `isActive=false`; assert no `apply` is called on any driver. -- [ ] `runCameraDrivers forwards cam and nowMs to apply` — assert the active +- [x] `runCameraDrivers forwards cam and nowMs to apply` — assert the active driver's `apply` received the exact `cam` reference and `nowMs` value passed in. -- [ ] Run the suite (`npm test -- cameraDrivers`) — fails (module absent). -- [ ] Implement `runCameraDrivers` with a didactic header: this is the ONE +- [x] Run the suite (`npm test -- cameraDrivers`) — fails (module absent). +- [x] Implement `runCameraDrivers` with a didactic header: this is the ONE camera-write site; single-writer arbitration (no cooperative blending — the spec bakes that in); precedence is data. Note it does not sort-mutate the input array (treat `drivers` as readonly). -- [ ] Main thread: `npm test -- cameraDrivers` → all pass; `npm run typecheck`. -- [ ] Commit. +- [x] Main thread: `npm test -- cameraDrivers` → all pass; `npm run typecheck`. +- [x] Commit. --- @@ -241,18 +241,18 @@ predicates, the yaw delta `0.000873`, and that `autoRotate.apply` calls **Tests** (wrapper mapping — each wrapper's predicate maps to its underlying piece): -- [ ] `input driver isActive reflects spaceMouse.hasAxes` — fake spaceMouse with +- [x] `input driver isActive reflects spaceMouse.hasAxes` — fake spaceMouse with toggleable `hasAxes`; assert the driver's `isActive` returns the same boolean both ways. -- [ ] `input driver apply forwards to spaceMouse.applyToCamera` — assert the fake's +- [x] `input driver apply forwards to spaceMouse.applyToCamera` — assert the fake's `applyToCamera` was called with the cam + nowMs. -- [ ] `tween driver isActive reflects tweenManager.isActive` — fake tween manager; +- [x] `tween driver isActive reflects tweenManager.isActive` — fake tween manager; assert both booleans. -- [ ] `tween driver apply forwards to tweenManager.advance` — assert `advance` +- [x] `tween driver apply forwards to tweenManager.advance` — assert `advance` called with cam + nowMs. -- [ ] `autoRotate driver isActive reflects settings.camera.autoRotate` — flip the +- [x] `autoRotate driver isActive reflects settings.camera.autoRotate` — flip the setting; assert the boolean tracks it. -- [ ] `autoRotate driver apply increments yaw and updates position` — assert +- [x] `autoRotate driver apply increments yaw and updates position` — assert `cam.yaw` increased by `0.000873` and `updatePosition` ran (spy or assert `position` recompute via a real-ish cam stub). @@ -267,12 +267,12 @@ predicates, the yaw delta `0.000873`, and that `autoRotate.apply` calls > when you read the live `EngineState`; prefer passing `state` and reading the > slices inside, matching how other subsystems close over `state`. -- [ ] Run the wrapper suite — fails (builder absent). -- [ ] Implement `buildCameraDrivers` in `cameraDrivers.ts`; wire `startLoop` to +- [x] Run the wrapper suite — fails (builder absent). +- [x] Implement `buildCameraDrivers` in `cameraDrivers.ts`; wire `startLoop` to call it and put the result on `frameDeps.drivers`; add the `drivers` field to `RunFrameDeps.d.ts`. -- [ ] Main thread: `npm test -- cameraDriver` (resolver + wrappers); `npm run typecheck`. -- [ ] Commit. +- [x] Main thread: `npm test -- cameraDriver` (resolver + wrappers); `npm run typecheck`. +- [x] Commit. --- @@ -314,24 +314,26 @@ fixture (`makeState()` at ~line 47); extend those — the new `drivers` field mu added to the deps fixture (use the real `buildCameraDrivers(state)` so the fixture exercises the production wiring, or hand-built fake drivers where that's simpler). -- [ ] `runFrame: tween active + autoRotate on → tween wins, autoRotate does not nudge yaw` +- [x] `runFrame: tween active + autoRotate on → tween wins, autoRotate does not nudge yaw` — fixture: `state.settings.camera.autoRotate = true`, tween manager `isActive` returns true and its `advance` sets cam to a known pose; spaceMouse `hasAxes` false. Run `runFrame`. Assert: `tween.advance` was called; `cam.yaw` equals the tween's pose (NOT pose + 0.000873) — i.e. auto-rotate did NOT add its delta on top. (This is the behaviour the deleted `!tweens.isActive()` guard encoded.) -- [ ] `runFrame: idle (no driver active, autoRotate off) → camera holds` — all +- [x] `runFrame: idle (no driver active, autoRotate off) → camera holds` — all driver predicates false; run `runFrame`; assert `cam.yaw` (and target/distance/ pitch) are unchanged. -- [ ] `runFrame: autoRotate on, nothing else active → yaw advances by 0.000873` +- [x] `runFrame: autoRotate on, nothing else active → yaw advances by 0.000873` — only `autoRotate.isActive` true; assert `cam.yaw` increased by exactly the delta and `updatePosition` ran. (Confirms the lower-priority driver still fires when it is the sole active one.) -- [ ] `runFrame: RoD stays awake iff a camera driver is active` — two sub-cases: - with one driver active (e.g. autoRotate on) assert `scheduler.requestRender` was - called; with no driver active AND all non-camera terms false assert it was NOT - called. (Mirrors the old camera terms; mock `reevaluateDemand` as the existing - file already does.) +- [x] `runFrame: RoD stays awake iff a camera driver is active` — covered by + equivalence rather than a direct test: the `stillAnimating` tail is reachable + only on the GPU-ready path (the lightweight fixture early-returns at the + renderer-null guard), and each driver's `isActive` maps one-to-one onto an old + term (proven by `cameraDriverWrappers.test.ts`), so `.some(isActive)` IS their + boolean OR. Documented as a comment in the test file; a heavyweight GPU fixture + to re-prove the identity would be disproportionate. > The existing `runFrame.test.ts` mocks `reevaluateDemand` (`:29`). Keep that mock. > The fixture short-circuits the GPU body via the renderer-null guard @@ -345,13 +347,13 @@ exercises the production wiring, or hand-built fake drivers where that's simpler > RoD assertions around the path the fixture actually takes — read the body > top-to-bottom against your fixture before asserting). -- [ ] Run `npm test -- runFrame` with the NEW tests against the OLD camera block +- [x] Run `npm test -- runFrame` with the NEW tests against the OLD camera block first if feasible (to confirm they encode current behaviour); then apply the swap and confirm still green. If running against old code first is impractical given the resolver dependency, write the tests and the swap together but keep each assertion tied to a documented current-behaviour fact above. -- [ ] Main thread: `npm test -- runFrame`; full `npm test`; `npm run typecheck`. -- [ ] Commit. +- [x] Main thread: `npm test -- runFrame`; full `npm test`; `npm run typecheck`. +- [x] Commit. --- @@ -362,20 +364,27 @@ exercises the production wiring, or hand-built fake drivers where that's simpler - `src/services/engine/phases/startLoop.ts` - `src/services/engine/camera/cameraDrivers.ts` -- [ ] Run the `entanglement-radar` skill over the diff. Confirm: precedence is now +- [x] Run the `entanglement-radar` skill over the diff. Confirm: precedence is now data (no `if (source/order)` chain), one camera-write site, the RoD camera terms - collapsed to the registry, no mirror of the driver list on `EngineState`. -- [ ] Grep for any remaining direct `state.subsystems.tweens.advance` / + collapsed to the registry, no mirror of the driver list on `EngineState`. — All + confirmed; the diff IS the un-braid (precedence×statement-order and the duplicated + "camera is moving" truth both dissolved). No new complecting found. +- [x] Grep for any remaining direct `state.subsystems.tweens.advance` / `spaceMouse.applyToCamera` / `cam.yaw += 0.000873` outside the driver wrappers — - there should be none in `runFrame.ts`. (Use Grep tool, not bash grep.) -- [ ] Confirm the SpaceMouse `cancelTween` mechanism (`spaceMouseSubsystem.ts:204`) + there should be none in `runFrame.ts`. — None; `0.000873` lives only in + `cameraDrivers.ts` (`AUTO_ROTATE_YAW_DELTA`). +- [x] Confirm the SpaceMouse `cancelTween` mechanism (`spaceMouseSubsystem.ts:204`) and the OrbitControls/engine.ts mouse-drag tween-cancel wiring are UNTOUCHED — cancellation is the interrupt mechanism; the registry only resolves the - same-frame race. The spec is explicit about this; do not "simplify" it away. -- [ ] Confirm no fov/near/far behaviour changed (the tween still does not touch - them; auto-rotate touches only yaw). -- [ ] Main thread: full `npm test`; `npm run typecheck`. All green. -- [ ] Commit any cleanup. + same-frame race. The spec is explicit about this; do not "simplify" it away. — + `cancelTween()` at `spaceMouseSubsystem.ts:204` intact; no changes outside the + four camera files. +- [x] Confirm no fov/near/far behaviour changed (the tween still does not touch + them; auto-rotate touches only yaw). — Confirmed; auto-rotate touches only `yaw`, + tween path unchanged. +- [x] Main thread: full `npm test`; `npm run typecheck`. All green. — 2456 tests + pass; typecheck clean. +- [x] Commit any cleanup. — No code cleanup needed (review found no new knots). ---