From eff1cee6a3c21b91089061615b15eaf759ade06f Mon Sep 17 00:00:00 2001 From: Kendall Garner <17521368+kgarner7@users.noreply.github.com> Date: Thu, 1 Feb 2024 04:12:39 +0000 Subject: [PATCH 1/2] [bugfix]: Fix repeated track scrobbling (#480) * [bugfix]: Fix repeated track scrobbling This commit fixes scrobbling tracks that are repeated in the queue (either due to a REPEAT_ONE or just enqueued multiple times). There are two extra cases: 1. The current player changes. The player alternates from 1 -> 2 -> 1 whenever playback finishes normally, so this is a sort of epoch that can be used to detect both repeat one and track being enqueued multiple times. 2. The current index changes. The player gets set back to 1 in a variety of cases (most often previous/next), so the check in (1) is not enough. However, the index changing will help identify that this is a new song. * use unique id instead --- src/renderer/features/player/hooks/use-scrobble.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/renderer/features/player/hooks/use-scrobble.ts b/src/renderer/features/player/hooks/use-scrobble.ts index 28549049..b2a7b9a3 100644 --- a/src/renderer/features/player/hooks/use-scrobble.ts +++ b/src/renderer/features/player/hooks/use-scrobble.ts @@ -315,12 +315,19 @@ export const useScrobble = () => { useEffect(() => { const unsubSongChange = usePlayerStore.subscribe( - (state) => [state.current.song, state.current.time], + (state) => [state.current.song, state.current.time, state.current.player], handleScrobbleFromSongChange, { // We need the current time to check the scrobble condition, but we only want to // trigger the callback when the song changes - equalityFn: (a, b) => (a[0] as QueueSong)?.id === (b[0] as QueueSong)?.id, + // There are two conditions where this should trigger: + // 1. The song actually changes (the common case) + // 2. The song does not change, but the player dows. This would either be + // a single track on repeat one, or one track added to the queue + // multiple times in a row and playback goes normally (no next/previous) + equalityFn: (a, b) => + (a[0] as QueueSong)?.uniqueId === (b[0] as QueueSong)?.uniqueId && + a[2] === b[2], }, ); From efa0d9ec35ac09f19eaf280f5b48c24de0bec410 Mon Sep 17 00:00:00 2001 From: Kendall Garner <17521368+kgarner7@users.noreply.github.com> Date: Thu, 1 Feb 2024 20:59:00 -0800 Subject: [PATCH 2/2] [enhancement]: larger player buttons, clearer random icon --- .../player/components/center-controls.tsx | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/renderer/features/player/components/center-controls.tsx b/src/renderer/features/player/components/center-controls.tsx index 6c2b50fd..d84685c0 100644 --- a/src/renderer/features/player/components/center-controls.tsx +++ b/src/renderer/features/player/components/center-controls.tsx @@ -6,7 +6,6 @@ import isElectron from 'is-electron'; import { useTranslation } from 'react-i18next'; import { IoIosPause } from 'react-icons/io'; import { - RiMenuAddFill, RiPlayFill, RiRepeat2Line, RiRepeatOneLine, @@ -17,6 +16,7 @@ import { RiSpeedFill, RiStopFill, } from 'react-icons/ri'; +import { BsDice3 } from 'react-icons/bs'; import styled from 'styled-components'; import { Text } from '/@/renderer/components'; import { useCenterControls } from '../hooks/use-center-controls'; @@ -171,17 +171,16 @@ export const CenterControls = ({ playersRef }: CenterControlsProps) => { } + icon={} tooltip={{ label: t('player.stop', { postProcess: 'sentenceCase' }), - openDelay: 500, }} variant="tertiary" onClick={handleStop} /> } + icon={} tooltip={{ label: shuffle === PlayerShuffle.NONE @@ -190,29 +189,26 @@ export const CenterControls = ({ playersRef }: CenterControlsProps) => { postProcess: 'sentenceCase', }) : t('player.shuffle', { postProcess: 'sentenceCase' }), - openDelay: 500, }} variant="tertiary" onClick={handleToggleShuffle} /> } + icon={} tooltip={{ label: t('player.previous', { postProcess: 'sentenceCase' }), - openDelay: 500, }} variant="secondary" onClick={handlePrevTrack} /> {skip?.enabled && ( } + icon={} tooltip={{ label: t('player.skip', { context: 'back', postProcess: 'sentenceCase', }), - openDelay: 500, }} variant="secondary" onClick={() => handleSkipBackward(skip?.skipBackwardSeconds)} @@ -232,30 +228,27 @@ export const CenterControls = ({ playersRef }: CenterControlsProps) => { status === PlayerStatus.PAUSED ? t('player.play', { postProcess: 'sentenceCase' }) : t('player.pause', { postProcess: 'sentenceCase' }), - openDelay: 500, }} variant="main" onClick={handlePlayPause} /> {skip?.enabled && ( } + icon={} tooltip={{ label: t('player.skip', { context: 'forward', postProcess: 'sentenceCase', }), - openDelay: 500, }} variant="secondary" onClick={() => handleSkipForward(skip?.skipForwardSeconds)} /> )} } + icon={} tooltip={{ label: t('player.next', { postProcess: 'sentenceCase' }), - openDelay: 500, }} variant="secondary" onClick={handleNextTrack} @@ -264,9 +257,9 @@ export const CenterControls = ({ playersRef }: CenterControlsProps) => { $isActive={repeat !== PlayerRepeat.NONE} icon={ repeat === PlayerRepeat.ONE ? ( - + ) : ( - + ) } tooltip={{ @@ -286,17 +279,15 @@ export const CenterControls = ({ playersRef }: CenterControlsProps) => { postProcess: 'sentenceCase', }) }`, - openDelay: 500, }} variant="tertiary" onClick={handleToggleRepeat} /> } + icon={} tooltip={{ label: t('player.playRandom', { postProcess: 'sentenceCase' }), - openDelay: 500, }} variant="tertiary" onClick={() =>