From ab17ba8add11f06ba735f6701286a3db7f882750 Mon Sep 17 00:00:00 2001 From: Kendall Garner <17521368+kgarner7@users.noreply.github.com> Date: Mon, 1 Apr 2024 22:13:06 -0700 Subject: [PATCH] [bugfix]: fix scrobble race conditions --- .../features/player/hooks/use-scrobble.ts | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/renderer/features/player/hooks/use-scrobble.ts b/src/renderer/features/player/hooks/use-scrobble.ts index b2a7b9a3..a93dad13 100644 --- a/src/renderer/features/player/hooks/use-scrobble.ts +++ b/src/renderer/features/player/hooks/use-scrobble.ts @@ -1,7 +1,7 @@ import { useEffect, useCallback, useState, useRef } from 'react'; import { QueueSong, ServerType } from '/@/renderer/api/types'; import { useSendScrobble } from '/@/renderer/features/player/mutations/scrobble-mutation'; -import { useCurrentStatus, usePlayerStore } from '/@/renderer/store'; +import { usePlayerStore } from '/@/renderer/store'; import { usePlaybackSettings } from '/@/renderer/store/settings.store'; import { PlayerStatus } from '/@/renderer/types'; @@ -52,7 +52,6 @@ const checkScrobbleConditions = (args: { }; export const useScrobble = () => { - const status = useCurrentStatus(); const scrobbleSettings = usePlaybackSettings().scrobble; const isScrobbleEnabled = scrobbleSettings?.enabled; const sendScrobble = useSendScrobble(); @@ -94,6 +93,7 @@ export const useScrobble = () => { if (progressIntervalId.current) { clearInterval(progressIntervalId.current); + progressIntervalId.current = null; } // const currentSong = current[0] as QueueSong | undefined; @@ -135,9 +135,13 @@ export const useScrobble = () => { clearTimeout(songChangeTimeoutId.current as ReturnType); songChangeTimeoutId.current = setTimeout(() => { const currentSong = current[0] as QueueSong | undefined; + // Get the current status from the state, not variable. This is because + // of a timing issue where, when playing the first track, the first + // event is song, and then the event is play + const currentStatus = usePlayerStore.getState().current.status; // Send start scrobble when song changes and the new song is playing - if (status === PlayerStatus.PLAYING && currentSong?.id) { + if (currentStatus === PlayerStatus.PLAYING && currentSong?.id) { sendScrobble.mutate({ query: { event: 'start', @@ -149,6 +153,12 @@ export const useScrobble = () => { }); if (currentSong?.serverType === ServerType.JELLYFIN) { + // It is possible that another function sets an interval. + // We only want one running, so clear the existing interval + if (progressIntervalId.current) { + clearInterval(progressIntervalId.current); + } + progressIntervalId.current = setInterval(() => { const currentTime = usePlayerStore.getState().current.time; handleScrobbleFromSeek(currentTime); @@ -163,7 +173,6 @@ export const useScrobble = () => { scrobbleSettings?.scrobbleAtPercentage, isCurrentSongScrobbled, sendScrobble, - status, handleScrobbleFromSeek, ], ); @@ -200,8 +209,14 @@ export const useScrobble = () => { }); if (currentSong?.serverType === ServerType.JELLYFIN) { + // It is possible that another function sets an interval. + // We only want one running, so clear the existing interval + if (progressIntervalId.current) { + clearInterval(progressIntervalId.current); + } + progressIntervalId.current = setInterval(() => { - const currentTime = currentTimeSec; + const currentTime = usePlayerStore.getState().current.time; handleScrobbleFromSeek(currentTime); }, 10000); } @@ -220,6 +235,7 @@ export const useScrobble = () => { if (progressIntervalId.current) { clearInterval(progressIntervalId.current as ReturnType); + progressIntervalId.current = null; } } else { const isLastTrackInQueue = usePlayerStore.getState().actions.checkIsLastTrack();