From 576c20edff8a8856fe2c4a58f7a0b6281e2c0e95 Mon Sep 17 00:00:00 2001 From: damontecres <154766448+damontecres@users.noreply.github.com> Date: Wed, 29 Oct 2025 11:02:21 -0400 Subject: [PATCH] Fixes & optimizations to series pages (#106) Simplifies the process for finding the right season id & episode id after clicking on an episode. This can also improve loading time for seasons that have a large number of episodes (hundreds). Also fixes handling for "unknown" seasons which don't have an index/season number. --- .../wholphin/data/model/BaseItem.kt | 28 ++-- .../damontecres/wholphin/ui/Extensions.kt | 5 + .../ui/detail/series/SeriesDetails.kt | 8 +- .../ui/detail/series/SeriesOverview.kt | 49 +++--- .../ui/detail/series/SeriesOverviewContent.kt | 4 +- .../ui/detail/series/SeriesViewModel.kt | 151 +++++++++--------- .../wholphin/ui/nav/Destination.kt | 3 +- 7 files changed, 130 insertions(+), 118 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/data/model/BaseItem.kt b/app/src/main/java/com/github/damontecres/wholphin/data/model/BaseItem.kt index e49f166b..ed468a90 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/data/model/BaseItem.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/data/model/BaseItem.kt @@ -1,6 +1,6 @@ package com.github.damontecres.wholphin.data.model -import com.github.damontecres.wholphin.ui.detail.series.SeasonEpisode +import com.github.damontecres.wholphin.ui.detail.series.SeasonEpisodeIds import com.github.damontecres.wholphin.ui.nav.Destination import com.github.damontecres.wholphin.util.seasonEpisode import kotlinx.serialization.Serializable @@ -54,27 +54,23 @@ data class BaseItem( // Redirect episodes & seasons to their series if possible when (type) { BaseItemKind.EPISODE -> { - indexNumber?.let { episode -> - data.parentIndexNumber?.let { season -> - Destination.SeriesOverview( - data.seriesId!!, - BaseItemKind.SERIES, - this, - SeasonEpisode(season, episode), - ) - } - } ?: Destination.MediaItem(id, type, this) - } - - BaseItemKind.SEASON -> - data.indexNumber?.let { season -> + data.seasonId?.let { seasonId -> Destination.SeriesOverview( data.seriesId!!, BaseItemKind.SERIES, this, - SeasonEpisode(season, 0), + SeasonEpisodeIds(seasonId, data.parentIndexNumber, id, indexNumber), ) } ?: Destination.MediaItem(id, type, this) + } + + BaseItemKind.SEASON -> + Destination.SeriesOverview( + data.seriesId!!, + BaseItemKind.SERIES, + this, + SeasonEpisodeIds(id, indexNumber, null, null), + ) else -> Destination.MediaItem(id, type, this) } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/Extensions.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/Extensions.kt index c21d9231..e8f6071f 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/Extensions.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/Extensions.kt @@ -375,3 +375,8 @@ suspend fun MutableLiveData.setValueOnMain(value: T) = withContext(Dispatchers.Main) { this@setValueOnMain.value = value } + +fun equalsNotNull( + a: Any?, + b: Any?, +) = a != null && b != null && a == b diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesDetails.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesDetails.kt index f14c7f53..e601303f 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesDetails.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesDetails.kt @@ -92,12 +92,12 @@ fun SeriesDetails( } } LaunchedEffect(Unit) { - viewModel.init(preferences, destination.itemId, destination.item, null, null) + viewModel.init(preferences, destination.itemId, destination.item, null) } val loading by viewModel.loading.observeAsState(LoadingState.Loading) val item by viewModel.item.observeAsState() - val seasons by viewModel.seasons.observeAsState(ItemListAndMapping.empty()) + val seasons by viewModel.seasons.observeAsState(listOf()) val people by viewModel.people.observeAsState(listOf()) val similar by viewModel.similar.observeAsState(listOf()) @@ -190,7 +190,7 @@ private const val SIMILAR_ROW = PEOPLE_ROW + 1 fun SeriesDetailsContent( preferences: UserPreferences, series: BaseItem, - seasons: ItemListAndMapping, + seasons: List, similar: List, people: List, played: Boolean, @@ -332,7 +332,7 @@ fun SeriesDetailsContent( item { ItemRow( title = "Seasons", - items = seasons.items, + items = seasons, onClickItem = { position = SEASONS_ROW onClickItem.invoke(it) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverview.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverview.kt index 4844caf9..04bd1f6a 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverview.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverview.kt @@ -1,3 +1,5 @@ +@file:UseSerializers(UUIDSerializer::class) + package com.github.damontecres.wholphin.ui.detail.series import androidx.compose.runtime.Composable @@ -26,15 +28,20 @@ import com.github.damontecres.wholphin.ui.components.chooseVersionParams import com.github.damontecres.wholphin.ui.data.ItemDetailsDialog import com.github.damontecres.wholphin.ui.data.ItemDetailsDialogInfo import com.github.damontecres.wholphin.ui.detail.buildMoreDialogItems +import com.github.damontecres.wholphin.ui.equalsNotNull +import com.github.damontecres.wholphin.ui.indexOfFirstOrNull import com.github.damontecres.wholphin.ui.nav.Destination import com.github.damontecres.wholphin.ui.tryRequestFocus import com.github.damontecres.wholphin.util.LoadingState import com.github.damontecres.wholphin.util.seasonEpisode import kotlinx.serialization.Serializable +import kotlinx.serialization.UseSerializers import org.jellyfin.sdk.model.api.ImageType import org.jellyfin.sdk.model.extensions.ticks +import org.jellyfin.sdk.model.serializer.UUIDSerializer import org.jellyfin.sdk.model.serializer.toUUID import timber.log.Timber +import java.util.UUID import kotlin.time.Duration @Serializable @@ -43,6 +50,14 @@ data class SeasonEpisode( val episode: Int, ) +@Serializable +data class SeasonEpisodeIds( + val seasonId: UUID, + val seasonNumber: Int?, + val episodeId: UUID?, + val episodeNumber: Int?, +) + @Serializable data class SeriesOverviewPosition( val seasonTabIndex: Int, @@ -55,7 +70,7 @@ fun SeriesOverview( destination: Destination.SeriesOverview, modifier: Modifier = Modifier, viewModel: SeriesViewModel = hiltViewModel(), - initialSeasonEpisode: SeasonEpisode? = null, + initialSeasonEpisode: SeasonEpisodeIds? = null, ) { LifecycleStartEffect(destination.itemId) { viewModel.maybePlayThemeSong( @@ -76,8 +91,7 @@ fun SeriesOverview( preferences, destination.itemId, destination.item, - initialSeasonEpisode?.season, - initialSeasonEpisode?.episode, + initialSeasonEpisode, ) initialLoadDone = true } @@ -85,9 +99,9 @@ fun SeriesOverview( val loading by viewModel.loading.observeAsState(LoadingState.Loading) val series by viewModel.item.observeAsState(null) - val seasons by viewModel.seasons.observeAsState(ItemListAndMapping.empty()) + val seasons by viewModel.seasons.observeAsState(listOf()) val episodes by viewModel.episodes.observeAsState(EpisodeList.Loading) - val episodeList = (episodes as? EpisodeList.Success)?.episodes?.items + val episodeList = (episodes as? EpisodeList.Success)?.episodes var position by rememberSaveable( destination, @@ -100,18 +114,18 @@ fun SeriesOverview( ) { mutableStateOf( SeriesOverviewPosition( - seasons.numberToIndex[initialSeasonEpisode?.season ?: 0] ?: 0, - (episodes as? EpisodeList.Success)?.episodes?.numberToIndex[ - initialSeasonEpisode?.episode - ?: 0, - ] ?: 0, + seasons.indexOfFirstOrNull { + equalsNotNull(it.id, initialSeasonEpisode?.seasonId) || + equalsNotNull(it.indexNumber, initialSeasonEpisode?.seasonNumber) + } ?: 0, + (episodes as? EpisodeList.Success)?.initialIndex ?: 0, ), ) } if (initialLoadDone) { LaunchedEffect(Unit) { - seasons.indexToNumber[position.seasonTabIndex]?.let { - viewModel.loadEpisodes(it) + seasons.getOrNull(position.seasonTabIndex)?.let { + viewModel.loadEpisodes(it.id) } } } @@ -123,10 +137,10 @@ fun SeriesOverview( LaunchedEffect(episodes) { episodes?.let { episodes -> if (episodes is EpisodeList.Success) { - if (episodes.episodes.items.isNotEmpty()) { + if (episodes.episodes.isNotEmpty()) { // TODO focus on first episode when changing seasons? // firstItemFocusRequester.requestFocus() - episodes.episodes.items.getOrNull(position.episodeRowIndex)?.let { + episodes.episodes.getOrNull(position.episodeRowIndex)?.let { viewModel.refreshEpisode(it.id, position.episodeRowIndex) } } @@ -137,7 +151,6 @@ fun SeriesOverview( LaunchedEffect(position) { (episodes as? EpisodeList.Success) ?.episodes - ?.items ?.getOrNull(position.episodeRowIndex) ?.let { viewModel.lookUpChosenTracks(it.id, it) @@ -230,7 +243,7 @@ fun SeriesOverview( SeriesOverviewContent( preferences = preferences, series = series, - seasons = seasons.items, + seasons = seasons, episodes = episodes, chosenStreams = chosenStreams, position = position, @@ -245,8 +258,8 @@ fun SeriesOverview( episodeRowFocusRequester = episodeRowFocusRequester, onFocus = { if (it.seasonTabIndex != position.seasonTabIndex) { - seasons.indexToNumber[it.seasonTabIndex]?.let { seasonNumber -> - viewModel.loadEpisodes(seasonNumber) + seasons.getOrNull(it.seasonTabIndex)?.let { season -> + viewModel.loadEpisodes(season.id) } } position = it diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt index 51495cac..db9402cf 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt @@ -85,7 +85,7 @@ fun SeriesOverviewContent( val tabRowFocusRequester = remember { FocusRequester() } val focusedEpisode = - (episodes as? EpisodeList.Success)?.episodes?.items?.getOrNull(position.episodeRowIndex) + (episodes as? EpisodeList.Success)?.episodes?.getOrNull(position.episodeRowIndex) Box( modifier = @@ -229,7 +229,7 @@ fun SeriesOverviewContent( .focusRestorer(firstItemFocusRequester) .focusRequester(episodeRowFocusRequester), ) { - itemsIndexed(eps.episodes.items) { episodeIndex, episode -> + itemsIndexed(eps.episodes) { episodeIndex, episode -> val interactionSource = remember { MutableInteractionSource() } if (interactionSource.collectIsFocusedAsState().value) { onFocus.invoke( diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt index 556aeeca..35a9c8c3 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt @@ -16,6 +16,7 @@ import com.github.damontecres.wholphin.preferences.ThemeSongVolume import com.github.damontecres.wholphin.preferences.UserPreferences import com.github.damontecres.wholphin.ui.SlimItemFields import com.github.damontecres.wholphin.ui.detail.ItemViewModel +import com.github.damontecres.wholphin.ui.equalsNotNull import com.github.damontecres.wholphin.ui.launchIO import com.github.damontecres.wholphin.ui.letNotEmpty import com.github.damontecres.wholphin.ui.nav.Destination @@ -68,7 +69,7 @@ class SeriesViewModel private lateinit var seriesId: UUID private lateinit var prefs: UserPreferences val loading = MutableLiveData(LoadingState.Loading) - val seasons = MutableLiveData(ItemListAndMapping.empty()) + val seasons = MutableLiveData>(listOf()) val episodes = MutableLiveData(EpisodeList.Loading) val people = MutableLiveData>(listOf()) val similar = MutableLiveData>() @@ -77,8 +78,7 @@ class SeriesViewModel prefs: UserPreferences, itemId: UUID, potential: BaseItem?, - season: Int?, - episode: Int?, + seasonEpisodeIds: SeasonEpisodeIds?, ) { this.seriesId = itemId this.prefs = prefs @@ -89,16 +89,28 @@ class SeriesViewModel ) + Dispatchers.IO, ) { val item = fetchItem(seriesId, potential) - val seasonsInfo = getSeasons(item) + val seasons = getSeasons(item) // If a particular season was requested, fetch those episodes, otherwise get the first season + val initialSeason = + if (seasonEpisodeIds != null) { + seasons.firstOrNull { + equalsNotNull(it.id, seasonEpisodeIds.seasonId) || + equalsNotNull(it.indexNumber, seasonEpisodeIds.seasonNumber) + } + } else { + seasons.firstOrNull() + } val episodeInfo = - (season ?: seasonsInfo.items.firstOrNull()?.indexNumber) - ?.let { seasonNum -> - loadEpisodesInternal(seasonNum) - } ?: EpisodeList.Error("Could not determine season") + initialSeason?.let { + loadEpisodesInternal( + it.id, + seasonEpisodeIds?.episodeId, + seasonEpisodeIds?.episodeNumber, + ) + } ?: EpisodeList.Error("Could not determine season for selected episode") withContext(Dispatchers.Main) { - seasons.value = seasonsInfo + this@SeriesViewModel.seasons.value = seasons episodes.value = episodeInfo loading.value = LoadingState.Success people.value = @@ -108,18 +120,20 @@ class SeriesViewModel }.orEmpty() } if (!similar.isInitialized) { - val similar = - api.libraryApi - .getSimilarItems( - GetSimilarItemsRequest( - userId = serverRepository.currentUser?.id, - itemId = itemId, - fields = SlimItemFields, - limit = 25, - ), - ).content.items - .map { BaseItem.from(it, api, true) } - this@SeriesViewModel.similar.setValueOnMain(similar) + viewModelScope.launchIO { + val similar = + api.libraryApi + .getSimilarItems( + GetSimilarItemsRequest( + userId = serverRepository.currentUser?.id, + itemId = itemId, + fields = SlimItemFields, + limit = 25, + ), + ).content.items + .map { BaseItem.from(it, api, true) } + this@SeriesViewModel.similar.setValueOnMain(similar) + } } } } @@ -163,7 +177,7 @@ class SeriesViewModel themeSongPlayer.stop() } - private suspend fun getSeasons(series: BaseItem): ItemListAndMapping { + private suspend fun getSeasons(series: BaseItem): List { val request = GetItemsRequest( parentId = series.id, @@ -178,30 +192,26 @@ class SeriesViewModel ItemFields.SEASON_USER_DATA, ), ) - val pager = - ApiRequestPager( - api, - request, - GetItemsRequestHandler, - viewModelScope, - ) - pager.init() - Timber.Forest.v("Loaded ${pager.size} seasons for series ${series.id}") - val pairs = - pager.mapIndexed { index, _ -> - val season = pager.getBlocking(index) - Pair(season?.indexNumber!!, index) + val seasons = + GetItemsRequestHandler.execute(api, request).content.items.map { + BaseItem.from( + it, + api, + ) } - val seasonNumToIndex = mapOf(*pairs.toTypedArray()) - val indexToSeasonNum = mapOf(*pairs.map { Pair(it.second, it.first) }.toTypedArray()) - return ItemListAndMapping(pager, seasonNumToIndex, indexToSeasonNum) + Timber.v("Loaded ${seasons.size} seasons for series ${series.id}") + return seasons } - private suspend fun loadEpisodesInternal(season: Int): EpisodeList { + private suspend fun loadEpisodesInternal( + seasonId: UUID, + episodeId: UUID?, + episodeNumber: Int?, + ): EpisodeList { val request = GetEpisodesRequest( - seriesId = item.value!!.id, - season = season, + seriesId = seriesId, + seasonId = seasonId, sortBy = ItemSortBy.INDEX_NUMBER, fields = listOf( @@ -215,18 +225,30 @@ class SeriesViewModel ) val pager = ApiRequestPager(api, request, GetEpisodesRequestHandler, viewModelScope) pager.init() - Timber.v("Loaded ${pager.size} episodes for season $season") - return EpisodeList.Success(convertPager(pager)) + val initialIndex = + if (episodeId != null || episodeNumber != null) { + pager + .indexOfBlocking { + equalsNotNull(it?.id, episodeId) || + equalsNotNull(it?.indexNumber, episodeNumber) + }.coerceAtLeast(0) + } else { + // Force the first page to to be fetched + pager.getBlocking(0) + 0 + } + Timber.v("Loaded ${pager.size} episodes for season $seasonId, initialIndex=$initialIndex") + return EpisodeList.Success(pager, initialIndex) } - fun loadEpisodes(season: Int) { + fun loadEpisodes(seasonId: UUID) { this@SeriesViewModel.episodes.value = EpisodeList.Loading - viewModelScope.launch(ExceptionHandler(true)) { + viewModelScope.launchIO(ExceptionHandler(true)) { val episodes = try { - loadEpisodesInternal(season) + loadEpisodesInternal(seasonId, null, null) } catch (e: Exception) { - Timber.e(e, "Error loading episodes for $seriesId for season $season") + Timber.e(e, "Error loading episodes for $seriesId for season $seasonId") EpisodeList.Error(e) } withContext(Dispatchers.Main) { @@ -295,13 +317,13 @@ class SeriesViewModel ) = viewModelScope.launch(ExceptionHandler() + Dispatchers.IO) { val base = api.userLibraryApi.getItem(itemId).content val item = BaseItem.Companion.from(base, api) - val eps = episodes.value!! + val eps = episodes.value if (eps is EpisodeList.Success) { val newItems = - eps.episodes.items.toMutableList().apply { + eps.episodes.toMutableList().apply { this[listIndex] = item } - val newValue = EpisodeList.Success(eps.episodes.copy(items = newItems)) + val newValue = eps.copy(episodes = newItems) withContext(Dispatchers.Main) { episodes.value = newValue } @@ -398,32 +420,6 @@ class SeriesViewModel } } -data class ItemListAndMapping( - val items: List, - val numberToIndex: Map, - val indexToNumber: Map, -) { - companion object { - fun empty() = ItemListAndMapping(listOf(), mapOf(), mapOf()) - } -} - -/** - * Calculate the index<->season/ep number pairings - * - * This allows for handling of missing seasons - */ -private suspend fun convertPager(pager: ApiRequestPager<*>): ItemListAndMapping { - val pairs = - pager.mapIndexed { index, _ -> - val item = pager.getBlocking(index) - Pair(item?.indexNumber ?: index, index) - } - val seasonNumToIndex = mapOf(*pairs.toTypedArray()) - val indexToSeasonNum = mapOf(*pairs.map { Pair(it.second, it.first) }.toTypedArray()) - return ItemListAndMapping(pager, seasonNumToIndex, indexToSeasonNum) -} - sealed interface EpisodeList { data object Loading : EpisodeList @@ -435,6 +431,7 @@ sealed interface EpisodeList { } data class Success( - val episodes: ItemListAndMapping, + val episodes: List, + val initialIndex: Int, ) : EpisodeList } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/nav/Destination.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/nav/Destination.kt index d62ca443..cb36866b 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/nav/Destination.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/nav/Destination.kt @@ -7,6 +7,7 @@ import com.github.damontecres.wholphin.data.model.BaseItem import com.github.damontecres.wholphin.data.model.GetItemsFilter import com.github.damontecres.wholphin.data.model.ItemPlayback import com.github.damontecres.wholphin.ui.detail.series.SeasonEpisode +import com.github.damontecres.wholphin.ui.detail.series.SeasonEpisodeIds import com.github.damontecres.wholphin.ui.preferences.PreferenceScreenOption import kotlinx.serialization.Serializable import kotlinx.serialization.Transient @@ -48,7 +49,7 @@ sealed class Destination( val itemId: UUID, val type: BaseItemKind, @Transient val item: BaseItem? = null, - val seasonEpisode: SeasonEpisode? = null, + val seasonEpisode: SeasonEpisodeIds? = null, ) : Destination() { override fun toString(): String = "SeriesOverview(itemId=$itemId, type=$type, seasonEpisode=$seasonEpisode)" }