From 45f5d236c13a554ddaa2079fca31f6129e243aee Mon Sep 17 00:00:00 2001 From: saopig1 <4x7sw862st@gmail.com> Date: Tue, 30 Jun 2026 16:17:53 +0800 Subject: [PATCH] fix(web): tick player time every frame and keep lyrics in sync (#107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `store.elapsed` is a Pinia getter (a cached Vue computed) that interpolates with `Date.now()`. Because `Date.now()` is not a reactive dependency, the computed only re-ran on WebSocket pushes / the 3s server poll, so the bottom progress bar jumped ~3s at a time and lyric highlighting lagged ~half a line — even though the consumers read it from a 60fps requestAnimationFrame loop. Add a pure `interpolateElapsed()` helper and a non-cached `liveElapsed()` store action. The per-frame consumers now call `liveElapsed()` so the value advances every frame instead of returning a frozen cache: - web/src/components/Player.vue (desktop progress bar, rAF) - web/src/App.vue (mobile progress bar, rAF) - web/src/views/Lyrics.vue (lyric highlight, 500ms interval) pause() now freezes at the live value rather than a possibly-stale cached one. The `elapsed` getter is refactored onto the same helper (behaviour unchanged). Adds web/src/stores/elapsed.test.ts covering the time-advancing interpolation, paused freeze, no-anchor, and duration-clamp cases. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/App.vue | 4 ++- web/src/components/Player.vue | 5 +-- web/src/stores/elapsed.test.ts | 44 ++++++++++++++++++++++++ web/src/stores/player.ts | 61 ++++++++++++++++++++++++++++++---- web/src/views/Lyrics.vue | 4 ++- 5 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 web/src/stores/elapsed.test.ts diff --git a/web/src/App.vue b/web/src/App.vue index e90c4dc..8a1e0cb 100644 --- a/web/src/App.vue +++ b/web/src/App.vue @@ -118,8 +118,10 @@ let mobileRaf: number | null = null; function updateMobileProgress() { const duration = currentSong.value?.duration ?? 0; + // liveElapsed() recomputes each frame; the cached `elapsed` getter would + // leave the mobile bar frozen between server pushes (#107). mobileProgressPct.value = duration > 0 - ? Math.min((playerStore.elapsed / duration) * 100, 100) + ? Math.min((playerStore.liveElapsed() / duration) * 100, 100) : 0; mobileRaf = requestAnimationFrame(updateMobileProgress); } diff --git a/web/src/components/Player.vue b/web/src/components/Player.vue index 99ad1b3..138013c 100644 --- a/web/src/components/Player.vue +++ b/web/src/components/Player.vue @@ -135,8 +135,9 @@ function formatTime(seconds: number): string { } function updateProgress() { - // Use store.elapsed which interpolates from server ground truth - currentElapsed.value = store.elapsed; + // liveElapsed() (an action, not the cached `elapsed` getter) re-interpolates + // from the server anchor on every frame so the clock ticks each second (#107). + currentElapsed.value = store.liveElapsed(); const duration = currentSong.value?.duration ?? 0; progressPercent.value = duration > 0 diff --git a/web/src/stores/elapsed.test.ts b/web/src/stores/elapsed.test.ts new file mode 100644 index 0000000..8a1c20e --- /dev/null +++ b/web/src/stores/elapsed.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { interpolateElapsed, type TimingState } from "./player.js"; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function timing(partial: Partial): TimingState { + return { serverElapsed: 0, serverSyncTime: 0, wasPlaying: false, ...partial }; +} + +describe("interpolateElapsed", () => { + it("returns serverElapsed before playback has a sync anchor", () => { + expect(interpolateElapsed(timing({ serverElapsed: 12, wasPlaying: false }), false, Infinity)).toBe(12); + // wasPlaying but no sync time yet + expect(interpolateElapsed(timing({ serverElapsed: 5, wasPlaying: true, serverSyncTime: 0 }), false, Infinity)).toBe(5); + }); + + it("advances with wall-clock time while playing (regression: must not be frozen)", () => { + const spy = vi.spyOn(Date, "now"); + const t = timing({ serverElapsed: 30, serverSyncTime: 10_000, wasPlaying: true }); + + spy.mockReturnValue(10_000); + expect(interpolateElapsed(t, false, Infinity)).toBeCloseTo(30, 5); + + spy.mockReturnValue(11_000); // +1s + expect(interpolateElapsed(t, false, Infinity)).toBeCloseTo(31, 5); + + spy.mockReturnValue(13_500); // +3.5s — distinct from the 1s reading + expect(interpolateElapsed(t, false, Infinity)).toBeCloseTo(33.5, 5); + }); + + it("freezes at serverElapsed while paused", () => { + vi.spyOn(Date, "now").mockReturnValue(99_000); + const t = timing({ serverElapsed: 42, serverSyncTime: 10_000, wasPlaying: true }); + expect(interpolateElapsed(t, true, Infinity)).toBe(42); + }); + + it("clamps to maxDuration", () => { + vi.spyOn(Date, "now").mockReturnValue(1_000_000); + const t = timing({ serverElapsed: 100, serverSyncTime: 1_000, wasPlaying: true }); + expect(interpolateElapsed(t, false, 180)).toBe(180); + }); +}); diff --git a/web/src/stores/player.ts b/web/src/stores/player.ts index 4dfb979..3773096 100644 --- a/web/src/stores/player.ts +++ b/web/src/stores/player.ts @@ -47,7 +47,7 @@ export interface FavoritePlaylist { createdAt: string; } -interface TimingState { +export interface TimingState { serverElapsed: number; serverSyncTime: number; wasPlaying: boolean; @@ -59,6 +59,33 @@ function defaultTiming(): TimingState { return { serverElapsed: 0, serverSyncTime: 0, wasPlaying: false }; } +/** + * Interpolate the live elapsed seconds from the last server anchor. + * + * This is a PURE function (its only time source is `Date.now()`), deliberately + * kept OUT of the Pinia getter so it can be called fresh every animation frame. + * The `elapsed` getter is a Vue `computed` and caches its result until a + * REACTIVE dependency changes — but `Date.now()` is not reactive, so a getter + * only re-runs on a WebSocket push / server poll (every few seconds). Reading + * the getter from a requestAnimationFrame loop therefore returns a frozen value + * and the clock appears to jump ~3s at a time (issue #107). Per-frame consumers + * must call this helper (via the `liveElapsed` action) instead. + */ +export function interpolateElapsed( + timing: TimingState, + isPaused: boolean, + maxDuration: number, +): number { + // No live anchor yet, or paused: report the frozen server position. + if (!timing.wasPlaying || timing.serverSyncTime === 0 || isPaused) { + return Math.min(timing.serverElapsed, maxDuration); + } + return Math.min( + timing.serverElapsed + (Date.now() - timing.serverSyncTime) / 1000, + maxDuration, + ); +} + export const usePlayerStore = defineStore('player', { state: () => ({ bots: [] as BotStatus[], @@ -111,15 +138,19 @@ export const usePlayerStore = defineStore('player', { if (!botId) return []; return this.queues[botId] ?? []; }, - /** Interpolated elapsed for the active bot */ + /** + * Interpolated elapsed for the active bot. NOTE: as a Pinia getter this is + * a Vue `computed` and is CACHED — it only re-runs when a reactive + * dependency changes, so it does NOT tick every second on its own. Use it + * for one-off reactive reads; per-frame consumers (progress bar, lyrics) + * must call the `liveElapsed` action so the clock advances smoothly (#107). + */ elapsed(): number { const botId = this.activeBotId ?? this.bots[0]?.id; if (!botId || !this.activeBot?.currentSong) return 0; const timing = this.timings[botId] ?? defaultTiming(); const maxDuration = this.activeBot.currentSong.duration || Infinity; - if (!timing.wasPlaying || timing.serverSyncTime === 0) return Math.min(timing.serverElapsed, maxDuration); - if (this.isPaused) return Math.min(timing.serverElapsed, maxDuration); - return Math.min(timing.serverElapsed + (Date.now() - timing.serverSyncTime) / 1000, maxDuration); + return interpolateElapsed(timing, this.isPaused, maxDuration); }, /** Sources that are currently logged in. Order: netease before qq. */ availableSources(): Source[] { @@ -131,6 +162,21 @@ export const usePlayerStore = defineStore('player', { }, actions: { + /** + * Live elapsed seconds for the active bot, recomputed on every call. Unlike + * the `elapsed` getter (a cached computed), this is an action, so it is NOT + * memoised — call it from requestAnimationFrame / interval loops so the + * progress bar and lyrics advance every frame instead of jumping on each + * server push (#107). + */ + liveElapsed(): number { + const botId = this.activeBotId ?? this.bots[0]?.id; + if (!botId || !this.activeBot?.currentSong) return 0; + const timing = this.timings[botId] ?? defaultTiming(); + const maxDuration = this.activeBot.currentSong.duration || Infinity; + return interpolateElapsed(timing, this.isPaused, maxDuration); + }, + _getTiming(botId: string): TimingState { if (!this.timings[botId]) { this.timings[botId] = defaultTiming(); @@ -403,9 +449,10 @@ export const usePlayerStore = defineStore('player', { async pause() { if (!this.activeBotId) return; - // Freeze elapsed at current interpolated value + // Freeze elapsed at the current LIVE interpolated value. Using the cached + // `elapsed` getter here could snapshot a value up to a few seconds stale. this._setTiming(this.activeBotId, { - serverElapsed: this.elapsed, + serverElapsed: this.liveElapsed(), wasPlaying: false, }); await axios.post(`/api/player/${this.activeBotId}/pause`); diff --git a/web/src/views/Lyrics.vue b/web/src/views/Lyrics.vue index a977c34..ab81e13 100644 --- a/web/src/views/Lyrics.vue +++ b/web/src/views/Lyrics.vue @@ -136,7 +136,9 @@ function scrollToActiveLine(idx: number) { function syncLyrics() { if (!store.isPlaying || lines.value.length === 0) return; - const elapsed = store.elapsed; + // liveElapsed() (action) is recomputed now; the cached `elapsed` getter only + // refreshed on server pushes, leaving highlights ~half a line behind (#107). + const elapsed = store.liveElapsed(); const idx = findActiveLine(elapsed); // Only update when the active line actually changes if (idx !== activeLine.value && idx >= 0) {