diff --git a/CHANGELOG.md b/CHANGELOG.md index 97baddf..5604da7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **The tuner now tracks what tuning your instrument is *actually* in, so it prompts you to retune in BOTH directions — down to a song's tuning, and back up when the next song needs it.** The coverage check used to compare each song against your fixed instrument-profile tuning, so it only ever prompted you *away* from "home" (e.g. E → Drop C#) and stayed silent coming back (Drop C# → E), even though you'd physically retuned. It now reads the host's live **per-instrument working tuning** (`window.feedBack.workingTuning`) — what your selected instrument is currently in — so coverage is measured against your *actual* tuning and fires both ways. When you clear an auto-opened tuner, the tuner publishes that song's tuning as your instrument's live working tuning (`assumed` — an explicit "I tuned / Skip" refines it in a later PR), so the next song is judged against where you now are. **Per-instrument** — your guitar's and bass's tunings are tracked separately (keyed like the selector), so switching instruments uses the right one. Feature-detected: on a host without the working-tuning capability it falls back to the static `/api/settings` tuning (today's behavior). `plugins/tuner/screen.js` (`_playerTuning` reads `workingTuning` keyed by the selected instrument; `_publishWorkingTuning` writes on clear). Builds on the host `workingTuning` foundation (PR 1 of the series) + the instrument→chart routing (PR 2). Tests: `tests/js/tuner_auto_open.test.js` (both-directions coverage via a live Drop-D working tuning; publish-on-clear targets the right instrument slot) — 29 pass. - **`.jsonc` support for feedpak data files** (feedpak-spec §8, FEP #3 / PR #13). Hand-edited packs may now use the `.jsonc` extension (JSON with C-style `//` line and `/* */` block comments) for any data file the manifest points at — arrangements, notation sidecars, `drum_tab`, `song_timeline`, `lyrics`, and `keys`. New shared `lib/jsonc.py` provides `parse_jsonc(text)` + `load_json(path)` (auto-detects `.jsonc` by suffix, string-aware so comment-like text inside JSON string values is preserved) and is now used by every reader in `lib/sloppak.py` (six side-file sites) and `scripts/lift_keys_notation.py` (three arrangement / song_timeline read sites). The strip regex mirrors the reference validator in `feedpak-spec/tools/validate.py`. This is an additive (MINOR) change: `.jsonc` is opt-in, so any pack that keeps its data files as `.json` is unaffected and needs no regeneration. Note that a `.jsonc` file containing real comments only loads on a reader that implements §8 — a pre-this-change reader calls bare `json.loads` and fails on the comments rather than ignoring them, so don't hand out `.jsonc` packs to older hosts. Tests: `tests/test_sloppak_jsonc_load.py` (covers all six side-file types, the lift helper, and the string-boundary preservation rule end-to-end). - **The highway now loads the part that matches your selected instrument — a bass player gets the Bass arrangement, not the default Lead/guitar chart.** When you open a song without an explicit arrangement, the WebSocket handler (`server.py` `highway_ws`) reads your selected `instrument` from `config.json` (the same file it already reads for your default-arrangement preference) and routes to the matching part: **bass → the Bass arrangement**; guitar — and any unknown/future instrument (drums, keys) — falls through to the existing preference/most-notes default, which already lands on a guitar part. Previously the instrument selector only fed the tuner, so a bass player was handed a guitar chart (and a tune/coverage check then compared a 4-string bass against a 6-string part). An **explicit arrangement request always wins** (a manual arrangement switch is untouched), and a bass player's saved default-arrangement preference is still honored **within** the bass parts (so a preferred `Bass 2` / `Alt. Bass` wins over the canonical Bass), so this only changes the *default* part chosen on load. Server-only — every launch path already flows through the WS, so there's no client change. This is the instrument↔chart-routing piece the working-tuning series leans on (otherwise coverage compares across instruments). Tests: `tests/test_highway_ws_instrument_routing.py` (bass→Bass, bass-honors-pref, bass-no-bass-part→guitar, guitar→default, explicit-wins). - **Host "working tuning" — a live, app-wide record of what tuning your instrument is *actually* in right now (foundation; no behavior change yet).** Introduces `window.feedBack.workingTuning`, a host-owned, session-lived state distinct from any one song's tuning and from a soft opt-in default: the offsets + string-count + reference pitch the player's instrument is currently in, plus an `assumed`/`verified` provenance flag. It's **per-instrument** — your guitar's current tuning and your bass's are kept *separately* (keyed like the instrument selector, e.g. `guitar-6` / `bass-4`), so switching instruments surfaces that instrument's own remembered tuning and you only ever deal with the one you've selected. It exists so a retune — or an instrument swap mid-session — is reflected **everywhere** (the highway, the library/song-picker, and plugins like the tuner, Virtuoso, and the minigames) instead of being re-derived per surface or wrongly assumed from a fixed profile. Modeled on the shipped `tuning` capability + the `feedBack.theme` read-API: a **synchronous `get(instrument?)`** (returns the selected instrument's state, defaulting to the seed until known), a `set(state, {provenance, instrument})` mutator (the tuner becomes the sole writer in the next change), `setCurrentInstrument()` for the selector, `resetToDefault()`, and a `working-tuning-changed` event that fires on every change **and once on hydration** (carrying which instrument changed) so a late-mounting consumer is never stuck on stale state. State is **in-memory, seeded from `/api/settings` on boot and reset on restart** — a stale "you're in drop-A" assumption is worse than re-asking. Registered as a separate `working-tuning` **exclusive-owner** capability (tuner = writer, the rest = requesters). This is the foundation (plumbing only — nothing writes to it yet) of the working-tuning series, which fixes the tuner gate only ever prompting *away from* a fixed "home" tuning (never back) and makes the current tuning a first-class signal the whole app shares. Offsets use the same per-string semitone vocabulary as song tunings, so fully custom/extended tunings (e.g. a drop-A 8-string) are first-class. Frontend-only: new `static/capabilities/working-tuning.js`, loaded from `static/index.html` + `static/v3/index.html`. @@ -52,6 +53,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._ +- **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.)_ - **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/routes.py b/plugins/tuner/routes.py index a58da10..e3a1dfd 100644 --- a/plugins/tuner/routes.py +++ b/plugins/tuner/routes.py @@ -56,7 +56,11 @@ def setup(app: FastAPI, context: dict): res["visualizationMode"] = str(data.get("visualizationMode", "default")) raw_mode = str(data.get("audioInputMode", "auto")) res["audioInputMode"] = raw_mode if raw_mode in ("auto", "browser") else "auto" - res["autoOpenOnTuningChange"] = bool(data.get("autoOpenOnTuningChange", False)) + # Fail closed: only a real JSON boolean enables the opt-in. A hand-edited / + # migrated / bad-client value (e.g. the string "false" or "0") must NOT be + # coerced to True by bool(). + _auto_open = data.get("autoOpenOnTuningChange", False) + res["autoOpenOnTuningChange"] = _auto_open if isinstance(_auto_open, bool) else False if not isinstance(res["customTunings"], dict): res["customTunings"] = {} diff --git a/plugins/tuner/screen.js b/plugins/tuner/screen.js index 638b153..ea30801 100644 --- a/plugins/tuner/screen.js +++ b/plugins/tuner/screen.js @@ -13,6 +13,10 @@ let _lastAutoOpenSessionKey = null; let _autoOpenDismissedSessionKey = null; let _autoOpenGeneration = 0; + // Bumped on every enable()/disable() so an in-flight open (which awaits audio start + // with the panel already visible) can detect it was dismissed mid-open and NOT flip + // _state.enabled on afterwards — avoiding a zombie enabled-but-hidden tuner. + let _openGen = 0; let _onAutoOpenSongLoading = null; let _onAutoOpenSongReady = null; @@ -167,66 +171,177 @@ 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. + // The player's CURRENT physical tuning. Prefer the host-owned live working + // tuning (window.feedBack.workingTuning) — what the instrument is *actually* in + // right now, which advances as the player retunes — so coverage prompts fire in + // BOTH directions (E→C# and back), not only away from a fixed profile. Feature- + // detected: on a host without the working-tuning capability, or before it's been + // set, we fall back to the static /api/settings instrument tuning (today's + // behavior). Returns { midis, refCents } or null. + // The SELECTED physical instrument's working-tuning slot identity, from /api/settings. + // The read (_playerTuning) and the write (_publishWorkingTuning) MUST agree on this + // key — the working tuning is a property of the player's selected instrument, not of + // any one song — or a published tuning lands in a slot coverage never reads back. + // Returns { isBass, sc, key }. + function _selectedInstrument(s) { + const isBass = !!(s && s.instrument === 'bass'); + const sc = (s && Number(s.string_count)) || (isBass ? 4 : 6); + return { isBass, sc, key: (isBass ? 'bass' : 'guitar') + '-' + sc }; + } + async function _playerTuning() { const u = window._tunerUtils; if (!u) return null; - let s; + // Instrument IDENTITY (which instrument is selected) + the static fallback + // tuning, from /api/settings. + let s = null; 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; + catch (_) { s = null; } + const { isBass, sc, key } = _selectedInstrument(s); + // Cache the resolved selection so the (synchronous) publish-on-clear writes to + // the EXACT slot this read path uses — no second /api/settings fetch that could + // race an instrument switch. Only cache when settings were actually read: on a + // fetch failure we keep the last confident selection (or none → publish skips) + // rather than recording a bogus default-instrument slot to publish into later. + // Coverage runs before any auto-open, so this is set by the time a clear can publish. + if (s) { + _state._playerSelected = { isBass, sc, key, refPitch: Number(s.reference_pitch) || 440 }; + } + // The LIVE per-instrument working tuning (advances as the player retunes) — + // this is what makes coverage prompt in BOTH directions, not only away from a + // fixed profile. Feature-detected; falls back to the static settings tuning + // when unset / no host capability. + const wt = (window.feedBack && window.feedBack.workingTuning + && typeof window.feedBack.workingTuning.get === 'function') + ? window.feedBack.workingTuning.get(key) : null; + const wtHasOffsets = !!(wt && Array.isArray(wt.offsets) && wt.offsets.length); let freqs = null; - if (Array.isArray(s.tuning)) { + if (wtHasOffsets) { + freqs = u.offsetsToFreqs(wt.offsets.slice(0, sc), isBass); + } else if (s && 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]; + } else if (s && typeof s.tuning === 'string') { + const named = _state._allTunings && _state._allTunings[key]; if (named && Array.isArray(named[s.tuning])) freqs = named[s.tuning]; } - if (!freqs) freqs = u.offsetsToFreqs(new Array(sc).fill(0), isBass); // standard fallback + if (!freqs) { + // No confident instrument identity — settings absent, OR present but carrying + // no instrument/string_count/tuning (a fresh profile: /api/settings omits them) + // — and no live working tuning. We can't tell the player's tuning, so fail + // toward prompting (null → not-covered) rather than silently assuming standard + // and suppressing a genuinely-needed prompt. + const hasIdentity = !!(s && (s.instrument || s.string_count || s.tuning)); + if (!hasIdentity && !wtHasOffsets) return null; + freqs = u.offsetsToFreqs(new Array(sc).fill(0), isBass); // standard fallback + } const midis = _openMidisFromFreqs(freqs); if (!midis) return null; + const refPitch = (wt && Number(wt.referencePitch)) || (s && Number(s.reference_pitch)) || 440; 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; + // PR: host workingTuning — when the player clears an AUTO-OPENED tuner we assume + // they tuned their (selected) instrument to the song, so publish the song's tuning + // as the live working tuning for that instrument ('assumed' — PR 4's explicit + // "I tuned / Skip" will replace this heuristic). After the instrument->chart routing + // PR the loaded arrangement matches the selected instrument, so the song's tuning IS + // the player's instrument's new tuning. + // + // We write to the SELECTED instrument's slot (the exact key _playerTuning reads), + // NOT a song-derived one, so the publish can't be stranded in a slot coverage never + // looks at. If the cleared song is a chart for the OTHER instrument (a manual switch + // to e.g. the bass part while guitar is selected), we skip — that isn't evidence the + // selected instrument was retuned, and writing it would pollute the wrong slot. + // + // Synchronous, off the selection _playerTuning last resolved (`_state._playerSelected`) + // — an auto-open always runs a coverage check first, so it's populated by the time a + // clear can publish. Reusing it (rather than re-fetching /api/settings here) keeps the + // write key identical to the read key and avoids racing an instrument switch. + function _publishWorkingTuning(songInfo) { + const wt = window.feedBack && window.feedBack.workingTuning; + if (!wt || typeof wt.set !== 'function') return; + if (!songInfo || !Array.isArray(songInfo.tuning) || !songInfo.tuning.length) return; + const sel = _state._playerSelected; + if (!sel || !sel.key || !(sel.sc > 0)) return; // coverage hasn't resolved the instrument yet + 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 songIsBass = (typeof window.feedBack?.isBassArrangement === 'function') + ? window.feedBack.isBassArrangement(ctx) + : (songInfo.arrangement || '').toLowerCase().includes('bass'); + if (songIsBass !== sel.isBass) return; // cross-instrument chart — don't pollute the selected slot + wt.set({ + offsets: songInfo.tuning.slice(0, sel.sc), + stringCount: sel.sc, + instrument: sel.isBass ? 'bass' : 'guitar', + referencePitch: sel.refPitch, + source: 'tuner', + }, { instrument: sel.key, provenance: 'assumed' }); } - // 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) { + // The retune the player would need to match this song, as a structured report: + // { covered, retune: [{ from, to }], reference, cantCover } + // — covered: the physical tuning already matches (the song's open strings are an + // exact contiguous run inside the player's strings) → no retune; + // — retune: the per-string note changes (e.g. { from:'B', to:'A' }) of the best + // contiguous alignment — what the badge cue names; + // — reference: a whole-instrument A4/centOffset mismatch (A440 vs A432, octave); + // — cantCover: the song needs more strings than the instrument has. + // Conservative: any missing data → { covered:false } so a needed prompt/cue is + // never silently dropped on a fetch hiccup. + async function _computeCoverageReport(songInfo) { + const u = window._tunerUtils; + const none = { covered: false, retune: [], reference: false, cantCover: false }; + if (!u) return none; const song = _songOpenMidis(songInfo); - if (!song || !song.length) return false; + if (!song || !song.length) return none; 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); + if (!player || !player.midis.length) return none; + const reference = Math.abs((Number(songInfo?.centOffset) || 0) - player.refCents) > 25; + if (player.midis.length < song.length) return { covered: false, retune: [], reference, cantCover: true }; + // Best contiguous alignment = the run with the fewest per-string mismatches + // (extended-range adds strings at the ends — match by pitch, not index). + let best = null; + for (let start = 0; start + song.length <= player.midis.length; start++) { + const diffs = []; + for (let i = 0; i < song.length; i++) { + const pm = player.midis[start + i]; + if (pm !== song[i]) diffs.push({ from: u.midiToNote(pm, false), to: u.midiToNote(song[i], false) }); + } + if (!best || diffs.length < best.length) best = diffs; + if (!diffs.length) break; + } + const covered = !reference && best.length === 0; + return { covered, retune: covered ? [] : best, reference, cantCover: false }; + } + + // Dedup the coverage computation (which fetches /api/settings): the auto-open gate + // AND the badge cue both call this on the same song:ready. Cache the in-flight/last + // result per song so they share ONE fetch. Invalidated when anything that changes the + // answer happens — a new song (song:loading), an instrument switch (instrument:changed), + // or a retune (working-tuning-changed) — so the cache can never go stale within a song. + let _coverageCache = null; // { key, promise } + function _coverageReport(songInfo) { + const key = _autoOpenSessionKey(songInfo) + '|' + + (songInfo && Array.isArray(songInfo.tuning) ? songInfo.tuning.join(',') : '') + + '|' + (songInfo && songInfo.centOffset != null ? songInfo.centOffset : ''); // coverage uses centOffset + if (_coverageCache && _coverageCache.key === key) return _coverageCache.promise; + const promise = _computeCoverageReport(songInfo); + _coverageCache = { key, promise }; + return promise; + } + function _invalidateCoverageCache() { _coverageCache = null; } + + // Boolean form used to gate the auto-open prompt. + async function _coveredByPlayerInstrument(songInfo) { + return (await _coverageReport(songInfo)).covered; } function _onAutoOpenSongLoadingHandler() { _autoOpenGeneration++; _autoOpenDismissedSessionKey = null; _lastAutoOpenSessionKey = null; + _invalidateCoverageCache(); } async function _maybeAutoOpenOnTuningChange() { @@ -289,6 +404,14 @@ _onAutoOpenSongReady = () => { _maybeAutoOpenOnTuningChange(); }; 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 + // switches instrument. Drop the cached selection so a publish-on-clear can't write + // to the previously-selected instrument's slot; the next coverage read re-resolves + // it. Until then _publishWorkingTuning skips (safe — no mis-slotted write). + window.feedBack.on('instrument:changed', () => { _state._playerSelected = null; _invalidateCoverageCache(); }); + // A retune (working tuning published on a tuner clear) changes coverage for the + // current song — drop the cached report so a re-evaluation recomputes it. + window.feedBack.on('working-tuning-changed', _invalidateCoverageCache); } // ── Player sync helpers ─────────────────────────────────────────── @@ -446,6 +569,7 @@ async function enable(opts) { if (_state.enabled) return; + const myOpen = ++_openGen; // this open's token; a disable()/newer open invalidates it // An AUTO-open (the "this song needs a different tuning" nudge) must // PERSIST: it is NOT dismissed by the autoplay song:play that follows // song entry, a stray click, or a same-screen re-emit — only by the @@ -518,6 +642,10 @@ { deviceId: _state.selectedDeviceId, channel: _state.selectedChannel, audioInputMode: _state.audioInputMode }, _tunerUIApi.updateUI ); + // The panel is visible (with ×/Skip) across the audio-start await above, so a + // dismiss can land here. If so, disable() already tore the panel down and + // bumped _openGen — do NOT flip enabled on (that would leave enabled-but-hidden). + if (myOpen !== _openGen) return; _state.enabled = true; if (window.tuner?.updateButtons) window.tuner.updateButtons(); } catch (e) { @@ -528,7 +656,9 @@ } function disable() { + _openGen++; // invalidate any in-flight enable() so it won't re-enable after this teardown const wasEnabled = _state.enabled; + const wasAutoOpened = _state.autoOpened; const onPlayer = document.getElementById('player')?.classList.contains('active'); _state.enabled = false; _state.autoOpened = false; @@ -550,7 +680,13 @@ } if (wasEnabled && onPlayer) { const songInfo = window.highway?.getSongInfo?.() || window.feedBack?.currentSong; - if (songInfo) _autoOpenDismissedSessionKey = _autoOpenSessionKey(songInfo); + if (songInfo) { + _autoOpenDismissedSessionKey = _autoOpenSessionKey(songInfo); + // Clearing an auto-opened tuner = the player tuned to this song: + // publish the song's tuning as their instrument's live working tuning + // so coverage stops nagging for it (and prompts on the way back). + if (wasAutoOpened) _publishWorkingTuning(songInfo); + } } } @@ -600,6 +736,9 @@ sessionKey: _autoOpenSessionKey, maybeAutoOpenOnTuningChange: _maybeAutoOpenOnTuningChange, coveredByPlayerInstrument: _coveredByPlayerInstrument, + coverageReport: _coverageReport, + playerTuning: _playerTuning, + publishWorkingTuning: _publishWorkingTuning, onSongLoading: _onAutoOpenSongLoadingHandler, getState() { return { diff --git a/static/v3/badges.js b/static/v3/badges.js index a816375..609d90b 100644 --- a/static/v3/badges.js +++ b/static/v3/badges.js @@ -25,6 +25,13 @@ // Tuning names per instrument key (e.g. 'guitar-6', 'bass-4'), loaded from // GET /api/tunings. Falls back to empty arrays until the fetch resolves. let _tuningsByKey = {}; + // Last instrument-coverage report for the current song (from the tuner plugin) — + // drives a passive "different tuning" cue on the tuner badge. null = covered / + // unknown / off the player. + let _lastCoverageReport = null; + // Monotonic token so a slow coverage fetch can't restore a stale cue after a newer + // song started loading / we left the player. Bumped on every refresh and clear. + let _coverageCueToken = 0; function _tuningsForKey(key) { return Object.keys(_tuningsByKey[key] || {}); } function _tuningsForInstrument(instrument, string_count) { return _tuningsForKey(instrument + '-' + string_count); @@ -257,6 +264,52 @@ openTuner(); }); _applyFrame(_lastFrame); + _applyCoverageCue(_lastCoverageReport); + } + + // Passive "different tuning" cue on the tuner badge: an amber ring + a tooltip + // naming the retune (e.g. "B→A"). The diff comes from the tuner plugin's coverage + // report; an absent plugin or a covered song → no cue. CSS-free (inline ring + + // native title) so it needs no Tailwind rebuild, and it never auto-opens the + // panel — it's advisory; the user taps the badge to tune. + function _applyCoverageCue(report) { + const btn = document.querySelector('#v3-badge-tuner [data-open-tuner]'); + if (!btn) return; + const needs = !!(report && !report.covered); + btn.style.boxShadow = needs ? '0 0 0 2px #fbbf24' : ''; + if (!needs) { btn.title = 'Open tuner'; return; } + const summary = report.cantCover ? 'a different instrument' + : (report.retune && report.retune.length) + ? report.retune.map((d) => d.from + '→' + d.to).join(', ') + : 'the reference pitch'; + btn.title = 'This song needs a different tuning — retune ' + summary + '. Click to tune.'; + } + + // A coverage report only drives the cue when it carries an actual signal: covered + // (clears the ring) or a nameable mismatch (retune / reference / cantCover). The + // plugin returns a conservative all-false report on a fetch hiccup / missing data — + // that's "unknown", NOT "needs retune", so collapse it to null (no cue) rather than + // painting an amber "retune the reference pitch" ring with no evidence. + function _meaningfulReport(report) { + if (!report) return null; + if (report.covered) return report; + if (report.cantCover || report.reference || (report.retune && report.retune.length)) return report; + return null; + } + + async function _refreshCoverageCue() { + const myToken = ++_coverageCueToken; + const songInfo = window.highway && window.highway.getSongInfo && window.highway.getSongInfo(); + const api = window._tunerAutoOpen; + if (!songInfo || !api || typeof api.coverageReport !== 'function') { + _lastCoverageReport = null; _applyCoverageCue(null); return; + } + let report = null; + try { report = await api.coverageReport(songInfo); } + catch (_e) { report = null; } + if (myToken !== _coverageCueToken) return; // superseded by a newer song / a clear + _lastCoverageReport = _meaningfulReport(report); + _applyCoverageCue(_lastCoverageReport); } // ── Instrument selector card (Stitch RightInstrumentSelector) ──────────-- @@ -374,6 +427,13 @@ await loadTunings(); renderInstrument(); }); + // Passive coverage cue: recompute when a song is ready; clear when a new + // song starts loading or we leave the player screen. + sm.on('song:ready', () => { _refreshCoverageCue(); }); + sm.on('song:loading', () => { _coverageCueToken++; _lastCoverageReport = null; _applyCoverageCue(null); }); + sm.on('screen:changed', (e) => { + if (!e || !e.detail || e.detail.id !== 'player') { _coverageCueToken++; _lastCoverageReport = null; _applyCoverageCue(null); } + }); } } if (document.readyState === 'loading') document.addEventListener('DOMContentLoaded', boot, { once: true }); diff --git a/tests/js/tuner_auto_open.test.js b/tests/js/tuner_auto_open.test.js index 3527090..33cf9b0 100644 --- a/tests/js/tuner_auto_open.test.js +++ b/tests/js/tuner_auto_open.test.js @@ -425,7 +425,188 @@ test('coverage: with no declared instrument it stays conservative (prompts as be assert.equal(sandbox.__enableCalls.length, 1); }); +// ── §4 coverage report + badge cue (E1.6) ────────────────────────────────── +test('coverage report names the string(s) to retune', async () => { + // Note: report objects come from the vm sandbox realm, so compare fields, not + // deepStrictEqual (which checks prototype identity across realms). + const sandbox = createTunerSandbox({ player: PLAYER_GUITAR_8_FS }); + const rep = (s) => sandbox.window._tunerAutoOpen.coverageReport(s); + const covered = await rep(E_STANDARD); + assert.equal(covered.covered, true); + assert.equal(covered.retune.length, 0); + assert.equal(covered.reference, false); + const dropA = await rep(SONG_DROP_A7); + assert.equal(dropA.covered, false); + assert.equal(dropA.retune.length, 1); + assert.equal(dropA.retune[0].from, 'B'); // the user's exact case → "retune B → A" + assert.equal(dropA.retune[0].to, 'A'); +}); + +test('coverage report flags a whole-instrument reference mismatch', async () => { + const sandbox = createTunerSandbox({ player: { ...PLAYER_GUITAR_6, reference_pitch: 432 } }); + const rep = await sandbox.window._tunerAutoOpen.coverageReport(E_STANDARD); + assert.equal(rep.covered, false); + assert.equal(rep.reference, true); +}); + +test('the tuner badge surfaces a passive coverage cue (badges.js)', () => { + const badgesSrc = fs.readFileSync( + path.join(__dirname, '..', '..', 'static', 'v3', 'badges.js'), 'utf8'); + // Recomputes via the tuner plugin's coverageReport on song:ready … + assert.match(badgesSrc, /api\.coverageReport/); + assert.match(badgesSrc, /sm\.on\('song:ready'/); + // … and shows an advisory ring + tooltip naming the retune (it never auto-opens). + assert.match(badgesSrc, /function _applyCoverageCue/); + assert.match(badgesSrc, /report\.retune/); + assert.match(badgesSrc, /boxShadow/); +}); + test('auto-open does not require app.js changes', () => { const appSrc = fs.readFileSync(APP_JS, 'utf8'); assert.doesNotMatch(appSrc, /_tunerAutoOpen|maybeAutoOpenOnTuningChange/); }); + +// ── PR 3: per-instrument live working tuning (the both-directions fix) ────── +test('coverage reads the live per-instrument working tuning, so it prompts BOTH directions', async () => { + // GUITAR-6 selected; the player's LIVE working tuning is Drop-D (they retuned). + const sandbox = createTunerSandbox({ player: { instrument: 'guitar', string_count: 6, tuning: 'Standard' } }); + sandbox.window.feedBack.workingTuning = { + get: () => ({ offsets: [-2, 0, 0, 0, 0, 0], stringCount: 6, instrument: 'guitar', referencePitch: 440 }), + set() {}, + }; + const rep = (s) => sandbox.window._tunerAutoOpen.coverageReport(s); + // The Drop-D song now MATCHES the live tuning → covered (no prompt). + assert.equal((await rep(DROP_D)).covered, true); + // An E-standard song NO LONGER matches (the player is in Drop-D) → not covered → + // prompts to tune the low string back UP to E. The old static-profile logic missed + // this "coming back" direction entirely. + const estd = await rep(E_STANDARD); + assert.equal(estd.covered, false); + assert.equal(estd.retune.length, 1); + assert.equal(estd.retune[0].from, 'D'); // player low string is D… + assert.equal(estd.retune[0].to, 'E'); // …song wants E → "tune D → E" (up) +}); + +// Wire a set-capturing workingTuning stub, then run a coverage read so the tuner caches +// the selected-instrument identity (publish is synchronous and writes to that cached +// slot — an auto-open always runs coverage first, so this mirrors real ordering). +async function _primeSets(sandbox, get = () => ({ offsets: null })) { + const sets = []; + sandbox.window.feedBack.workingTuning = { get, set: (state, opts) => sets.push({ state, opts }) }; + await sandbox.window._tunerAutoOpen.playerTuning(); + return sets; +} + +test('clearing the auto-opened tuner publishes the song tuning to the right instrument slot', async () => { + const sandbox = createTunerSandbox({ player: { instrument: 'guitar', string_count: 6, tuning: 'Standard' } }); + const sets = await _primeSets(sandbox); + sandbox.window._tunerAutoOpen.publishWorkingTuning(DROP_D); + assert.equal(sets.length, 1); + assert.deepEqual(sets[0].state.offsets, [-2, 0, 0, 0, 0, 0]); // the song's tuning + assert.equal(sets[0].state.instrument, 'guitar'); + assert.equal(sets[0].opts.instrument, 'guitar-6'); // targets the guitar slot + assert.equal(sets[0].opts.provenance, 'assumed'); // a guess, not mic-verified +}); + +test('publish targets the SELECTED instrument slot, not a song-derived one (string-count mismatch)', async () => { + // A 5-string bass is selected; the cleared song is a 4-string bass chart. The publish + // must land in bass-5 (what coverage reads), NOT bass-4 (where it would be stranded). + const sandbox = createTunerSandbox({ player: { instrument: 'bass', string_count: 5, tuning: 'Standard' } }); + const sets = await _primeSets(sandbox); + sandbox.window._tunerAutoOpen.publishWorkingTuning(BASS_EADG); + assert.equal(sets.length, 1); + assert.equal(sets[0].opts.instrument, 'bass-5'); // the selected instrument's slot + assert.equal(sets[0].state.instrument, 'bass'); +}); + +test('publish skips a cross-instrument chart (bass arrangement while guitar is selected)', async () => { + // Guitar selected, but the player manually opened the Bass arrangement and cleared. + // That is not evidence the guitar was retuned — do not pollute either slot. + const sandbox = createTunerSandbox({ player: { instrument: 'guitar', string_count: 6, tuning: 'Standard' } }); + const sets = await _primeSets(sandbox); + sandbox.window._tunerAutoOpen.publishWorkingTuning(BASS_EADG); + assert.equal(sets.length, 0); +}); + +test('publish carries the player reference pitch so the slot is self-consistent', async () => { + const sandbox = createTunerSandbox({ player: { instrument: 'guitar', string_count: 6, tuning: 'Standard', reference_pitch: 432 } }); + const sets = await _primeSets(sandbox); + sandbox.window._tunerAutoOpen.publishWorkingTuning(DROP_D); + assert.equal(sets.length, 1); + assert.equal(sets[0].state.referencePitch, 432); +}); + +test('publish skips when the instrument could not be confidently resolved (settings unreadable)', async () => { + // No player settings → /api/settings reports not-ok → we never cached a confident + // selection, so publish must NOT write to a guessed default slot. + const sandbox = createTunerSandbox(); // no player + const sets = await _primeSets(sandbox); + sandbox.window._tunerAutoOpen.publishWorkingTuning(DROP_D); + assert.equal(sets.length, 0); +}); + +// ── #655 fix: transactional open — a dismiss during the audio-start await must not +// re-enable the tuner afterward (no zombie enabled-but-hidden state). See issue #675. +test('dismiss during the audio-start await does not leave a zombie enabled tuner', async () => { + const sandbox = createTunerSandbox({ player: { instrument: 'guitar', string_count: 6, tuning: 'Standard' } }); + // Minimal UI so real enable() gets past the panel-show line to the audio-start await + // (the default sandbox _tunerUI is a no-op that never creates uiContainer). + const el = () => ({ + classList: { add() {}, remove() {}, toggle() {}, contains: () => false }, + querySelector: () => null, appendChild() {}, remove() {}, style: {}, + }); + const origTunerUI = sandbox.window._tunerUI; // the sandbox's full no-op method set + sandbox.window._tunerUI = (state, actions) => { + const api = origTunerUI(state, actions); + state.uiContainer = el(); + state.vizContainer = el(); + state.skipBtn = el(); + api.showMicError = api.showMicError || (() => {}); + return api; + }; + // The OPEN's audio start resolves only AFTER a ×/Skip dismiss has landed — i.e. the + // user dismissed while audio was starting. (disable()'s own background-audio resume + // is a later call and resolves at once.) + let firstStart = true; + sandbox.window._tunerAudio.start = () => { + if (!firstStart) return Promise.resolve(); + firstStart = false; + return Promise.resolve().then(() => { sandbox.window.tuner.disable(); }); + }; + await sandbox.window.tuner.enable({ auto: true }); + assert.equal(sandbox.window._tunerAutoOpen.getState().enabled, false, + 'a mid-open dismiss must win — the tuner stays disabled, not enabled-but-hidden'); +}); +// ── #656 fix: coverage stays conservative when the instrument identity is unknown. +// A fresh profile (/api/settings omits instrument/string_count/tuning) must NOT be +// assumed to be standard guitar and silently suppress the prompt. See issue #677. +test('coverage is conservative when settings carry no instrument identity', async () => { + const sandbox = createTunerSandbox({ player: {} }); // settings object, but no instrument fields + const covered = await sandbox.window._tunerAutoOpen.coveredByPlayerInstrument(E_STANDARD); + assert.equal(covered, false, 'unknown instrument → not covered → still prompt'); +}); + +test('a configured standard-guitar player still covers a standard song', async () => { + const sandbox = createTunerSandbox({ player: { instrument: 'guitar', string_count: 6, tuning: 'Standard' } }); + const covered = await sandbox.window._tunerAutoOpen.coveredByPlayerInstrument(E_STANDARD); + assert.equal(covered, true, 'a known standard guitar covers a standard song (no regression)'); +}); +// ── #657 fix (#680): coverage is deduped — the auto-open gate and the badge cue both +// call coverageReport() on the same song:ready; they must share ONE /api/settings fetch. +test('coverage reports for the same song share one settings fetch, and a new song refetches', async () => { + const sandbox = createTunerSandbox({ player: { instrument: 'guitar', string_count: 6, tuning: 'Standard' } }); + let settingsFetches = 0; + const origFetch = sandbox.window.fetch; + sandbox.window.fetch = (url) => { + if (String(url).includes('/api/settings')) settingsFetches += 1; + return origFetch(url); + }; + const api = sandbox.window._tunerAutoOpen; + const [a, b] = await Promise.all([api.coverageReport(DROP_D), api.coverageReport(DROP_D)]); + assert.equal(settingsFetches, 1, 'concurrent reports for the same song share one fetch'); + assert.deepEqual(a, b); + // A new song invalidates the cache → a fresh fetch. + api.onSongLoading(); + await api.coverageReport(E_STANDARD); + assert.equal(settingsFetches, 2, 'a new song refetches'); +}); diff --git a/tests/plugins/tuner/test_config.py b/tests/plugins/tuner/test_config.py index be176ac..ad405de 100644 --- a/tests/plugins/tuner/test_config.py +++ b/tests/plugins/tuner/test_config.py @@ -95,6 +95,20 @@ class TestConfigPersistence: client.post("/api/plugins/tuner/config", json={"audioInputMode": "browser"}) assert client.get("/api/plugins/tuner/config").json()["audioInputMode"] == "browser" + def test_auto_open_defaults_false(self, client): + assert client.get("/api/plugins/tuner/config").json()["autoOpenOnTuningChange"] is False + + def test_auto_open_true_accepted(self, client): + client.post("/api/plugins/tuner/config", json={"autoOpenOnTuningChange": True}) + assert client.get("/api/plugins/tuner/config").json()["autoOpenOnTuningChange"] is True + + def test_auto_open_fail_closed_on_non_bool(self, client): + # A hand-edited / bad-client non-boolean (e.g. the string "false") must NOT be + # coerced to True by bool() — the opt-in stays off. + for bad in ("false", "0", "1", "yes", 1, {}): + client.post("/api/plugins/tuner/config", json={"autoOpenOnTuningChange": bad}) + assert client.get("/api/plugins/tuner/config").json()["autoOpenOnTuningChange"] is False, bad + def test_disabled_tunings_strips_entries_without_colon(self, client): client.post("/api/plugins/tuner/config", json={ "disabledTunings": ["guitar-6:Drop D", "legacy-entry", "bass-4:Standard"]