feat(tuner): gate playback until you've tuned — hold autoplay + no-trap Skip/Back/Esc (working-tuning PR 4) (#666)

* 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 <noreply@anthropic.com>
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) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
This commit is contained in:
ChrisBeWithYou
2026-07-01 11:18:29 +02:00
committed by GitHub
co-authored by Claude Opus 4.8 byrongamatos
parent 48f435408f
commit 115c96a3f0
6 changed files with 197 additions and 21 deletions
+1
View File
@@ -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 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._ - **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.)_ - **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 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`. - **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. - **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.
+37 -2
View File
@@ -19,6 +19,11 @@
let _openGen = 0; let _openGen = 0;
let _onAutoOpenSongLoading = null; let _onAutoOpenSongLoading = null;
let _onAutoOpenSongReady = 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) ── // ── Shared mutable state (read/written by screen.js; UI reads via closure) ──
const _state = { const _state = {
@@ -362,11 +367,23 @@
return (await _coverageReport(songInfo)).covered; return (await _coverageReport(songInfo)).covered;
} }
function _releaseGate() {
if (_autoplayRelease) { try { _autoplayRelease(); } catch (_) { /* */ } _autoplayRelease = null; }
}
function _onAutoOpenSongLoadingHandler() { function _onAutoOpenSongLoadingHandler() {
_autoOpenGeneration++; _autoOpenGeneration++;
_autoOpenDismissedSessionKey = null; _autoOpenDismissedSessionKey = null;
_lastAutoOpenSessionKey = null; _lastAutoOpenSessionKey = null;
_invalidateCoverageCache(); _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() { async function _maybeAutoOpenOnTuningChange() {
@@ -412,6 +429,10 @@
try { try {
await window.tuner.enable({ auto: true }); await window.tuner.enable({ auto: true });
if (myGen !== _autoOpenGeneration) return; 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) { } catch (e) {
console.warn('Tuner: auto-open failed:', e && e.message ? e.message : e); console.warn('Tuner: auto-open failed:', e && e.message ? e.message : e);
if (_lastAutoOpenSessionKey === sessionKey) _lastAutoOpenSessionKey = null; if (_lastAutoOpenSessionKey === sessionKey) _lastAutoOpenSessionKey = null;
@@ -426,7 +447,14 @@
function _installAutoOpenListeners() { function _installAutoOpenListeners() {
if (_onAutoOpenSongLoading || !window.feedBack?.on) return; if (_onAutoOpenSongLoading || !window.feedBack?.on) return;
_onAutoOpenSongLoading = _onAutoOpenSongLoadingHandler; _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:loading', _onAutoOpenSongLoading);
window.feedBack.on('song:ready', _onAutoOpenSongReady); window.feedBack.on('song:ready', _onAutoOpenSongReady);
// The badge (static/v3/badges.js) emits this on the feedBack bus when the player // 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 // "Skip" is the auto-open nudge's explicit dismiss; hidden for a manual
// open (the × / click-away already close those). // open (the × / click-away already close those).
if (_state.skipBtn) _state.skipBtn.classList.toggle('hidden', !auto); 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 // Close when clicking outside the panel. Deferred so the badge's opening
// click doesn't bubble up to the document and fire immediately. Skipped // 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 // 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 (!auto) {
if (_outsideClickClose) document.removeEventListener('click', _outsideClickClose); if (_outsideClickClose) document.removeEventListener('click', _outsideClickClose);
_outsideClickClose = () => { if (_state.enabled) disable(); }; _outsideClickClose = () => { if (_state.enabled) disable(); };
@@ -689,6 +723,7 @@
const onPlayer = document.getElementById('player')?.classList.contains('active'); const onPlayer = document.getElementById('player')?.classList.contains('active');
_state.enabled = false; _state.enabled = false;
_state.autoOpened = false; _state.autoOpened = false;
_releaseGate(); // dismissing a gated auto-open releases playback (it starts now)
_state.manualTargetFreq = null; _state.manualTargetFreq = null;
if (_outsideClickClose) { document.removeEventListener('click', _outsideClickClose); _outsideClickClose = null; } if (_outsideClickClose) { document.removeEventListener('click', _outsideClickClose); _outsideClickClose = null; }
if (_state.activeViz) { _state.activeViz.destroy(); _state.activeViz = null; } if (_state.activeViz) { _state.activeViz.destroy(); _state.activeViz = null; }
+18 -1
View File
@@ -573,6 +573,7 @@ window._tunerUI = function(state, actions) {
closeBtn.title = 'Close'; closeBtn.title = 'Close';
closeBtn.textContent = '×'; closeBtn.textContent = '×';
closeBtn.onclick = () => actions.disable(); closeBtn.onclick = () => actions.disable();
state.closeBtn = closeBtn;
header.appendChild(closeBtn); header.appendChild(closeBtn);
const settingsBtn = document.createElement('button'); const settingsBtn = document.createElement('button');
@@ -639,11 +640,27 @@ window._tunerUI = function(state, actions) {
const skipBtn = document.createElement('button'); 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.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.textContent = 'Skip';
skipBtn.title = 'Dismiss the tuner for this song'; skipBtn.title = "I've tuned — play the song";
skipBtn.onclick = () => actions.disable(); skipBtn.onclick = () => actions.disable();
state.skipBtn = skipBtn; state.skipBtn = skipBtn;
state.uiContainer.appendChild(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); document.body.appendChild(state.uiContainer);
state.uiContainer.addEventListener('click', (e) => e.stopPropagation()); state.uiContainer.addEventListener('click', (e) => e.stopPropagation());
} }
+74 -18
View File
@@ -5990,6 +5990,56 @@ function _resolvePlayerOrigin() {
// next song:ready. song:ready also fires on arrangement switches / seeks, // next song:ready. song:ready also fires on arrangement switches / seeks,
// which never arm the flag, so those don't auto-restart. // which never arm the flag, so those don't auto-restart.
let _pendingAutostart = false; 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', () => { window.feedBack.on('song:ready', () => {
if (!_pendingAutostart) return; if (!_pendingAutostart) return;
_pendingAutostart = false; _pendingAutostart = false;
@@ -6014,27 +6064,30 @@ window.feedBack.on('song:ready', () => {
if (authors.length) _creditsTimer = setTimeout(hideSongCreditsOverlay, _CREDITS_HOLD_MS); if (authors.length) _creditsTimer = setTimeout(hideSongCreditsOverlay, _CREDITS_HOLD_MS);
return; return;
} }
// "Countdown before song": play a 4-beat count-in, then start. Otherwise // The actual auto-start: a count-in (which handles HTML5 + _juceMode) or the
// reuse the Play button's start path directly (handles HTML5 + _juceMode). // Play path directly. Guarded so a manual Play during a gate / credits hold
if (_countdownBeforeSongEnabled()) { // can't double-toggle, and so a stale (released-after-leaving) start never
// The count-in (~2.5s) gives the credits their on-screen dwell. // begins playback off the player.
Promise.resolve(startSongCountIn()).catch((err) => console.warn('[app] song count-in failed:', err)); const start = () => {
} else if (authors.length) { if (isPlaying) return;
// No count-in window — hold the credits a couple seconds, then start. if (!document.getElementById('player')?.classList.contains('active')) { hideSongCreditsOverlay(); return; }
// _cancelCountIn() and changeArrangement() both clear _creditsTimer, so if (_countdownBeforeSongEnabled()) {
// a teardown / arrangement switch during the hold cancels this play. Promise.resolve(startSongCountIn()).catch((err) => console.warn('[app] song count-in failed:', err));
_creditsTimer = setTimeout(() => { } else {
_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.
Promise.resolve(togglePlay()) Promise.resolve(togglePlay())
.then(() => { if (!isPlaying) hideSongCreditsOverlay(); }) .then(() => { if (!isPlaying) hideSongCreditsOverlay(); })
.catch((err) => { console.warn('[app] autoplay failed:', err); 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 ──────────────────────────────────────────────────── // ── Resume last session ────────────────────────────────────────────────────
@@ -6331,6 +6384,9 @@ async function playSong(filename, arrangement, options) {
if (!options || options.bridge !== false) { if (!options || options.bridge !== false) {
_recordPlaybackBridge('playback.window-play-song', 'window.playSong', 'legacy playSong entry point used'); _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 }); window.feedBack.emit('song:loading', { filename, arrangement: arrangement ?? null });
// Cancel any pending art/metadata requests // Cancel any pending art/metadata requests
+1
View File
@@ -148,6 +148,7 @@ function loadPlaySong(sandbox) {
var _playerOriginScreen = null; var _playerOriginScreen = null;
var _pendingAutostart = false; var _pendingAutostart = false;
function _clearAutoExit() {} function _clearAutoExit() {}
function _clearAutoplayHold() {}
function _resolvePlayerOrigin() { return 'home'; } function _resolvePlayerOrigin() { return 'home'; }
function _recordPlaybackBridge() {} function _recordPlaybackBridge() {}
function _cancelCountIn() {} function _cancelCountIn() {}
+66
View File
@@ -9,6 +9,7 @@ const vm = require('node:vm');
const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js'); const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
const TUNER_SCREEN_JS = path.join(__dirname, '..', '..', 'plugins', 'tuner', 'screen.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 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() { function loadTuningHelpers() {
const src = fs.readFileSync(APP_JS, 'utf8'); 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/); 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', () => { test('auto-open does not require app.js changes', () => {
const appSrc = fs.readFileSync(APP_JS, 'utf8'); const appSrc = fs.readFileSync(APP_JS, 'utf8');
assert.doesNotMatch(appSrc, /_tunerAutoOpen|maybeAutoOpenOnTuningChange/); assert.doesNotMatch(appSrc, /_tunerAutoOpen|maybeAutoOpenOnTuningChange/);