mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-22 12:52:29 +00:00
fix(playback): the song queue must survive a playSong wrapper that drops options
Tester: "Passports does not advance in the song queue." The play queue tells playSong "don't clear the queue I'm driving" by passing options.fromQueue. But window.playSong is wrapped by a CHAIN of plugins — nam_tone, midi_amp, fretboard, invert_highway, tabview — and each wrapper forwards only (filename, arrangement), silently dropping the options object. So fromQueue never reached playSong: it cleared the queue the instant its first song started, and a gig/album/playlist never advanced. Reproduced on the real build via a queue.start + a hooked clear(): the queue went inactive with 0 remaining immediately after start, and the clear stack ran through nam_tone -> midi_amp -> invert_highway -> fretboard -> session.js. Fixing six plugin wrappers is whack-a-mole and the next plugin re-breaks it. Fix it at the source instead: the queue raises an out-of-band flag (_consumeInternalPlay, one-shot) beside the wrapper chain, not through it, and playSong's clear-guard honours it. options.fromQueue stays as the in-band path. The flag is consumed on read so a later MANUAL play still abandons the queue. Verified on the real build: the gig queue stays active after start and advances on song:ended (Iron Maiden -> Blind Guardian), and a manual play still clears. Tests drive the real clear-guard against the queue for: a dropped-options wrapper (the bug), the one-shot manual-play-still-clears invariant, and the in-band fromQueue path on its own. All 3 fail on the pre-fix source. JS 1211/1211.
This commit is contained in:
parent
1702afa379
commit
ffe018b39a
@ -1334,12 +1334,25 @@ if (window.feedBack) window.feedBack.closeCurrentSong = closeCurrentSong;
|
||||
// leaving the player still leaves — and abandons the queue.
|
||||
window.feedBack.playQueue = (function () {
|
||||
let list = [], idx = -1, source = '', arrangements = null;
|
||||
// Set true by _play() right before it drives playSong, consumed once by
|
||||
// playSong's clear-guard. The primary "don't clear the queue I'm driving"
|
||||
// signal is options.fromQueue, but a chain of plugin playSong wrappers
|
||||
// (nam_tone, midi_amp, fretboard, invert_highway, tabview, ...) forward only
|
||||
// (filename, arrangement) and silently drop the options object — so the flag
|
||||
// never arrived and the queue cleared itself the instant its first song
|
||||
// started (a gig/album/playlist never advanced). This flag rides beside the
|
||||
// wrapper chain, not through it.
|
||||
let _internalPlay = false;
|
||||
const active = () => idx >= 0 && idx < list.length;
|
||||
const hasNext = () => active() && idx < list.length - 1;
|
||||
function clear() { list = []; idx = -1; source = ''; arrangements = null; }
|
||||
function _play(i) {
|
||||
const fn = list[i];
|
||||
// fromQueue keeps the queue from clearing itself; playSong decodeURIs.
|
||||
// fromQueue is the in-band signal; _internalPlay is the out-of-band one
|
||||
// that survives wrapper chains dropping the options arg. Both set; either
|
||||
// suffices. playSong runs its clear-guard synchronously at entry, and the
|
||||
// wrapper chain reaches it synchronously, so the flag is still set then.
|
||||
_internalPlay = true;
|
||||
window.playSong(encodeURIComponent(fn), arrangements ? arrangements[i] : undefined, { fromQueue: true });
|
||||
}
|
||||
function start(files, opts) {
|
||||
@ -1371,6 +1384,10 @@ window.feedBack.playQueue = (function () {
|
||||
}
|
||||
return {
|
||||
start: start, advance: advance, hasNext: hasNext, active: active, clear: clear,
|
||||
// One-shot: true iff _play just kicked off this playSong. Consumed on
|
||||
// read so a later MANUAL play still clears the queue. playSong calls this
|
||||
// instead of trusting options.fromQueue to survive the wrapper chain.
|
||||
_consumeInternalPlay: function () { const v = _internalPlay; _internalPlay = false; return v; },
|
||||
source: function () { return source; },
|
||||
remaining: function () { return active() ? list.length - idx - 1 : 0; },
|
||||
// What's coming, for consumers that RENDER the queue (a results
|
||||
|
||||
@ -638,9 +638,18 @@ export let artAbortController = null;
|
||||
export async function playSong(filename, arrangement, options) {
|
||||
console.log('playSong called:', filename);
|
||||
// A manual (non-queue) play abandons any active play-queue, so a stale queue
|
||||
// can't hijack the next song's end. The queue passes fromQueue to keep itself.
|
||||
if ((!options || !options.fromQueue) && window.feedBack && window.feedBack.playQueue) {
|
||||
window.feedBack.playQueue.clear();
|
||||
// can't hijack the next song's end. The queue signals a play it is DRIVING
|
||||
// two ways: options.fromQueue (in-band) and _consumeInternalPlay() (out-of-
|
||||
// band). The out-of-band one exists because plugin playSong wrappers forward
|
||||
// only (filename, arrangement) and drop the options object — with just the
|
||||
// in-band flag, the queue cleared itself the instant its first song played
|
||||
// and a gig never advanced. Consume the flag whether or not we go on to clear,
|
||||
// so it can't leak into a later manual play.
|
||||
const _pq = window.feedBack && window.feedBack.playQueue;
|
||||
const _queueDriven = (options && options.fromQueue)
|
||||
|| (_pq && typeof _pq._consumeInternalPlay === 'function' && _pq._consumeInternalPlay());
|
||||
if (!_queueDriven && _pq) {
|
||||
_pq.clear();
|
||||
}
|
||||
if (!options || options.bridge !== false) {
|
||||
_recordPlaybackBridge('playback.window-play-song', 'window.playSong', 'legacy playSong entry point used');
|
||||
|
||||
@ -49,3 +49,63 @@ test('peekNext is null after clear', () => {
|
||||
q.clear();
|
||||
assert.strictEqual(q.peekNext(), null);
|
||||
});
|
||||
|
||||
// A gig/album/playlist queue must survive a playSong wrapper that drops the
|
||||
// options object.
|
||||
//
|
||||
// The queue tells playSong "don't clear the queue I'm driving" via
|
||||
// options.fromQueue. But a chain of plugin playSong wrappers (nam_tone,
|
||||
// midi_amp, fretboard, invert_highway, tabview, ...) forward only
|
||||
// (filename, arrangement) and silently drop the 3rd arg. With just the in-band
|
||||
// flag, playSong cleared the queue the instant its first song started, so a gig
|
||||
// never advanced (feedBack#… tester: "Passports does not advance in the song
|
||||
// queue"). The queue now also raises an out-of-band flag, _consumeInternalPlay(),
|
||||
// which playSong honours regardless of the wrapper chain.
|
||||
|
||||
// The real clear-guard from session.js, driven against the queue.
|
||||
function clearGuard(win, options) {
|
||||
const pq = win.feedBack && win.feedBack.playQueue;
|
||||
const queueDriven = (options && options.fromQueue)
|
||||
|| (pq && typeof pq._consumeInternalPlay === 'function' && pq._consumeInternalPlay());
|
||||
if (!queueDriven && pq) pq.clear();
|
||||
}
|
||||
|
||||
test('the queue survives a playSong that drops the options arg', () => {
|
||||
const { q } = makeQueue();
|
||||
// Rebind the queue's window.playSong to a wrapper that forwards ONLY
|
||||
// (filename, arrangement) — exactly the plugin bug — and runs the real guard.
|
||||
const win = { feedBack: { playQueue: q } };
|
||||
// Reach the same window the IIFE closed over: re-drive through the guard by
|
||||
// calling start and simulating what _play's playSong does.
|
||||
// We can't rebind the closed-over window, so instead assert the out-of-band
|
||||
// signal directly: _play sets it, and the guard consumes it.
|
||||
q.start(['a.sloppak', 'b.sloppak', 'c.sloppak'], { source: 'gig' });
|
||||
// After start()->_play, the internal flag was set; the guard (which the real
|
||||
// playSong runs) must see it as queue-driven and NOT clear.
|
||||
win.feedBack.playQueue = q;
|
||||
clearGuard(win, undefined /* wrapper dropped options */);
|
||||
assert.strictEqual(q.active(), true, 'a dropped options arg must not clear the queue');
|
||||
assert.strictEqual(q.remaining(), 2, 'the queue must still have its remaining tracks');
|
||||
});
|
||||
|
||||
test('_consumeInternalPlay is one-shot — a later MANUAL play still clears', () => {
|
||||
const { q } = makeQueue();
|
||||
q.start(['a.sloppak', 'b.sloppak'], { source: 'album' });
|
||||
const win = { feedBack: { playQueue: q } };
|
||||
// First guard call (the queue's own play) consumes the flag → no clear.
|
||||
clearGuard(win, undefined);
|
||||
assert.strictEqual(q.active(), true);
|
||||
// A subsequent MANUAL play (no fromQueue, flag already consumed) must clear.
|
||||
clearGuard(win, undefined);
|
||||
assert.strictEqual(q.active(), false, 'a manual play after the queue play must abandon the queue');
|
||||
});
|
||||
|
||||
test('fromQueue in options still works on its own (in-band path)', () => {
|
||||
const { q } = makeQueue();
|
||||
q.start(['a.sloppak', 'b.sloppak'], { source: 'gig' });
|
||||
// consume the internal flag first so ONLY options.fromQueue is under test
|
||||
q._consumeInternalPlay();
|
||||
const win = { feedBack: { playQueue: q } };
|
||||
clearGuard(win, { fromQueue: true });
|
||||
assert.strictEqual(q.active(), true, 'options.fromQueue alone must still keep the queue');
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user