From e2b1a5e055ef0199b56c9d43956b129d56574607 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Apr 2026 15:02:53 +0000 Subject: [PATCH] fix: prevent skipped/duplicate songs in Random mode edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three corner cases fixed: 1. Removing the currently-playing song caused the next song in the array to be silently marked as "played" and skipped. Root cause: playedIndices was updated in next() by marking currentIndex, but after remove() shifts currentIndex, it pointed to the wrong song. Fix: mark songs as played at play-time (in play/playAt/next/prev) instead of at next-request-time. 2. Using prev() in Random mode could cause a song to play twice — the song navigated to via prev() was not recorded in playedIndices, so next() could randomly select it again. Fix: prev() now marks the returned song as played. 3. Switching to Random mode mid-playback could cause the current song to repeat because setMode() cleared playedIndices without preserving the currently-playing song. Fix: setMode() now re-adds currentIndex after clearing. https://claude.ai/code/session_01W3ZncxL5VfdZeB4qqYWDmY --- src/audio/queue.test.ts | 69 +++++++++++++++++++++++++++++++++++++++++ src/audio/queue.ts | 10 ++++-- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/audio/queue.test.ts b/src/audio/queue.test.ts index e956d6e..dc58ce5 100644 --- a/src/audio/queue.test.ts +++ b/src/audio/queue.test.ts @@ -176,6 +176,75 @@ describe("PlayQueue", () => { expect(queue.next()).toBeNull(); }); + it("random mode: removing currently-playing song does not skip others", () => { + queue.setMode(PlayMode.Random); + queue.add(makeSong("A")); + queue.add(makeSong("B")); + queue.add(makeSong("C")); + queue.add(makeSong("D")); + queue.play(); // plays A (index 0) + const second = queue.next()!; // plays some song + // Remove the currently-playing song + const curIdx = queue.getCurrentIndex(); + queue.remove(curIdx); + // Remaining songs (excluding A and the removed song) should all be reachable + const played = new Set(); + played.add("A"); // already played via play() + played.add(second.id); // played and then removed + let song = queue.next(); + while (song) { + played.add(song.id); + song = queue.next(); + } + // All 4 original songs should have been played or accounted for + expect(played).toEqual(new Set(["A", "B", "C", "D"])); + }); + + it("random mode: prev does not cause duplicate plays", () => { + queue.setMode(PlayMode.Random); + queue.add(makeSong("A")); + queue.add(makeSong("B")); + queue.add(makeSong("C")); + queue.play(); // plays A + queue.next(); // plays B or C + queue.prev(); // go back — this song is now marked as played + // Exhaust remaining songs + const ids: string[] = []; + let song = queue.next(); + while (song) { + ids.push(song.id); + song = queue.next(); + } + // No song ID should appear more than once across the entire session + expect(new Set(ids).size).toBe(ids.length); + }); + + it("random mode: adding song mid-playback includes the new song", () => { + queue.setMode(PlayMode.Random); + queue.add(makeSong("A")); + queue.add(makeSong("B")); + queue.play(); // plays A + queue.next(); // plays B + // Add a new song while all existing songs have been played + queue.add(makeSong("C")); + const song = queue.next(); + expect(song).not.toBeNull(); + expect(song!.id).toBe("C"); + // After C, should stop + expect(queue.next()).toBeNull(); + }); + + it("random mode: setMode preserves current song as played", () => { + queue.add(makeSong("A")); + queue.add(makeSong("B")); + queue.play(); // plays A in sequential mode + queue.setMode(PlayMode.Random); // switch to random — A should be marked played + // next() should only return B, never A again + const song = queue.next(); + expect(song?.id).toBe("B"); + expect(queue.next()).toBeNull(); + }); + it("random-loop mode never returns null", () => { queue.setMode(PlayMode.RandomLoop); queue.add(makeSong("1")); diff --git a/src/audio/queue.ts b/src/audio/queue.ts index 7a6f36f..b97444d 100644 --- a/src/audio/queue.ts +++ b/src/audio/queue.ts @@ -61,6 +61,7 @@ export class PlayQueue { if (this.songs.length === 0) return null; this.playedIndices.clear(); this.currentIndex = 0; + this.playedIndices.add(0); return this.songs[0]; } @@ -68,6 +69,7 @@ export class PlayQueue { if (index < 0 || index >= this.songs.length) return null; this.playedIndices.clear(); this.currentIndex = index; + this.playedIndices.add(index); return this.songs[index]; } @@ -86,9 +88,6 @@ export class PlayQueue { return this.songs[this.currentIndex]; } case PlayMode.Random: { - if (this.currentIndex >= 0) { - this.playedIndices.add(this.currentIndex); - } const unplayed: number[] = []; for (let i = 0; i < this.songs.length; i++) { if (!this.playedIndices.has(i)) unplayed.push(i); @@ -97,6 +96,7 @@ export class PlayQueue { const nextIndex = unplayed[Math.floor(Math.random() * unplayed.length)]; this.currentIndex = nextIndex; + this.playedIndices.add(nextIndex); return this.songs[nextIndex]; } case PlayMode.RandomLoop: { @@ -124,6 +124,7 @@ export class PlayQueue { } else { this.currentIndex = prevIndex; } + this.playedIndices.add(this.currentIndex); return this.songs[this.currentIndex]; } @@ -152,6 +153,9 @@ export class PlayQueue { setMode(mode: PlayMode): void { this.mode = mode; this.playedIndices.clear(); + if (this.currentIndex >= 0) { + this.playedIndices.add(this.currentIndex); + } } getCurrentIndex(): number {