refactor(engine): extract per-frame body to runFrame.ts#44
Merged
Conversation
Per-frame body lifted verbatim from engine.ts:1407-1708 into runFrame.ts.
Closure-captured locals threaded through RunFrameDeps; the mutable
`lastReportedFps` becomes a {current} ref so its writes round-trip into
engine.ts across the module boundary.
`updateScaleBar` and `setHovered` are passed as dep functions rather
than ref-ifying their internal `let`s (lastScaleSig, hovered, etc.) —
the helpers already close over those locals and the frame body never
reads/writes them directly.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Frame body collapses to a single runFrame(state, frameDeps, now) call.
The forward-declared `frame` arrow now just dispatches to runFrame; the
~310-line per-frame body that used to live inline in engine.ts has been
relocated wholesale to runFrame.ts.
Closure-captured locals are threaded through `RunFrameDeps`:
- `lastReportedFps` becomes a `{current}` ref (only mutated entry).
- `fpsCounter`, `updateScaleBar`, `setHovered`, `cssToTexPx`,
`milkyWayITimeEpochMs` ride as read-only refs / values.
- IIFE-local renderers (`device`, `context`, `milkyWayRenderer`,
`filamentRenderer`, `quadRenderer`, `diskRenderer`) ride as
read-only refs.
The keep-rendering predicate (formerly the "stillAnimating" tail of
the frame body) lives inside `runFrame` itself in this phase — there
is no separate `scheduleFrameTail` helper to keep behind in engine.ts.
The plan mentioned one as a possibility; in practice it's a single
condition that can stay in the lifted body without losing readability.
Removes now-unused imports: `computeViewProj`, `autoLodMask`,
`renderFrame`, `PROCEDURAL_DISK_FADE_START_PX`,
`PROCEDURAL_DISK_FADE_END_PX`.
engine.ts: 2225 → 1953 lines (-272).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
skymap | 8882cfd | Commit Preview URL Branch Preview URL |
May 08 2026, 12:02 AM |
…redicate Per code-quality review nit: the `if (visibleSources.length === 0)` early-return inside the hover-pick block bypasses the keep-rendering predicate at the end of runFrame. In engine.ts pre-extraction the two were close together and the asymmetry was easier to spot; the relocation pushed them ~80 lines apart. Acknowledge the by-design behaviour with a comment so a fresh reader doesn't think it's a bug. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 3 of Spec B's engine internal restructure. The ~310-line per-frame body has been lifted out of
engine.tsinto a dedicatedrunFrame.tsmodule.src/services/engine/runFrame.ts(new): the per-frame body, lifted verbatim. No behaviour changes.src/services/engine/engine.ts: forward-declaredframearrow now dispatches torunFrame(state, frameDeps, performance.now()). Net diff: -272 lines (2225 → 1953).tests/services/engine/runFrame.test.ts(new): focused integration test covering the FPS-counter wiring through the newRunFrameDeps.Closure captures threaded through
RunFrameDepscanvascbfpsCounterlastReportedFpslet{current}refdevicecontextmilkyWayRendererfilamentRendererquadRendererdiskRenderermilkyWayITimeEpochMscssToTexPxsetHoveredupdateScaleBarOnly
lastReportedFpsneeded the{current}ref pattern — it's the only mutable closure value the body writes directly.lastScaleSig(also alet) is encapsulated insideupdateScaleBar()'s closure, so it stays inline; same story for the other helpers' internal state.Deviations from the plan
scheduleFrameTail()helper that "stays in engine.ts". In practice the keep-rendering predicate is a single 8-line conditional with no other call sites, so it lives insiderunFramerather than as a separate helper. Splitting it out would have been pure ceremony.Test plan
runFrame.test.tspasses (3/3 — FPS round-trip, dedup, bootstrap)npm run typecheckclean (bothsrcandtoolsconfigs)Spec:
docs/superpowers/specs/2026-05-08-engine-internal-restructure-design.md🤖 Generated with Claude Code