Skip to content

Commit a5bc8f9

Browse files
committed
Make the landing snow actually render on an idle screen
Three independent gates kept the falling-snow decoration over the landing mountain from ever drawing: the mount-time paint was still=true, which the old snowOn check tied directly to snow visibility; paintLanding early-returned whenever it wasn't re-entered with animating=true, which never happens while idle; and its only caller was the turn monitor, which deliberately stops ticking once idle. Fix: snow visibility no longer depends on `still` (that flag now only freezes the mountain's own draw/fill/fade timeline, not the flakes over it), the early-return is gone so idle repaints actually happen, and the shell's createAppShell now repaints the landing off the renderer's own FRAME event whenever the landing is up and not mid-turn-animation. That event is already scoped to shell lifetime (wired at construction, unwired in dispose) and paintLanding already no-ops once the landing tears down, so this needed no new timer to arm or leak, and it doesn't touch the turn monitor's cadence at all. The alternative — a separate timer armed from createLandingAbove and stopped in the landing teardown — was rejected: it would duplicate the monitor's own cadence-management responsibility for no real gain, since the FRAME event already has the right lifetime. Added a real-mount-path regression test in landing.test.ts that goes through createAppShell and lets the renderer's FRAME event drive the repaint, unlike every existing test in that file, which drives the mark by calling paintLanding directly with a hand-picked clock. That gap is exactly why this shipped broken and nobody caught it. This supersedes PR #380, which added the snow-drawing code but never made it reachable; that PR should stay open until this one is reviewed and can then be closed in favor of this one.
1 parent 093284f commit a5bc8f9

4 files changed

Lines changed: 115 additions & 24 deletions

File tree

src/tui-opentui/landing.test.ts

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
import { LOCKUP_WORDMARK } from "./lockup"
3939
import pkg from "../../package.json" with { type: "json" }
4040
import { MARK_LARGE, MARK_MID, MARK_SMALL } from "./mark-shape"
41+
import { SNOW_CHAR } from "./mark-anim"
4142
import { UI } from "./theme"
4243

