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 ad13b80e..75668c2c 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt @@ -58,10 +58,12 @@ data class QueueSnapshot( val shuffled: Boolean get() = shuffleOrder != null val anchorPlay: Long = - when (val order = shuffleOrder) { - null -> upNextAnchor - else -> order.indexOf(upNextAnchor).takeIf { it >= 0 }?.toLong() ?: 0L - } + ( + when (val order = shuffleOrder) { + null -> upNextAnchor + else -> order.indexOf(upNextAnchor).takeIf { it >= 0 }?.toLong() ?: 0L + } + ).coerceAtMost(contextSize - 1L) fun isUpNext(position: Long): Boolean = upNextSize > 0L && position > anchorPlay && position <= anchorPlay + upNextSize diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 570b15f8..413d8d55 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -349,15 +349,8 @@ class PlaybackController( if (position != current) { rebuildWindow(queueRepository.window(updated, target, QUEUE_WINDOW_RADIUS), target) refresh(target) - } else if (target - windowStart in 0 until player.itemCount) { - updating = true - player.removeAt((target - windowStart).toInt()) - updating = false - windowEnd-- - shiftWindowLocked(target) - refresh(target) } else { - loadWindow(target, autoplay = playing) + loadWindow(target, autoplay = playing, startPositionMs = 0) } return@withLock } @@ -400,10 +393,17 @@ class PlaybackController( val cursor = queueRepository.cursor() val entries = snapshot.entries.map { it.entry } val upNext = snapshot.upNext.map { it.entry } + val wasUpNext = snapshot.isUpNext(current) if (!queueRepository.move(snapshot, from, to)) return@withLock val updated = readSnapshot() this.snapshot = updated - val target = movedCursor(current, from, to) + val currentId = player.currentItem?.id + val target = + ( + currentId?.let { + if (wasUpNext) queueRepository.combinedIndexOf(updated, it) else queueRepository.combinedContextIndexOf(updated, it) + } ?: movedCursor(current, from, to) + ).coerceIn(0, updated.size - 1) lastPosition = target windowJob?.cancel() val fromInWindow = from in windowStart..windowEnd 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 95c82c8c..2e354436 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -134,26 +134,26 @@ class QueueViewModel( } fun reconcile() { - viewModelScope.launch { - mutex.withLock { - if (!ready) return@withLock - val snapshot = queueRepository.snapshot() - if (snapshot.size == 0L) { - rows.clear() - return@withLock - } - val start = (rows.firstOrNull()?.position ?: 0L).coerceIn(0, snapshot.size - 1) - val end = (start + rows.size.coerceAtLeast(QUEUE_CHUNK) - 1).coerceAtMost(snapshot.size - 1) - val loaded = queueRepository.range(snapshot, start, end) - val reused = reusedIds() - val updated = loaded.map { rowFor(it, reused) } - if (updated.size == rows.size) { - updated.forEachIndexed { index, row -> if (rows[index] != row) rows[index] = row } - } else { - rows.clear() - rows.addAll(updated) - } - } + viewModelScope.launch { mutex.withLock { reconcileLocked() } } + } + + private suspend fun reconcileLocked() { + if (!ready) return + val snapshot = queueRepository.snapshot() + if (snapshot.size == 0L) { + rows.clear() + return + } + val start = (rows.firstOrNull()?.position ?: 0L).coerceIn(0, snapshot.size - 1) + val end = (start + rows.size.coerceAtLeast(QUEUE_CHUNK) - 1).coerceAtMost(snapshot.size - 1) + val loaded = queueRepository.range(snapshot, start, end) + val reused = reusedIds() + val updated = loaded.map { rowFor(it, reused) } + if (updated.size == rows.size) { + updated.forEachIndexed { index, row -> if (rows[index] != row) rows[index] = row } + } else { + rows.clear() + rows.addAll(updated) } } @@ -228,24 +228,9 @@ class QueueViewModel( viewModelScope.launch { mutex.withLock { val index = rows.indexOfFirst { it.position == position } - if (index >= 0) { - rows.removeAt(index) - for (i in index until rows.size) { - val row = rows[i] - rows[i] = row.copy(position = row.position - 1) - } - } + if (index >= 0) rows.removeAt(index) playbackController.removeAt(position) - val snapshot = queueRepository.snapshot() - if (rows.isEmpty()) { - reload() - return@withLock - } - val next = rows.last().position + 1 - if (next <= snapshot.size - 1) { - queueRepository.range(snapshot, next, next).firstOrNull()?.let { rows.add(newRow(it)) } - } - while (rows.size > QUEUE_WINDOW_ROWS) rows.removeAt(0) + reconcileLocked() } } } diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 191527ff..3f76a33a 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -926,6 +926,81 @@ class PlaybackControllerTest { assertFalse(handle.operations.any { it.startsWith("setWindow") }) } + @Test + fun removingTheLastContextTrackWithAQueuedTrackKeepsTheQueue() { + seedAlbum(1, sourceId = 1) + seedSong("x1", "al2") + + controller.playAlbum(1, "al1", 0) + await { + controller.state.value.item + ?.id == "s1" + } + controller.addToQueue(1, QueueKind.Song, "x1") + await { runBlocking { controller.upcomingItem()?.id } == "x1" } + + runBlocking { controller.removeAt(0) } + await { + controller.state.value.item + ?.id == "x1" + } + + val snapshot = runBlocking { queues.snapshot() } + assertEquals(1L, snapshot.size) + assertTrue(runBlocking { queues.range(snapshot, 0, 0) }.isNotEmpty()) + } + + @Test + fun movingTheAnchorKeepsThePlayingTrackPositionInSync() { + seedAlbum(4, sourceId = 1) + seedSong("x1", "al2") + + controller.playAlbum(1, "al1", 1) + await { + controller.state.value.item + ?.id == "s2" + } + controller.addToQueue(1, QueueKind.Song, "x1") + await { runBlocking { controller.upcomingItem()?.id } == "x1" } + controller.next() + await { + controller.state.value.item + ?.id == "x1" + } + + runBlocking { controller.move(1, 0) } + await { + controller.state.value.item + ?.id == "x1" + } + + val snapshot = runBlocking { queues.snapshot() } + assertEquals(runBlocking { queues.combinedIndexOf(snapshot, "x1") }, controller.state.value.position) + } + + @Test + fun removingThePlayingAnchorTrackPlaysTheModelNextTrack() { + seedAlbum(4, sourceId = 1) + seedSong("x1", "al2") + + controller.playAlbum(1, "al1", 1) + await { + controller.state.value.item + ?.id == "s2" + } + controller.addToQueue(1, QueueKind.Song, "x1") + await { runBlocking { controller.upcomingItem()?.id } == "x1" } + + runBlocking { controller.removeAt(1) } + await { + controller.state.value.item + ?.id == "s3" + } + + val snapshot = runBlocking { queues.snapshot() } + assertEquals(runBlocking { queues.combinedIndexOf(snapshot, "s3") }, controller.state.value.position) + } + @Test fun nextFollowsTheReorderedQueue() { seedAlbum(3, sourceId = 1) diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt index 06562d07..ac3a5c31 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt @@ -6,6 +6,7 @@ import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.db.SubtracksDatabase +import com.subtracks.data.model.QueueKind import com.subtracks.data.model.Song import com.subtracks.data.model.SongListItem import com.subtracks.data.model.Source @@ -225,6 +226,37 @@ class QueueViewModelTest { assertEquals((220L..399L).toList(), viewModel.rows.map { it.position }) } + @Test + fun removingATrackUnderTheBlockReloadsRows() { + runBlocking { seedSongs(10) } + runBlocking { + db.libraryDao().upsertSongs( + listOf(Song(1, "x1", "al2", "ar1", "X", "Other Album", "Artist", 100, 1, 1, null, null)), + ) + } + controller.playAlbum(1, "al1", 2) + await { controller.state.value.position == 2L } + controller.addToQueue(1, QueueKind.Song, "x1") + await { runBlocking { controller.upcomingItem()?.id } == "x1" } + + viewModel.open() + await { viewModel.ready } + + viewModel.remove(2) + await { + val positions = viewModel.rows.map { it.position } + positions.isNotEmpty() && positions == (positions.first()..positions.last()).toList() + } + + val snapshot = runBlocking { queues.snapshot() } + val expected = + runBlocking { + (viewModel.rows.first().position..viewModel.rows.last().position) + .mapNotNull { queues.itemAt(snapshot, it)?.song?.id } + } + assertEquals(expected, viewModel.rows.map { it.song.song.id }) + } + @Test fun removeAndUndoKeepTheBoundedWindowConsistent() { runBlocking { seedSongs(400) }