diff --git a/lib/media/episode_collection.dart b/lib/media/episode_collection.dart index 9d2817f7..3aff89fc 100644 --- a/lib/media/episode_collection.dart +++ b/lib/media/episode_collection.dart @@ -54,18 +54,21 @@ Future fetchFirstEpisodeForSeason( return null; } -/// A season number of 0 (or missing) denotes the Specials folder, which the -/// app treats as a last resort for "what to watch next" — see -/// [defaultPlaybackSeasonIndex], [firstUnwatchedSeasonIndex] and -/// [compareEpisodesByWatchOrder]. +/// A season number of 0 (or missing) denotes the Specials folder. Season +/// selection treats it as a last resort for "what to watch next" — see +/// [defaultPlaybackSeasonIndex] and [firstUnwatchedSeasonIndex], which open on +/// the first regular season. Episode ordering ([compareEpisodesByWatchOrder]) +/// instead places Specials by air date, only falling back to Specials-last when +/// an episode has no air date. bool isSpecialSeasonNumber(int? seasonNumber) => (seasonNumber ?? 0) == 0; /// Prefer the first regular season over specials, falling back to the first /// season row when a show only has specials or lacks season indexes. int defaultPlaybackSeasonIndex(List seasons) { if (seasons.isEmpty) return 0; - final regularSeasonIndex = - seasons.indexWhere((season) => season.kind == MediaKind.season && !isSpecialSeasonNumber(season.index)); + final regularSeasonIndex = seasons.indexWhere( + (season) => season.kind == MediaKind.season && !isSpecialSeasonNumber(season.index), + ); if (regularSeasonIndex != -1) return regularSeasonIndex; final firstSeasonIndex = seasons.indexWhere((season) => season.kind == MediaKind.season); return firstSeasonIndex == -1 ? 0 : firstSeasonIndex; @@ -109,18 +112,34 @@ MediaItem? firstUnwatchedEpisode(List episodes) { return null; } -/// Orders episodes the way the app selects "what to watch next": regular -/// seasons first, Specials (season 0) last, then by season number, then -/// episode number. Mirrors the "specials are a last resort" convention used by -/// [defaultPlaybackSeasonIndex] / [firstUnwatchedSeasonIndex] and the offline -/// continue-watching sort, so a count-capped "next N" selection (download / -/// sync rule) takes the next regular episodes instead of the whole Specials -/// folder first (#1414). +/// Orders episodes into the **aired watch order** — the sequence they're meant +/// to be played in: primarily by air date ([MediaItem.originallyAvailableAt]), +/// so a Special that aired between two regular episodes is played between them, +/// the way Plex's own play queue and clients do (#1416). This is the single +/// shared definition of episode order, used by the offline next/prev queue, the +/// Jellyfin online queue, the offline OnDeck list, and the count-capped +/// "download / sync next N" selection — keeping streaming, offline, and +/// download order consistent across both backends. /// -/// The trailing id comparison keeps the order deterministic for episodes that -/// share a season/episode index — Dart's [List.sort] is not stable — so the -/// "next N" cut is stable across runs. +/// Episodes without a usable air date sort *after* dated ones, falling back to +/// season → episode order with Specials last. So undated Specials never wedge +/// into the middle of the aired run, and a "next N" cut still leads with regular +/// episodes — preserving the #1414 guarantee that the whole Specials folder is +/// never front-loaded. The trailing id comparison keeps ties deterministic +/// (Dart's [List.sort] is not stable) so the "next N" cut is stable across runs. int compareEpisodesByWatchOrder(MediaItem a, MediaItem b) { + final aDate = _airDateKey(a); + final bDate = _airDateKey(b); + if (aDate != null && bDate != null) { + final byDate = aDate.compareTo(bDate); + if (byDate != 0) return byDate; + } else if (aDate == null && bDate != null) { + return 1; // undated episodes sort after dated ones + } else if (aDate != null && bDate == null) { + return -1; + } + // Same air date, or both undated: regular seasons before Specials, then by + // season number, episode number, and id. final aSpecial = isSpecialSeasonNumber(a.parentIndex); final bSpecial = isSpecialSeasonNumber(b.parentIndex); if (aSpecial != bSpecial) return aSpecial ? 1 : -1; @@ -131,6 +150,15 @@ int compareEpisodesByWatchOrder(MediaItem a, MediaItem b) { return a.id.compareTo(b.id); } +/// Air date used to order episodes in [compareEpisodesByWatchOrder], or null +/// when absent. Both backends normalize [MediaItem.originallyAvailableAt] to +/// `YYYY-MM-DD` (Plex natively, Jellyfin from `PremiereDate`), so a plain +/// lexicographic comparison is chronological. +String? _airDateKey(MediaItem episode) { + final date = episode.originallyAvailableAt; + return (date == null || date.isEmpty) ? null : date; +} + /// In-place sort by [compareEpisodesByWatchOrder]. See that function for the /// ordering rationale. void sortEpisodesByWatchOrder(List episodes) => episodes.sort(compareEpisodesByWatchOrder); @@ -229,10 +257,12 @@ Future _collectPlayable( final leaves = await client.fetchPlayableDescendants(parentId); // Collect into a local list and order it before handing back: the backend // returns episodes in raw container order (Plex /grandchildren puts S00 - // first), and order-capped callers ("next N unwatched" download, sync-rule - // deficit) slice the front — so without this they'd grab Specials ahead of - // regular episodes (#1414). Sort the per-call slice, not the shared `out` - // accumulator, so multi-container callers don't interleave across shows. + // first). Sorting into aired watch order means order-capped callers ("next N + // unwatched" download, sync-rule deficit) slice the next episodes in the order + // they're meant to be watched — Specials interleaved by air date, never the + // whole Specials folder front-loaded (#1414). Sort the per-call slice, not the + // shared `out` accumulator, so multi-container callers don't interleave across + // shows. final collected = []; for (final ep in leaves) { if (ep.kind != MediaKind.episode) continue; diff --git a/lib/screens/video_player/parts/episode_queue.dart b/lib/screens/video_player/parts/episode_queue.dart index 452565b4..3b8e885f 100644 --- a/lib/screens/video_player/parts/episode_queue.dart +++ b/lib/screens/video_player/parts/episode_queue.dart @@ -119,10 +119,10 @@ extension _VideoPlayerEpisodeQueueMethods on VideoPlayerScreenState { if (episodes.isEmpty) return; - // Regular seasons first, Specials last, then season/episode — the shared - // watch order, so offline next/prev matches what "download next N" selects - // and the offline continue-watching list use (#1414). Copy first so the - // provider's cached list isn't reordered. + // Aired watch order (Specials interleaved by air date) — the shared + // episode order, so offline next/prev matches streaming, what "download + // next N" selects, and the offline OnDeck list (#1416/#1414). Copy first + // so the provider's cached list isn't reordered. final sorted = List.from(episodes)..sort(compareEpisodesByWatchOrder); final currentIdx = sorted.indexWhere((ep) => ep.id == _currentMetadata.id); diff --git a/lib/services/jellyfin_client/parts/browse.dart b/lib/services/jellyfin_client/parts/browse.dart index 09f075af..eb7c1155 100644 --- a/lib/services/jellyfin_client/parts/browse.dart +++ b/lib/services/jellyfin_client/parts/browse.dart @@ -70,11 +70,13 @@ const _folderRowFields = 'SortName'; /// Even slimmer set used by [fetchClientSideEpisodeQueue]. Queue rows /// only need title, thumbnail (`ImageTags['Primary']`), season/episode -/// index, and watched state. Title + indices come back without any -/// `Fields` request; we only need to ask for `UserData` for the -/// watched indicator. Drops `Overview` etc. so that even a thousand- -/// episode shounen show fits comfortably in one response. -const _queueFields = 'UserData'; +/// index, watched state, and the air date that drives the watch order. +/// Title + indices come back without any `Fields` request; we ask for +/// `UserData` (watched indicator) and `PremiereDate` (air-date sort, so +/// Specials interleave — see [compareEpisodesByWatchOrder]). Drops +/// `Overview` etc. so even a thousand-episode shounen show fits in one +/// response. +const _queueFields = 'UserData,PremiereDate'; /// Page size for [fetchClientSideEpisodeQueue]. Keeps each server response /// bounded while still returning the full series queue. @@ -915,16 +917,17 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { return _pagedMediaItems(response.data, offset: offset, requestedSize: pageSize); } - /// All episodes of a series in the app's watch order — regular seasons - /// first, Specials (season 0) last — so the client-side next/previous queue - /// matches what downloads and offline playback use (#1414). The server sort - /// ([_episodeOrderQueryParameters]) keeps paging stable (and lists Specials - /// first); [sortEpisodesByWatchOrder] then normalizes the assembled list to - /// the shared convention, leaving a single definition of "episode order". + /// All episodes of a series in the app's **aired watch order** — primarily by + /// air date, so Specials interleave between regular episodes the way Plex's + /// own play queue does — so the client-side next/previous queue matches + /// streaming, downloads, and offline playback (#1416/#1414). The server sort + /// ([_episodeOrderQueryParameters]) only keeps paging stable; + /// [sortEpisodesByWatchOrder] then orders the assembled list, leaving a single + /// definition of "episode order". /// - /// Uses [_queueFields] (only `UserData`) instead of the browse field - /// set so the response stays small even for shows with thousands of - /// episodes. + /// Uses [_queueFields] (`UserData` + `PremiereDate`) instead of the full + /// browse field set so the response stays small even for shows with thousands + /// of episodes. /// /// Paged in [_episodeQueuePageSize] chunks so long-running shows still get /// a complete client-side next/previous queue without one huge response. @@ -960,8 +963,8 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { startIndex += page.length; } - // Server lists Specials first (ParentIndexNumber asc); reorder to the - // shared watch order so online next/prev matches offline + downloads. + // Server lists Specials first (ParentIndexNumber asc); reorder into the + // shared aired watch order so online next/prev matches offline + downloads. sortEpisodesByWatchOrder(all); return all; } diff --git a/test/utils/episode_collection_test.dart b/test/utils/episode_collection_test.dart index 5e1216a2..af62d3d8 100644 --- a/test/utils/episode_collection_test.dart +++ b/test/utils/episode_collection_test.dart @@ -28,6 +28,7 @@ MediaItem _episode( int? viewCount, int? viewOffsetMs, int? durationMs, + String? originallyAvailableAt, }) => MediaItem( id: id, backend: MediaBackend.plex, @@ -41,6 +42,7 @@ MediaItem _episode( viewCount: viewCount, viewOffsetMs: viewOffsetMs, durationMs: durationMs, + originallyAvailableAt: originallyAvailableAt, ); MediaItem _clip(String id) => MediaItem(id: id, backend: MediaBackend.plex, kind: MediaKind.clip, title: 'Clip'); @@ -191,7 +193,24 @@ void main() { ); }); - test('sortEpisodesByWatchOrder puts regular seasons first and Specials last', () { + test('sortEpisodesByWatchOrder interleaves Specials into aired order by air date', () { + // Mirrors a real interleaved-Specials show (e.g. The Eminence in Shadow): + // S00E01 aired between S01E02 and S01E05, so it plays there — not after the + // whole season. This is what Plex's own play queue returns (#1416). + final s1e1 = _episode('s1e1', parentIndex: 1, index: 1, originallyAvailableAt: '2022-10-05'); + final s1e2 = _episode('s1e2', parentIndex: 1, index: 2, originallyAvailableAt: '2022-10-12'); + final s0e1 = _episode('s0e1', parentIndex: 0, index: 1, originallyAvailableAt: '2022-10-27'); + final s1e5 = _episode('s1e5', parentIndex: 1, index: 5, originallyAvailableAt: '2022-11-02'); + final s0e2 = _episode('s0e2', parentIndex: 0, index: 2, originallyAvailableAt: '2022-11-03'); + + // Raw container order would clump the Specials folder first. + final episodes = [s0e1, s0e2, s1e1, s1e2, s1e5]; + sortEpisodesByWatchOrder(episodes); + + expect(episodes.map((e) => e.id), ['s1e1', 's1e2', 's0e1', 's1e5', 's0e2']); + }); + + test('sortEpisodesByWatchOrder falls back to Specials-last when episodes have no air dates', () { final s0e1 = _episode('s0e1', parentIndex: 0, index: 1); final s0e2 = _episode('s0e2', parentIndex: 0, index: 2); final s1e1 = _episode('s1e1', parentIndex: 1, index: 1); @@ -202,10 +221,23 @@ void main() { final episodes = [s0e1, s0e2, s1e1, s1e2, s2e1]; sortEpisodesByWatchOrder(episodes); - // A "next 2 unwatched" cut now takes S01E01/S01E02, not the Specials. + // With no air dates, a "next 2 unwatched" cut still takes S01E01/S01E02. expect(episodes.map((e) => e.id), ['s1e1', 's1e2', 's2e1', 's0e1', 's0e2']); }); + test('sortEpisodesByWatchOrder trails undated Specials after dated episodes', () { + // A dated regular run with an undated Special: the Special can't be placed + // in the aired timeline, so it sorts last rather than wedging into the run. + final s1e1 = _episode('s1e1', parentIndex: 1, index: 1, originallyAvailableAt: '2022-10-05'); + final s1e2 = _episode('s1e2', parentIndex: 1, index: 2, originallyAvailableAt: '2022-10-12'); + final s0e1 = _episode('s0e1', parentIndex: 0, index: 1); + + final episodes = [s0e1, s1e2, s1e1]; + sortEpisodesByWatchOrder(episodes); + + expect(episodes.map((e) => e.id), ['s1e1', 's1e2', 's0e1']); + }); + test('compareEpisodesByWatchOrder breaks index ties on id for deterministic cuts', () { final a = _episode('a', parentIndex: 1, index: 1); final b = _episode('b', parentIndex: 1, index: 1); @@ -228,10 +260,7 @@ void main() { // Watched with no resume point — counts as done. expect(_episode('b', viewCount: 1).isUnwatchedOrInProgress, isFalse); // Watched but still resumable (re-watching) — counts as to-watch. - expect( - _episode('c', viewCount: 1, viewOffsetMs: 500, durationMs: 1000).isUnwatchedOrInProgress, - isTrue, - ); + expect(_episode('c', viewCount: 1, viewOffsetMs: 500, durationMs: 1000).isUnwatchedOrInProgress, isTrue); }); test('fetchFirstEpisodeForSeason requests only the first children page', () async {