diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 6dca6ad2..47d39f98 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -25,8 +25,6 @@ import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import kotlin.random.Random -internal const val PLAYBACK_RESTART_THRESHOLD_MS = 3_000L - data class QueueItem( val id: String, val title: String, @@ -747,7 +745,7 @@ class PlaybackController( fun previous() { val player = player ?: return - if (player.currentPositionMs > PLAYBACK_RESTART_THRESHOLD_MS) { + if (player.currentPositionMs > RESTART_THRESHOLD_MS) { scope.launch { startLock.withLock { player.seekTo(0) } refresh() @@ -788,6 +786,26 @@ class PlaybackController( } } + fun previousTrack() { + val snapshot = snapshot ?: return + if (snapshot.size == 0L) return + val current = currentPosition() ?: return + if (offline) { + scope.launch { + downloads.awaitLoaded() + val target = + downloadedAtOrBefore(snapshot, current - 1) + ?: if (repeatMode == RepeatMode.All) downloadedAtOrBefore(snapshot, snapshot.size - 1) else null + target?.let { jumpTo(it) } + } + return + } + when { + current > 0 -> jumpTo(current - 1) + repeatMode == RepeatMode.All -> jumpTo(snapshot.size - 1) + } + } + fun addToQueue( sourceId: Long, kind: QueueKind, @@ -906,15 +924,13 @@ class PlaybackController( suspend fun previousItem(): QueueItem? { var current: QueueSnapshot? = null var position: Long? = null - var restarting = false startLock.withLock { current = snapshot position = currentPosition() - restarting = (player?.currentPositionMs ?: 0L) > PLAYBACK_RESTART_THRESHOLD_MS } val snapshot = current ?: return null val at = position ?: return null - if (at <= 0 || restarting) return null + if (at <= 0) return null return queueRepository.itemAt(snapshot, at - 1)?.toQueueItem() } @@ -1513,6 +1529,7 @@ class PlaybackController( const val BUFFERING_INDICATOR_DELAY_MS = 1_000L const val QUEUE_WINDOW_RADIUS = 25L const val WINDOW_SHIFT_DELAY_MS = 400L + const val RESTART_THRESHOLD_MS = 3_000L const val SEED_PIN_TRIES = 1_000_000 const val FILE_SCHEME = "file:" diff --git a/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt b/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt index da1ad155..4164aa39 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt @@ -77,7 +77,6 @@ import com.subtracks.data.model.Song import com.subtracks.data.repo.DownloadRepository import com.subtracks.data.repo.LibraryRepository import com.subtracks.data.source.StarType -import com.subtracks.playback.PLAYBACK_RESTART_THRESHOLD_MS import com.subtracks.playback.PlaybackController import com.subtracks.playback.PlaybackState import com.subtracks.playback.QueueItem @@ -153,6 +152,21 @@ internal fun rotateStrip( listOf(null, base.getOrNull(0), base.getOrNull(1)) } +internal fun handBackReady( + frozen: List, + itemId: String?, + previousArt: NowPlayingArt?, + nextArt: NowPlayingArt?, +): Boolean { + val centre = frozen.getOrNull(1)?.id ?: return false + if (centre != itemId) return false + return when { + frozen.getOrNull(2) == null -> previousArt?.id == frozen.getOrNull(0)?.id + frozen.getOrNull(0) == null -> nextArt?.id == frozen.getOrNull(2)?.id + else -> true + } +} + @Composable fun NowPlayingRoute( onBack: () -> Unit, @@ -211,8 +225,7 @@ fun NowPlayingRoute( name = item.title, ) var adjacent by remember { mutableStateOf(AdjacentArt()) } - val previousReachable = positionMs <= PLAYBACK_RESTART_THRESHOLD_MS - LaunchedEffect(state.item?.id, state.layout, previousReachable) { + LaunchedEffect(state.item?.id, state.layout) { if (state.item == null) { adjacent = AdjacentArt() return@LaunchedEffect @@ -230,8 +243,9 @@ fun NowPlayingRoute( title = sourceTitle.value, coverArt = art, thumbnailRef = thumbnail, - previousArt = if (previousReachable) adjacent.previous else null, + previousArt = adjacent.previous, nextArt = adjacent.next, + onSkipPrevious = controller::previousTrack, artwork = rememberArtworkColors(thumbnail ?: art, fallbackName = state.item?.title), onBack = onBack, onQueue = onQueue, @@ -263,6 +277,7 @@ fun NowPlayingScreen( onPlayPause: () -> Unit, onNext: () -> Unit, onPrevious: () -> Unit, + onSkipPrevious: () -> Unit = onPrevious, onShuffle: () -> Unit = {}, onRepeat: () -> Unit = {}, onMore: () -> Unit = {}, @@ -317,18 +332,19 @@ fun NowPlayingScreen( fun commitSwipe(direction: Int) { frozenStrip = rotateStrip(strip, direction) swipeOffsetX += direction * coverWidthPx - if (direction > 0) onNext() else onPrevious() + if (direction > 0) onNext() else onSkipPrevious() animateSwipe(0f) } - // The rotated strip keeps the slide continuous while the controller catches up. Hand back once - // it reports the target track, or after a grace period when the action did not move there (a - // restart, an offline skip), so a frozen strip can never stay desynced from playback. - LaunchedEffect(state.item?.id, frozenStrip, dragging) { + // The rotated strip keeps the slide continuous while the controller catches up. It only hands + // back once the live props agree with the frozen slots, so a stale neighbour can never flash + // during the slide; the timeout covers an action that does not move (an offline skip). + LaunchedEffect(state.item?.id, frozenStrip, dragging, previousArt, nextArt) { val pending = frozenStrip ?: return@LaunchedEffect - if (pending.getOrNull(1)?.id == state.item?.id) { + if (dragging) return@LaunchedEffect + if (handBackReady(pending, state.item?.id, previousArt, nextArt)) { frozenStrip = null - } else if (!dragging) { + } else { delay(HAND_BACK_TIMEOUT_MS) if (frozenStrip === pending) frozenStrip = null } @@ -422,7 +438,7 @@ fun NowPlayingScreen( } SwipeAction.Previous -> { - onPrevious() + onSkipPrevious() animateSwipe(0f) } diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index f60248ea..29400916 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -213,7 +213,7 @@ class PlaybackControllerTest { } @Test - fun previousItemIsNullOncePreviousWouldRestartTheTrack() { + fun previousTrackMovesBackEvenOncePreviousWouldRestart() { seedAlbum(3, sourceId = 1) controller.playAlbum(1, "al1", 1) await { @@ -222,8 +222,12 @@ class PlaybackControllerTest { } handle.positionMs = 3_100 + controller.previousTrack() - assertNull(runBlocking { controller.previousItem() }) + await { + controller.state.value.item + ?.id == "s1" + } } @Test diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt index 2365ce99..26fd869b 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt @@ -56,6 +56,7 @@ class NowPlayingTapsTest { onArtistClick: (() -> Unit)? = null, onNext: () -> Unit = {}, onPrevious: () -> Unit = {}, + onSkipPrevious: () -> Unit = onPrevious, previousArt: NowPlayingArt? = null, nextArt: NowPlayingArt? = null, ) { @@ -72,6 +73,7 @@ class NowPlayingTapsTest { onPlayPause = {}, onNext = onNext, onPrevious = onPrevious, + onSkipPrevious = onSkipPrevious, onAlbumClick = onAlbumClick, onArtistClick = onArtistClick, onSeek = {}, @@ -163,12 +165,34 @@ class NowPlayingTapsTest { fun swipingRightOnTheCoverGoesToThePreviousTrack() { var next = false var previous = false - render(onNext = { next = true }, onPrevious = { previous = true }) + var skipPrevious = false + render( + onNext = { next = true }, + onPrevious = { previous = true }, + onSkipPrevious = { skipPrevious = true }, + ) composeRule.onNodeWithTag(NOW_PLAYING_COVER_TAG).performTouchInput { swipeRight() } - assertTrue("a right swipe should go to the previous track", previous) + assertTrue("a right swipe should skip to the previous track", skipPrevious) assertTrue("a right swipe should not skip to the next track", !next) + assertTrue("a right swipe should not use the restart-on-previous button", !previous) + } + + @Test + fun theStripHandsBackOnlyWhenTheLivePropsAgree() { + val before = adjacentArt("s0") + val current = adjacentArt("s1") + val next = adjacentArt("s2") + val after = adjacentArt("s3") + val frozenNext = listOf(current, next, null) + val frozenPrevious = listOf(null, before, current) + + assertTrue(!handBackReady(frozenNext, itemId = "s1", previousArt = before, nextArt = after)) + assertTrue(!handBackReady(frozenNext, itemId = "s2", previousArt = before, nextArt = after)) + assertTrue(handBackReady(frozenNext, itemId = "s2", previousArt = current, nextArt = after)) + assertTrue(handBackReady(frozenPrevious, itemId = "s0", previousArt = null, nextArt = current)) + assertTrue(!handBackReady(frozenPrevious, itemId = "s1", previousArt = null, nextArt = current)) } @Test