diff --git a/src/audio/queue.test.ts b/src/audio/queue.test.ts index b6bd9f0..3aa8ced 100644 --- a/src/audio/queue.test.ts +++ b/src/audio/queue.test.ts @@ -605,4 +605,141 @@ describe("PlayQueue", () => { expect(q.list().map((s) => s.id)).toEqual(["A"]); }); }); + + // Issue #141: in Random/RandomLoop, next() picks from the shuffle bag and + // ignores array order, so a song spliced in by addNext (!pn) was NOT played + // next — it just waited for its random turn like any other song. addNext now + // records the insert slot on the forward stack, which next() honours first. + describe("addNext in random modes (issue #141)", () => { + for (const mode of [PlayMode.Random, PlayMode.RandomLoop]) { + it(`plays the inserted song next in ${mode} mode`, () => { + queue.setMode(mode); + for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id)); + queue.play(); // current = 0 (a) + queue.addNext(makeSong("x")); + expect(queue.next()?.id).toBe("x"); + }); + } + + it("plays consecutive inserts in the order the queue displays them", () => { + queue.setMode(PlayMode.RandomLoop); + for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id)); + queue.play(); // current = 0 (a) + queue.addNext(makeSong("x")); + queue.addNext(makeSong("y")); // splices in front of x, as in sequential + expect(queue.list().map((s) => s.id)).toEqual(["a", "y", "x", "b", "c", "d"]); + expect(queue.next()?.id).toBe("y"); + expect(queue.next()?.id).toBe("x"); + }); + + it("honours the insert even after the shuffle bag is exhausted", () => { + // Random (non-loop) returns null once every song has played. Songs added + // afterwards must still be reachable via !pn — and with TWO of them the + // order can only come from the forward stack, not from the bag having a + // single remaining candidate. + queue.setMode(PlayMode.Random); + for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id)); + queue.play(); + for (let i = 0; i < 3; i++) queue.next(); + expect(queue.next()).toBeNull(); // bag exhausted + queue.addNext(makeSong("x")); + queue.addNext(makeSong("y")); + queue.addNext(makeSong("z")); + expect(queue.next()?.id).toBe("z"); + expect(queue.next()?.id).toBe("y"); + expect(queue.next()?.id).toBe("x"); + }); + + it("pops past a prev() marker to reach the pending insert", () => { + // prev() shares the forward stack, and in random mode with no history it + // pushes the current index and then returns null. next() must walk past + // those self-referencing markers instead of consuming one and giving up + // to the shuffle bag. + queue.setMode(PlayMode.Random); + for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id)); + queue.play(); // a + queue.addNext(makeSong("x")); + expect(queue.prev()).toBeNull(); + expect(queue.prev()).toBeNull(); + expect(queue.next()?.id).toBe("x"); + }); + + it("plays each song exactly once — the insert is not replayed later", () => { + queue.setMode(PlayMode.Random); + for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id)); + queue.play(); // a + queue.addNext(makeSong("x")); + queue.addNext(makeSong("y")); + + const played = [queue.current()!.id]; + for (let i = 0; i < 5; i++) played.push(queue.next()!.id); + expect(queue.next()).toBeNull(); // bag exhausted + expect(played.slice(0, 3)).toEqual(["a", "y", "x"]); + expect(new Set(played).size).toBe(6); + }); + + it("keeps the insert reachable after an earlier song is removed", () => { + queue.setMode(PlayMode.RandomLoop); + for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id)); + queue.playAt(2); // current = 2 (c) + queue.addNext(makeSong("x")); // [a, b, c, x, d] + queue.remove(0); // [b, c, x, d] — x slides from 3 to 2 + expect(queue.next()?.id).toBe("x"); + }); + + it("drops the entry when the inserted song is itself removed", () => { + // Leaving the stale entry behind would not throw — index 2 still exists + // after the removal, it just points at a different song. So the queue is + // arranged with exactly one song the shuffle bag can legally return: + // anything else means the dead forward entry was honoured. + queue.setMode(PlayMode.RandomLoop); + for (const id of ["a", "b", "c"]) queue.add(makeSong(id)); + queue.playAt(0); // current = 0 (a), played = {0} + queue.next(); // b or c — two of the three are now played + const remaining = queue.list().find((s) => s.id !== "a" && s.id !== queue.current()!.id)!; + queue.addNext(makeSong("x")); // spliced at currentIndex+1 + queue.remove(queue.getCurrentIndex() + 1); // …and removed again + expect(queue.list().map((s) => s.id)).not.toContain("x"); + expect(queue.next()?.id).toBe(remaining.id); + }); + + it("never yields a stale index under interleaved inserts and removals", () => { + // The forward stack holds array indices, so every splice has to shift + // them. next() returning `undefined` here (an out-of-range index) reads + // as end-of-queue to BotInstance.playNext and silently stops playback. + queue.setMode(PlayMode.RandomLoop); + for (let i = 0; i < 6; i++) queue.add(makeSong(`s${i}`)); + queue.play(); + for (let step = 0; step < 200; step++) { + const roll = step % 4; + if (roll === 0) queue.addNext(makeSong(`x${step}`)); + else if (roll === 1 && queue.size() > 1) queue.remove(step % queue.size()); + else { + const song = queue.next(); + expect(song === null || song === queue.current()).toBe(true); + if (song !== null) expect(song).toBeDefined(); + } + } + }); + + it("leaves sequential/loop behaviour untouched", () => { + queue.setMode(PlayMode.Sequential); + for (const id of ["a", "b", "c"]) queue.add(makeSong(id)); + queue.play(); // a + queue.addNext(makeSong("x")); + expect(queue.next()?.id).toBe("x"); + expect(queue.next()?.id).toBe("b"); + expect(queue.next()?.id).toBe("c"); + expect(queue.next()).toBeNull(); + }); + + it("still appends (no forward entry) when nothing is playing", () => { + queue.setMode(PlayMode.Random); + queue.add(makeSong("a")); + queue.addNext(makeSong("x")); // currentIndex is still -1 → plain push + expect(queue.list().map((s) => s.id)).toEqual(["a", "x"]); + queue.play(); // a — a stray forward entry would have hijacked this + expect(queue.current()?.id).toBe("a"); + }); + }); }); diff --git a/src/audio/queue.ts b/src/audio/queue.ts index 6274a05..bc309ae 100644 --- a/src/audio/queue.ts +++ b/src/audio/queue.ts @@ -60,8 +60,12 @@ export class PlayQueue { * or queue empty), so the existing "add → idle bot starts playing" * flow continues to work. * - * Shifts playedIndices and history entries > currentIndex by +1 so - * their references stay valid after the splice. + * Shifts playedIndices, history and forwardStack entries > currentIndex + * by +1 so their references stay valid after the splice. + * + * In the random modes the array position alone means nothing — next() + * picks from the shuffle bag — so the insert slot is also recorded on + * the forward stack, which next() consults first (issue #141). */ addNext(song: QueuedSong): void { if (this.currentIndex < 0 || this.songs.length === 0) { @@ -80,6 +84,23 @@ export class PlayQueue { this.history = this.history.map((i) => i > this.currentIndex ? i + 1 : i, ); + + this.forwardStack = this.forwardStack.map((i) => + i > this.currentIndex ? i + 1 : i, + ); + + // Push AFTER the shift, or the slot we just claimed would be shifted + // too. Stacking makes repeated !pn play in the order the queue shows + // them (each insert lands in front of the previous one), matching what + // sequential mode does with the same array. Bounded like history: drop the + // OLDEST pending entry rather than refusing the newest, so the song the + // user just asked for is always the one that gets honoured. + if (this.mode === PlayMode.Random || this.mode === PlayMode.RandomLoop) { + this.forwardStack.push(insertAt); + if (this.forwardStack.length > PlayQueue.HISTORY_LIMIT) { + this.forwardStack.shift(); + } + } } remove(index: number): QueuedSong | null { @@ -106,6 +127,13 @@ export class PlayQueue { .filter((idx) => idx !== index) .map((idx) => (idx > index ? idx - 1 : idx)); + // …and for the forward stack, which now also carries !pn insert slots + // (issue #141). Left unshifted, a removal elsewhere in the queue would + // silently repoint the entry at whatever song slid into that slot. + this.forwardStack = this.forwardStack + .filter((idx) => idx !== index) + .map((idx) => (idx > index ? idx - 1 : idx)); + return removed; } @@ -158,15 +186,22 @@ export class PlayQueue { } case PlayMode.Random: case PlayMode.RandomLoop: { - // 优先回到前进栈记录的位置(prev 退回的歌) - if (this.forwardStack.length > 0) { + // 优先回到前进栈记录的位置(prev 退回的歌,或 !pn 插入的歌)。 + // Keep popping past entries that no longer point anywhere useful, + // the way prev() walks past stale history entries. Without the loop a + // prev() that pushed the current index would swallow the pending !pn + // entry behind it. The range check is belt-and-braces — addNext and + // remove keep the stack in sync — but an out-of-range index here would + // set currentIndex out of bounds and hand back `undefined`, which + // BotInstance.playNext reads as end-of-queue and stops playback. + while (this.forwardStack.length > 0) { const target = this.forwardStack.pop()!; - if (target !== this.currentIndex) { - this.pushHistory(this.currentIndex); - this.currentIndex = target; - this.playedIndices.add(target); - return this.songs[target]; - } + if (target < 0 || target >= this.songs.length) continue; + if (target === this.currentIndex) continue; + this.pushHistory(this.currentIndex); + this.currentIndex = target; + this.playedIndices.add(target); + return this.songs[target]; } // Shuffle bag: pick uniformly from the songs not yet played this