diff --git a/lib/providers/playback_state_provider.dart b/lib/providers/playback_state_provider.dart index 2f97edeb..cd1934c3 100644 --- a/lib/providers/playback_state_provider.dart +++ b/lib/providers/playback_state_provider.dart @@ -69,11 +69,14 @@ class PlaybackStateProvider with ChangeNotifier, DisposableChangeNotifierMixin { /// Whether [item] belongs to the currently active queue. True for Plex /// items the server-side queue stamped with a `playQueueItemId`, and for - /// items present in a Jellyfin local queue (synthetic id). Gates the - /// player's "preserve vs. wipe launcher-set queue" decision in both - /// [VideoPlayerScreen.initState] and `_ensurePlayQueue`, so a playlist - /// or collection queue survives entry into the player instead of being - /// replaced with a show queue. + /// items present in a Jellyfin local queue (synthetic id). Membership for + /// local queues is by object identity — [MediaItem] is `@Freezed(equal: + /// false)` — so only the exact instances stored in the queue match. + /// Gates the player's "preserve vs. wipe launcher-set queue" decision in + /// [VideoPlayerScreen.initState], `_ensurePlayQueue`, and + /// [EpisodeNavigationService]'s `_ensureLocalEpisodeQueue`, so a + /// playlist, collection, or shuffled show queue survives entry into the + /// player instead of being replaced with a sequential show queue. bool isItemInActiveQueue(MediaItem item) => isQueueActive && playQueueItemIdFor(item) != null; /// The context key (show/season/playlist ratingKey) for the current session @@ -88,6 +91,11 @@ class PlaybackStateProvider with ChangeNotifier, DisposableChangeNotifierMixin { /// The current play queue item ID int? get currentPlayQueueItemID => _currentPlayQueueItemID; + /// The queue item the cursor currently points at, or null when no queue + /// is active or the cursor is outside the loaded window. + MediaItem? get currentQueueItem => + _currentPlayQueueItemID == null ? null : _findLoadedItem(_currentPlayQueueItemID!); + /// Set the client reference for loading more items void setPlayQueueWindowFetcher(PlayQueueWindowFetcher? fetcher) { _windowFetcher = fetcher; diff --git a/lib/screens/video_player_screen.dart b/lib/screens/video_player_screen.dart index 3b6f7249..d5c79d85 100644 --- a/lib/screens/video_player_screen.dart +++ b/lib/screens/video_player_screen.dart @@ -547,10 +547,10 @@ class VideoPlayerScreenState extends State with WidgetsBindin WidgetsBinding.instance.addPostFrameCallback((_) { // Keep the queue when this item belongs to it — that covers both // server-side queues (Plex `playQueueItemId`) and client-side - // launcher-seeded queues (Jellyfin playlist/collection, with - // synthetic ids tracked in the provider). For genuine standalone - // playback (continue-watching, direct episode tap with no queue - // launcher) clear any stale queue so prev/next stays consistent. + // launcher-seeded queues (Jellyfin playlist/collection/shuffled + // show, with synthetic ids tracked in the provider). For genuine + // standalone playback (continue-watching, direct episode tap with no + // queue launcher) clear any stale queue so prev/next stays consistent. final meta = _currentMetadata; if (playbackState.isItemInActiveQueue(meta)) { playbackState.setCurrentItem(meta); diff --git a/lib/services/episode_navigation_service.dart b/lib/services/episode_navigation_service.dart index ca442057..4bfccca0 100644 --- a/lib/services/episode_navigation_service.dart +++ b/lib/services/episode_navigation_service.dart @@ -33,7 +33,7 @@ class AdjacentEpisodes { /// Plex episodes navigate through the server-side `/playQueues` queue; /// Jellyfin (and any other backend whose /// [MediaServerClient.fetchClientSideEpisodeQueue] returns rows) builds -/// a centred 21-item local queue here and publishes it through +/// a full-series local queue here and publishes it through /// [PlaybackStateProvider] so the rest of the player reads prev/next from /// the same source. class EpisodeNavigationService { @@ -70,8 +70,8 @@ class EpisodeNavigationService { final serverManager = context.read().serverManager; final playbackState = context.read(); - // For Jellyfin, build (or refresh) the centered 21-item window and - // publish it into PlaybackStateProvider so the rest of this method — + // For Jellyfin, make sure a local queue covering the current item is + // published into PlaybackStateProvider so the rest of this method — // and the queue button/sheet — can read prev/next from the same // place Plex does. Plex playback comes in here with its server-side // queue already populated by `_ensurePlayQueue` so this branch is @@ -94,10 +94,13 @@ class EpisodeNavigationService { } } - /// Ensure [PlaybackStateProvider] holds a centered 21-item window of - /// the current series. Cached per-series, so jumping anywhere in the - /// show only triggers one wire fetch per session. No-op for movies, - /// items without a series anchor, or backends whose + /// Ensure [PlaybackStateProvider] holds a queue covering the current + /// item. A queue the item already belongs to (launcher-seeded shuffle, + /// playlist, collection, or an earlier series build) is preserved as-is; + /// otherwise the full series episode list is published, anchored at the + /// current episode. Episode lists are cached per-series, so jumping + /// anywhere in the show only triggers one wire fetch per session. No-op + /// for movies, items without a series anchor, or backends whose /// [MediaServerClient.fetchClientSideEpisodeQueue] returns null (Plex's /// queue lives server-side and is populated elsewhere). Future _ensureLocalEpisodeQueue( @@ -109,10 +112,29 @@ class EpisodeNavigationService { return; } final seriesId = metadata.grandparentId!; - // Don't replace a playlist/collection queue with a series queue. - // The launcher (e.g. [JellyfinSequentialLauncher]) sets contextKey to - // the playlist/collection id; a series rebuild here would clobber it - // and prev/next would walk the show instead of the user's list. + // Preserve any queue this item already belongs to — a launcher-seeded + // shuffled show queue (contextKey == seriesId), a playlist/collection + // queue, or a series queue this method built earlier. setCurrentItem + // re-anchors the cursor, replacing the re-anchor the rebuild used to + // provide. Without this gate a shuffled show queue was clobbered by a + // sequential rebuild after the first episode (#1466). + if (playbackState.isItemInActiveQueue(metadata)) { + playbackState.setCurrentItem(metadata); + return; + } + // Same-episode reload with a fresh object: a source/quality switch hands + // _reloadMediaInPlace a copyWith clone of the playing item, and MediaItem + // compares by identity, so the membership gate above misses. The cursor + // already points at this episode — the queue (and any shuffled order) + // must survive. + if (playbackState.isQueueActive && playbackState.currentQueueItem?.globalKey == metadata.globalKey) { + return; + } + // The playing item isn't in the active queue. Still don't replace a + // playlist/collection queue with a series queue: the launcher (e.g. + // [JellyfinSequentialLauncher]) sets contextKey to the playlist or + // collection id; a series rebuild here would clobber it and prev/next + // would walk the show instead of the user's list. final activeKey = playbackState.shuffleContextKey; if (playbackState.isQueueActive && activeKey != null && activeKey != seriesId) { return; diff --git a/lib/services/media_list_playback_launcher.dart b/lib/services/media_list_playback_launcher.dart index 69ed5a9f..9c05eff3 100644 --- a/lib/services/media_list_playback_launcher.dart +++ b/lib/services/media_list_playback_launcher.dart @@ -200,7 +200,12 @@ abstract class MediaListPlaybackLauncher { await navigateForTesting(itemToPlay); } else { if (!context.mounted) return const PlayQueueError('Context not mounted'); - await navigateToVideoPlayer(context, metadata: itemToPlay); + // The queue holds these exact instances and the player's initState gate + // matches by identity — a WatchStateStore clone here would wipe the + // launcher-set queue on entry. The items were fetched from the server + // in this same user action, so session watch patches are already + // reflected. + await navigateToVideoPlayer(context, metadata: itemToPlay, resolveWatchState: false); } return const PlayQueueSuccess(); } diff --git a/test/services/episode_navigation_service_test.dart b/test/services/episode_navigation_service_test.dart index 76510a1b..1e0902ac 100644 --- a/test/services/episode_navigation_service_test.dart +++ b/test/services/episode_navigation_service_test.dart @@ -237,4 +237,126 @@ void main() { expect(result!.previous?.id, 'ep1'); }); }); + + // =========================================================== + // loadAdjacentEpisodes: shuffled same-series queue (#1466) + // =========================================================== + + group('loadAdjacentEpisodes with a shuffled same-series queue', () { + // Mirrors JellyfinSequentialLauncher.launchShuffledShow: the full series + // episode list, locally shuffled, published with contextKey == seriesId. + // The regression under test: _ensureLocalEpisodeQueue used to rebuild a + // sequential series queue whenever contextKey == seriesId, so shuffle + // held for exactly one episode (#1466). + final ep1 = _jfEpisode('ep1', seriesId: 'series-A'); + final ep2 = _jfEpisode('ep2', seriesId: 'series-A'); + final ep3 = _jfEpisode('ep3', seriesId: 'series-A'); + final ep4 = _jfEpisode('ep4', seriesId: 'series-A'); + final ep5 = _jfEpisode('ep5', seriesId: 'series-A'); + final shuffledOrder = [ep3, ep1, ep5, ep2, ep4]; + const shuffledIds = ['ep3', 'ep1', 'ep5', 'ep2', 'ep4']; + + // Stub server answers with the sequential list, so an (unwanted) queue + // rebuild is observable both via seriesQueueCalls and via reordered + // loadedItems. + (PlaybackStateProvider, _RecordingClient, MultiServerProvider) buildShuffledSession() { + final playback = PlaybackStateProvider(); + addTearDown(playback.dispose); + playback.setPlaybackFromLocalQueue( + LocalPlayQueue( + id: 'jellyfin:series-A', + items: shuffledOrder, + currentIndex: 0, + shuffled: true, + backendId: MediaBackend.jellyfin.id, + ), + contextKey: 'series-A', + ); + final client = _RecordingClient(seriesEpisodes: [ep1, ep2, ep3, ep4, ep5]); + final manager = _StubManager(client); + final serverProvider = MultiServerProvider(manager, DataAggregationService(manager)); + addTearDown(serverProvider.dispose); + return (playback, client, serverProvider); + } + + Future probe( + WidgetTester tester, + PlaybackStateProvider playback, + MultiServerProvider serverProvider, + MediaItem metadata, + ) async { + AdjacentEpisodes? result; + await tester.pumpWidget( + MultiProvider( + providers: [ + ChangeNotifierProvider.value(value: playback), + ChangeNotifierProvider.value(value: serverProvider), + ], + child: _ProbeWidget(metadata: metadata, onResult: (r) => result = r), + ), + ); + await tester.pump(); + await tester.pump(); + return result; + } + + testWidgets('preserves the shuffled order instead of rebuilding sequentially', (tester) async { + final (playback, client, serverProvider) = buildShuffledSession(); + + final result = await probe(tester, playback, serverProvider, ep3); + + expect(client.seriesQueueCalls, isEmpty); + expect(playback.loadedItems.map((e) => e.id), shuffledIds); + expect(playback.isShuffleActive, isTrue); + expect(playback.shuffleContextKey, 'series-A'); + expect(result!.next?.id, 'ep1'); + expect(result.previous, isNull); + }); + + testWidgets('continues the shuffled order after advancing to the next episode', (tester) async { + final (playback, client, serverProvider) = buildShuffledSession(); + // What _reloadMediaInPlace does when the player swaps to the next item. + playback.setCurrentItem(ep1); + + final result = await probe(tester, playback, serverProvider, ep1); + + expect(client.seriesQueueCalls, isEmpty); + expect(playback.isShuffleActive, isTrue); + expect(result!.next?.id, 'ep5'); + expect(result.previous?.id, 'ep3'); + }); + + testWidgets('same-episode clone from a source switch does not clobber the queue', (tester) async { + final (playback, client, serverProvider) = buildShuffledSession(); + + // _switchPlaybackSource reloads with a copyWith clone of the playing + // item; MediaItem compares by identity, so queue membership misses and + // only the cursor-globalKey gate keeps the queue alive. + final result = await probe(tester, playback, serverProvider, ep3.copyWith(viewOffsetMs: 42)); + + expect(client.seriesQueueCalls, isEmpty); + expect(playback.loadedItems.map((e) => e.id), shuffledIds); + expect(playback.isShuffleActive, isTrue); + expect(result!.next?.id, 'ep1'); + }); + + testWidgets('still builds a sequential series queue when no queue is active', (tester) async { + // Direct episode tap with no launcher: the preserve gates must not get + // in the way of the normal series-queue build. + final playback = PlaybackStateProvider(); + addTearDown(playback.dispose); + final client = _RecordingClient(seriesEpisodes: [ep1, ep2, ep3, ep4, ep5]); + final manager = _StubManager(client); + final serverProvider = MultiServerProvider(manager, DataAggregationService(manager)); + addTearDown(serverProvider.dispose); + + final result = await probe(tester, playback, serverProvider, ep3); + + expect(client.seriesQueueCalls, ['series-A']); + expect(playback.loadedItems.map((e) => e.id), ['ep1', 'ep2', 'ep3', 'ep4', 'ep5']); + expect(playback.isShuffleActive, isFalse); + expect(result!.next?.id, 'ep4'); + expect(result.previous?.id, 'ep2'); + }); + }); }