diff --git a/CHANGELOG.md b/CHANGELOG.md index 657cb783..d9be1b5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -213,6 +213,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * The currently-playing track in any tracklist (**AlbumDetail**, **ArtistDetail**, **PlaylistDetail**, **Favorites**, **RandomMix**) ran an **`opacity` pulse** on the entire row plus three **`transform` keyframe** EQ-bar siblings — both compositor properties, but on **WebKitGTK without compositing** (Linux + NVIDIA proprietary + `WEBKIT_DISABLE_COMPOSITING_MODE=1`) every animated row falls back to a full **software repaint** of the subtree per frame. On AlbumDetail the combined cost held the WebProcess at **~80 % CPU** for the duration of playback; CPU dropped immediately on pause/stop. * `.track-row.active` keeps the **accent-tinted background** but no longer pulses. The "now playing" indicator becomes a single Lucide **`AudioLines` icon** (one SVG per active row instead of three animated spans). Cleanup: dead `track-pulse` + `eq-bounce` keyframes and a duplicate, shadowed `.eq-bar` block in `theme.css`. +### Radio — queue navigation, dedup, and similar-first variety + +**By [@Psychotoxical](https://github.com/Psychotoxical), reported by netherguy4, PR [#503](https://github.com/Psychotoxical/psysonic/pull/503)** + +* **Queue navigation through duplicates**: `playTrack` re-resolved the active slot via `findIndex(... .id === ...)`, which returns the **first** matching id. Reaching a track's second occurrence snapped `queueIndex` back to the earlier slot — highlight visibly jumped and the next auto-advance played the wrong follow-up. `next()`, `previous()`, the `audio:ended` repeat-one path, queue-row click and the queue-item Play Now now pass an explicit target index through `playTrack`. +* **Queue duplicates**: `enqueueRadio` didn't dedupe incoming tracks; the `next()` top-up deduped against the live queue but trimmed the played tail down to **5 history entries**, so songs heard a few advances ago could be re-added by a later Last.fm / topSongs response; and the `.filter(...)` pass admitted intra-batch repeats (top + similar overlap is common) because it read the dedup set before mutating it. A radio-session-scoped seen-set, reset on artist change and `clearQueue`, closes all three paths. +* **Variety**: Starting Radio on a track no longer queues five top tracks of the seed artist before any similar-artist material plays. The seed path and both top-up paths lead with similar songs and only fall back to top tracks when similar comes back empty (no Last.fm / small library). + ### Track preview — volume slider ignored during preview **By [@Psychotoxical](https://github.com/Psychotoxical), reported by netherguy4, PR [#502](https://github.com/Psychotoxical/psysonic/pull/502)** diff --git a/src/components/ContextMenu.tsx b/src/components/ContextMenu.tsx index a1091e11..b2146fea 100644 --- a/src/components/ContextMenu.tsx +++ b/src/components/ContextMenu.tsx @@ -1391,19 +1391,20 @@ export default function ContextMenu() { } // Load radio queue in background — enqueueRadio replaces any pending radio // tracks so clicking "Start Radio" again never stacks duplicate batches. - // Shuffle so the follow-up tracks feel fresh instead of always being the - // same "Top 5" in the same order every time. + // Lead with similar songs (other artists) so the listener doesn't get a + // wall of the seed artist's own top tracks before anything else plays. + // Top tracks stay as a fallback for setups without Last.fm / small + // libraries where similar comes back empty (issue #500). try { const [similar, top] = await Promise.all([getSimilarSongs2(artistId), getTopSongs(artistName)]); - // Keep artist top songs and similar-by-artist in two blocks (each shuffled), not one blended pile — - // otherwise this feels the same as Instant Mix (track-based similar only). - const topTracks = shuffleArray( - top.map(songToTrack).filter(t => t.id !== seedTrack.id).map(t => ({ ...t, radioAdded: true as const })) - ); const similarTracks = shuffleArray( similar.map(songToTrack).filter(t => t.id !== seedTrack.id).map(t => ({ ...t, radioAdded: true as const })) ); - const radioTracks = [...topTracks, ...similarTracks]; + const radioTracks = similarTracks.length > 0 + ? similarTracks + : shuffleArray( + top.map(songToTrack).filter(t => t.id !== seedTrack.id).map(t => ({ ...t, radioAdded: true as const })) + ); if (radioTracks.length > 0) usePlayerStore.getState().enqueueRadio(radioTracks, artistId); } catch (e) { console.error('Failed to load radio queue', e); @@ -2142,7 +2143,7 @@ export default function ContextMenu() { const song = item as Track; return ( <> -
handleAction(() => playTrack(song, queue))}> +
handleAction(() => playTrack(song, queue, undefined, undefined, contextMenu.queueIndex))}> {t('contextMenu.playNow')}
handleAction(() => { diff --git a/src/components/QueuePanel.tsx b/src/components/QueuePanel.tsx index 16747744..5b1707c8 100644 --- a/src/components/QueuePanel.tsx +++ b/src/components/QueuePanel.tsx @@ -1063,7 +1063,9 @@ function QueuePanelHostOrSolo() { className={`queue-item ${isPlaying ? 'active' : ''} ${contextMenu.isOpen && contextMenu.type === 'queue-item' && contextMenu.queueIndex === idx ? 'context-active' : ''}`} onClick={() => { suppressNextAutoScrollRef.current = true; - playTrack(track, queue); + // Pass the row index so a click on a duplicate track lands on + // *this* slot, not the first occurrence (issue #500). + playTrack(track, queue, undefined, undefined, idx); }} onContextMenu={(e) => { e.preventDefault(); diff --git a/src/store/playerStore.ts b/src/store/playerStore.ts index a581bebc..b141b3e0 100644 --- a/src/store/playerStore.ts +++ b/src/store/playerStore.ts @@ -235,8 +235,14 @@ interface PlayerState { playRadio: (station: InternetRadioStation) => void; /** `_orbitConfirmed` is an internal bypass flag — callers outside the - * orbit bulk-gate should leave it `undefined`. */ - playTrack: (track: Track, queue?: Track[], manual?: boolean, _orbitConfirmed?: boolean) => void; + * orbit bulk-gate should leave it `undefined`. + * `targetQueueIndex` lets callers that already know the exact target + * position (next()/previous()/queue-row click) bypass the `findIndex` + * by-id fallback, which otherwise resolves to the *first* occurrence + * and breaks navigation when the same track appears multiple times in + * the queue (issue #500). Ignored if out of range or if the track id + * at that position doesn't match. */ + playTrack: (track: Track, queue?: Track[], manual?: boolean, _orbitConfirmed?: boolean, targetQueueIndex?: number) => void; /** Queue becomes `[track]` only; if already on this track, does not restart `audio_play`. */ reseedQueueForInstantMix: (track: Track) => void; pause: () => void; @@ -409,6 +415,13 @@ let radioFetching = false; // Artist ID used to start the current radio session — persists across track // advances so proactive loading works even when songs lack artistId. let currentRadioArtistId: string | null = null; +// Track ids the current radio session has already enqueued — *including* +// entries that were trimmed off the front of the queue when it grew too long +// (`HISTORY_KEEP` in next()'s top-up path). Without this the queue's own +// id-set wasn't enough to dedupe: a song played 8 tracks ago is gone from +// the queue and the next Last.fm/topSongs response could re-add it. Reset +// on `setRadioArtistId(other)` and on `clearQueue()`. Issue #500. +let radioSessionSeenIds = new Set(); let cachedLoudnessGainByTrackId: Record = {}; let stableLoudnessGainByTrackId: Record = {}; let lastNormalizationUiUpdateAtMs = 0; @@ -1538,7 +1551,7 @@ function handleAudioEnded() { return; } - const { repeatMode, currentTrack, queue } = usePlayerStore.getState(); + const { repeatMode, currentTrack, queue, queueIndex } = usePlayerStore.getState(); isAudioPaused = false; usePlayerStore.setState({ isPlaying: false, @@ -1560,7 +1573,8 @@ function handleAudioEnded() { authState.hotCacheDownloadDir || null, ); } - usePlayerStore.getState().playTrack(currentTrack, queue, false); + // Pin to the current slot — the track may appear elsewhere in the queue. + usePlayerStore.getState().playTrack(currentTrack, queue, false, false, queueIndex); } else { usePlayerStore.getState().next(false); } @@ -2466,7 +2480,7 @@ export const usePlayerStore = create()( }, // ── playTrack ──────────────────────────────────────────────────────────── - playTrack: (track, queue, manual = true, _orbitConfirmed = false) => { + playTrack: (track, queue, manual = true, _orbitConfirmed = false, targetQueueIndex) => { // Orbit bulk-gate: only gate when the `queue` argument *replaces* // the current queue (Play All / Play Album / Play Playlist / Hero // play buttons). Navigation calls — queue-row click, next(), @@ -2527,7 +2541,18 @@ export const usePlayerStore = create()( seekFallbackVisualTarget = null; } const newQueue = queue ?? state.queue; - const idx = newQueue.findIndex(t => sameQueueTrackId(t.id, track.id)); + // Prefer an explicit target index from the caller (next/previous/queue-row + // click already know the exact slot). `findIndex` returns the *first* + // matching id, which jumps backwards when the queue contains the same + // track twice — breaking radio playback (issue #500). + const explicitIdxValid = + typeof targetQueueIndex === 'number' + && targetQueueIndex >= 0 + && targetQueueIndex < newQueue.length + && sameQueueTrackId(newQueue[targetQueueIndex]?.id, track.id); + const idx = explicitIdxValid + ? (targetQueueIndex as number) + : newQueue.findIndex(t => sameQueueTrackId(t.id, track.id)); if (manual) { pushQueueUndoFromGetter(get); } @@ -2985,7 +3010,7 @@ export const usePlayerStore = create()( applySkipStarOnManualNext(currentTrack, manual); const nextIdx = queueIndex + 1; if (nextIdx < queue.length) { - get().playTrack(queue[nextIdx], queue, manual); + get().playTrack(queue[nextIdx], queue, manual, false, nextIdx); // Proactively top up auto-added tracks when ≤ 2 remain ahead, // so the queue never runs dry without a visible loading pause. const { infiniteQueueEnabled } = useAuthStore.getState(); @@ -3014,17 +3039,25 @@ export const usePlayerStore = create()( Promise.all([getSimilarSongs2(artistId), getTopSongs(artistName)]) .then(([similar, top]) => { const existingIds = new Set(get().queue.map(t => t.id)); - const fresh: Track[] = [...top, ...similar] - .map(songToTrack) - .filter(t => !existingIds.has(t.id)) - .slice(0, 10) - .map(t => ({ ...t, radioAdded: true as const })); + // Lead with similar (other artists) for variety; top tracks + // of the upcoming artist are only a fallback when similar + // is empty. Single-pass loop dedupes against the live queue, + // the session seen-set, and intra-batch overlap (issue #500). + const sourceList = similar.length > 0 ? similar : top; + const fresh: Track[] = []; + for (const raw of sourceList) { + if (fresh.length >= 10) break; + const t = songToTrack(raw); + if (existingIds.has(t.id) || radioSessionSeenIds.has(t.id)) continue; + radioSessionSeenIds.add(t.id); + fresh.push({ ...t, radioAdded: true as const }); + } if (fresh.length > 0) { // Trim played tracks from the front to keep the queue bounded. // Without trimming the queue grows unboundedly, making every // Zustand persist write larger and causing UI freezes over time. // Keep the last HISTORY_KEEP played tracks so the user can still - // navigate backwards a few songs. + // navigate backwards a few songs. Trimmed ids stay in the seen-set. const HISTORY_KEEP = 5; set(state => { const trimStart = Math.max(0, state.queueIndex - HISTORY_KEEP); @@ -3041,7 +3074,7 @@ export const usePlayerStore = create()( } } } else if (repeatMode === 'all' && queue.length > 0) { - get().playTrack(queue[0], queue, manual); + get().playTrack(queue[0], queue, manual, false, 0); } else { // Queue exhausted. Check radio first (independent of infinite queue setting), // then infinite queue, then stop. @@ -3053,15 +3086,21 @@ export const usePlayerStore = create()( .then(([similar, top]) => { radioFetching = false; const existingIds = new Set(get().queue.map(t => t.id)); - const fresh: Track[] = [...top, ...similar] - .map(songToTrack) - .filter(t => !existingIds.has(t.id)) - .slice(0, 10) - .map(t => ({ ...t, radioAdded: true as const })); + // Same source preference + dedup contract as the proactive + // top-up: similar first, top only as a fallback (issue #500). + const sourceList = similar.length > 0 ? similar : top; + const fresh: Track[] = []; + for (const raw of sourceList) { + if (fresh.length >= 10) break; + const t = songToTrack(raw); + if (existingIds.has(t.id) || radioSessionSeenIds.has(t.id)) continue; + radioSessionSeenIds.add(t.id); + fresh.push({ ...t, radioAdded: true as const }); + } if (fresh.length > 0) { const currentQueue = get().queue; const newQueue = [...currentQueue, ...fresh]; - get().playTrack(fresh[0], newQueue, false); + get().playTrack(fresh[0], newQueue, false, false, currentQueue.length); } else { invoke('audio_stop').catch(console.error); isAudioPaused = false; @@ -3124,7 +3163,7 @@ export const usePlayerStore = create()( return; } const prevIdx = queueIndex - 1; - if (prevIdx >= 0) get().playTrack(queue[prevIdx], queue); + if (prevIdx >= 0) get().playTrack(queue[prevIdx], queue, true, false, prevIdx); }, // ── seek ───────────────────────────────────────────────────────────────── @@ -3236,23 +3275,56 @@ export const usePlayerStore = create()( }); }, - setRadioArtistId: (artistId) => { currentRadioArtistId = artistId; }, + setRadioArtistId: (artistId) => { + if (artistId !== currentRadioArtistId) { + radioSessionSeenIds = new Set(); + } + currentRadioArtistId = artistId; + }, enqueueRadio: (tracks, artistId) => { - if (artistId) currentRadioArtistId = artistId; + if (artistId !== undefined) { + if (artistId !== currentRadioArtistId) { + radioSessionSeenIds = new Set(); + } + currentRadioArtistId = artistId; + } pushQueueUndoFromGetter(get); set(state => { // Drop all upcoming (not yet played) radio tracks — clicking "Start Radio" // again replaces the pending radio batch instead of stacking on top. const beforeAndCurrent = state.queue.slice(0, state.queueIndex + 1); const upcoming = state.queue.slice(state.queueIndex + 1).filter(t => !t.radioAdded); + // Tracks about to leave the queue here. Callers like ContextMenu.startRadio + // pass the previous pending radio back in `tracks` to merge with new + // similars — the seen-set must not block those re-introductions. + const droppedRadioIds = state.queue + .slice(state.queueIndex + 1) + .filter(t => t.radioAdded) + .map(t => t.id); + for (const id of droppedRadioIds) radioSessionSeenIds.delete(id); + // Capture surviving queue ids in the seen-set so the next radio top-up + // can dedupe against the seed track + already-queued non-radio items. + for (const t of beforeAndCurrent) radioSessionSeenIds.add(t.id); + for (const t of upcoming) radioSessionSeenIds.add(t.id); + // Drop incoming tracks already seen earlier this session AND + // intra-batch duplicates (top + similar Last.fm responses commonly + // overlap). The seen-set is mutated inside the loop so a repeated + // id later in `tracks` is rejected by the same pass that admitted + // the first occurrence (issue #500). + const dedupedTracks: Track[] = []; + for (const t of tracks) { + if (radioSessionSeenIds.has(t.id)) continue; + radioSessionSeenIds.add(t.id); + dedupedTracks.push(t); + } // Insert new radio tracks before any autoAdded tracks in the upcoming section. const firstAutoIdx = upcoming.findIndex(t => t.autoAdded); const merged = firstAutoIdx === -1 - ? [...upcoming, ...tracks] + ? [...upcoming, ...dedupedTracks] : [ ...upcoming.slice(0, firstAutoIdx), - ...tracks, + ...dedupedTracks, ...upcoming.slice(firstAutoIdx), ]; const newQueue = [...beforeAndCurrent, ...merged]; @@ -3307,6 +3379,8 @@ export const usePlayerStore = create()( isAudioPaused = false; clearSeekFallbackRetry(); if (seekDebounce) { clearTimeout(seekDebounce); seekDebounce = null; } clearSeekTarget(); + radioSessionSeenIds = new Set(); + currentRadioArtistId = null; set({ queue: [], queueIndex: 0, currentTrack: null, isPlaying: false, progress: 0, buffered: 0, currentTime: 0 }); syncQueueToServer([], null, 0); },