4344
const SIZE = { width: 80, height: 24 } as const
@@ -229,11 +230,14 @@ describe("landing screen", () => {
229230
try {
230231
await settle(h)
231232
const still = markRows(h).join("\n")
233+
const stripSnow = (text: string) => text.replaceAll(SNOW_CHAR, " ")
232234

233-
// Idle re-entry holds the filled frame however far the clock moves.
235+
// Idle re-entry holds the mountain's filled frame however far the
236+
// clock moves — but the snow drifting over it is not still, since the
237+
// idle landing screen is exactly where it needs to animate.
234238
paintLanding(shell, 1_700, false)
235239
await settle(h)
236-
expect(markRows(h).join("\n")).toBe(still)
240+
expect(stripSnow(markRows(h).join("\n"))).toBe(stripSnow(still))
237241

238242
const frames = new Set<string>()
239243
for (const nowMs of [0, 500, 1_100, 1_900, 2_600, 3_400]) {
@@ -248,6 +252,49 @@ describe("landing screen", () => {
248252
}, SIZE)
249253
})
250254

255+
test(
256+
"an idle mount keeps the snow drifting on the renderer's own frame event",
257+
async () => {
258+
// Regression for CL-5737: every other test in this file drives the mark
259+
// by calling `paintLanding` directly with a hand-picked clock. That is
260+
// exactly why the landing snow shipped completely unreachable — none of
261+
// those tests go through the real driver a running session actually
262+
// uses. This one mounts the shell for real and lets the renderer's own
263+
// FRAME event (wired in `createAppShell`, unwired in `shell.dispose`)
264+
// drive the repaint, the same as production, with no direct calls to
265+
// `paintLanding` or `renderMark`.
266+
await withTestRenderer(async (h) => {
267+
const shell = createAppShell(h.renderer, {
268+
terminal: { columns: 80, rows: 24 },
269+
wireKeys: false,
270+
run: "idle",
271+
})
272+
try {
273+
await settle(h)
274+
const before = markRows(h).join("\n")
275+
276+
// Real elapsed time, not an injected clock: the production driver
277+
// reads the wall clock, so this is the only way to exercise it.
278+
// At the fall speed in mark-anim.ts a few real seconds is enough
279+
// for at least one active flake column to cross a row boundary.
280+
const start = Date.now()
281+
while (Date.now() - start < 5_000) {
282+
await h.renderOnce()
283+
}
284+
const after = markRows(h).join("\n")
285+
286+
expect(after).not.toBe(before)
287+
288+
const stripSnow = (text: string) => text.replaceAll(SNOW_CHAR, " ")
289+
expect(stripSnow(after)).toBe(stripSnow(before))
290+
} finally {
291+
shell.dispose()
292+
}
293+
}, SIZE)
294+
},
295+
15_000,
296+
)
297+
251298
test("a starter key fills the prompt; a typed prompt keeps its digits", async () => {
252299
await withTestRenderer(async (h) => {
253300
const shell = createAppShell(h.renderer, {

src/tui-opentui/mark-anim.test.ts

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,19 +106,38 @@ describe("renderMark", () => {
106106
const grid = renderMark({ nowMs: 0, still: true, grid: MARK_LARGE })
107107
grid.forEach((row, y) => {
108108
row.forEach((cell, x) => {
109-
// Still mode has no snow — only space or mountain blocks.
110-
expect(` ${MOUNTAIN_CHARS}`).toContain(cell.char)
109+
// Still mode freezes the mountain, but snow still drifts over the sky.
110+
expect(` ${MOUNTAIN_CHARS}${SNOW_CHAR}`).toContain(cell.char)
111111
if ((MARK_LARGE.coverage[y]?.[x] ?? 0) === 1) expect(cell.char).toBe("█")
112112
})
113113
})
114114
})
115115

116-
test("the still frame is clock-independent and has no snow", () => {
117-
const a = markText(renderMark({ nowMs: 0, still: true }))
118-
const b = markText(renderMark({ nowMs: 987_654, still: true }))
116+
test("still holds the mountain fixed while the clock advances", () => {
117+
// Snow moves with the clock even in still mode (the idle landing screen),
118+
// so isolate the mountain by stripping snow before comparing.
119+
const stripSnow = (text: string) => text.replaceAll(SNOW_CHAR, " ")
120+
const a = stripSnow(markText(renderMark({ nowMs: 0, still: true })))
121+
const b = stripSnow(markText(renderMark({ nowMs: 987_654, still: true })))
119122
expect(b).toBe(a)
120123
expect(a.replace(/[\s\n]/g, "").length).toBeGreaterThan(0)
121-
expect(a.includes(SNOW_CHAR)).toBe(false)
124+
})
125+
126+
test("snow keeps drifting in still mode while the mountain stays frozen", () => {
127+
const times = [0, 1500, 3000, 4500, 6000, 7500]
128+
const snowSets = times.map((nowMs) => {
129+
const grid = renderMark({ nowMs, still: true, grid: MARK_LARGE })
130+
const snow: string[] = []
131+
grid.forEach((row, y) => {
132+
row.forEach((cell, x) => {
133+
if (isSnow(cell.char)) snow.push(`${y},${x}`)
134+
})
135+
})
136+
return snow.join("|")
137+
})
138+
const withSnow = snowSets.filter((s) => s.length > 0)
139+
expect(withSnow.length).toBeGreaterThan(1)
140+
expect(new Set(withSnow).size).toBeGreaterThan(1)
122141
})
123142

124143
test("the animated frame advances with the injected clock", () => {
@@ -203,11 +222,12 @@ describe("renderMark", () => {
203222
expect(mountains).toBeGreaterThan(flakes)
204223
})
205224

206-
test("still mode freezes the mark with no snow motion", () => {
225+
test("still mode freezes the mountain but not the snow", () => {
207226
const a = renderMark({ nowMs: 0, still: true, grid: MARK_SMALL })
208227
const b = renderMark({ nowMs: 50_000, still: true, grid: MARK_SMALL })
209-
expect(markText(b)).toBe(markText(a))
210-
expect(a.flat().some((cell) => isSnow(cell.char))).toBe(false)
228+
const mountainText = (grid: typeof a) =>
229+
grid.map((row) => row.map((cell) => (isMountain(cell.char) ? cell.char : " ")).join("")).join("\n")
230+
expect(mountainText(b)).toBe(mountainText(a))
211231
})
212232

213233
test("snow drops out during the fade-out phase, matching the mark", () => {

src/tui-opentui/mark-anim.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@
99
*
1010
* Over the sky (zero-coverage cells) a sparse field of pixel snow falls on the
1111
* same injected clock. Density and speed stay low so the ridgeline keeps its
12-
* silhouette; `still` (idle or reduced motion) freezes the mark and drops the
13-
* snow entirely. Mountain cells always win over flakes.
12+
* silhouette; `still` (idle or reduced motion) freezes the mountain's own
13+
* draw/fill/fade timeline to its fully-filled frame, but snow keeps drifting —
14+
* the landing screen is idle by definition, so tying snow to the same flag
15+
* that freezes the mountain would mean it never falls. Mountain cells always
16+
* win over flakes.
1417
*
1518
* Everything here is pure and clock-injected: `nowMs` is the only time source,
1619
* so the caller's existing 250 ms status tick drives the animation and tests
@@ -94,7 +97,11 @@ export type MarkCell = {
9497

9598
export type MarkInput = {
9699
readonly nowMs: number
97-
/** Hold the mark still: idle session, or reduced motion. */
100+
/**
101+
* Hold the mountain's draw/fill/fade timeline on its fully-filled frame:
102+
* idle session, or reduced motion. Snow is not gated by this — see
103+
* `snowOn` in `renderMark`.
104+
*/
98105
readonly still: boolean
99106
/** Which baked rasterization to composite. Defaults to the compact grid. */
100107
readonly grid?: MarkGrid
@@ -149,9 +156,10 @@ export function renderMark(input: MarkInput): readonly (readonly MarkCell[])[] {
149156
const { drawProg, fillProg, alpha } = markFrame(seconds, input.still)
150157
const revealed = drawProg * shape.cols
151158
const fillLine = shape.rows * (1 - fillProg)
152-
// Fade out drops the snow too so the decoration doesn't outlast the mark
153-
// it drifts over.
154-
const snowOn = !input.still && alpha === 1
159+
// Independent of `still`: the mountain can be frozen full while snow still
160+
// drifts (the idle landing screen). Fade out drops the snow too so the
161+
// decoration doesn't outlast the mark it drifts over.
162+
const snowOn = alpha === 1
155163

156164
const grid: MarkCell[][] = []
157165
for (let row = 0; row < shape.rows; row++) {

src/tui-opentui/shell.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2556,13 +2556,15 @@ function clearLandingMark(shell: AppShell): void {
25562556
}
25572557

25582558
/**
2559-
* Repaint the landing mark for `nowMs`. `animating` runs the draw/fill/fade
2560-
* timeline; anything else holds the filled frame. No-op once the landing is
2561-
* gone, so the caller can drive it unconditionally.
2559+
* Repaint the landing mark for `nowMs`. `animating` runs the mountain's
2560+
* draw/fill/fade timeline; anything else holds its filled frame. No-op once
2561+
* the landing is gone, so the caller can drive it unconditionally.
25622562
*
2563-
* A still mark draws the same frame for every clock value, so repainting it
2564-
* only dirties renderables; the guard mirrors `setLockupFrame` and lets an idle
2565-
* session sit without touching the paint tree.
2563+
* Always repaints while the landing is up, even when `animating` is false:
2564+
* the landing is idle by definition (no turn processing), and snow still
2565+
* needs to drift across a frozen mountain. `paintLandingMark`/`renderMark`
2566+
* only touch the rows whose content actually changed, so the unchanging
2567+
* mountain rows cost nothing extra here.
25662568
*/
25672569
export function paintLanding(
25682570
shell: AppShell,
@@ -2572,7 +2574,6 @@ export function paintLanding(
25722574
const bag = internals.get(shell)
25732575
const landing = bag?.landing
25742576
if (bag === undefined || landing === null || landing === undefined) return
2575-
if (!animating && !bag.landingAnimating) return
25762577
bag.landingAnimating = animating
25772578
bag.landingNowMs = nowMs
25782579
paintLandingMark(landing.above, nowMs, !animating)
@@ -5599,6 +5600,21 @@ export function createAppShell(
55995600
// starves that pass of room to lay the row out in.
56005601
syncTranscriptSpacer(shell)
56015602
syncNoticeAfterLayout(shell)
5603+
// The landing's snow needs a frame source that keeps running while the
5604+
// turn monitor is deliberately quiet (idle, no session yet). The renderer
5605+
// FRAME event is already scoped to shell lifetime (wired here, unwired in
5606+
// `dispose` below) and `paintLanding` no-ops once the landing tears down,
5607+
// so riding it costs no extra timer to arm or leak.
5608+
//
5609+
// Only re-enters while idle (`landingAnimating` already false): while a
5610+
// turn is processing, `paintPhaseAt` in runtime-bridge.ts drives the
5611+
// mountain's own draw/fill/fade loop off the turn monitor's clock, and
5612+
// this re-entry must not stomp that with an unrelated real-clock value
5613+
// every render pass.
5614+
const landingBag = internals.get(shell)
5615+
if (landingBag?.landing != null && !landingBag.landingAnimating) {
5616+
paintLanding(shell, Date.now(), false)
5617+
}
56025618
}
56035619

56045620
const onResize = (width: number, height: number): void => {

0 commit comments

Comments
 (0)