diff --git a/CHANGELOG.md b/CHANGELOG.md index 01ce67d..97baddf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **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._ - **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 072b5af..638b153 100644 --- a/plugins/tuner/screen.js +++ b/plugins/tuner/screen.js @@ -133,6 +133,96 @@ return filename + '::' + arr; } + // ── §4 instrument-coverage ──────────────────────────────────────────────── + // FeedBack is tune-to-song: the highway draws tab in the SONG's tuning, so the + // player tunes their instrument to match. We therefore only auto-open when the + // player's CURRENT physical tuning doesn't already cover the song — i.e. the + // song's open-string tuning isn't an exact contiguous run inside the player's + // strings, OR the global reference differs. So an 8-string F# player isn't + // nagged for a 6-/7-string standard song (its top strings already match), but a + // Drop-A song whose dropped open string the player lacks still prompts. + function _openMidisFromFreqs(freqs) { + const u = window._tunerUtils; + if (!u || !Array.isArray(freqs)) return null; + return freqs.map((f) => Math.round(u.freqToMidi(f))); + } + + // The song's open-string MIDI (at A440; centOffset handled separately as a + // global). Mirrors _tuningIdentityKey's isBass / string-count derivation. + function _songOpenMidis(songInfo) { + const u = window._tunerUtils; + if (!u || !songInfo || !Array.isArray(songInfo.tuning) || !songInfo.tuning.length) return null; + const ctx = (typeof window.feedBack?.songTuningContext === 'function') + ? window.feedBack.songTuningContext(songInfo) + : { stringCount: songInfo.stringCount, arrangement: songInfo.arrangement, arrangement_smart_name: songInfo.arrangement_smart_name }; + const isBass = (typeof window.feedBack?.isBassArrangement === 'function') + ? window.feedBack.isBassArrangement(ctx) + : (songInfo.arrangement || '').toLowerCase().includes('bass'); + const sc = (typeof window.feedBack?.effectiveStringCount === 'function') + ? window.feedBack.effectiveStringCount(songInfo.tuning, ctx) + : (songInfo.stringCount || songInfo.tuning.length); + if (!sc || sc <= 0) return null; + const offsets = songInfo.tuning.slice(0, sc); + if (!offsets.length) return null; + return _openMidisFromFreqs(u.offsetsToFreqs(offsets, isBass)); + } + + // The player's PHYSICAL instrument from core /api/settings (the v3 instrument + // badge's selection — a STABLE reference, NOT the tuner's song-tracking + // _syncCurrentTuning). Returns { midis, refCents } or null. + async function _playerTuning() { + const u = window._tunerUtils; + if (!u) return null; + let s; + try { s = await fetch('/api/settings').then((r) => (r && r.ok ? r.json() : null)); } + catch (_) { return null; } + if (!s) return null; + const isBass = s.instrument === 'bass'; + const sc = Number(s.string_count) || (isBass ? 4 : 6); + const refPitch = Number(s.reference_pitch) || 440; + let freqs = null; + if (Array.isArray(s.tuning)) { + freqs = u.offsetsToFreqs(s.tuning.slice(0, sc), isBass); + } else if (typeof s.tuning === 'string') { + const named = _state._allTunings && _state._allTunings[(isBass ? 'bass' : 'guitar') + '-' + sc]; + if (named && Array.isArray(named[s.tuning])) freqs = named[s.tuning]; + } + if (!freqs) freqs = u.offsetsToFreqs(new Array(sc).fill(0), isBass); // standard fallback + const midis = _openMidisFromFreqs(freqs); + if (!midis) return null; + return { midis, refCents: 1200 * Math.log2(refPitch / 440) }; + } + + // The song's open strings must appear as an exact, contiguous, order-preserving + // run inside the player's strings (extended-range adds strings at the low/high + // ends — match by pitch, never by string index). + function _contiguousRunMatch(songMidis, playerMidis) { + if (playerMidis.length < songMidis.length) return false; + for (let start = 0; start + songMidis.length <= playerMidis.length; start++) { + let ok = true; + for (let i = 0; i < songMidis.length; i++) { + if (playerMidis[start + i] !== songMidis[i]) { ok = false; break; } + } + if (ok) return true; + } + return false; + } + + // True when the player's current physical tuning already covers the song + // (→ suppress auto-open). Conservative: any missing data returns false, so a + // genuinely-needed prompt is never silently dropped on a fetch hiccup. + async function _coveredByPlayerInstrument(songInfo) { + const song = _songOpenMidis(songInfo); + if (!song || !song.length) return false; + const player = await _playerTuning(); + if (!player || !player.midis.length) return false; + const songCents = Number(songInfo?.centOffset) || 0; + // Global reference / octave: a difference fretting can't absorb (A440 vs + // A432 ≈ 32¢, or an octave-down centOffset) → NOT covered. + if (Math.abs(songCents - player.refCents) > 25) return false; + return _contiguousRunMatch(song, player.midis); + } + function _onAutoOpenSongLoadingHandler() { _autoOpenGeneration++; _autoOpenDismissedSessionKey = null; @@ -171,6 +261,13 @@ if (_lastAutoOpenSessionKey === sessionKey) return; if (!window.tuner || typeof window.tuner.enable !== 'function') return; + // §4: skip the prompt when the player's physical instrument already covers + // this song's tuning (e.g. an 8-string F# playing a 6-/7-string standard + // song). Async (fetches /api/settings) — re-check the generation after. + const covered = await _coveredByPlayerInstrument(songInfo); + if (myGen !== _autoOpenGeneration) return; + if (covered) return; + _lastAutoOpenSessionKey = sessionKey; try { await window.tuner.enable({ auto: true }); @@ -502,6 +599,7 @@ tuningIdentityKey: _tuningIdentityKey, sessionKey: _autoOpenSessionKey, maybeAutoOpenOnTuningChange: _maybeAutoOpenOnTuningChange, + coveredByPlayerInstrument: _coveredByPlayerInstrument, onSongLoading: _onAutoOpenSongLoadingHandler, getState() { return { diff --git a/tests/js/tuner_auto_open.test.js b/tests/js/tuner_auto_open.test.js index 0754950..3527090 100644 --- a/tests/js/tuner_auto_open.test.js +++ b/tests/js/tuner_auto_open.test.js @@ -8,6 +8,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'); function loadTuningHelpers() { const src = fs.readFileSync(APP_JS, 'utf8'); @@ -30,6 +31,10 @@ function createTunerSandbox(opts) { // Auto-open is opt-in (default off in prod). The sandbox defaults it ON so the // behaviour tests exercise the feature; pass { autoOpen: false } to gate it off. const autoOpen = !opts || opts.autoOpen !== false; + // The player's physical instrument for the §4 coverage check (core /api/settings). + // Absent → the endpoint reports not-ok → coverage stays conservative (can't + // decide → don't suppress the prompt), preserving the pre-coverage behaviour. + const playerSettings = (opts && opts.player) || null; const enableCalls = []; let playerActive = true; let songInfo = null; @@ -41,7 +46,13 @@ function createTunerSandbox(opts) { setTimeout(fn) { fn(); return 0; }, clearTimeout() {}, fetch(url) { - if (String(url).includes('/config')) { + const _u = String(url); + if (_u.includes('/api/settings')) { + return Promise.resolve(playerSettings + ? { ok: true, json: () => Promise.resolve(playerSettings) } + : { ok: false, json: () => Promise.resolve({}) }); + } + if (_u.includes('/config')) { return Promise.resolve({ json: () => Promise.resolve({ showFloatingButton: true, @@ -124,12 +135,8 @@ function createTunerSandbox(opts) { sandbox.window.highway = { getSongInfo: () => songInfo, }; - sandbox.window._tunerUtils = { - preferFlats: () => false, - offsetsToFreqs: (offsets) => offsets.map((o, i) => 80 + i * 10), - freqToMidi: () => 40, - midiToNote: () => 'E', - }; + // _tunerUtils comes from the REAL tuning-utils.js (loaded into the sandbox + // below) so the §4 coverage check runs real pitch math, not stubbed values. sandbox.window._tunerUI = () => ({ addButton() {}, initUI() {}, @@ -156,6 +163,7 @@ function createTunerSandbox(opts) { }); vm.createContext(sandbox); + vm.runInContext(fs.readFileSync(TUNING_UTILS_JS, 'utf8'), sandbox); vm.runInContext(fs.readFileSync(TUNER_SCREEN_JS, 'utf8'), sandbox); const realEnable = sandbox.window.tuner.enable.bind(sandbox.window.tuner); @@ -359,6 +367,64 @@ test('screen.js registers song:loading and song:ready auto-open listeners at boo assert.doesNotMatch(src, /restartCurrentSong/); }); +// ── §4 instrument-coverage (E1.5) ────────────────────────────────────────── +// Player physical instruments (core /api/settings shape: instrument/string_count/ +// tuning offsets/reference_pitch). +const PLAYER_GUITAR_8_FS = { instrument: 'guitar', string_count: 8, tuning: [0, 0, 0, 0, 0, 0, 0, 0], reference_pitch: 440 }; // F# standard +const PLAYER_GUITAR_6 = { instrument: 'guitar', string_count: 6, tuning: [0, 0, 0, 0, 0, 0], reference_pitch: 440 }; // E standard +const SONG_7B = { filename: '7b.sloppak', arrangement: 'Lead', arrangement_index: 0, stringCount: 7, tuning: [0, 0, 0, 0, 0, 0, 0] }; // B standard 7 +const SONG_DROP_A7 = { filename: 'dropa7.sloppak', arrangement: 'Lead', arrangement_index: 0, stringCount: 7, tuning: [-2, 0, 0, 0, 0, 0, 0] }; // Drop A 7 + +test('coverage: an 8-string F# player is NOT prompted for a covered 6-/7-string standard song', async () => { + const sandbox = createTunerSandbox({ player: PLAYER_GUITAR_8_FS }); + sandbox.window._tunerAutoOpen.resetState(); + await ready(sandbox, DROP_D); // first song → sets lastTuningKey, no open + await ready(sandbox, E_STANDARD); // 6-E lives on the 8-string's top 6 → suppressed + assert.equal(sandbox.__enableCalls.length, 0); + await ready(sandbox, SONG_7B); // 7-B lives on the 8-string's top 7 → suppressed + assert.equal(sandbox.__enableCalls.length, 0); +}); + +test('coverage: a Drop-A 7-string song STILL prompts the 8-string F# player (dropped string absent)', async () => { + const sandbox = createTunerSandbox({ player: PLAYER_GUITAR_8_FS }); + sandbox.window._tunerAutoOpen.resetState(); + await ready(sandbox, E_STANDARD); // first → no open + await ready(sandbox, SONG_DROP_A7); // needs an open A1; the 8-string has F#1/B1, not A1 → prompt + assert.equal(sandbox.__enableCalls.length, 1); +}); + +test('coverage: a 6-string-standard player IS prompted for Drop D', async () => { + const sandbox = createTunerSandbox({ player: PLAYER_GUITAR_6 }); + sandbox.window._tunerAutoOpen.resetState(); + await ready(sandbox, E_STANDARD); // first → no open + await ready(sandbox, DROP_D); // low E must drop to D → not covered → prompt + assert.equal(sandbox.__enableCalls.length, 1); +}); + +test('coverage: a reference-pitch mismatch (A432 player vs A440 song) prompts even when the shape matches', async () => { + const sandbox = createTunerSandbox({ player: { ...PLAYER_GUITAR_6, reference_pitch: 432 } }); + sandbox.window._tunerAutoOpen.resetState(); + await ready(sandbox, DROP_D); // first → no open + await ready(sandbox, E_STANDARD); // shape matches, but the whole instrument is ~32¢ flat → prompt + assert.equal(sandbox.__enableCalls.length, 1); +}); + +test('coverage: covered/uncovered is decided by contiguous pitch alignment (direct)', async () => { + const sandbox = createTunerSandbox({ player: PLAYER_GUITAR_8_FS }); + const cover = (song) => sandbox.window._tunerAutoOpen.coveredByPlayerInstrument(song); + assert.equal(await cover(E_STANDARD), true); // 6-E is a run inside 8-string F# + assert.equal(await cover(SONG_7B), true); // 7-B is a run inside 8-string F# + assert.equal(await cover(SONG_DROP_A7), false); // Drop-A's low A1 isn't an open string on it +}); + +test('coverage: with no declared instrument it stays conservative (prompts as before)', async () => { + const sandbox = createTunerSandbox(); // no /api/settings instrument + sandbox.window._tunerAutoOpen.resetState(); + await ready(sandbox, CUSTOM_GUITAR); + await ready(sandbox, E_STANDARD); + assert.equal(sandbox.__enableCalls.length, 1); +}); + test('auto-open does not require app.js changes', () => { const appSrc = fs.readFileSync(APP_JS, 'utf8'); assert.doesNotMatch(appSrc, /_tunerAutoOpen|maybeAutoOpenOnTuningChange/);