diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 5ad3b3bc..075e37f7 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -253,15 +253,13 @@ class PlaybackController( return@withLock } val start = randomIndex(snap.contextSize, avoid) - val seed = Random.nextLong() - val position = Shuffle.toSequence(seed, snap.contextSize, start) - queueRepository.setShuffle(seed) + queueRepository.setShuffle(pinnedSeed(snap.contextSize, start)) shuffleEnabled = true queueSourceId = entry.sourceId - queueRepository.setCursor(position) + queueRepository.setCursor(0) snap = queueRepository.snapshot() this@PlaybackController.snapshot = snap - loadWindow(position, autoplay = true) + loadWindow(0, autoplay = true) } private fun randomIndex( @@ -273,6 +271,21 @@ class PlaybackController( return if (avoid != null && index == avoid) (index + 1L) % size else index } + // A seed whose permutation puts `flat` at play position 0, so the shuffled queue starts on the + // track that should play instead of somewhere in the middle. Each random seed hits with + // probability 1/size; fall back to a random seed for queues too large to search. + private fun pinnedSeed( + size: Long, + flat: Long, + ): Long { + if (size <= 1L) return Random.nextLong() + repeat(SEED_PIN_TRIES) { + val seed = Random.nextLong() + if (Shuffle.toFlat(seed, size, 0L) == flat) return seed + } + return Random.nextLong() + } + fun playAt(position: Long) { scope.launch { startLock.withLock { @@ -333,21 +346,21 @@ class PlaybackController( } // Derive the current track's new position from the edit. Resolving its song id after // the edit can match an identical song elsewhere (a duplicate in the context or the - // block), so use positions throughout; a context removal can move the anchor, so read - // the combined position back from the updated snapshot. + // block), so use canonical positions throughout: a shuffled removal re-derives the + // permutation at the smaller size, so the current track's play position is a remap of + // its flat index, not a simple shift. var target = when { position == current && snapshot.shuffled && !wasUpNext -> { // The permutation is re-derived at the smaller size, so there is no stable // "next"; play whatever now fills the removed track's slot. - val seed = snapshot.shuffleSeed!! val removedFlat = snapshot.flatContext(snapshot.contextPlay(position)) ?: 0L val newContextSize = (snapshot.contextSize - 1).coerceAtLeast(0) if (newContextSize == 0L) { 0L } else { val slot = removedFlat.coerceIn(0, newContextSize - 1) - updated.combined(Shuffle.toSequence(seed, newContextSize, slot)) + updated.combined(updated.playContext(slot) ?: 0L) } } @@ -363,16 +376,18 @@ class PlaybackController( } else -> { - val play = snapshot.contextPlay(current) - val newPlay = - if (!snapshot.isUpNext(position) && snapshot.contextPlay(position) < play) play - 1 else play - updated.combined(newPlay) + // The current is a context track. Map its flat index through the (possibly + // re-derived) order, decremented when a context track before it was removed. + val flat = snapshot.flatContext(snapshot.contextPlay(current)) ?: 0L + val removedFlat = + if (snapshot.isUpNext(position)) null else snapshot.flatContext(snapshot.contextPlay(position)) + val movedFlat = if (removedFlat != null && removedFlat < flat) flat - 1 else flat + updated.combined(if (snapshot.shuffled) updated.playContext(movedFlat) ?: 0L else movedFlat) } }.coerceIn(0, updated.size - 1) - // Removing the playing context track while shuffled leaves the block anchored to some - // other track; re-anchor it to the track the cursor landed on so the queued tracks - // still play next rather than being stranded behind the cursor. - if (snapshot.shuffled && !wasUpNext && position == current && updated.upNextSize > 0L) { + // Keep the up-next block next: after a removal the cursor may have landed on a context + // track that no longer carries the block (its anchor moved), so re-anchor it there. + if (updated.upNextSize > 0L && !updated.isUpNext(target)) { val reanchored = queueRepository.reanchorFor(updated, target) if (reanchored.changed) { updated = readSnapshot() @@ -762,9 +777,8 @@ class PlaybackController( val flat = snapshot.flatContext(snapshot.anchorContextPlay(current)) ?: return@withLock windowJob?.cancel() val enabling = !shuffleEnabled - val seed = if (enabling) Random.nextLong() else 0L if (enabling) { - queueRepository.setShuffle(seed) + queueRepository.setShuffle(pinnedSeed(snapshot.contextSize, flat)) } else { queueRepository.setShuffle(null) } @@ -774,7 +788,7 @@ class PlaybackController( val position = when { wasUpNext -> reordered.anchorPlay + 1 + blockIndex - enabling -> reordered.combined(Shuffle.toSequence(seed, reordered.contextSize, flat)) + enabling -> reordered.combined(0L) else -> reordered.combined(flat) } val anchor = queueRepository.reanchorFor(reordered, position) @@ -844,9 +858,8 @@ class PlaybackController( val start = startPosition.coerceIn(0, snapshot.contextSize - 1) val position = if (shuffleEnabled) { - val seed = Random.nextLong() - queueRepository.setShuffle(seed) - Shuffle.toSequence(seed, snapshot.contextSize, start) + queueRepository.setShuffle(pinnedSeed(snapshot.contextSize, start)) + 0L } else { start } @@ -868,9 +881,9 @@ class PlaybackController( var snapshot = queueRepository.snapshot() if (shuffleEnabled && !snapshot.shuffled && snapshot.contextSize > 0L) { val start = snapshot.anchorContextPlay(queueRepository.cursor()).coerceIn(0, snapshot.contextSize - 1) - val seed = Random.nextLong() - queueRepository.setShuffle(seed) - queueRepository.setCursor(Shuffle.toSequence(seed, snapshot.contextSize, start)) + val flat = snapshot.flatContext(start) ?: start + queueRepository.setShuffle(pinnedSeed(snapshot.contextSize, flat)) + queueRepository.setCursor(0) snapshot = queueRepository.snapshot() } else if (!snapshot.shuffled) { shuffleEnabled = false @@ -1235,5 +1248,6 @@ class PlaybackController( 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 } } diff --git a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt index 20db62c3..a616108f 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -285,6 +285,7 @@ fun QueueRoute( initialIndex = viewModel.initialIndex, generation = viewModel.generation, currentPosition = playback.position, + currentSongId = playback.item?.id, shuffle = playback.shuffle, contextTitle = sourceTitle.value, onClearUpNext = controller::clearUpNext, @@ -307,6 +308,7 @@ fun QueueScreen( rows: List, ready: Boolean, currentPosition: Long?, + currentSongId: String?, coverArt: (String?, Boolean) -> CoverArtRef?, onBack: () -> Unit, onPlay: (Long) -> Unit, @@ -434,7 +436,7 @@ fun QueueScreen( ) { isDragging -> QueueRowItem( row = row, - isPlaying = row.position == currentPosition, + isPlaying = row.position == currentPosition && row.song.song.id == currentSongId, floating = isDragging, coverArt = coverArt, dragHandle = diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 3f8cb471..7e87ea52 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -2003,6 +2003,76 @@ class PlaybackControllerTest { ) } + @Test + fun shufflePlayStartsAtTheTopOfTheQueue() { + seedAlbum(20, sourceId = 1) + + controller.shuffleAlbum(1, "al1") + await { controller.state.value.item != null && controller.state.value.shuffle } + + assertEquals(0L, controller.state.value.position) + val snapshot = runBlocking { queues.snapshot() } + assertEquals( + controller.state.value.item + ?.id, + runBlocking { queues.itemAt(snapshot, 0)?.song?.id }, + ) + } + + @Test + fun enablingShufflePinsThePlayingTrackToTheTop() { + seedAlbum(20, sourceId = 1) + + controller.playAlbum(1, "al1", 7) + await { + controller.state.value.item + ?.id == "s8" + } + + controller.toggleShuffle() + await { controller.state.value.shuffle } + + assertEquals(0L, controller.state.value.position) + assertEquals( + "s8", + controller.state.value.item + ?.id, + ) + } + + @Test + fun removingAContextTrackWhileShuffledKeepsThePlayingTrackAndTheBlock() { + seedAlbum(6, sourceId = 1) + seedSong("x1", "al2") + + controller.playAlbum(1, "al1", 2) + await { + controller.state.value.item + ?.id == "s3" + } + controller.addToQueue(1, QueueKind.Song, "x1") + await { runBlocking { controller.upcomingItem()?.id } == "x1" } + controller.toggleShuffle() + await { controller.state.value.shuffle && runBlocking { controller.upcomingItem()?.id } == "x1" } + + val currentId = + controller.state.value.item!! + .id + val before = runBlocking { queues.snapshot() } + val currentPosition = controller.state.value.position!! + val removePosition = (0 until before.size).first { !before.isUpNext(it) && it != currentPosition } + + runBlocking { controller.removeAt(removePosition) } + + await { + val snap = runBlocking { queues.snapshot() } + controller.state.value.item + ?.id == currentId && + runBlocking { queues.itemAt(snap, controller.state.value.position!!)?.song?.id } == currentId + } + await { runBlocking { controller.upcomingItem()?.id } == "x1" } + } + @Test fun shufflingTheSameAlbumAgainPicksADifferentTrack() { seedAlbum(5, sourceId = 1) diff --git a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt index 962f74b6..e46f52e7 100644 --- a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt @@ -416,6 +416,7 @@ class AppScreenshotTest { Fixtures.songItems.mapIndexed { index, item -> QueueRow(index.toLong(), index.toLong(), item) }, ready = true, currentPosition = 1L, + currentSongId = Fixtures.songItems[1].song.id, coverArt = { id, _ -> id?.let { CoverArtRef(it, "test:$it") } }, onBack = {}, onPlay = {},