fix shuffled queue edits: flat remap, pinned shuffle start, stable highlight

The previous commit assumed a removed/moved track only shifts play
positions by one, but a shuffled context removal re-derives the
permutation, so the current track's play position is a remap of its flat
index. `removeAt` now maps the current's flat through the updated order
(shuffled) or shifts it (unshuffled), and re-anchors the up-next block to
the track the cursor lands on. This fixes removing any track while
shuffled picking a random track and the anchor removal not advancing to
the block.

Restore "shuffle starts at the top": pick the seed so the requested start
track maps to play position 0 (`pinnedSeed`), instead of leaving the
current at its derived position. Applies to shuffle-play, tapping a track
while shuffled, toggling shuffle on, and restore.

The queue view's playing highlight now requires both the combined position
and the song id, so a transient position mismatch no longer flashes the
row above as playing.
This commit is contained in:
austinried
2026-10-04 00:30:01 +00:00
parent e142148b5a
commit 6d25b31a9a
4 changed files with 114 additions and 27 deletions
@@ -253,15 +253,13 @@ class PlaybackController(
return@withLock return@withLock
} }
val start = randomIndex(snap.contextSize, avoid) val start = randomIndex(snap.contextSize, avoid)
val seed = Random.nextLong() queueRepository.setShuffle(pinnedSeed(snap.contextSize, start))
val position = Shuffle.toSequence(seed, snap.contextSize, start)
queueRepository.setShuffle(seed)
shuffleEnabled = true shuffleEnabled = true
queueSourceId = entry.sourceId queueSourceId = entry.sourceId
queueRepository.setCursor(position) queueRepository.setCursor(0)
snap = queueRepository.snapshot() snap = queueRepository.snapshot()
this@PlaybackController.snapshot = snap this@PlaybackController.snapshot = snap
loadWindow(position, autoplay = true) loadWindow(0, autoplay = true)
} }
private fun randomIndex( private fun randomIndex(
@@ -273,6 +271,21 @@ class PlaybackController(
return if (avoid != null && index == avoid) (index + 1L) % size else index 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) { fun playAt(position: Long) {
scope.launch { scope.launch {
startLock.withLock { startLock.withLock {
@@ -333,21 +346,21 @@ class PlaybackController(
} }
// Derive the current track's new position from the edit. Resolving its song id after // 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 // 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 // block), so use canonical positions throughout: a shuffled removal re-derives the
// the combined position back from the updated snapshot. // 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 = var target =
when { when {
position == current && snapshot.shuffled && !wasUpNext -> { position == current && snapshot.shuffled && !wasUpNext -> {
// The permutation is re-derived at the smaller size, so there is no stable // The permutation is re-derived at the smaller size, so there is no stable
// "next"; play whatever now fills the removed track's slot. // "next"; play whatever now fills the removed track's slot.
val seed = snapshot.shuffleSeed!!
val removedFlat = snapshot.flatContext(snapshot.contextPlay(position)) ?: 0L val removedFlat = snapshot.flatContext(snapshot.contextPlay(position)) ?: 0L
val newContextSize = (snapshot.contextSize - 1).coerceAtLeast(0) val newContextSize = (snapshot.contextSize - 1).coerceAtLeast(0)
if (newContextSize == 0L) { if (newContextSize == 0L) {
0L 0L
} else { } else {
val slot = removedFlat.coerceIn(0, newContextSize - 1) 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 -> { else -> {
val play = snapshot.contextPlay(current) // The current is a context track. Map its flat index through the (possibly
val newPlay = // re-derived) order, decremented when a context track before it was removed.
if (!snapshot.isUpNext(position) && snapshot.contextPlay(position) < play) play - 1 else play val flat = snapshot.flatContext(snapshot.contextPlay(current)) ?: 0L
updated.combined(newPlay) 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) }.coerceIn(0, updated.size - 1)
// Removing the playing context track while shuffled leaves the block anchored to some // Keep the up-next block next: after a removal the cursor may have landed on a context
// other track; re-anchor it to the track the cursor landed on so the queued tracks // track that no longer carries the block (its anchor moved), so re-anchor it there.
// still play next rather than being stranded behind the cursor. if (updated.upNextSize > 0L && !updated.isUpNext(target)) {
if (snapshot.shuffled && !wasUpNext && position == current && updated.upNextSize > 0L) {
val reanchored = queueRepository.reanchorFor(updated, target) val reanchored = queueRepository.reanchorFor(updated, target)
if (reanchored.changed) { if (reanchored.changed) {
updated = readSnapshot() updated = readSnapshot()
@@ -762,9 +777,8 @@ class PlaybackController(
val flat = snapshot.flatContext(snapshot.anchorContextPlay(current)) ?: return@withLock val flat = snapshot.flatContext(snapshot.anchorContextPlay(current)) ?: return@withLock
windowJob?.cancel() windowJob?.cancel()
val enabling = !shuffleEnabled val enabling = !shuffleEnabled
val seed = if (enabling) Random.nextLong() else 0L
if (enabling) { if (enabling) {
queueRepository.setShuffle(seed) queueRepository.setShuffle(pinnedSeed(snapshot.contextSize, flat))
} else { } else {
queueRepository.setShuffle(null) queueRepository.setShuffle(null)
} }
@@ -774,7 +788,7 @@ class PlaybackController(
val position = val position =
when { when {
wasUpNext -> reordered.anchorPlay + 1 + blockIndex wasUpNext -> reordered.anchorPlay + 1 + blockIndex
enabling -> reordered.combined(Shuffle.toSequence(seed, reordered.contextSize, flat)) enabling -> reordered.combined(0L)
else -> reordered.combined(flat) else -> reordered.combined(flat)
} }
val anchor = queueRepository.reanchorFor(reordered, position) val anchor = queueRepository.reanchorFor(reordered, position)
@@ -844,9 +858,8 @@ class PlaybackController(
val start = startPosition.coerceIn(0, snapshot.contextSize - 1) val start = startPosition.coerceIn(0, snapshot.contextSize - 1)
val position = val position =
if (shuffleEnabled) { if (shuffleEnabled) {
val seed = Random.nextLong() queueRepository.setShuffle(pinnedSeed(snapshot.contextSize, start))
queueRepository.setShuffle(seed) 0L
Shuffle.toSequence(seed, snapshot.contextSize, start)
} else { } else {
start start
} }
@@ -868,9 +881,9 @@ class PlaybackController(
var snapshot = queueRepository.snapshot() var snapshot = queueRepository.snapshot()
if (shuffleEnabled && !snapshot.shuffled && snapshot.contextSize > 0L) { if (shuffleEnabled && !snapshot.shuffled && snapshot.contextSize > 0L) {
val start = snapshot.anchorContextPlay(queueRepository.cursor()).coerceIn(0, snapshot.contextSize - 1) val start = snapshot.anchorContextPlay(queueRepository.cursor()).coerceIn(0, snapshot.contextSize - 1)
val seed = Random.nextLong() val flat = snapshot.flatContext(start) ?: start
queueRepository.setShuffle(seed) queueRepository.setShuffle(pinnedSeed(snapshot.contextSize, flat))
queueRepository.setCursor(Shuffle.toSequence(seed, snapshot.contextSize, start)) queueRepository.setCursor(0)
snapshot = queueRepository.snapshot() snapshot = queueRepository.snapshot()
} else if (!snapshot.shuffled) { } else if (!snapshot.shuffled) {
shuffleEnabled = false shuffleEnabled = false
@@ -1235,5 +1248,6 @@ class PlaybackController(
const val QUEUE_WINDOW_RADIUS = 25L const val QUEUE_WINDOW_RADIUS = 25L
const val WINDOW_SHIFT_DELAY_MS = 400L const val WINDOW_SHIFT_DELAY_MS = 400L
const val RESTART_THRESHOLD_MS = 3_000L const val RESTART_THRESHOLD_MS = 3_000L
const val SEED_PIN_TRIES = 1_000_000
} }
} }
@@ -285,6 +285,7 @@ fun QueueRoute(
initialIndex = viewModel.initialIndex, initialIndex = viewModel.initialIndex,
generation = viewModel.generation, generation = viewModel.generation,
currentPosition = playback.position, currentPosition = playback.position,
currentSongId = playback.item?.id,
shuffle = playback.shuffle, shuffle = playback.shuffle,
contextTitle = sourceTitle.value, contextTitle = sourceTitle.value,
onClearUpNext = controller::clearUpNext, onClearUpNext = controller::clearUpNext,
@@ -307,6 +308,7 @@ fun QueueScreen(
rows: List<QueueRow>, rows: List<QueueRow>,
ready: Boolean, ready: Boolean,
currentPosition: Long?, currentPosition: Long?,
currentSongId: String?,
coverArt: (String?, Boolean) -> CoverArtRef?, coverArt: (String?, Boolean) -> CoverArtRef?,
onBack: () -> Unit, onBack: () -> Unit,
onPlay: (Long) -> Unit, onPlay: (Long) -> Unit,
@@ -434,7 +436,7 @@ fun QueueScreen(
) { isDragging -> ) { isDragging ->
QueueRowItem( QueueRowItem(
row = row, row = row,
isPlaying = row.position == currentPosition, isPlaying = row.position == currentPosition && row.song.song.id == currentSongId,
floating = isDragging, floating = isDragging,
coverArt = coverArt, coverArt = coverArt,
dragHandle = dragHandle =
@@ -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 @Test
fun shufflingTheSameAlbumAgainPicksADifferentTrack() { fun shufflingTheSameAlbumAgainPicksADifferentTrack() {
seedAlbum(5, sourceId = 1) seedAlbum(5, sourceId = 1)
@@ -416,6 +416,7 @@ class AppScreenshotTest {
Fixtures.songItems.mapIndexed { index, item -> QueueRow(index.toLong(), index.toLong(), item) }, Fixtures.songItems.mapIndexed { index, item -> QueueRow(index.toLong(), index.toLong(), item) },
ready = true, ready = true,
currentPosition = 1L, currentPosition = 1L,
currentSongId = Fixtures.songItems[1].song.id,
coverArt = { id, _ -> id?.let { CoverArtRef(it, "test:$it") } }, coverArt = { id, _ -> id?.let { CoverArtRef(it, "test:$it") } },
onBack = {}, onBack = {},
onPlay = {}, onPlay = {},