fix resuming after removing the playing track in a shuffled queue

The "play whatever now fills the removed track's slot" branch was written
for the old re-derive model: it mapped the removed canonical slot through
the new permutation, which now (with the fixed shuffle order) picks an
unrelated track. Removing the playing track should just resume at the same
slot, which holds whatever was next.

- Drop the special slot branch: `position == current` resumes at `current`
  for shuffled and unshuffled alike. Without a block that is the next
  shuffled track; with one, re-anchor the block from the removed track onto
  the track now preceding that slot so the block takes the slot and plays
  next (previously it was stranded behind a random track).
- Tests assert the immediate playing track, not just what is upcoming, which
  is why they missed this. Both new tests fail on the previous commit.
- Cover advancing out of a block track and moving a block track while
  shuffled (the last items from the QA review).
This commit is contained in:
austinried
2026-10-04 00:30:01 +00:00
parent eb6b08ff56
commit 72330387c8
3 changed files with 77 additions and 76 deletions
@@ -437,6 +437,8 @@ class QueueRepository(
return Reanchored(play, true)
}
suspend fun setUpNextAnchor(anchor: Long) = cursorMutex.withLock { dao.setCursor(cursorRow().copy(upNextAnchor = anchor)) }
suspend fun clearUpNext() {
writeUpNext(emptyList())
}
@@ -344,26 +344,24 @@ class PlaybackController(
lastEdit = ReloadUndo(entries, cursor, snapshot.shuffleSeed, upNext, snapshot.upNextAnchor)
return@withLock
}
// The block floats behind the currently playing context track, so removing that track
// removes its anchor. Re-anchor the block to the track that now precedes the removed
// slot in play order, so it takes the slot and plays next, rather than the anchor
// landing wherever `adjustAnchorOnRemove` left it.
if (snapshot.shuffled && position == current && !wasUpNext && updated.upNextSize > 0L) {
val play = snapshot.contextPlay(current)
val predecessor = play - 1L
val anchor = if (predecessor < 0L) -1L else updated.flatContext(predecessor) ?: -1L
queueRepository.setUpNextAnchor(anchor)
updated = readSnapshot()
this.snapshot = updated
}
// 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 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.
// block), so use positions throughout. Removing the playing track resumes at the same
// slot, which now holds whatever was next (the block, if one is queued).
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 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(updated.playContext(slot) ?: 0L)
}
}
position == current -> {
current
}
@@ -1640,63 +1640,6 @@ class PlaybackControllerTest {
assertEquals(5, remaining.distinct().size)
}
@Test
fun removingTheCurrentTrackWhileShuffledPlaysTheSongThatFillsItsSlot() {
seedAlbum(6, sourceId = 1)
controller.playAlbum(1, "al1", 2)
await {
controller.state.value.item
?.id == "s3"
}
controller.toggleShuffle()
await { controller.state.value.shuffle }
val current = controller.state.value.position!!
val before = runBlocking { queues.snapshot() }
val removedFlat = before.flatContext(current)!!
runBlocking { controller.removeAt(current) }
val after = runBlocking { queues.snapshot() }
val newContextSize = (before.contextSize - 1).coerceAtLeast(0)
val slot = removedFlat.coerceIn(0, newContextSize - 1)
assertEquals(5L, after.size)
assertEquals(
// The song now occupying the removed track's canonical slot is the one playing.
slot,
after.flatContext(after.contextPlay(controller.state.value.position!!)),
)
assertEquals(
runBlocking { queues.itemAt(after, controller.state.value.position!!)?.song?.id },
controller.state.value.item
?.id,
)
}
@Test
fun removingThePlayingAnchorWhileShuffledPlaysTheQueuedTrackNext() {
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 }
await { runBlocking { controller.upcomingItem()?.id } == "x1" }
val current = controller.state.value.position!!
runBlocking { controller.removeAt(current) }
await { runBlocking { controller.upcomingItem()?.id } == "x1" }
}
@Test
fun removingThePlayingTrackWhileShuffledLeavesTheBlockReachable() {
seedAlbum(6, sourceId = 1)
@@ -1714,14 +1657,34 @@ class PlaybackControllerTest {
val current = controller.state.value.position!!
runBlocking { controller.removeAt(current) }
// The block was queued behind the removed track, so it takes the slot and plays now.
await {
controller.state.value.item
?.id == "x1"
}
val after = runBlocking { queues.snapshot() }
assertTrue(after.shuffled)
assertEquals(
controller.state.value.item
?.id,
"x1",
runBlocking { queues.itemAt(after, controller.state.value.position!!)?.song?.id },
)
await { runBlocking { controller.upcomingItem()?.id } == "x1" }
}
@Test
fun removingThePlayingTrackWhileShuffledPlaysTheNextShuffledTrack() {
seedShuffledQueue(seed = 12345L)
val orderBefore = resolvedIds()
val current = controller.state.value.position!!
val expected = orderBefore.toMutableList().apply { removeAt(current.toInt()) }[current.toInt()]
runBlocking { controller.removeAt(current) }
await {
controller.state.value.item
?.id == expected
}
val after = runBlocking { queues.snapshot() }
assertEquals(expected, runBlocking { queues.itemAt(after, controller.state.value.position!!)?.song?.id })
}
@Test
@@ -1740,6 +1703,44 @@ class PlaybackControllerTest {
assertEquals(currentId, runBlocking { queues.itemAt(after, controller.state.value.position!!)?.song?.id })
}
@Test
fun advancingOutOfABlockTrackWhileShuffledPlaysTheNextBlockTrack() {
seedShuffledQueue(seed = 5L)
seedSong("x1", "al2")
seedSong("x2", "al2")
controller.addToQueue(1, QueueKind.Song, "x1")
controller.addToQueue(1, QueueKind.Song, "x2")
await { runBlocking { controller.upcomingItem()?.id } == "x1" }
controller.playAt(1)
await {
controller.state.value.item
?.id == "x1"
}
controller.next()
await {
controller.state.value.item
?.id == "x2"
}
}
@Test
fun movingABlockTrackWhileShuffledReordersTheBlock() {
seedShuffledQueue(seed = 5L)
seedSong("x1", "al2")
seedSong("x2", "al2")
controller.addToQueue(1, QueueKind.Song, "x1")
controller.addToQueue(1, QueueKind.Song, "x2")
await { runBlocking { controller.upcomingItem()?.id } == "x1" }
runBlocking { controller.move(2, 1) }
val after = runBlocking { queues.snapshot() }
assertEquals("x2", runBlocking { queues.itemAt(after, after.anchorPlay + 1)?.song?.id })
assertEquals("x1", runBlocking { queues.itemAt(after, after.anchorPlay + 2)?.song?.id })
}
@Test
fun undoingARemovalRestoresTheShuffledOrder() {
seedShuffledQueue(seed = 42L)