From 7c51cadef74887a89d1714f0c6eaf01bc5dda3c7 Mon Sep 17 00:00:00 2001 From: Kamil Krzywda Date: Fri, 18 Sep 2026 12:10:40 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20v0.12.1=20=E2=80=94=20ISS-001=20time-sca?= =?UTF-8?q?le=20slider=20no=20longer=20snaps=20back=20to=20the=20stale=20v?= =?UTF-8?q?alue=20on=20speed=20change=20(optimistic=20scale=20kept=20until?= =?UTF-8?q?=20a=20snapshot=20confirms=20it);=20sim-control=20commit=20mach?= =?UTF-8?q?inery=20hardened=20(synchronous=20busy=20ref,=20in-flight=20que?= =?UTF-8?q?ue=20flush=20loop,=20sent/queued/failed=20outcomes=20so=20queue?= =?UTF-8?q?d=20commits=20never=20drop=20the=20display)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 7 ++++ components/LeftPane.tsx | 79 ++++++++++++++++++++++++++---------- components/left-pane.test.ts | 24 +++++++++++ e2e/panes.spec.ts | 45 +++++++++++++++++++- package.json | 2 +- 5 files changed, 132 insertions(+), 25 deletions(-) create mode 100644 components/left-pane.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ebc53fe..03ddeee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## v0.12.1 — 2026-09-18 11:13 +- **Bugfix (ISS-001): time-scale slider pointer jumped back to the old value, then forward again, on every speed change.** Root cause in `components/LeftPane.tsx`: `commitNow` cleared the optimistic `pendingScale` *before* the server confirmed it — so for one poll cycle the displayed scale fell back to the stale snapshot's value (pointer snapped back), then jumped forward when the confirming snapshot landed. Fix: keep the chosen value on screen until a rendered snapshot actually CONFIRMS it (`serverTimeScale === pendingScale`) and no commit is still in flight; drop it immediately only if the POST genuinely fails, so the display falls back to server truth. The confirmation rule is extracted as a pure exported `pendingScaleConfirmed(pending, server)` for unit testing (same pattern as RightPane's picker rules); the busy guard prevents an old snapshot from coincidentally "confirming" a quick drag back to the previous scale before its own commit fires +- **Hardened the sim-control commit machinery while fixing it** (`components/LeftPane.tsx`): the in-flight gate moved from React state to a synchronous ref (closures can no longer see a stale `busy` and re-queue instead of send, which could silently drop a commit), the single queue slot is now flushed by a loop inside the in-flight call (last intent always wins, multiple queued commits all sent), and `applySimControls` reports `'sent' | 'queued' | 'failed'` so an optimistic value is only dropped on genuine failure — never when a commit merely got queued behind another +- Tests: unit — new `components/left-pane.test.ts` (no pending → false; stale-scale window stays false; confirms exactly when server carries the pending value). e2e — new `panes.spec.ts` test commits 64× via keyboard and samples the displayed value ~30× across the whole commit→confirm window, asserting it never snaps back to a stale scale (the old bug), then verifies the server confirms +- Verified: unit suite 228 → 231 green; `tsc --noEmit` clean; e2e — full suite green on the initial v0.12.1 deploy against the live compose build (incl. the new no-snap-back test); after the commit-machinery restructure, the time-control specs (`panes` + `sim`) verified green in 3 consecutive rounds. Full-suite re-run deferred: the live instance is under active human play and the destructive self-heal specs must not touch it + + ## v0.12.0 — 2026-09-18 09:17 - **Gameplay pivot (user-driven): contracts → packages** — the job model is rebuilt end-to-end around cargo *packages* and manual flight, replacing the old accept→dispatch flow in one coherent increment. A **package** is a cargo unit origin→dest with a reward € and a game-time deadline; planes carry up to `capacity` packages (ATR 72-600 = **6 slots**, new `capacity` field on catalog entries exposed via `/api/state`). The loop: **Load** an open package onto a grounded plane at its origin → **Fly** the plane to any known airport (the destination of a loaded package, or pure repositioning) → on arrival every in-deadline package bound for that airport is **delivered** (cash += reward + rep per package); packages destined elsewhere stay loaded for a later leg - **New player actions** (`lib/game/actions.ts`, all under the sim lock via `engine.runAction()` — tick first, then validate+mutate): `loadPackage` (open → loaded; plane must be GROUNDED at the origin with a free slot — expired packages still occupy theirs), `flyPlane` (grounded plane → any dataset ICAO; 400 unknown airport, 409 flying / insufficient fuel where burnPerKm × great-circle km > fuel), `abandonPackage` (loaded or already-expired package while its plane is GROUNDED — 409 if in flight). Removed: `acceptContract`, `dispatchPlane` diff --git a/components/LeftPane.tsx b/components/LeftPane.tsx index 460fe8a..d76d046 100644 --- a/components/LeftPane.tsx +++ b/components/LeftPane.tsx @@ -34,44 +34,65 @@ function loadoutFor(state: StateSnapshot, p: PlaneState): string { return `${used}/${capacity} pkgs`; } +// ISS-001 — an optimistic slider value is cleared exactly when a rendered snapshot CONFIRMS it +// (server scale === pending), never earlier: clearing at commit time would snap the pointer back +// to the stale scale for one poll cycle, then jump forward again on the confirming snapshot. +export function pendingScaleConfirmed(pendingScale: number | null, serverTimeScale: number): boolean { + return pendingScale !== null && serverTimeScale === pendingScale; +} + export default function LeftPane({ state, loading, error, onRefresh }: LeftPaneProps) { const [busy, setBusy] = useState(false); // POST /api/sim, then refetch the full snapshot immediately so the pane reflects the new // controls without waiting for the next poll tick. On failure keep the stale UI — the // running poll retries on its own and surfaces the error state. A commit that lands while - // another POST is in flight is held in a single slot and flushed right after, so the last - // user intent always reaches the server (the server's [1,64] clamp stays as backstop). + // another POST is in flight is held in a single slot (last intent wins) and flushed by the + // in-flight call, so every user intent reaches the server (the [1,64] clamp stays as backstop). + // The gate is a REF, not React state: closures never see a stale "busy" value, which used to + // let a flush re-queue instead of send and silently drop a commit. const queuedRef = useRef<{ timeScale?: number; paused?: boolean } | null>(null); + const busyRef = useRef(false); + type SimControlsOutcome = 'sent' | 'queued' | 'failed'; + + // 'sent': this body reached the server (a confirming snapshot follows via onRefresh). + // 'queued': an in-flight call holds it — its flush will send it. + // 'failed': the POST for this body never succeeded; callers drop optimistic values only then. const applySimControls = useCallback( - async (body: { timeScale?: number; paused?: boolean }) => { - if (busy) { - queuedRef.current = body; - return; + async (body: { timeScale?: number; paused?: boolean }): Promise => { + if (busyRef.current) { + queuedRef.current = body; // last intent wins — the in-flight call's flush sends it + return 'queued'; } + busyRef.current = true; setBusy(true); + let sentOwn = false; try { - const res = await fetch('/api/sim', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(body), - cache: 'no-store', - }); - if (!res.ok) throw new Error(`POST /api/sim → HTTP ${res.status}`); - onRefresh(); + let current: { timeScale?: number; paused?: boolean } | null = body; + while (current !== null) { + const res = await fetch('/api/sim', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(current), + cache: 'no-store', + }); + if (!res.ok) throw new Error(`POST /api/sim → HTTP ${res.status}`); + onRefresh(); + if (current === body) sentOwn = true; // object identity — queued bodies are fresh literals + current = queuedRef.current; // pick up anything that arrived while we were in flight + queuedRef.current = null; + } } catch (err) { console.error('failed to apply sim controls', err); + queuedRef.current = null; // don't replay a stale intent after a failed round-trip } finally { + busyRef.current = false; setBusy(false); - const queued = queuedRef.current; - if (queued !== null) { - queuedRef.current = null; - void applySimControls(queued); - } } + return sentOwn ? 'sent' : 'failed'; }, - [busy, onRefresh], + [onRefresh], ); const paused = state?.simState.paused ?? false; @@ -97,10 +118,24 @@ export default function LeftPane({ state, loading, error, onRefresh }: LeftPaneP } const s = pendingRef.current; if (s === null) return; - setPending(null); - void applySimControls({ timeScale: s }); + // ISS-001: keep the chosen value on screen until a snapshot CONFIRMS it — clearing at + // commit time would snap the slider back to the stale scale for one poll cycle, then jump + // forward again when the confirming snapshot lands. 'queued' also keeps it (the in-flight + // call flushes it); only a genuine failure drops it so the display falls back to truth. + void applySimControls({ timeScale: s }).then((outcome) => { + if (outcome === 'failed') setPending(null); + }); }, [applySimControls, setPending]); + // Confirmation: once a rendered snapshot carries the pending scale AND no commit is in + // flight, the optimistic value has been superseded by server truth — clear it (display is + // unchanged either way). The busy guard matters: while a commit POST is still out, an OLD + // snapshot that happens to match the pending value is coincidence, not confirmation (e.g. a + // quick drag back to the previous scale must not be swallowed before its own commit fires). + useEffect(() => { + if (!busy && state && pendingScaleConfirmed(pendingScale, state.simState.timeScale)) setPending(null); + }, [state, pendingScale, busy, setPending]); + const onSliderInput = useCallback( (e: ChangeEvent) => { setPending(sliderToScale(Number(e.target.value))); diff --git a/components/left-pane.test.ts b/components/left-pane.test.ts new file mode 100644 index 0000000..c6b0b03 --- /dev/null +++ b/components/left-pane.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest'; +import { pendingScaleConfirmed } from './LeftPane'; + +// ISS-001 — the optimistic slider value is cleared exactly when a rendered snapshot confirms +// it (server scale === pending), never earlier. Clearing at commit time would snap the pointer +// back to the stale scale for one poll cycle, then jump forward again on the confirming snapshot. + +describe('pendingScaleConfirmed', () => { + it('is false while there is no pending value', () => { + expect(pendingScaleConfirmed(null, 1)).toBe(false); + expect(pendingScaleConfirmed(null, 64)).toBe(false); + }); + + it('stays false while the server still reports the stale scale (the ISS-001 window)', () => { + // User committed 64×; the last rendered snapshot is still at 1× → must NOT clear yet. + expect(pendingScaleConfirmed(64, 1)).toBe(false); + expect(pendingScaleConfirmed(32, 8)).toBe(false); + }); + + it('becomes true exactly when a snapshot carries the pending scale', () => { + expect(pendingScaleConfirmed(64, 64)).toBe(true); + expect(pendingScaleConfirmed(1, 1)).toBe(true); // committing the current value confirms immediately + }); +}); diff --git a/e2e/panes.spec.ts b/e2e/panes.spec.ts index 76f4d5e..1779173 100644 --- a/e2e/panes.spec.ts +++ b/e2e/panes.spec.ts @@ -44,11 +44,21 @@ test('left pane renders wallet, reputation and the starter fleet at Frankfurt', await expect(page.getByTestId('reputation')).toBeVisible(); expect(Number((await page.getByTestId('reputation').textContent()) ?? 'NaN')).toBeGreaterThanOrEqual(0); - // Fleet: the starter ATR 72-600 at Frankfurt (EDDF). + // Fleet: the starter ATR 72-600 — location asserted against /api/state truth, because a + // human player (or an earlier spec) may have moved it anywhere by now. const row = page.getByTestId('fleet-list').locator('.plane-row').first(); await expect(row, 'starter plane row appears once the first snapshot lands').toBeVisible({ timeout: 15_000 }); await expect(row).toContainText('ATR 72-600'); - await expect(row).toContainText('Frankfurt'); + const truth = (await getState(page)).planes.find((p) => p.id === 'plane-starter') as unknown as { + status: string; + locationName: string; + destName: string | null; + }; + if (truth.status === 'flying' && truth.destName) { + await expect(row).toContainText(`${truth.locationName} → ${truth.destName}`); // in-flight leg + } else { + await expect(row).toContainText(truth.locationName); + } // v0.12.0 loadout: "n/capacity pkgs" (capacity from the snapshot catalog — ATR 72-600 = 6). const loadout = page.getByTestId('plane-loadout-plane-starter'); @@ -94,6 +104,37 @@ test('time controls: pause freezes the sim and the power-of-two slider sets time expect(after.simState.paused).toBe(false); }); +test('slider commit keeps the chosen scale on screen until confirmed — no snap-back (ISS-001)', async ({ page }) => { + test.setTimeout(60_000); + await page.goto('/'); + const slider = page.getByTestId('time-scale-slider'); + await expect(slider).toBeVisible({ timeout: 15_000 }); + + // Commit to max via keyboard (input + change → immediate commitNow POST)… + await slider.focus(); + await page.keyboard.press('End'); + + // …and sample the displayed value rapidly across the whole commit→confirm window. With the + // ISS-001 fix the optimistic value stays until a snapshot confirms it, so EVERY sample must + // be '64×' — never snapping back to the previous scale for one poll cycle (the old bug). + const seen: string[] = []; + for (let i = 0; i < 30; i++) { + seen.push(((await page.getByTestId('time-scale-value').textContent()) ?? '').trim()); + await new Promise((r) => setTimeout(r, 25)); + } + expect( + seen.every((v) => v === '64×'), + `displayed scale must stay 64× during the commit window (saw: ${[...new Set(seen)].join(', ')})`, + ).toBe(true); + + // …and the server confirms it. + await expect.poll(async () => (await getState(page)).simState.timeScale, { timeout: 10_000 }).toBe(64); + + // Restore to min for later tests (afterEach restores scale anyway). + await page.keyboard.press('Home'); + await expect.poll(async () => (await getState(page)).simState.timeScale, { timeout: 10_000 }).toBe(1); +}); + test('right pane placeholder exists with the Jobs & Market heading', async ({ page }) => { await page.goto('/'); const right = page.getByTestId('right-pane'); diff --git a/package.json b/package.json index c138e11..c1c713f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "flight-ops", - "version": "0.12.0", + "version": "0.12.1", "private": true, "scripts": { "dev": "next dev -p 3000",