From 978ac27115fee95a217b366425d160ab9ac0c034 Mon Sep 17 00:00:00 2001 From: Johnny Shankman`` Date: Sat, 16 Nov 2024 15:37:48 -0500 Subject: [PATCH 1/7] Fix: removeTrack causes index to reset to 0 at times --- gapless5.js | 36 +++++++++++++----------------------- package.json | 2 +- types/gapless5.d.ts | 2 +- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/gapless5.js b/gapless5.js index ab1e4f0..4f08e81 100644 --- a/gapless5.js +++ b/gapless5.js @@ -1331,35 +1331,25 @@ function Gapless5(options = {}, deprecated = {}) { // eslint-disable-line no-unu * @param {number | string} pointOrPath - audio path or playlist index */ this.removeTrack = (pointOrPath) => { - const point = this.playlist.indexFromTrack(pointOrPath); - if (!isValidIndex(point)) { - log.warn(`Cannot remove missing track: ${pointOrPath}`); + const index = typeof pointOrPath === 'number' ? pointOrPath : this.findTrack(pointOrPath); + if (index < 0 || index >= this.playlist.numTracks()) { + log.warn(`Cannot remove missing track at index: ${index}`); return; } - const deletedPlaying = point === this.playlist.trackNumber; - const { source: curSource } = this.playlist.getSourceIndexed(point); - if (!curSource) { - return; - } - let wasPlaying = false; - - if (curSource.state === Gapless5State.Loading) { - curSource.unload(); - } else if (curSource.inPlayState(true)) { - wasPlaying = true; - curSource.stop(); + // If removing a track before the current track, decrement trackNumber + if (index < this.playlist.trackNumber) { + this.playlist.trackNumber--; } - - this.playlist.remove(point); - - if (deletedPlaying) { - this.next(); // Don't stop after a delete - if (wasPlaying) { - this.play(); - } + // If removing the current track or a track after it, but trackNumber would exceed new length + else if (this.playlist.trackNumber >= this.playlist.numTracks() - 1) { + this.playlist.trackNumber = Math.max(0, this.playlist.numTracks() - 2); } + // Otherwise keep the same trackNumber + this.playlist.sources[index].unload(); + this.playlist.sources.splice(index, 1); + this.playlist.shuffledIndices = []; this.uiDirty = true; }; diff --git a/package.json b/package.json index 29c0769..f599c5f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@regosen/gapless-5", - "version": "1.5.5", + "version": "1.5.6", "description": "A gapless JavaScript audio player for HTML5", "main": "gapless5.js", "style": "gapless5.css", diff --git a/types/gapless5.d.ts b/types/gapless5.d.ts index 104c2a8..de46405 100644 --- a/types/gapless5.d.ts +++ b/types/gapless5.d.ts @@ -331,7 +331,7 @@ declare class Gapless5Source { isPlayActive: (checkStarting: any) => boolean; getPosition: () => number; getLength: () => number; - play: (syncPosition: any) => void; + play: (syncPosition: any, skipCallback: any) => void; setPlaybackRate: (rate: any) => void; tick: (updateLoopState: any) => number; getSeekablePercent: () => number; From 292eb6f4d9ac8604a9a7be91ec6ea70ba12c7fea Mon Sep 17 00:00:00 2001 From: Johnny Shankman`` Date: Sat, 16 Nov 2024 16:09:15 -0500 Subject: [PATCH 2/7] use isValidIndex --- gapless5.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gapless5.js b/gapless5.js index 4f08e81..0de0e3e 100644 --- a/gapless5.js +++ b/gapless5.js @@ -1332,8 +1332,8 @@ function Gapless5(options = {}, deprecated = {}) { // eslint-disable-line no-unu */ this.removeTrack = (pointOrPath) => { const index = typeof pointOrPath === 'number' ? pointOrPath : this.findTrack(pointOrPath); - if (index < 0 || index >= this.playlist.numTracks()) { - log.warn(`Cannot remove missing track at index: ${index}`); + if (!isValidIndex(index)) { + log.error(`Cannot remove missing track at index: ${index}`); return; } From da8eb360fb1bea2470b77dcc445067696b47a761 Mon Sep 17 00:00:00 2001 From: Johnny Shankman`` Date: Sat, 16 Nov 2024 16:21:46 -0500 Subject: [PATCH 3/7] Update: handle removing the currentlyPlayingSong --- gapless5.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/gapless5.js b/gapless5.js index 0de0e3e..eef3ca8 100644 --- a/gapless5.js +++ b/gapless5.js @@ -1337,6 +1337,14 @@ function Gapless5(options = {}, deprecated = {}) { // eslint-disable-line no-unu return; } + const isCurrentTrack = index === this.playlist.trackNumber; + const wasPlayingCurrentTrack = isCurrentTrack && this.isPlaying(); + + // If it's the current track, pause playback + if (wasPlayingCurrentTrack) { + this.pause(); + } + // If removing a track before the current track, decrement trackNumber if (index < this.playlist.trackNumber) { this.playlist.trackNumber--; @@ -1351,6 +1359,11 @@ function Gapless5(options = {}, deprecated = {}) { // eslint-disable-line no-unu this.playlist.sources.splice(index, 1); this.playlist.shuffledIndices = []; this.uiDirty = true; + + // If we were playing and there are tracks remaining, play the next available track + if (wasPlaying && this.playlist.numTracks() > 0) { + this.play(); + } }; /** From f9f81e0dbf3cbb1d9c16d0e7a17ac537e38d4cb0 Mon Sep 17 00:00:00 2001 From: Johnny Shankman`` Date: Wed, 20 Nov 2024 00:41:21 -0500 Subject: [PATCH 4/7] fix --- gapless5.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapless5.js b/gapless5.js index eef3ca8..93505d3 100644 --- a/gapless5.js +++ b/gapless5.js @@ -1361,7 +1361,7 @@ function Gapless5(options = {}, deprecated = {}) { // eslint-disable-line no-unu this.uiDirty = true; // If we were playing and there are tracks remaining, play the next available track - if (wasPlaying && this.playlist.numTracks() > 0) { + if (wasPlayingCurrentTrack && this.playlist.numTracks() > 0) { this.play(); } }; From 2146f9b04df175eeac33e6f6ab1f9b79057a8136 Mon Sep 17 00:00:00 2001 From: Johnny Shankman`` Date: Wed, 20 Nov 2024 00:43:14 -0500 Subject: [PATCH 5/7] Add regression test --- gapless5.test.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/gapless5.test.js b/gapless5.test.js index 5ce152b..862e8f6 100644 --- a/gapless5.test.js +++ b/gapless5.test.js @@ -152,6 +152,29 @@ describe('Gapless-5 object with tracklist', () => { player.stop(); expect(player.onstop).toHaveBeenCalledWith(TRACKS[0]); }); + + it('maintains correct index after removing tracks', () => { + const testTracks = ['track1.mp3', 'track2.mp3', 'track3.mp3']; + const player = new Gapless5({ + ...INIT_OPTIONS, + tracks: testTracks, + }); + + // Verify initial state + expect(player.getIndex()).toBe(0); + expect(player.getTracks()).toStrictEqual(testTracks); + + // Move to track 1 + player.gotoTrack(1); + expect(player.getIndex()).toBe(1); + + // Remove track at index 2 + player.removeTrack(2); + + // Check index is still 1 and track list is updated + expect(player.getIndex()).toBe(1); + expect(player.getTracks()).toStrictEqual(['track1.mp3', 'track2.mp3']); + }); }); describe('Gapless-5 object with load limit', () => { @@ -179,3 +202,5 @@ describe('Gapless-5 object with load limit', () => { expect(loadedTracks.size).toBe(0); }); }); + + From 8b9472b1d4e22d56de0062dd8af5b266783b435d Mon Sep 17 00:00:00 2001 From: Johnny Shankman`` Date: Wed, 20 Nov 2024 00:46:18 -0500 Subject: [PATCH 6/7] regression test --- gapless5.test.js | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/gapless5.test.js b/gapless5.test.js index 862e8f6..dcc64af 100644 --- a/gapless5.test.js +++ b/gapless5.test.js @@ -153,7 +153,7 @@ describe('Gapless-5 object with tracklist', () => { expect(player.onstop).toHaveBeenCalledWith(TRACKS[0]); }); - it('maintains correct index after removing tracks', () => { + it('regression: maintains correct index after removing tracks that come after the current track', () => { const testTracks = ['track1.mp3', 'track2.mp3', 'track3.mp3']; const player = new Gapless5({ ...INIT_OPTIONS, @@ -175,6 +175,48 @@ describe('Gapless-5 object with tracklist', () => { expect(player.getIndex()).toBe(1); expect(player.getTracks()).toStrictEqual(['track1.mp3', 'track2.mp3']); }); + + it('regression: maintains correct index when removing the currently playing track', () => { + const testTracks = ['track1.mp3', 'track2.mp3', 'track3.mp3']; + const player = new Gapless5({ + ...INIT_OPTIONS, + tracks: testTracks, + }); + + // Verify initial state + expect(player.getIndex()).toBe(0); + expect(player.getTracks()).toStrictEqual(testTracks); + + // Move to track 1 + player.gotoTrack(1); + expect(player.getIndex()).toBe(1); + player.play(); + + // Remove the currently playing track + player.removeTrack(1); + + // Check index is still 1 and track list is updated, therefore we move to the next available track + expect(player.getIndex()).toBe(1); + expect(player.getTracks()).toStrictEqual(['track1.mp3', 'track3.mp3']); + + player.play(); + + // Remove the currently playing track + player.removeTrack(1); + + // Check index is still 0 and track list is updated, therefore we move to the next available track + expect(player.getIndex()).toBe(0); + expect(player.getTracks()).toStrictEqual(['track1.mp3']); + + player.play(); + + // Remove the only remaining track + player.removeTrack(0); + + // Check index is still 0 and track list is updated + expect(player.getIndex()).toBe(0); + expect(player.getTracks()).toStrictEqual([]); + }); }); describe('Gapless-5 object with load limit', () => { From 7cd5bd445cd209dc104b51c041f8441de0ac9875 Mon Sep 17 00:00:00 2001 From: Johnny Shankman`` Date: Wed, 20 Nov 2024 00:46:39 -0500 Subject: [PATCH 7/7] better words --- gapless5.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapless5.test.js b/gapless5.test.js index dcc64af..49e694f 100644 --- a/gapless5.test.js +++ b/gapless5.test.js @@ -153,7 +153,7 @@ describe('Gapless-5 object with tracklist', () => { expect(player.onstop).toHaveBeenCalledWith(TRACKS[0]); }); - it('regression: maintains correct index after removing tracks that come after the current track', () => { + it('regression: maintains correct index when removing tracks that come after the currently playing track', () => { const testTracks = ['track1.mp3', 'track2.mp3', 'track3.mp3']; const player = new Gapless5({ ...INIT_OPTIONS,