From 65b045df03cdd007c84fca98d9c30a64cc6dc012 Mon Sep 17 00:00:00 2001
From: Kendall Garner <17521368+kgarner7@users.noreply.github.com>
Date: Sat, 30 Mar 2024 21:48:09 -0700
Subject: [PATCH] [bugfix]: Resolve MPV next/prev race condition

Resolves #536.

With the previous implementation, next/previous would first update
the current queue and then call next/previous. However, since these were
asynchronous calls it was very likely that the second calls would fail
(and a test of adding delay showed that it actually caused a double skip).
This PR resolves this by just removing the prev/next.

Small other fixes:
- setQueue + pause -> setQueue(..., true)
- make MPV and web player have the same behavior for (pause/stop) where appropriate
---
 .../player/hooks/use-center-controls.ts       | 56 +++++++++----------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/src/renderer/features/player/hooks/use-center-controls.ts b/src/renderer/features/player/hooks/use-center-controls.ts
index b5c0eed8..872d2fd7 100644
--- a/src/renderer/features/player/hooks/use-center-controls.ts
+++ b/src/renderer/features/player/hooks/use-center-controls.ts
@@ -311,7 +311,6 @@ export const useCenterControls = (args: { playersRef: any }) => {
                 const playerData = next();
                 mprisUpdateSong({ song: playerData.current.song, status: PlayerStatus.PLAYING });
                 mpvPlayer!.setQueue(playerData);
-                mpvPlayer!.next();
             },
             web: () => {
                 const playerData = next();
@@ -324,8 +323,7 @@ export const useCenterControls = (args: { playersRef: any }) => {
                 if (isLastTrack) {
                     const playerData = setCurrentIndex(0);
                     mprisUpdateSong({ song: playerData.current.song, status: PlayerStatus.PAUSED });
-                    mpvPlayer!.setQueue(playerData);
-                    mpvPlayer!.pause();
+                    mpvPlayer!.setQueue(playerData, true);
                     pause();
                 } else {
                     const playerData = next();
@@ -334,7 +332,6 @@ export const useCenterControls = (args: { playersRef: any }) => {
                         status: PlayerStatus.PLAYING,
                     });
                     mpvPlayer!.setQueue(playerData);
-                    mpvPlayer!.next();
                 }
             },
             web: () => {
@@ -359,10 +356,14 @@ export const useCenterControls = (args: { playersRef: any }) => {
 
         const handleRepeatOne = {
             local: () => {
-                const playerData = next();
-                mprisUpdateSong({ song: playerData.current.song, status: PlayerStatus.PLAYING });
-                mpvPlayer!.setQueue(playerData);
-                mpvPlayer!.next();
+                if (!isLastTrack) {
+                    const playerData = next();
+                    mprisUpdateSong({
+                        song: playerData.current.song,
+                        status: PlayerStatus.PLAYING,
+                    });
+                    mpvPlayer!.setQueue(playerData);
+                }
             },
             web: () => {
                 if (!isLastTrack) {
@@ -429,7 +430,6 @@ export const useCenterControls = (args: { playersRef: any }) => {
                         status: PlayerStatus.PLAYING,
                     });
                     mpvPlayer!.setQueue(playerData);
-                    mpvPlayer!.previous();
                 } else {
                     const playerData = setCurrentIndex(queue.length - 1);
                     mprisUpdateSong({
@@ -437,7 +437,6 @@ export const useCenterControls = (args: { playersRef: any }) => {
                         status: PlayerStatus.PLAYING,
                     });
                     mpvPlayer!.setQueue(playerData);
-                    mpvPlayer!.previous();
                 }
             },
             web: () => {
@@ -461,13 +460,19 @@ export const useCenterControls = (args: { playersRef: any }) => {
 
         const handleRepeatNone = {
             local: () => {
-                const playerData = previous();
-                remote?.updateSong({
-                    currentTime: usePlayerStore.getState().current.time,
-                    song: playerData.current.song,
-                });
-                mpvPlayer!.setQueue(playerData);
-                mpvPlayer!.previous();
+                if (isFirstTrack) {
+                    const playerData = setCurrentIndex(0);
+                    mprisUpdateSong({ song: playerData.current.song, status: PlayerStatus.PAUSED });
+                    mpvPlayer!.setQueue(playerData, true);
+                    pause();
+                } else {
+                    const playerData = previous();
+                    remote?.updateSong({
+                        currentTime: usePlayerStore.getState().current.time,
+                        song: playerData.current.song,
+                    });
+                    mpvPlayer!.setQueue(playerData);
+                }
             },
             web: () => {
                 if (isFirstTrack) {
@@ -487,17 +492,12 @@ export const useCenterControls = (args: { playersRef: any }) => {
 
         const handleRepeatOne = {
             local: () => {
-                if (!isFirstTrack) {
-                    const playerData = previous();
-                    mprisUpdateSong({
-                        song: playerData.current.song,
-                        status: PlayerStatus.PLAYING,
-                    });
-                    mpvPlayer!.setQueue(playerData);
-                    mpvPlayer!.previous();
-                } else {
-                    mpvPlayer!.stop();
-                }
+                const playerData = previous();
+                mprisUpdateSong({
+                    song: playerData.current.song,
+                    status: PlayerStatus.PLAYING,
+                });
+                mpvPlayer!.setQueue(playerData);
             },
             web: () => {
                 const playerData = previous();