From 115c96a3f0a948656b4c774a8f7ae0d6ed174d73 Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Wed, 1 Jul 2026 04:18:29 -0500 Subject: [PATCH] =?UTF-8?q?feat(tuner):=20gate=20playback=20until=20you've?= =?UTF-8?q?=20tuned=20=E2=80=94=20hold=20autoplay=20+=20no-trap=20Skip/Bac?= =?UTF-8?q?k/Esc=20(working-tuning=20PR=204)=20(#666)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(tuner): gate playback until you've tuned — hold autoplay + no-trap Skip/Back/Esc (working-tuning PR 4) When the opt-in auto-open fires because a song needs a different tuning, playback now WAITS behind the tuner instead of starting underneath it — the "tune before you play" model. Built on a new generic core hook window.feedBack.holdAutoplay() (mirrors holdAutoExit): the tuner claims the hold synchronously on song:loading (beating the song:ready autostart) and releases it — or a 12s fail-open backstop does — so a wedged plugin can never strand a song. Generation-guarded; manual Play always wins. No one-way trap: - Skip = "I've tuned" -> plays and records the song's tuning as the instrument's current working tuning (the explicit write-point PR 3 left as 'assumed'). - Back to library / Esc -> leave the song, record nothing (reuses requestExitSong; Esc is the existing player shortcut). - The in-panel x is dropped for an auto-open — Skip/Back/Esc are the dismiss surface. This also keeps the write honest: Skip is the only on-player dismiss that records, so leaving never falsely records a tuning. Stacked on #660 (working-tuning PR 3). Core app.js gains only the generic hook (a test asserts it never references the tuner's internals); shell-agnostic. Needs a desktop smoke-test that the tuner mic doesn't contend with note_detect's scoring input under ASIO/exclusive mode (per the design charrette). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF * fix(tuner): backstop can't cut off tuning + gate race/token hardening (PR #666 review) Review fixes for the autoplay gate: - The 12s fail-open backstop could start playback UNDER a legitimately-open tuner (a slow / mic-verify retune > 12s). holdAutoplay()'s release now carries a .settle() that cancels the backstop; the tuner calls it once the tuner is confirmed open (_gateClaimed), so the hold becomes deliberate and only a dismiss / song switch releases it. (Fail-open still covers "claimed but wedged before deciding".) - The async song:ready handler could release a NEWER song's gate after its await (global _gateClaimed, no guard). It now snapshots _autoOpenGeneration and bails if a newer song took over. - holdAutoplay guarded by song generation, not per-hold — a stale release from an earlier hold could clear a later one. Each hold now mints a unique token that release()/settle() must match. Tests: source-level assertions for the token, settle(), the settle-on-open call, and the song:ready gen-guard. 45 tuner+speed tests green. Codex-reviewed. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 Co-authored-by: byrongamatos --- CHANGELOG.md | 1 + plugins/tuner/screen.js | 39 +++++++++++++- plugins/tuner/utils/ui.js | 19 ++++++- static/app.js | 92 +++++++++++++++++++++++++------- tests/js/speed_reset.test.js | 1 + tests/js/tuner_auto_open.test.js | 66 +++++++++++++++++++++++ 6 files changed, 197 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5604da7..f7d1651 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Tuner auto-open is now opt-in and persists instead of flashing open-then-shut.** When you entered a song (or switched arrangement) whose tuning differed from the last, the tuner auto-opened and — for some testers — vanished ~1s later (reported macOS+Windows, 0.3.0). Root cause: the tuner closes itself on `song:play` (`plugins/tuner/utils/ui.js` — you don't tune while playing), so a **song switch** fired autoplay → `song:play` → the just-auto-opened tuner closed; an **arrangement switch** (which never arms autoplay) had no `song:play`, so it stayed open — exactly why two testers saw opposite behaviour (it wasn't the mic). Now: (1) the feature is a **new opt-in setting** ("Auto-open on tuning change", in the tuner's Settings panel, persisted as `autoOpenOnTuningChange`, **default OFF**); (2) an **auto**-opened tuner *persists* — it ignores the autoplay `song:play`, stray outside-clicks, and same-screen re-emits, closing only via the new in-panel **`×`** / **"Skip"** buttons or when you leave the song. A *manually* opened tuner keeps its classic click-away / play-to-close behaviour. The panel previously had no in-box close at all; this adds one (`×` + contextual Skip). All in the tuner plugin (`routes.py` config, `screen.js` gate + persist, `utils/ui.js` buttons + `song:play` guard, `settings.html` toggle) — **no core `app.js` changes**. Tests: `tests/js/tuner_auto_open.test.js` (opt-in gate, `{ auto: true }` persist mode, play/click-proofing). **Default (opt-in vs opt-out) is teed up for Byron to decide — flip one boolean.** - **Tuner auto-open is now tuning-coverage-aware — extended-range players aren't nagged for songs their instrument already covers.** With the opt-in auto-open on, it now prompts only when your **current physical tuning** (from your instrument selection in Settings) doesn't already cover the song. FeedBack is tune-to-song — the highway draws tab in the song's tuning — so the check aligns the song's open-string tuning string-for-string against your instrument: an **8-string F♯-standard** player gets **no** prompt for a 6- or 7-string standard song (its top strings already match those tunings), while a song needing an open string you don't have (e.g. a **Drop-A 7-string**, whose low A isn't an open string on an F♯ 8-string) **still** prompts. A whole-instrument reference difference also prompts — A440 vs A432, or an octave-down `centOffset` (which the auto-open now accounts for; it was previously ignored). The player's instrument is read from core **`/api/settings`** (the v3 instrument selector — a stable physical reference, not the tuner's song-tracking selection); when nothing's declared or the lookup is unavailable it falls back to a conservative prompt, so a real retune is never silently skipped. **v3-only** (the instrument selector is v3). All in the tuner plugin (`plugins/tuner/screen.js`) — **no core changes**. Tests: `tests/js/tuner_auto_open.test.js` (covered vs uncovered, the Drop-A case, reference-pitch mismatch, contiguous alignment). _Follow-up (E1.6): a passive "different tuning" badge cue that names the string(s) to retune, plus the splitscreen / no-usable-input guards._ - **The tuner badge now passively flags when a song needs a different tuning — and names the retune.** Building on the coverage check: when you enter a song your current instrument doesn't cover, the topbar tuner badge gets an amber ring and a tooltip that **names the change** — e.g. *"retune B→A"* for a Drop-A song on an F♯ 8-string, or *"the reference pitch"* for an A440-vs-A432 mismatch. It's purely **advisory** (it never auto-opens the panel — tap the badge to tune), recomputed on `song:ready` and cleared when a new song loads or you leave the player. The retune diff comes from the tuner plugin's coverage report (`window._tunerAutoOpen.coverageReport` → `{ covered, retune: [{ from, to }], reference, cantCover }`); the cue is CSS-free (an inline ring + native tooltip — no Tailwind rebuild) and no-ops when the tuner plugin isn't installed. **v3-only.** Touches `static/v3/badges.js` (the cue) + `plugins/tuner/screen.js` (the report). Tests: `tests/js/tuner_auto_open.test.js` (the report names the strings; reference mismatch; the badge wiring). _(The splitscreen-suppress and no-usable-input guards move to the playback-gate stage, where they matter for its no-trap rule.)_ +- **Tuner auto-open can now gate playback until you've tuned — the "tune before you play" model — via a new core `holdAutoplay()` hook.** With the opt-in auto-open on, when a song needs a retune the tuner opens and **playback waits** for your choice — **Skip** (you've tuned → play, and record the song's tuning as your instrument's current working tuning), **Back to library** / **Esc** (leave the song; a gated retune is never a one-way trap), or press **Play** (always wins). For an auto-open the in-panel **×** is dropped — Skip / Back to library / Esc are its dismiss surface. Previously the song played with the tuner overlaid; now it holds — which also definitively kills the original flash, since autoplay's `song:play` can't fire while playback is held. Implemented as a small **core hook** `window.feedBack.holdAutoplay()` (mirrors the existing `holdAutoExit()`): a plugin claims it **synchronously on `song:loading`** (so it beats the `song:ready` autostart), and `release()` — or a **12-second fail-open backstop** — runs the deferred start. **Generation-guarded** (a new song invalidates a stale hold) and **fail-open** (a wedged or crashed plugin can never permanently strand a song); **manual Play always wins** (it doesn't flow through the autostart path). The tuner claims the gate only when the feature is on, and **releases it the instant** it decides not to open (song already covered / tuning unchanged) or when you Skip. Touches core `static/app.js` (the hook + an autostart refactor) and the tuner plugin (`plugins/tuner/screen.js` — the claim/release; `plugins/tuner/utils/ui.js` — the Skip / Back-to-library buttons, × dropped on auto-open); the hook is generic and shell-agnostic (a test asserts `app.js` still doesn't reference the tuner's internals). Tests: `tests/js/tuner_auto_open.test.js` (claim on `song:loading`, release on dismiss, feature-off no-claim, the core hook + fail-open backstop, the Skip / Back-to-library / Esc escape-hatch) + a `speed_reset.test.js` stub. ⚠️ **Needs a manual smoke-test before shipping** — this is a core playback change; verify on desktop that the tuner mic doesn't contend with note_detect's scoring input (ASIO/exclusive mode), per the design charrette. - **v3 player: opening another rail popover now closes the Section Practice popover (no more two stacked popovers).** Opening the **Practice** pill's popover and then clicking a different player-rail icon (e.g. **Plugins**) left the Practice popover open underneath the new one — looked broken (reported on macOS, 0.3.0 / 2026-06-28). The rail icons call `e.stopPropagation()` in their click handler (`static/v3/player-chrome.js`), which killed bubbling before it reached the Practice popover's outside-click dismiss bound on `document`. The dismiss (`_installSectionPracticeDismiss` in `static/app.js`) now binds in the **capture phase**, which runs before the target's handler so a descendant's `stopPropagation()` can't swallow it — mirroring how the audio-mixer popover already dismisses. Esc handling stays bubble-phase (the player's Escape-to-exit ordering is unchanged). v2 shares `app.js` and is only hardened (no rail `stopPropagation` there). Tests: `tests/js/section_practice_dismiss.test.js`. - **v3 UI no longer lets you accidentally text-select the chrome.** Dragging or double-clicking across the interface used to marquee-highlight buttons, labels, the sidebar, the transport, and the note-highway HUD — which looks broken (reported on Mac + Windows). The v3 shell now defaults to `user-select: none` on `html` (`static/v3/v3.css`), then opts *content* back in — so chrome is non-selectable but the text you actually copy still works. Decided by a 4-lens panel (UX / accessibility / dev-ops / plugin-ecosystem); the guardrails are deliberate: **form fields are always re-enabled** (never break the caret / IME — no `* { user-select:none }`, which trips a WebKit input bug); **plugin screens (`.screen[id^="plugin-"]`) stay selectable by default** so a plugin's copyable text (lyrics, chord names, results) — including community plugins that don't know about this — isn't silently locked; and **core read-only content opts back in by container** via a new hand-authored **`.fb-selectable`** class — applied to the whole **Settings** panel (paths, device names, version, diagnostics, About — answering "is settings still copyable?": yes), the **now-playing song metadata** (with `pointer-events` re-enabled so the HUD text is actually reachable), and the focused **modals / dialogs / toasts / scan banner** that carry copyable errors, IDs, paths, and file names. It's cosmetic only (it protects nothing) and never used to lock copy-worthy text — errors, IDs, paths, versions, and metadata stay selectable per WCAG 2.2 (copy-paste as a permitted mechanism). Dense card lists (library grid, dashboard, profile) stay non-selectable by design — making them selectable would reintroduce the marquee-mess across cards. **v3-only** (v2 unchanged); plain CSS, no Tailwind rebuild; no desktop changes (standard OS-framed window). Plugin authors: `.fb-selectable` is documented in `CLAUDE.md` for re-enabling copyable content rendered outside a plugin screen. Tests: `tests/js/v3_user_select_policy.test.js`. - **Input-setup wizard no longer collapses an audio device's driver-type variants into one entry.** On Windows the desktop engine enumerates the same interface once per host API (ASIO / Windows Audio / DirectSound), and the wizard's audio picker (`plugins/input_setup/screen.js`) de-duped the source list by display **label** — so the variants (which share a name) collapsed to a single choice, silently keeping whichever sorted first (often *not* the low-latency ASIO one the player wants). The audio-input capability already collapses true duplicates by `logicalSourceKey` (`_visibleInputSources` in `static/capabilities/audio-session.js`), and the variants each have a **distinct** key, so the wizard's extra label-collapse was redundant for real dupes and destructive for these — it also could drop the variant that was actually `selected`. Removed it; the picker now lists every selectable input. Pairs with feedBack-desktop's change to label each source with its driver type (e.g. "Focusrite (ASIO)") so the now-distinct entries are legible. diff --git a/plugins/tuner/screen.js b/plugins/tuner/screen.js index 0845550..6d4f738 100644 --- a/plugins/tuner/screen.js +++ b/plugins/tuner/screen.js @@ -19,6 +19,11 @@ let _openGen = 0; let _onAutoOpenSongLoading = null; let _onAutoOpenSongReady = null; + // Autoplay gate (E2): when the feature is on, hold playback on song:loading and + // release it once we know we won't open (covered / unchanged) or the tuner is + // dismissed — so playback waits behind a genuinely-needed retune. + let _autoplayRelease = null; + let _gateClaimed = false; // ── Shared mutable state (read/written by screen.js; UI reads via closure) ── const _state = { @@ -362,11 +367,23 @@ return (await _coverageReport(songInfo)).covered; } + function _releaseGate() { + if (_autoplayRelease) { try { _autoplayRelease(); } catch (_) { /* */ } _autoplayRelease = null; } + } + function _onAutoOpenSongLoadingHandler() { _autoOpenGeneration++; _autoOpenDismissedSessionKey = null; _lastAutoOpenSessionKey = null; _invalidateCoverageCache(); + // Claim the autoplay gate NOW (synchronously, before song:ready) when the + // feature is on, so playback can wait behind a needed retune. Released on + // song:ready if we don't open, or when the tuner is dismissed. + _releaseGate(); + _gateClaimed = false; + _autoplayRelease = (_state._serverConfig && _state._serverConfig.autoOpenOnTuningChange + && window.feedBack && typeof window.feedBack.holdAutoplay === 'function') + ? window.feedBack.holdAutoplay() : null; } async function _maybeAutoOpenOnTuningChange() { @@ -412,6 +429,10 @@ try { await window.tuner.enable({ auto: true }); if (myGen !== _autoOpenGeneration) return; + _gateClaimed = true; // tuner is open → keep the autoplay gate until it's dismissed + // The hold is now intentional and user-dismissable — cancel the fail-open + // backstop so it can't start playback while the player is still tuning. + if (_autoplayRelease && typeof _autoplayRelease.settle === 'function') _autoplayRelease.settle(); } catch (e) { console.warn('Tuner: auto-open failed:', e && e.message ? e.message : e); if (_lastAutoOpenSessionKey === sessionKey) _lastAutoOpenSessionKey = null; @@ -426,7 +447,14 @@ function _installAutoOpenListeners() { if (_onAutoOpenSongLoading || !window.feedBack?.on) return; _onAutoOpenSongLoading = _onAutoOpenSongLoadingHandler; - _onAutoOpenSongReady = () => { _maybeAutoOpenOnTuningChange(); }; + _onAutoOpenSongReady = async () => { + const myGen = _autoOpenGeneration; + await _maybeAutoOpenOnTuningChange(); + // A newer song:loading may have superseded us while awaiting — it owns the + // gate/_gateClaimed now, so don't release its hold based on our stale view. + if (myGen !== _autoOpenGeneration) return; + if (!_gateClaimed) _releaseGate(); // not gating this song → let it play + }; window.feedBack.on('song:loading', _onAutoOpenSongLoading); window.feedBack.on('song:ready', _onAutoOpenSongReady); // The badge (static/v3/badges.js) emits this on the feedBack bus when the player @@ -635,11 +663,17 @@ // "Skip" is the auto-open nudge's explicit dismiss; hidden for a manual // open (the × / click-away already close those). if (_state.skipBtn) _state.skipBtn.classList.toggle('hidden', !auto); + // Auto-open shows the "Back to library" escape hatch and hides the ×: + // the Skip / Back buttons + Esc are the auto-open's dismiss surface, so a + // gated retune always offers a way forward AND a way out. + if (_state.backBtn) _state.backBtn.classList.toggle('hidden', !auto); + if (_state.closeBtn) _state.closeBtn.classList.toggle('hidden', !!auto); // Close when clicking outside the panel. Deferred so the badge's opening // click doesn't bubble up to the document and fire immediately. Skipped // for an auto-open: the user never clicked to open it, so their first - // unrelated click must not dismiss it (it persists until Skip/×/leave). + // unrelated click must not dismiss it (it persists until Skip / Back to + // library / Esc). if (!auto) { if (_outsideClickClose) document.removeEventListener('click', _outsideClickClose); _outsideClickClose = () => { if (_state.enabled) disable(); }; @@ -689,6 +723,7 @@ const onPlayer = document.getElementById('player')?.classList.contains('active'); _state.enabled = false; _state.autoOpened = false; + _releaseGate(); // dismissing a gated auto-open releases playback (it starts now) _state.manualTargetFreq = null; if (_outsideClickClose) { document.removeEventListener('click', _outsideClickClose); _outsideClickClose = null; } if (_state.activeViz) { _state.activeViz.destroy(); _state.activeViz = null; } diff --git a/plugins/tuner/utils/ui.js b/plugins/tuner/utils/ui.js index 681f0ab..f071f0d 100644 --- a/plugins/tuner/utils/ui.js +++ b/plugins/tuner/utils/ui.js @@ -573,6 +573,7 @@ window._tunerUI = function(state, actions) { closeBtn.title = 'Close'; closeBtn.textContent = '×'; closeBtn.onclick = () => actions.disable(); + state.closeBtn = closeBtn; header.appendChild(closeBtn); const settingsBtn = document.createElement('button'); @@ -639,11 +640,27 @@ window._tunerUI = function(state, actions) { const skipBtn = document.createElement('button'); skipBtn.className = 'tuner-skip-btn hidden w-full mt-3 text-[11px] text-fb-textDim hover:text-fb-text border border-fb-border/40 hover:border-fb-border/70 rounded-lg py-1.5 transition-colors'; skipBtn.textContent = 'Skip'; - skipBtn.title = 'Dismiss the tuner for this song'; + skipBtn.title = "I've tuned — play the song"; skipBtn.onclick = () => actions.disable(); state.skipBtn = skipBtn; state.uiContainer.appendChild(skipBtn); + // Auto-open escape hatch: leave the song entirely instead of committing to + // the play-now choice — so a gated retune is never a one-way trap. Mirrors + // Escape (the player's "Back to library" shortcut) and, like Escape, does + // NOT record a tuning (you're leaving, not asserting you tuned). + const backBtn = document.createElement('button'); + backBtn.className = 'tuner-back-btn hidden w-full mt-2 text-[11px] text-fb-textDim hover:text-fb-text border border-fb-border/40 hover:border-fb-border/70 rounded-lg py-1.5 transition-colors'; + backBtn.textContent = 'Back to library'; + backBtn.title = 'Leave the song (Esc)'; + backBtn.onclick = () => { + const exit = window.feedBack && window.feedBack.requestExitSong; + if (typeof exit === 'function') exit(); + else if (typeof window.requestExitSong === 'function') window.requestExitSong(); + }; + state.backBtn = backBtn; + state.uiContainer.appendChild(backBtn); + document.body.appendChild(state.uiContainer); state.uiContainer.addEventListener('click', (e) => e.stopPropagation()); } diff --git a/static/app.js b/static/app.js index 6c00304..025edcc 100644 --- a/static/app.js +++ b/static/app.js @@ -5990,6 +5990,56 @@ function _resolvePlayerOrigin() { // next song:ready. song:ready also fires on arrangement switches / seeks, // which never arm the flag, so those don't auto-restart. let _pendingAutostart = false; +// Autoplay gate (window.feedBack.holdAutoplay): a plugin (the tuner) can defer the +// auto-start of a freshly-loaded song until it's cleared — "tune before you play". +// The hold is claimed synchronously on song:loading (so it beats this song:ready +// autostart); release() — or a fail-open backstop — runs the deferred start. +// Generation-guarded so a newer song invalidates a stale hold. Manual Play never +// flows through here, so Play always wins. +let _autoplayHeld = false; +let _autoplayStart = null; +let _autoplayGen = 0; +let _autoplayBackstop = null; +const AUTOPLAY_HOLD_BACKSTOP_MS = 12000; +function _clearAutoplayHold() { + if (_autoplayBackstop) { clearTimeout(_autoplayBackstop); _autoplayBackstop = null; } + _autoplayHeld = false; + _autoplayStart = null; + _autoplayGen++; +} +function _releaseAutoplay(gen) { + if (gen !== _autoplayGen) return; // a newer song superseded this hold + if (_autoplayBackstop) { clearTimeout(_autoplayBackstop); _autoplayBackstop = null; } + _autoplayHeld = false; + const start = _autoplayStart; + _autoplayStart = null; + if (typeof start === 'function') start(); +} +let _autoplayHoldToken = 0; +window.feedBack.holdAutoplay = function () { + const gen = _autoplayGen; + const token = ++_autoplayHoldToken; // this hold's identity — a stale release from an earlier hold is a no-op + _autoplayHeld = true; + if (_autoplayBackstop) clearTimeout(_autoplayBackstop); + // Fail-open: a hold that's never released (a plugin that claimed but wedged before + // it could decide) must never permanently block the song. Once the holder commits + // to an intentional, user-dismissable hold it calls release.settle() to cancel this + // — so the backstop can't cut off e.g. a user still tuning past the timeout. + _autoplayBackstop = setTimeout(() => _releaseAutoplay(gen), AUTOPLAY_HOLD_BACKSTOP_MS); + let released = false; + function release() { + if (released || gen !== _autoplayGen || token !== _autoplayHoldToken) return; + released = true; + _releaseAutoplay(gen); + } + // Cancel the fail-open backstop WITHOUT releasing: the holder has taken explicit + // responsibility for releasing (on dismiss), and a song switch clears the hold anyway. + release.settle = function () { + if (gen !== _autoplayGen || token !== _autoplayHoldToken) return; + if (_autoplayBackstop) { clearTimeout(_autoplayBackstop); _autoplayBackstop = null; } + }; + return release; +}; window.feedBack.on('song:ready', () => { if (!_pendingAutostart) return; _pendingAutostart = false; @@ -6014,27 +6064,30 @@ window.feedBack.on('song:ready', () => { if (authors.length) _creditsTimer = setTimeout(hideSongCreditsOverlay, _CREDITS_HOLD_MS); return; } - // "Countdown before song": play a 4-beat count-in, then start. Otherwise - // reuse the Play button's start path directly (handles HTML5 + _juceMode). - if (_countdownBeforeSongEnabled()) { - // The count-in (~2.5s) gives the credits their on-screen dwell. - Promise.resolve(startSongCountIn()).catch((err) => console.warn('[app] song count-in failed:', err)); - } else if (authors.length) { - // No count-in window — hold the credits a couple seconds, then start. - // _cancelCountIn() and changeArrangement() both clear _creditsTimer, so - // a teardown / arrangement switch during the hold cancels this play. - _creditsTimer = setTimeout(() => { - _creditsTimer = null; - // If playback doesn't actually start (e.g. HTML5 autoplay rejection), - // song:play never fires — clear the credits promptly rather than - // waiting for the backstop. On success the song:play listener owns it. + // The actual auto-start: a count-in (which handles HTML5 + _juceMode) or the + // Play path directly. Guarded so a manual Play during a gate / credits hold + // can't double-toggle, and so a stale (released-after-leaving) start never + // begins playback off the player. + const start = () => { + if (isPlaying) return; + if (!document.getElementById('player')?.classList.contains('active')) { hideSongCreditsOverlay(); return; } + if (_countdownBeforeSongEnabled()) { + Promise.resolve(startSongCountIn()).catch((err) => console.warn('[app] song count-in failed:', err)); + } else { Promise.resolve(togglePlay()) .then(() => { if (!isPlaying) hideSongCreditsOverlay(); }) .catch((err) => { console.warn('[app] autoplay failed:', err); hideSongCreditsOverlay(); }); - }, _CREDITS_HOLD_MS); - } else { - Promise.resolve(togglePlay()).catch((err) => console.warn('[app] autoplay failed:', err)); - } + } + }; + // A plugin (the tuner) may gate playback until it's cleared. The hold was + // claimed on song:loading; stash the start and let release()/the backstop run + // it. _cancelCountIn()/changeArrangement() clear _creditsTimer below, so a + // teardown during the credits dwell still cancels a non-gated play. + if (_autoplayHeld) { _autoplayStart = start; return; } + // Not gated: a count-in starts now (it owns its on-screen dwell); otherwise + // let the credits dwell a couple seconds first, then start. + if (_countdownBeforeSongEnabled() || !authors.length) start(); + else _creditsTimer = setTimeout(() => { _creditsTimer = null; start(); }, _CREDITS_HOLD_MS); }); // ── Resume last session ──────────────────────────────────────────────────── @@ -6331,6 +6384,9 @@ async function playSong(filename, arrangement, options) { if (!options || options.bridge !== false) { _recordPlaybackBridge('playback.window-play-song', 'window.playSong', 'legacy playSong entry point used'); } + // Invalidate any prior song's autoplay gate before plugins re-claim it on the + // song:loading emit below. + _clearAutoplayHold(); window.feedBack.emit('song:loading', { filename, arrangement: arrangement ?? null }); // Cancel any pending art/metadata requests diff --git a/tests/js/speed_reset.test.js b/tests/js/speed_reset.test.js index 28f8912..20c7d0d 100644 --- a/tests/js/speed_reset.test.js +++ b/tests/js/speed_reset.test.js @@ -148,6 +148,7 @@ function loadPlaySong(sandbox) { var _playerOriginScreen = null; var _pendingAutostart = false; function _clearAutoExit() {} + function _clearAutoplayHold() {} function _resolvePlayerOrigin() { return 'home'; } function _recordPlaybackBridge() {} function _cancelCountIn() {} diff --git a/tests/js/tuner_auto_open.test.js b/tests/js/tuner_auto_open.test.js index d0ea0c8..1baa185 100644 --- a/tests/js/tuner_auto_open.test.js +++ b/tests/js/tuner_auto_open.test.js @@ -9,6 +9,7 @@ const vm = require('node:vm'); const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js'); const TUNER_SCREEN_JS = path.join(__dirname, '..', '..', 'plugins', 'tuner', 'screen.js'); const TUNING_UTILS_JS = path.join(__dirname, '..', '..', 'plugins', 'tuner', 'utils', 'tuning-utils.js'); +const TUNER_UI_JS = path.join(__dirname, '..', '..', 'plugins', 'tuner', 'utils', 'ui.js'); function loadTuningHelpers() { const src = fs.readFileSync(APP_JS, 'utf8'); @@ -461,6 +462,71 @@ test('the tuner badge surfaces a passive coverage cue (badges.js)', () => { assert.match(badgesSrc, /boxShadow/); }); +// ── Autoplay gate (E2) ───────────────────────────────────────────────────── +test('gate: claims an autoplay hold on song:loading and releases it on dismiss', async () => { + const sandbox = createTunerSandbox({ player: PLAYER_GUITAR_6 }); + let holds = 0, releases = 0; + sandbox.window.feedBack.holdAutoplay = () => { holds++; return () => { releases++; }; }; + const api = sandbox.window._tunerAutoOpen; + await ready(sandbox, DROP_D); // loads config (feature on) + api.resetState(); + api.onSongLoading(); // song:loading → claim the gate + assert.equal(holds, 1); + assert.equal(releases, 0); + sandbox.window.tuner.disable(); // dismiss → release (playback proceeds) + assert.equal(releases, 1); +}); + +test('gate: does not claim a hold when the feature is off', async () => { + const sandbox = createTunerSandbox({ player: PLAYER_GUITAR_6, autoOpen: false }); + let holds = 0; + sandbox.window.feedBack.holdAutoplay = () => { holds++; return () => {}; }; + const api = sandbox.window._tunerAutoOpen; + await ready(sandbox, DROP_D); // loads config (feature OFF) + api.onSongLoading(); + assert.equal(holds, 0); +}); + +test('the autoplay gate is a generic core hook with a fail-open backstop (app.js)', () => { + const appSrc = fs.readFileSync(APP_JS, 'utf8'); + assert.match(appSrc, /window\.feedBack\.holdAutoplay = function/); + assert.match(appSrc, /AUTOPLAY_HOLD_BACKSTOP_MS/); // fail-open: never strand the song + assert.match(appSrc, /if \(_autoplayHeld\) \{ _autoplayStart = start;/); // a gated start is stashed + assert.match(appSrc, /_clearAutoplayHold\(\);[\s\S]{0,160}emit\('song:loading'/); // reset before plugins re-claim + // Per-hold identity: a stale release from an earlier hold must not clear a later one. + assert.match(appSrc, /token !== _autoplayHoldToken/); + // settle(): a committed holder can cancel the fail-open backstop (no timed release). + assert.match(appSrc, /release\.settle = function/); + // The hook is generic — app.js still doesn't reference the tuner's internals. + assert.doesNotMatch(appSrc, /_tunerAutoOpen|maybeAutoOpenOnTuningChange/); +}); + +test('the tuner gates playback via holdAutoplay (screen.js)', () => { + const src = fs.readFileSync(TUNER_SCREEN_JS, 'utf8'); + assert.match(src, /window\.feedBack\.holdAutoplay\(\)/); // claimed on song:loading + assert.match(src, /_gateClaimed = true/); // kept when the tuner opens + assert.match(src, /if \(!_gateClaimed\) _releaseGate\(\)/); // released when we don't open + assert.match(src, /_releaseGate\(\);[\s\S]{0,80}dismissing a gated/); // released on dismiss + // Once open, the tuner settles the hold so the 12s backstop can't start playback + // while the player is still tuning. + assert.match(src, /_autoplayRelease\.settle\(\)/); + // The async song:ready handler is generation-guarded so it can't release a NEWER + // song's gate after its await. + assert.match(src, /_onAutoOpenSongReady = async \(\) => \{[\s\S]{0,320}myGen !== _autoOpenGeneration\) return;[\s\S]{0,120}_releaseGate/); +}); + +test('gate escape hatch: auto-open offers "Back to library" (+ Esc) and hides the × (ui.js/screen.js)', () => { + const ui = fs.readFileSync(TUNER_UI_JS, 'utf8'); + assert.match(ui, /Back to library/); // an explicit way out, not only play-now + assert.match(ui, /requestExitSong/); // reuses the standard song-exit path (mirrors Esc) + assert.match(ui, /state\.backBtn =/); + assert.match(ui, /state\.closeBtn =/); // × captured so it can be toggled + + const screen = fs.readFileSync(TUNER_SCREEN_JS, 'utf8'); + assert.match(screen, /_state\.backBtn[\s\S]{0,60}toggle\('hidden', !auto\)/); // shown on auto-open + assert.match(screen, /_state\.closeBtn[\s\S]{0,60}toggle\('hidden', !!auto\)/); // × hidden on auto-open +}); + test('auto-open does not require app.js changes', () => { const appSrc = fs.readFileSync(APP_JS, 'utf8'); assert.doesNotMatch(appSrc, /_tunerAutoOpen|maybeAutoOpenOnTuningChange/);