From 4d08b8fb1eaffe7a9f05063626fee4bf43746141 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 18 Oct 2026 08:15:44 +0000 Subject: [PATCH] playback: share edge wrapping with the art predictions Lift the wrap arithmetic out of next(), previous() and previousTrack() into nextPosition()/previousPosition() so upcomingItem()/previousItem() cannot drift from the actions again. A one-track queue no longer predicts itself as its own neighbour, online or offline: the wrap would land on the current position, and the actions are a no-op there, so the strip offered a dead swipe. --- .../subtracks/playback/PlaybackController.kt | 75 +++++++++---------- .../playback/PlaybackControllerTest.kt | 32 ++++++++ 2 files changed, 68 insertions(+), 39 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index ed426a78..5def03de 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -693,12 +693,29 @@ class PlaybackController( } return } - when { - current < snapshot.size - 1 -> jumpTo(current + 1) - repeatMode != RepeatMode.Off -> jumpTo(0) - } + nextPosition(snapshot, current)?.let { jumpTo(it) } } + private fun nextPosition( + snapshot: QueueSnapshot, + current: Long, + ): Long? = + when { + current < snapshot.size - 1 -> current + 1 + repeatMode != RepeatMode.Off -> 0L + else -> null + } + + private fun previousPosition( + snapshot: QueueSnapshot, + current: Long, + ): Long? = + when { + current > 0 -> current - 1 + repeatMode == RepeatMode.All -> snapshot.size - 1 + else -> null + } + private suspend fun consumeUpNext(position: Long) = startLock.withLock { val snapshot = snapshot ?: return@withLock @@ -784,19 +801,12 @@ class PlaybackController( } return } - when { - current > 0 -> { - jumpTo(current - 1) - } - - repeatMode == RepeatMode.All -> { - jumpTo(snapshot.size - 1) - } - - else -> { - player.seekTo(0) - refresh() - } + val target = previousPosition(snapshot, current) + if (target != null) { + jumpTo(target) + } else { + player.seekTo(0) + refresh() } } @@ -814,10 +824,7 @@ class PlaybackController( } return } - when { - current > 0 -> jumpTo(current - 1) - repeatMode == RepeatMode.All -> jumpTo(snapshot.size - 1) - } + previousPosition(snapshot, current)?.let { jumpTo(it) } } fun addToQueue( @@ -935,21 +942,15 @@ class PlaybackController( val snapshot = current ?: return null val at = position ?: return null if (!isOffline) { - val next = - if (at + 1 < snapshot.size) { - at + 1 - } else if (repeatMode != RepeatMode.Off) { - 0L - } else { - null - } - return next?.let { queueRepository.itemAt(snapshot, it)?.toQueueItem() } + return nextPosition(snapshot, at) + ?.takeIf { it != at } + ?.let { queueRepository.itemAt(snapshot, it)?.toQueueItem() } } downloads.awaitLoaded() val target = downloadedAtOrAfter(snapshot, at + 1) ?: if (repeatMode != RepeatMode.Off) downloadedAtOrAfter(snapshot, 0L) else null - return target?.let { queueRepository.itemAt(snapshot, it)?.toQueueItem() } + return target?.takeIf { it != at }?.let { queueRepository.itemAt(snapshot, it)?.toQueueItem() } } suspend fun previousItem(): QueueItem? { @@ -964,19 +965,15 @@ class PlaybackController( val snapshot = current ?: return null val at = position ?: return null if (!isOffline) { - val previous = - when { - at > 0 -> at - 1 - repeatMode == RepeatMode.All -> snapshot.size - 1 - else -> return null - } - return queueRepository.itemAt(snapshot, previous)?.toQueueItem() + return previousPosition(snapshot, at) + ?.takeIf { it != at } + ?.let { queueRepository.itemAt(snapshot, it)?.toQueueItem() } } downloads.awaitLoaded() val target = downloadedAtOrBefore(snapshot, at - 1) ?: if (repeatMode == RepeatMode.All) downloadedAtOrBefore(snapshot, snapshot.size - 1) else null - return target?.let { queueRepository.itemAt(snapshot, it)?.toQueueItem() } + return target?.takeIf { it != at }?.let { queueRepository.itemAt(snapshot, it)?.toQueueItem() } } private fun play( diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 2b1e6a33..222863b4 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -2262,6 +2262,38 @@ class PlaybackControllerTest { ) } + @Test + fun aSingleTrackQueueHasNoAdjacentArtwork() { + seedAlbum(1, sourceId = 1) + controller.playAlbum(1, "al1", 0) + await { + controller.state.value.item + ?.id == "s1" + } + controller.cycleRepeat() + assertEquals(RepeatMode.All, controller.state.value.repeat) + + assertNull(runBlocking { controller.upcomingItem() }) + assertNull(runBlocking { controller.previousItem() }) + } + + @Test + fun aSingleDownloadedTrackHasNoAdjacentArtworkWhenOffline() { + seedAlbum(1, sourceId = 1) + markDownloaded(1, "s1") + setOffline() + controller.playAlbum(1, "al1", 0) + await { + controller.state.value.item + ?.id == "s1" + } + controller.cycleRepeat() + assertEquals(RepeatMode.All, controller.state.value.repeat) + + assertNull(runBlocking { controller.upcomingItem() }) + assertNull(runBlocking { controller.previousItem() }) + } + @Test fun repeatAllLoopsAfterEndingWithRepeatOff() { seedAlbum(2, sourceId = 1)