From 87321c7efdbb6917d0dbd045282f1a4760d31981 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:12:21 +0700 Subject: [PATCH] fix(player): preserve playlist context when playing a track Queue the full available Library collection from the selected row instead of replacing playback with one track. Preserve playlist order and map the selected position after skipping unavailable and virtual CUE entries, so Next and Previous retain their context. Cover both themes, repeated tracks, list boundaries, unavailable audio, and external-player behavior. Refs #592. --- lib/providers/playback_provider.dart | 33 +- lib/screens/library_tracks_folder_screen.dart | 24 +- test/playlist_playback_test.dart | 282 ++++++++++++++++++ 3 files changed, 317 insertions(+), 22 deletions(-) create mode 100644 test/playlist_playback_test.dart diff --git a/lib/providers/playback_provider.dart b/lib/providers/playback_provider.dart index e28f0c26..28c6ebf2 100644 --- a/lib/providers/playback_provider.dart +++ b/lib/providers/playback_provider.dart @@ -145,20 +145,24 @@ class PlaybackController extends Notifier { Future playTrackList(List tracks, {int startIndex = 0}) async { if (tracks.isEmpty) return; - final orderedTracks = _orderedTracksFromStartIndex(tracks, startIndex); - final resolvedPaths = await _resolveTrackPaths(orderedTracks); + final safeStart = startIndex.clamp(0, tracks.length - 1); + final resolvedPaths = await resolveTrackFilePaths(tracks); if (await _useInternalPlayer()) { final queue = []; + int? initialIndex; var skippedCueVirtualTrack = false; - for (var index = 0; index < orderedTracks.length; index++) { - final track = orderedTracks[index]; + for (var index = 0; index < tracks.length; index++) { + final track = tracks[index]; final resolvedPath = resolvedPaths[index]; if (resolvedPath == null) continue; if (isCueVirtualPath(resolvedPath)) { skippedCueVirtualTrack = true; continue; } + // Keep the playlist's original order so Previous can reach earlier + // tracks and reaching its end still respects the player's repeat mode. + if (index >= safeStart) initialIndex ??= queue.length; queue.add( PlayableMedia( id: resolvedPath, @@ -177,7 +181,9 @@ class PlaybackController extends Notifier { if (queue.isNotEmpty) { _log.d('Playing ${queue.length} tracks in the internal player'); - await ref.read(musicPlayerControllerProvider).playAll(queue); + await ref + .read(musicPlayerControllerProvider) + .playAll(queue, initialIndex: initialIndex ?? 0); return; } if (skippedCueVirtualTrack) { @@ -189,8 +195,9 @@ class PlaybackController extends Notifier { } var skippedCueVirtualTrack = false; - for (var index = 0; index < orderedTracks.length; index++) { - final track = orderedTracks[index]; + for (var offset = 0; offset < tracks.length; offset++) { + final index = (safeStart + offset) % tracks.length; + final track = tracks[index]; final resolvedPath = resolvedPaths[index]; if (resolvedPath == null) { continue; @@ -222,18 +229,6 @@ class PlaybackController extends Notifier { Future> resolveTrackFilePaths(List tracks) => _resolveTrackPaths(tracks); - List _orderedTracksFromStartIndex(List tracks, int startIndex) { - final safeStart = startIndex.clamp(0, tracks.length - 1); - if (safeStart == 0) { - return List.from(tracks, growable: false); - } - - return [ - ...tracks.sublist(safeStart), - ...tracks.sublist(0, safeStart), - ]; - } - Future> _resolveTrackPaths(List tracks) async { if (tracks.isEmpty) return const []; final localFuture = LibraryDatabase.instance.findExistingBatch([ diff --git a/lib/screens/library_tracks_folder_screen.dart b/lib/screens/library_tracks_folder_screen.dart index a52e433c..6c5b4673 100644 --- a/lib/screens/library_tracks_folder_screen.dart +++ b/lib/screens/library_tracks_folder_screen.dart @@ -351,6 +351,7 @@ class _LibraryTracksFolderScreenState mode: widget.mode, playlistId: widget.playlistId, folderTracks: folderTracks, + trackIndex: index, isInHistory: isInHistory, isSelectionMode: isSelectionMode, isSelected: isSelected, @@ -910,6 +911,7 @@ class _CollectionTrackTile extends ConsumerWidget { final LibraryTracksFolderMode mode; final String? playlistId; final List folderTracks; + final int trackIndex; final bool isInHistory; final bool isSelectionMode; final bool isSelected; @@ -921,6 +923,7 @@ class _CollectionTrackTile extends ConsumerWidget { required this.mode, required this.playlistId, required this.folderTracks, + required this.trackIndex, required this.isInHistory, this.isSelectionMode = false, this.isSelected = false, @@ -1023,9 +1026,7 @@ class _CollectionTrackTile extends ConsumerWidget { trailing: isInHistory || isInLocalLibrary ? IconButton( tooltip: context.l10n.tooltipPlay, - onPressed: () { - ref.read(playbackProvider.notifier).playTrackList([track]); - }, + onPressed: () => _playFromHere(context, ref), icon: Icon(Icons.play_arrow, color: colorScheme.primary), style: IconButton.styleFrom( minimumSize: Size.square(context.tokens.minTouchTarget), @@ -1049,6 +1050,23 @@ class _CollectionTrackTile extends ConsumerWidget { ); } + Future _playFromHere(BuildContext context, WidgetRef ref) async { + try { + await ref + .read(playbackProvider.notifier) + .playTrackList(folderTracks, startIndex: trackIndex); + } catch (error) { + if (!context.mounted) return; + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text( + context.l10n.snackbarCannotOpenFile(context.friendlyError(error)), + ), + ), + ); + } + } + Widget _buildTrackCover(BuildContext context, String coverUrl, double size) { final colorScheme = Theme.of(context).colorScheme; Widget placeholder() => Container( diff --git a/test/playlist_playback_test.dart b/test/playlist_playback_test.dart new file mode 100644 index 00000000..429b441f --- /dev/null +++ b/test/playlist_playback_test.dart @@ -0,0 +1,282 @@ +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:spotiflac_android/l10n/l10n.dart'; +import 'package:spotiflac_android/models/settings.dart'; +import 'package:spotiflac_android/models/track.dart'; +import 'package:spotiflac_android/providers/download_queue_provider.dart'; +import 'package:spotiflac_android/providers/library_collections_provider.dart'; +import 'package:spotiflac_android/providers/local_library_provider.dart'; +import 'package:spotiflac_android/providers/music_player_provider.dart'; +import 'package:spotiflac_android/providers/playback_provider.dart'; +import 'package:spotiflac_android/providers/settings_provider.dart'; +import 'package:spotiflac_android/screens/library_tracks_folder_screen.dart'; +import 'package:spotiflac_android/services/music_player_service.dart'; +import 'package:spotiflac_android/theme/app_theme.dart'; +import 'package:spotiflac_android/theme/mornye_theme.dart'; +import 'package:spotiflac_android/widgets/track_card.dart'; + +final _tracks = [ + for (final name in ['First', 'Unavailable', 'Selected', 'Last']) + Track( + id: name, + name: name, + artistName: 'Artist', + albumName: 'Album', + duration: 180, + ), +]; + +class _Settings extends SettingsNotifier { + @override + AppSettings build() => const AppSettings(playerMode: 'internal'); + + @override + void setPlayerMode(String mode) => state = state.copyWith(playerMode: mode); +} + +class _Handler extends Fake implements MusicPlayerHandler {} + +class _Player extends MusicPlayerController { + List queue = []; + int? index; + + @override + Future ensureInitialized() async => _Handler(); + + @override + Future playAll( + List items, { + int initialIndex = 0, + }) async { + queue = items; + index = initialIndex; + } +} + +class _Playback extends PlaybackController { + final Map paths = { + 'First': 'content://library/first', + 'Selected': 'content://library/selected', + 'Last': 'content://library/last', + }; + + @override + Future> resolveTrackFilePaths(List tracks) async => [ + for (final track in tracks) paths[track.id], + ]; +} + +class _Collections extends LibraryCollectionsNotifier { + @override + LibraryCollectionsState build() => LibraryCollectionsState( + isLoaded: true, + playlists: [ + UserPlaylistCollection( + id: 'playlist', + name: 'Playlist', + createdAt: DateTime(2026), + updatedAt: DateTime(2026), + tracks: [ + for (final track in _tracks) + CollectionTrackEntry( + key: track.id, + track: track, + addedAt: DateTime(2026), + ), + ], + ), + ], + ); + + @override + Future ensurePlaylistLoaded(String playlistId) async {} +} + +class _Library extends LocalLibraryNotifier { + @override + LocalLibraryState build() => LocalLibraryState(); +} + +class _History extends DownloadHistoryNotifier { + @override + DownloadHistoryState build() => DownloadHistoryState(); +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + setUp(() => SharedPreferences.setMockInitialValues({})); + + for (final mornye in [false, true]) { + testWidgets('playlist row queues its neighbors (Mornye: $mornye)', ( + tester, + ) async { + final player = _Player(); + await tester.pumpWidget( + ProviderScope( + overrides: [ + settingsProvider.overrideWith(_Settings.new), + musicPlayerControllerProvider.overrideWithValue(player), + playbackProvider.overrideWith(_Playback.new), + libraryCollectionsProvider.overrideWith(_Collections.new), + localLibraryProvider.overrideWith(_Library.new), + downloadHistoryProvider.overrideWith(_History.new), + downloadHistoryVisibleBatchExistsProvider.overrideWith( + (ref, request) => { + for (final track in request.tracks) + if (track.spotifyId != 'Unavailable') track.lookupKey, + }, + ), + localLibraryCoverProvider.overrideWith( + (ref, request) async => null, + ), + localLibraryFirstCoverProvider.overrideWith( + (ref, request) async => null, + ), + ], + child: MaterialApp( + theme: mornye + ? MornyeTheme.build(Brightness.dark) + : AppTheme.light(), + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + home: const LibraryTracksFolderScreen( + mode: LibraryTracksFolderMode.playlist, + playlistId: 'playlist', + ), + ), + ), + ); + await tester.pumpAndSettle(); + final row = find.ancestor( + of: find.text('Selected'), + matching: find.byType(TrackCard), + ); + await tester.scrollUntilVisible(row, 200); + await tester.tap( + find.descendant(of: row, matching: find.byIcon(Icons.play_arrow)), + ); + await tester.pumpAndSettle(); + + expect(player.queue.map((item) => item.title), [ + 'First', + 'Selected', + 'Last', + ]); + expect(player.index, 1); + expect(tester.takeException(), isNull); + }); + } + + late ProviderContainer container; + late _Player player; + late _Playback playback; + setUp(() { + player = _Player(); + playback = _Playback(); + container = ProviderContainer( + overrides: [ + settingsProvider.overrideWith(_Settings.new), + musicPlayerControllerProvider.overrideWithValue(player), + playbackProvider.overrideWith(() => playback), + ], + ); + addTearDown(container.dispose); + }); + + test( + 'starting at the last track does not rotate earlier tracks after it', + () async { + await container + .read(playbackProvider.notifier) + .playTrackList(_tracks, startIndex: 3); + expect(player.queue.map((item) => item.title), [ + 'First', + 'Selected', + 'Last', + ]); + expect(player.index, player.queue.length - 1); + }, + ); + + test( + 'missing and virtual CUE tracks do not shift the selected track', + () async { + playback.paths['Unavailable'] = '/album.cue#track02'; + await container + .read(playbackProvider.notifier) + .playTrackList(_tracks, startIndex: 2); + expect(player.queue.map((item) => item.title), [ + 'First', + 'Selected', + 'Last', + ]); + expect(player.index, 1); + }, + ); + + test('repeated tracks retain the selected occurrence', () async { + await container.read(playbackProvider.notifier).playTrackList([ + _tracks.first, + _tracks.last, + _tracks.first, + _tracks[2], + ], startIndex: 2); + expect(player.queue.map((item) => item.title), [ + 'First', + 'Last', + 'First', + 'Selected', + ]); + expect(player.index, 2); + }); + + test('unavailable selection starts the next playable track', () async { + await container + .read(playbackProvider.notifier) + .playTrackList(_tracks, startIndex: 1); + expect(player.queue[player.index!].title, 'Selected'); + }); + + test('empty playlist leaves existing playback alone', () async { + await container.read(playbackProvider.notifier).playTrackList([]); + expect(player.index, isNull); + }); + + test( + 'no playable files reports an error without replacing playback', + () async { + playback.paths.clear(); + await expectLater( + container.read(playbackProvider.notifier).playTrackList(_tracks), + throwsA(isA()), + ); + expect(player.index, isNull); + }, + ); + + test('external mode opens only the selected available file', () async { + container.read(settingsProvider.notifier).setPlayerMode('external'); + final calls = []; + const channel = MethodChannel('com.zarz.spotiflac/backend'); + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + messenger.setMockMethodCallHandler(channel, (call) async { + calls.add(call); + return null; + }); + addTearDown(() => messenger.setMockMethodCallHandler(channel, null)); + + await container + .read(playbackProvider.notifier) + .playTrackList(_tracks, startIndex: 2); + expect(calls.single.method, 'openContentUri'); + expect( + calls.single.arguments, + containsPair('uri', 'content://library/selected'), + ); + expect(player.index, isNull); + }); +}