From 8341d2141672f085ffcb48c62dc577c424fb0b1e Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Sat, 18 Jul 2026 23:36:57 -0500 Subject: [PATCH] Use instrument tuning in playlist checks --- lib/metadata_db.py | 15 ++++++++++-- static/v3/playlists.js | 30 +++++++++++------------ tests/js/v3_playlist_tuning_check.test.js | 22 ++++++++++++++++- tests/test_playlists_api.py | 20 +++++++++++++++ 4 files changed, 69 insertions(+), 18 deletions(-) diff --git a/lib/metadata_db.py b/lib/metadata_db.py index 2ebf3d0..c877381 100644 --- a/lib/metadata_db.py +++ b/lib/metadata_db.py @@ -2715,7 +2715,9 @@ class MetadataDB: rows = self.conn.execute( f"""SELECT ps.filename, ps.position, s.title, s.artist, s.tuning_name, ps.arrangement, ps.work_key, s.arrangements, - (s.filename IS NULL) AS dead, s.tuning_offsets + (s.filename IS NULL) AS dead, s.tuning_offsets, + s.bass_tuning_name, s.bass_tuning_offsets, + s.rhythm_tuning_name, s.rhythm_tuning_offsets FROM playlist_songs ps LEFT JOIN songs s ON s.filename = ps.filename WHERE ps.playlist_id = ? {dead_filter} ORDER BY ps.position, ps.filename""", @@ -2733,6 +2735,10 @@ class MetadataDB: # rows are different tunings), and coverage needs to know whether to # measure against bass or guitar base pitches. "tuning_offsets": r[9] or "", + "bass_tuning_name": r[10] or "", + "bass_tuning_offsets": r[11] or "", + "rhythm_tuning_name": r[12] or "", + "rhythm_tuning_offsets": r[13] or "", "bass_only": _arrangements_all_bass(r[7]), "art_url": f"/api/song/{quote(r[0])}/art", } @@ -2762,7 +2768,8 @@ class MetadataDB: self._ensure_work_display() row = self.conn.execute( "SELECT wd.filename, s.title, s.artist, s.tuning_name, s.arrangements, " - "s.tuning_offsets " + "s.tuning_offsets, s.bass_tuning_name, s.bass_tuning_offsets, " + "s.rhythm_tuning_name, s.rhythm_tuning_offsets " "FROM work_display wd JOIN songs s ON s.filename = wd.filename " "WHERE wd.effective_work_key = ? AND wd.is_group_representative = 1", (work_key,)).fetchone() @@ -2777,6 +2784,10 @@ class MetadataDB: return {"resolved_filename": row[0], "title": row[1] or row[0], "artist": row[2] or "", "tuning_name": row[3] or "", "tuning_offsets": row[5] or "", + "bass_tuning_name": row[6] or "", + "bass_tuning_offsets": row[7] or "", + "rhythm_tuning_name": row[8] or "", + "rhythm_tuning_offsets": row[9] or "", "bass_only": _arrangements_all_bass(row[4]), "arrangements": arrs, "art_url": f"/api/song/{quote(row[0])}/art", diff --git a/static/v3/playlists.js b/static/v3/playlists.js index 3774f77..9527b54 100644 --- a/static/v3/playlists.js +++ b/static/v3/playlists.js @@ -61,25 +61,25 @@ // without stopping. This flags them. It is READ-ONLY: nothing here edits a // playlist — removal is a separate, explicit, itemised action. - // ── SEAM — repoint HERE when `feat/v3-tuning-filter-instrument` lands ──── - // The tuning this row should be scored against for the player's instrument. - // main indexes exactly ONE tuning per chart, derived from the guitar - // arrangement, so today a bass player is scored against the guitar chart's - // tuning — the very data source behind the reported bug. The sibling PR adds - // per-instrument tuning columns (`bass_tuning_name` / `bass_tuning_offsets`) - // plus a `libInstrument()` perspective; when it lands, this ONE function - // picks the bass tuning for a bass player and everything downstream (the - // chips, the summary, the filter, the remove list) follows with no other - // change. Kept as a function, not an inline read, for exactly that reason. + // Pick the indexed perspective that matches the player's live instrument. + // #1003 supplies bass-specific columns; when a song has no bass chart we + // deliberately fall back to the historical song-level guitar tuning. function rowTuningForCheck(s) { + let wantsBass = false; + try { + const wt = window.feedBack && window.feedBack.workingTuning; + const cur = wt && typeof wt.get === 'function' ? wt.get() : null; + wantsBass = !!cur && cur.instrument === 'bass'; + } catch (_) { /* capability errors degrade to the song-level tuning */ } + const hasBassTuning = wantsBass && !!s.bass_tuning_offsets; return { - offsets: s.tuning_offsets || s.tuning_name, - // Score a bass-only chart against bass base pitches — a 4-string bass - // tuning read as guitar can false-match a guitarist. - isBass: !!s.bass_only, + offsets: hasBassTuning + ? s.bass_tuning_offsets : (s.tuning_offsets || s.tuning_name), + // The selected bass perspective uses bass base pitches. A bass-only + // fallback row does too; every other fallback is the lead chart. + isBass: hasBassTuning || !!s.bass_only, }; } - // A coverage report says "not covered" BOTH for a real mismatch and for // "I couldn't work it out" (missing settings/tuner data → an all-empty // report). Only a report carrying an actual reason — named string changes, diff --git a/tests/js/v3_playlist_tuning_check.test.js b/tests/js/v3_playlist_tuning_check.test.js index b34f95d..db17d91 100644 --- a/tests/js/v3_playlist_tuning_check.test.js +++ b/tests/js/v3_playlist_tuning_check.test.js @@ -60,7 +60,7 @@ function loadChecker(opts) { const calls = []; const window = { parseRawTuningOffsets: loadParseRawTuningOffsets(), - feedBack: opts.noWorkingTuning ? {} : { workingTuning: { get: () => ({ instrument: 'bass' }) } }, + feedBack: opts.noWorkingTuning ? {} : { workingTuning: { get: () => ({ instrument: opts.instrument || 'bass' }) } }, _tunerAutoOpen: opts.noCoverage ? undefined : { coverageReport: async (info) => { calls.push(info); @@ -176,6 +176,26 @@ test('a coverage call that throws degrades to unknown, not mismatch', async () = assert.deepEqual(plain(out.map((r) => r.state)), ['unknown']); }); +test('the bass perspective uses #1003 bass offsets instead of guitar offsets', async () => { + const checker = loadChecker({ instrument: 'bass', coverage: async () => REPORT_COVERED }); + await checker.checkPlaylistTuning([song({ + tuning_offsets: '0 0 0 0 0 0', + bass_tuning_offsets: '-2 -2 -2 -2 -2 -2', + })]); + assert.deepEqual(plain(checker.calls[0].tuning), [-2, -2, -2, -2, -2, -2]); + assert.equal(checker.calls[0].arrangement, 'Bass'); +}); + +test("the guitar perspective ignores a song's bass offsets", async () => { + const checker = loadChecker({ instrument: 'guitar', coverage: async () => REPORT_COVERED }); + await checker.checkPlaylistTuning([song({ + tuning_offsets: '0 0 0 0 0 0', + bass_tuning_offsets: '-2 -2 -2 -2 -2 -2', + })]); + assert.deepEqual(plain(checker.calls[0].tuning), [0, 0, 0, 0, 0, 0]); + assert.equal(checker.calls[0].arrangement, 'Lead'); +}); + test('a bass-only chart is scored against bass base pitches', async () => { // Otherwise a 4-string bass tuning read as guitar can false-match — the // cross-instrument confusion this whole feature exists to undo. diff --git a/tests/test_playlists_api.py b/tests/test_playlists_api.py index 0518d91..8e2984f 100644 --- a/tests/test_playlists_api.py +++ b/tests/test_playlists_api.py @@ -285,6 +285,26 @@ def test_playlist_songs_carry_tuning_offsets_for_the_check(client, server): assert song["tuning_name"] == "Drop D" +def test_playlist_songs_carry_role_specific_tunings(client, server): + db = server.meta_db + db.put("roles.archive", 0, 0, { + "title": "Roles", + "tuning_name": "E Standard", + "tuning_offsets": "0 0 0 0 0 0", + "bass_tuning_name": "A Standard", + "bass_tuning_offsets": "-2 -2 -2 -2 -2 -2", + "rhythm_tuning_name": "Drop D", + "rhythm_tuning_offsets": "-2 0 0 0 0 0", + }) + pid = client.post("/api/playlists", json={"name": "Roles"}).json()["id"] + client.post(f"/api/playlists/{pid}/songs", json={"filename": "roles.archive"}) + song = client.get(f"/api/playlists/{pid}").json()["songs"][0] + assert song["bass_tuning_name"] == "A Standard" + assert song["bass_tuning_offsets"] == "-2 -2 -2 -2 -2 -2" + assert song["rhythm_tuning_name"] == "Drop D" + assert song["rhythm_tuning_offsets"] == "-2 0 0 0 0 0" + + def test_playlist_songs_flag_bass_only_charts(client, server): # Every arrangement a bass part → bass_only, so coverage scores the row # against bass strings. A chart that ALSO has a guitar part must not be