Fix bug finding the episode to scroll to on Series Overview (#1708)
## Description Fixes an off-by-one error when searching for the right episode (or season) to scroll to on the series overview page. While the code works most of the time, on cases where a special appears _after_ a season is included, the final episode in the season not be scrolled to. For example, if Special S00E04 comes after Season 2 (which has 10 episodes), clicking on S02E10 on the home page won't scroll to that episode on the series overview, instead S02E01 will be focused. And S00E04 will appear at the end of the list. This should also be a very, very tiny performance boost since the "search the whole list" fallback won't be triggered as often. I also added some comments to explain the function better. ### Related issues N/A, just noticed it yesterday ### Testing Emulator in the situation described above ## Screenshots N/A ## AI or LLM usage None
This commit is contained in:
Damontecres
GitHub
parent
46b2f2839f
commit
84f6035103
2 changed files with 177 additions and 23 deletions
|
|
@ -37,6 +37,7 @@ import com.github.damontecres.wholphin.ui.lt
|
|||
import com.github.damontecres.wholphin.ui.nav.Destination
|
||||
import com.github.damontecres.wholphin.ui.showToast
|
||||
import com.github.damontecres.wholphin.util.ApiRequestPager
|
||||
import com.github.damontecres.wholphin.util.BlockingList
|
||||
import com.github.damontecres.wholphin.util.DataLoadingState
|
||||
import com.github.damontecres.wholphin.util.ExceptionHandler
|
||||
import com.github.damontecres.wholphin.util.GetEpisodesRequestHandler
|
||||
|
|
@ -165,7 +166,7 @@ class SeriesViewModel
|
|||
viewModelScope.launchIO {
|
||||
val index =
|
||||
(seasons as? ApiRequestPager<*>)?.let {
|
||||
findIndexOf(
|
||||
findIndexByNumberOrId(
|
||||
seasonEpisodeIds.seasonNumber,
|
||||
seasonEpisodeIds.seasonId,
|
||||
it,
|
||||
|
|
@ -361,7 +362,7 @@ class SeriesViewModel
|
|||
pager.init(episodeNumber ?: 0)
|
||||
val initialIndex =
|
||||
if (episodeId != null || episodeNumber != null) {
|
||||
findIndexOf(episodeNumber, episodeId, pager)
|
||||
findIndexByNumberOrId(episodeNumber, episodeId, pager)
|
||||
.coerceAtLeast(0)
|
||||
} else {
|
||||
// Force the first page to to be fetched
|
||||
|
|
@ -704,43 +705,67 @@ enum class SeriesPageType {
|
|||
OVERVIEW,
|
||||
}
|
||||
|
||||
private suspend fun findIndexOf(
|
||||
private fun checkNumberOrId(
|
||||
targetNum: Int?,
|
||||
targetId: UUID?,
|
||||
pager: ApiRequestPager<*>,
|
||||
indexNumber: Int?,
|
||||
id: UUID?,
|
||||
): Boolean =
|
||||
if (targetId != null) {
|
||||
equalsNotNull(targetId, id)
|
||||
} else {
|
||||
equalsNotNull(indexNumber, targetNum)
|
||||
}
|
||||
|
||||
/**
|
||||
* Find the index in the [list] where the item's `indexNumber`==[targetNum] or `id`==[targetId]
|
||||
*
|
||||
* This is necessary in cases where items are missing. E.g. if looking for episode 4 but
|
||||
* episodes 2 & 3 is missing, then the index in the list for episode 4 will be `1`.
|
||||
*
|
||||
* @param targetNum the 1-index season or episode number
|
||||
* @param targetId the season or episode ID
|
||||
*
|
||||
* @return the index within [list] that matches, or zero if no match is found
|
||||
*/
|
||||
suspend fun findIndexByNumberOrId(
|
||||
targetNum: Int?,
|
||||
targetId: UUID?,
|
||||
list: BlockingList<BaseItem?>,
|
||||
): Int {
|
||||
// Adjust for 1-based numbers
|
||||
val listIndex = targetNum?.minus(1)?.coerceAtLeast(0)
|
||||
val index =
|
||||
if (targetId != null && (targetNum == null || targetNum !in pager.indices)) {
|
||||
if (targetId != null && (targetNum == null || listIndex !in list.indices)) {
|
||||
// No hint info, so have to check everything
|
||||
pager.indexOfBlocking {
|
||||
equalsNotNull(it?.indexNumber, targetNum) ||
|
||||
equalsNotNull(it?.id, targetId)
|
||||
}
|
||||
} else if (targetNum != null && targetNum in pager.indices) {
|
||||
// Start searching from the season number and choose direction from there
|
||||
val num = pager.getBlocking(targetNum)?.indexNumber
|
||||
list
|
||||
.indexOfBlocking {
|
||||
checkNumberOrId(targetNum, targetId, it?.indexNumber, it?.id)
|
||||
}.coerceAtLeast(0)
|
||||
} else if (listIndex != null && listIndex in list.indices) {
|
||||
// Start searching from the target number and choose direction from there
|
||||
val num = list.getBlocking(listIndex)?.indexNumber
|
||||
if (num.lt(targetNum)) {
|
||||
for (i in targetNum + 1 until pager.lastIndex) {
|
||||
val season = pager.getBlocking(i)
|
||||
if (equalsNotNull(season?.indexNumber, targetNum) ||
|
||||
equalsNotNull(season?.id, targetId)
|
||||
) {
|
||||
for (i in listIndex + 1 until list.lastIndex) {
|
||||
val item = list.getBlocking(i)
|
||||
if (checkNumberOrId(targetNum, targetId, item?.indexNumber, item?.id)) {
|
||||
return i
|
||||
}
|
||||
}
|
||||
return 0
|
||||
} else if (num.gt(targetNum)) {
|
||||
for (i in targetNum - 1 downTo 0) {
|
||||
val season = pager.getBlocking(i)
|
||||
if (equalsNotNull(season?.indexNumber, targetNum) ||
|
||||
equalsNotNull(season?.id, targetId)
|
||||
) {
|
||||
for (i in listIndex - 1 downTo 0) {
|
||||
val item = list.getBlocking(i)
|
||||
if (checkNumberOrId(targetNum, targetId, item?.indexNumber, item?.id)) {
|
||||
return i
|
||||
}
|
||||
}
|
||||
return 0
|
||||
} else {
|
||||
targetNum
|
||||
list
|
||||
.indexOfBlocking {
|
||||
checkNumberOrId(targetNum, targetId, it?.indexNumber, it?.id)
|
||||
}.coerceAtLeast(0)
|
||||
}
|
||||
} else {
|
||||
0
|
||||
|
|
|
|||
|
|
@ -0,0 +1,129 @@
|
|||
package com.github.damontecres.wholphin.test
|
||||
|
||||
import com.github.damontecres.wholphin.data.model.BaseItem
|
||||
import com.github.damontecres.wholphin.ui.detail.series.findIndexByNumberOrId
|
||||
import com.github.damontecres.wholphin.util.BlockingList
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.jellyfin.sdk.model.UUID
|
||||
import org.jellyfin.sdk.model.api.BaseItemDto
|
||||
import org.jellyfin.sdk.model.api.BaseItemKind
|
||||
import org.junit.Assert
|
||||
import org.junit.Test
|
||||
|
||||
class TestFindIndexByNumberOrId {
|
||||
fun create(
|
||||
indexNumber: Int,
|
||||
parentIndexNumber: Int,
|
||||
) = BaseItem(
|
||||
BaseItemDto(
|
||||
id = UUID.randomUUID(),
|
||||
type = BaseItemKind.EPISODE,
|
||||
indexNumber = indexNumber,
|
||||
parentIndexNumber = parentIndexNumber,
|
||||
),
|
||||
)
|
||||
|
||||
val epS02E01 = create(1, 2)
|
||||
val epS02E02 = create(2, 2)
|
||||
val epS02E03 = create(3, 2)
|
||||
val epS02E04 = create(4, 2)
|
||||
val epS02E05 = create(5, 2)
|
||||
val epS00E08 = create(8, 0)
|
||||
val epS00E02 = create(2, 0)
|
||||
|
||||
private val episodes =
|
||||
listOf(
|
||||
epS02E01,
|
||||
epS02E02,
|
||||
epS02E03,
|
||||
)
|
||||
|
||||
private val missingEpisodes =
|
||||
listOf(
|
||||
epS02E01,
|
||||
epS02E04,
|
||||
epS02E05,
|
||||
)
|
||||
|
||||
private val withSpecials =
|
||||
listOf(
|
||||
epS02E01,
|
||||
epS02E02,
|
||||
epS02E03,
|
||||
epS00E08,
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `Basic tests`() =
|
||||
runTest {
|
||||
findIndexByNumberOrId(targetNum = 1, targetId = epS02E01.id, list = BlockingList.of(episodes)).let { index ->
|
||||
Assert.assertEquals(0, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 2, targetId = epS02E02.id, list = BlockingList.of(episodes)).let { index ->
|
||||
Assert.assertEquals(1, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 3, targetId = epS02E03.id, list = BlockingList.of(episodes)).let { index ->
|
||||
Assert.assertEquals(2, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 100, targetId = UUID.randomUUID(), list = BlockingList.of(episodes)).let { index ->
|
||||
Assert.assertEquals(0, index)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `Test missing episodes`() =
|
||||
runTest {
|
||||
findIndexByNumberOrId(targetNum = 1, targetId = epS02E01.id, list = BlockingList.of(missingEpisodes)).let { index ->
|
||||
Assert.assertEquals(0, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 2, targetId = epS02E02.id, list = BlockingList.of(missingEpisodes)).let { index ->
|
||||
Assert.assertEquals(0, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 4, targetId = epS02E04.id, list = BlockingList.of(missingEpisodes)).let { index ->
|
||||
Assert.assertEquals(1, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 5, targetId = epS02E05.id, list = BlockingList.of(missingEpisodes)).let { index ->
|
||||
Assert.assertEquals(2, index)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `Test missing episodes without ID`() =
|
||||
runTest {
|
||||
findIndexByNumberOrId(targetNum = 1, targetId = null, list = BlockingList.of(missingEpisodes)).let { index ->
|
||||
Assert.assertEquals(0, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 2, targetId = null, list = BlockingList.of(missingEpisodes)).let { index ->
|
||||
Assert.assertEquals(0, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 4, targetId = null, list = BlockingList.of(missingEpisodes)).let { index ->
|
||||
Assert.assertEquals(0, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 5, targetId = null, list = BlockingList.of(missingEpisodes)).let { index ->
|
||||
Assert.assertEquals(0, index)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `Test with special`() =
|
||||
runTest {
|
||||
findIndexByNumberOrId(targetNum = 1, targetId = epS02E01.id, list = BlockingList.of(withSpecials)).let { index ->
|
||||
Assert.assertEquals(0, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 2, targetId = epS02E02.id, list = BlockingList.of(withSpecials)).let { index ->
|
||||
Assert.assertEquals(1, index)
|
||||
}
|
||||
findIndexByNumberOrId(targetNum = 3, targetId = epS02E03.id, list = BlockingList.of(withSpecials)).let { index ->
|
||||
Assert.assertEquals(2, index)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `Test with special matching number`() =
|
||||
runTest {
|
||||
val list = listOf(epS02E01, epS00E02, epS02E02)
|
||||
findIndexByNumberOrId(targetNum = 2, targetId = epS02E02.id, list = BlockingList.of(list)).let { index ->
|
||||
Assert.assertEquals(2, index)
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue