diff --git a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt index 73f66649..fbb3cf4d 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt @@ -61,7 +61,7 @@ data class QueueSnapshot( ( when (val order = shuffleOrder) { null -> upNextAnchor - else -> order.indexOf(upNextAnchor).takeIf { it >= 0 }?.toLong() ?: 0L + else -> if (upNextAnchor < 0L) -1L else order.indexOf(upNextAnchor).takeIf { it >= 0 }?.toLong() ?: 0L } ).coerceAtMost(contextSize - 1L) @@ -411,33 +411,28 @@ class QueueRepository( if (order == null) { val removed = compact(removeEntry(snapshot.entries, play)) write(removed.map { it.entry }) - adjustAnchorOnRemove(play, removed.sumOf { it.length }) + adjustAnchorOnRemove(play) } else { val flat = order.getOrNull(play.toInt()) ?: return snapshot - val removed = compact(removeEntry(snapshot.entries, flat)) - write(removed.map { it.entry }) + write(compact(removeEntry(snapshot.entries, flat)).map { it.entry }) val reordered = order.toMutableList().apply { removeAt(play.toInt()) } for (index in reordered.indices) if (reordered[index] > flat) reordered[index] -= 1 setShuffle(true, reordered.toLongArray()) - adjustAnchorOnRemove(flat, removed.sumOf { it.length }) + adjustAnchorOnRemove(flat) } return snapshot() } - private suspend fun adjustAnchorOnRemove( - removedFlat: Long, - newSize: Long, - ) = cursorMutex.withLock { - val row = cursorRow() - val anchor = row.upNextAnchor - val adjusted = - when { - anchor > removedFlat -> anchor - 1 - anchor == removedFlat -> removedFlat.coerceIn(0, (newSize - 1).coerceAtLeast(0)) - else -> anchor - } - if (adjusted != anchor) dao.setCursor(row.copy(upNextAnchor = adjusted)) - } + private suspend fun adjustAnchorOnRemove(removedFlat: Long) = + cursorMutex.withLock { + val row = cursorRow() + val anchor = row.upNextAnchor + // Removing the track the block sits behind leaves the block next in line, so move the + // anchor back to the preceding track (or in front of the whole context, -1, when the + // anchor was the first track) rather than letting the following track play first. + val adjusted = if (anchor >= removedFlat) anchor - 1 else anchor + if (adjusted != anchor) dao.setCursor(row.copy(upNextAnchor = adjusted)) + } private suspend fun adjustAnchorOnMove( flatFrom: Long, 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 11c6ad65..482619e8 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -332,19 +332,20 @@ fun QueueScreen( // One reorder state per run of rows: sh.calvin.reorderable only targets keys registered with // the state that started the drag, so a drag can never reach a row in another run. The context // sits on both sides of the up-next block, and splitting it there stops a context drag crossing - // the block (which would move a track over the queued songs and relocate the anchor). The - // up-next state disables auto-scroll (scrollThreshold 0): its block is followed by context - // rows the scroller would otherwise keep scrolling towards with nowhere to drop. + // the block (which would move a track over the queued songs and relocate the anchor). + // scrollThreshold 0 disables auto-scroll: the library keeps scrolling past a run's last valid + // target with nowhere to drop, and the dragged item can be left off-screen. Scroll to where + // you want to drop first, then drag. val upNextReorder = rememberReorderableLazyListState(listState, scrollThreshold = 0.dp) { from, to -> onReorder(rows.rowIndexOf(from.key), rows.rowIndexOf(to.key)) } val contextBeforeReorder = - rememberReorderableLazyListState(listState) { from, to -> + rememberReorderableLazyListState(listState, scrollThreshold = 0.dp) { from, to -> onReorder(rows.rowIndexOf(from.key), rows.rowIndexOf(to.key)) } val contextAfterReorder = - rememberReorderableLazyListState(listState) { from, to -> + rememberReorderableLazyListState(listState, scrollThreshold = 0.dp) { from, to -> onReorder(rows.rowIndexOf(from.key), rows.rowIndexOf(to.key)) } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt index feddbc26..49032676 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt @@ -465,6 +465,19 @@ class QueueRepositoryTest { assertEquals(listOf("s1", "s4", "s2", "s3"), resolveAll(repository.snapshot())) } + @Test + fun removingTheAnchorTrackPutsTheQueueFirst() = + runTest { + seedLibrary() + val snapshot = repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"))) + repository.addUpNext(snapshot, currentPosition = 0, entry = repository.songEntry(1, "s4"), playNext = false) + assertEquals(listOf("s1", "s4", "s2", "s3"), resolveAll(repository.snapshot())) + + repository.removeAt(repository.snapshot(), 0) + + assertEquals(listOf("s4", "s2", "s3"), resolveAll(repository.snapshot())) + } + @Test fun removingTheLastAnchorTrackKeepsTheQueueResolvable() = runTest { diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 538148f6..5f2b52fb 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -519,7 +519,8 @@ class PlaybackControllerTest { ?.id == "x1" } - assertEquals(1L, runBlocking { queues.cursor() }) + val snapshot = runBlocking { queues.snapshot() } + assertEquals(runBlocking { queues.combinedIndexOf(snapshot, "x1") }, controller.state.value.position) } @Test @@ -979,7 +980,7 @@ class PlaybackControllerTest { } @Test - fun removingThePlayingAnchorTrackPlaysTheModelNextTrack() { + fun removingThePlayingAnchorTrackPlaysTheQueuedTrackNext() { seedAlbum(4, sourceId = 1) seedSong("x1", "al2") @@ -994,11 +995,11 @@ class PlaybackControllerTest { runBlocking { controller.removeAt(1) } await { controller.state.value.item - ?.id == "s3" + ?.id == "x1" } val snapshot = runBlocking { queues.snapshot() } - assertEquals(runBlocking { queues.combinedIndexOf(snapshot, "s3") }, controller.state.value.position) + assertEquals(runBlocking { queues.combinedIndexOf(snapshot, "x1") }, controller.state.value.position) } @Test