From 62af97afc5077211e333de674c61e8fe8ea2fdf6 Mon Sep 17 00:00:00 2001 From: Jason Henriquez Date: Wed, 22 May 2024 12:25:19 -0500 Subject: [PATCH 1/7] Remove un-utilized allowPlayingVideoRemoval disabled option --- .../watch-video-playlist.js | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/renderer/components/watch-video-playlist/watch-video-playlist.js b/src/renderer/components/watch-video-playlist/watch-video-playlist.js index 8da0c0819a702..7da7b5a0c17f9 100644 --- a/src/renderer/components/watch-video-playlist/watch-video-playlist.js +++ b/src/renderer/components/watch-video-playlist/watch-video-playlist.js @@ -160,12 +160,12 @@ export default defineComponent({ }, selectedUserPlaylistVideoCount () { // Re-fetch from local store when current user playlist updated - this.parseUserPlaylist(this.selectedUserPlaylist, { allowPlayingVideoRemoval: true }) + this.parseUserPlaylist(this.selectedUserPlaylist) this.shufflePlaylistItems() }, selectedUserPlaylistLastUpdatedAt () { // Re-fetch from local store when current user playlist updated - this.parseUserPlaylist(this.selectedUserPlaylist, { allowPlayingVideoRemoval: true }) + this.parseUserPlaylist(this.selectedUserPlaylist) }, videoId: function (newId, oldId) { // Check if next video is from the shuffled list or if the user clicked a different video @@ -466,22 +466,12 @@ export default defineComponent({ }) }, - parseUserPlaylist: function (playlist, { allowPlayingVideoRemoval = true } = {}) { + parseUserPlaylist: function (playlist) { this.playlistTitle = playlist.playlistName this.channelName = '' this.channelId = '' - if (this.playlistItems.length === 0 || allowPlayingVideoRemoval) { - this.playlistItems = playlist.videos - } else { - // `this.currentVideo` relies on `playlistItems` - const latestPlaylistContainsCurrentVideo = playlist.videos.some(v => v.playlistItemId === this.playlistItemId) - // Only update list of videos if latest video list still contains currently playing video - if (latestPlaylistContainsCurrentVideo) { - this.playlistItems = playlist.videos - } - } - + this.playlistItems = playlist.videos if (this.reversePlaylist) { this.playlistItems = this.playlistItems.toReversed() } From d63bec6c805d62b7e5a204b367fa1f8e4bf36cc0 Mon Sep 17 00:00:00 2001 From: Jason Henriquez Date: Wed, 22 May 2024 14:07:57 -0500 Subject: [PATCH 2/7] Have currently deleted video entry become the prior entry in the playlist --- .../watch-video-playlist.js | 53 ++++++++++++------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/src/renderer/components/watch-video-playlist/watch-video-playlist.js b/src/renderer/components/watch-video-playlist/watch-video-playlist.js index 7da7b5a0c17f9..8aa630d5cfa18 100644 --- a/src/renderer/components/watch-video-playlist/watch-video-playlist.js +++ b/src/renderer/components/watch-video-playlist/watch-video-playlist.js @@ -48,6 +48,7 @@ export default defineComponent({ loopEnabled: false, reversePlaylist: false, pauseOnCurrentVideo: false, + prevVideoBeforeDeletion: null, channelName: '', channelId: '', playlistTitle: '', @@ -85,15 +86,7 @@ export default defineComponent({ }, currentVideoIndexZeroBased: function () { - return this.playlistItems.findIndex((item) => { - if (item.playlistItemId != null && this.playlistItemId != null) { - return item.playlistItemId === this.playlistItemId - } else if (item.videoId != null) { - return item.videoId === this.videoId - } else { - return item.id === this.videoId - } - }) + return this.findIndexOfCurrentVideoInPlaylist(this.playlistItems) }, currentVideoIndexOneBased: function () { return this.currentVideoIndexZeroBased + 1 @@ -123,16 +116,7 @@ export default defineComponent({ videoIndexInPlaylistItems: function () { const playlistItems = this.shuffleEnabled ? this.randomizedPlaylistItems : this.playlistItems - - return playlistItems.findIndex((item) => { - if (item.playlistItemId != null && this.playlistItemId != null) { - return item.playlistItemId === this.playlistItemId - } else if (item.videoId != null) { - return item.videoId === this.videoId - } else { - return item.id === this.videoId - } - }) + return this.findIndexOfCurrentVideoInPlaylist(playlistItems) }, videoIsFirstPlaylistItem: function () { return this.videoIndexInPlaylistItems === 0 @@ -186,6 +170,7 @@ export default defineComponent({ }, watchViewLoading: function (newVal, oldVal) { // This component is loaded/rendered before watch view loaded + this.prevVideoBeforeDeletion = null if (oldVal && !newVal) { // Scroll after watch view loaded, otherwise doesn't work // Mainly for Local API @@ -232,6 +217,18 @@ export default defineComponent({ } }, methods: { + findIndexOfCurrentVideoInPlaylist: function (playlist) { + return playlist.findIndex((item) => { + if (item.playlistItemId != null && (this.playlistItemId != null || this.prevVideoBeforeDeletion?.playlistItemId != null)) { + return item.playlistItemId === this.playlistItemId || item.playlistItemId === this.prevVideoBeforeDeletion?.playlistItemId + } else if (item.videoId != null) { + return item.videoId === this.videoId || item.videoId === this.prevVideoBeforeDeletion?.videoId + } else { + return item.id === this.videoId || item.id === this.prevVideoBeforeDeletion?.videoId + } + }) + }, + getPlaylistInfoWithDelay: function () { if (this.getPlaylistInfoRun) { return } @@ -348,8 +345,20 @@ export default defineComponent({ playlistType: this.playlistType, } - const videoIndex = this.videoIndexInPlaylistItems + let videoIndex = this.videoIndexInPlaylistItems + + /* + * When the current video being watched in the playlist is deleted, + * the previous video is shown as the "current" one. + * So if we want to play the previous video, in this case, + * we actually want to actually play the "current" video. + */ + if (this.prevVideoBeforeDeletion) { + videoIndex++ + } + const targetVideoIndex = (this.videoIsFirstPlaylistItem || this.videoIsNotPlaylistItem) ? this.playlistItems.length - 1 : videoIndex - 1 + if (this.shuffleEnabled) { const targetPlaylistItem = this.randomizedPlaylistItems[targetVideoIndex] @@ -471,6 +480,10 @@ export default defineComponent({ this.channelName = '' this.channelId = '' + if (this.findIndexOfCurrentVideoInPlaylist(playlist.videos) === -1) { + this.prevVideoBeforeDeletion = this.playlistItems[this.currentVideoIndexZeroBased - 1] + } + this.playlistItems = playlist.videos if (this.reversePlaylist) { this.playlistItems = this.playlistItems.toReversed() From 7e398a8c70b3ca698469928b4c06f241ef2a90e8 Mon Sep 17 00:00:00 2001 From: Jason Henriquez Date: Wed, 29 May 2024 19:35:50 -0500 Subject: [PATCH 3/7] Fix issue where first video is skipped when current video after removal is zeroth & loop is enabled --- .../components/watch-video-playlist/watch-video-playlist.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/renderer/components/watch-video-playlist/watch-video-playlist.js b/src/renderer/components/watch-video-playlist/watch-video-playlist.js index 8aa630d5cfa18..3068b891ed9c6 100644 --- a/src/renderer/components/watch-video-playlist/watch-video-playlist.js +++ b/src/renderer/components/watch-video-playlist/watch-video-playlist.js @@ -118,9 +118,6 @@ export default defineComponent({ const playlistItems = this.shuffleEnabled ? this.randomizedPlaylistItems : this.playlistItems return this.findIndexOfCurrentVideoInPlaylist(playlistItems) }, - videoIsFirstPlaylistItem: function () { - return this.videoIndexInPlaylistItems === 0 - }, videoIsLastPlaylistItem: function () { return this.videoIndexInPlaylistItems === (this.playlistItems.length - 1) }, @@ -357,7 +354,8 @@ export default defineComponent({ videoIndex++ } - const targetVideoIndex = (this.videoIsFirstPlaylistItem || this.videoIsNotPlaylistItem) ? this.playlistItems.length - 1 : videoIndex - 1 + // Wrap around to the end of the playlist only if there are no remaining earlier videos + const targetVideoIndex = (videoIndex === 0 || this.videoIsNotPlaylistItem) ? this.playlistItems.length - 1 : videoIndex - 1 if (this.shuffleEnabled) { const targetPlaylistItem = this.randomizedPlaylistItems[targetVideoIndex] From d00bc83ac1d3d5a4003ca0ad6adb8296b89cef20 Mon Sep 17 00:00:00 2001 From: Jason Henriquez Date: Fri, 31 May 2024 06:01:55 -0500 Subject: [PATCH 4/7] Handle first video in playlist being removed --- .../watch-video-playlist.js | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/renderer/components/watch-video-playlist/watch-video-playlist.js b/src/renderer/components/watch-video-playlist/watch-video-playlist.js index 3068b891ed9c6..c4f2b6926906a 100644 --- a/src/renderer/components/watch-video-playlist/watch-video-playlist.js +++ b/src/renderer/components/watch-video-playlist/watch-video-playlist.js @@ -165,9 +165,11 @@ export default defineComponent({ } } }, + playlistItemId: function () { + this.prevVideoBeforeDeletion = null + }, watchViewLoading: function (newVal, oldVal) { // This component is loaded/rendered before watch view loaded - this.prevVideoBeforeDeletion = null if (oldVal && !newVal) { // Scroll after watch view loaded, otherwise doesn't work // Mainly for Local API @@ -478,13 +480,24 @@ export default defineComponent({ this.channelName = '' this.channelId = '' - if (this.findIndexOfCurrentVideoInPlaylist(playlist.videos) === -1) { - this.prevVideoBeforeDeletion = this.playlistItems[this.currentVideoIndexZeroBased - 1] + const isCurrentVideoInParsedPlaylist = this.findIndexOfCurrentVideoInPlaylist(playlist.videos) !== -1 + if (!isCurrentVideoInParsedPlaylist) { + // grab 2nd video if the 1st one is current & deleted + // or the prior video in the list before the current video's deletion + const targetVideoIndex = (this.currentVideoIndexZeroBased === 0 ? 1 : this.currentVideoIndexZeroBased - 1) + this.prevVideoBeforeDeletion = this.playlistItems[targetVideoIndex] } - this.playlistItems = playlist.videos + let playlistItems = playlist.videos if (this.reversePlaylist) { - this.playlistItems = this.playlistItems.toReversed() + playlistItems = this.playlistItems.toReversed() + } + this.playlistItems = playlistItems + + // grab the first video of the parsed playlit if the current video is not in either the current or parsed data + // (e.g., reloading the page after the current video has already been removed from the playlist) + if (!isCurrentVideoInParsedPlaylist && this.prevVideoBeforeDeletion == null) { + this.prevVideoBeforeDeletion = this.playlistItems[0] } this.isLoading = false From 0247aca146ef7f3db0b5a9528bc37b2a73bb26dc Mon Sep 17 00:00:00 2001 From: Jason Henriquez Date: Fri, 31 May 2024 16:14:45 -0500 Subject: [PATCH 5/7] Fix shuffle prevVideo issue --- .../components/watch-video-playlist/watch-video-playlist.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/renderer/components/watch-video-playlist/watch-video-playlist.js b/src/renderer/components/watch-video-playlist/watch-video-playlist.js index c4f2b6926906a..0b3c3d7298256 100644 --- a/src/renderer/components/watch-video-playlist/watch-video-playlist.js +++ b/src/renderer/components/watch-video-playlist/watch-video-playlist.js @@ -351,8 +351,10 @@ export default defineComponent({ * the previous video is shown as the "current" one. * So if we want to play the previous video, in this case, * we actually want to actually play the "current" video. + * The only exception is when shuffle is enabled, as we don't actually + * want to play the last sequential video with shuffle. */ - if (this.prevVideoBeforeDeletion) { + if (this.prevVideoBeforeDeletion && !this.shuffleEnabled) { videoIndex++ } From 4f37130be66210d59cc9c4c2d0ea57f74e9c6b80 Mon Sep 17 00:00:00 2001 From: Jason Henriquez Date: Sat, 1 Jun 2024 08:50:46 -0500 Subject: [PATCH 6/7] Fix wrong variable for reversal bug --- .../components/watch-video-playlist/watch-video-playlist.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/components/watch-video-playlist/watch-video-playlist.js b/src/renderer/components/watch-video-playlist/watch-video-playlist.js index 0b3c3d7298256..4b39a270e8f65 100644 --- a/src/renderer/components/watch-video-playlist/watch-video-playlist.js +++ b/src/renderer/components/watch-video-playlist/watch-video-playlist.js @@ -492,7 +492,7 @@ export default defineComponent({ let playlistItems = playlist.videos if (this.reversePlaylist) { - playlistItems = this.playlistItems.toReversed() + playlistItems = playlistItems.toReversed() } this.playlistItems = playlistItems From 257681eda510fe901d960c4d6846d6fadf55f4f0 Mon Sep 17 00:00:00 2001 From: Jason Henriquez Date: Tue, 4 Jun 2024 17:04:47 -0500 Subject: [PATCH 7/7] Implement suggested optimization --- .../watch-video-playlist.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/renderer/components/watch-video-playlist/watch-video-playlist.js b/src/renderer/components/watch-video-playlist/watch-video-playlist.js index 4b39a270e8f65..535ad18a16b0c 100644 --- a/src/renderer/components/watch-video-playlist/watch-video-playlist.js +++ b/src/renderer/components/watch-video-playlist/watch-video-playlist.js @@ -217,14 +217,19 @@ export default defineComponent({ }, methods: { findIndexOfCurrentVideoInPlaylist: function (playlist) { + const playlistItemId = this.playlistItemId + const prevVideoBeforeDeletion = this.prevVideoBeforeDeletion + const videoId = this.videoId return playlist.findIndex((item) => { - if (item.playlistItemId != null && (this.playlistItemId != null || this.prevVideoBeforeDeletion?.playlistItemId != null)) { - return item.playlistItemId === this.playlistItemId || item.playlistItemId === this.prevVideoBeforeDeletion?.playlistItemId - } else if (item.videoId != null) { - return item.videoId === this.videoId || item.videoId === this.prevVideoBeforeDeletion?.videoId - } else { - return item.id === this.videoId || item.id === this.prevVideoBeforeDeletion?.videoId + if (item.playlistItemId && (playlistItemId || prevVideoBeforeDeletion?.playlistItemId)) { + return item.playlistItemId === playlistItemId || item.playlistItemId === prevVideoBeforeDeletion?.playlistItemId + } else if (item.videoId) { + return item.videoId === videoId || item.videoId === prevVideoBeforeDeletion?.videoId + } else if (item.id) { + return item.id === videoId || item.id === prevVideoBeforeDeletion?.videoId } + + return false }) },