Makussu/subtracks
1
0
Fork 0
mirror of https://github.com/austinried/subtracks.git synced 2026-11-10 15:11:58 +01:00
Code Issues Packages Projects Releases Wiki Activity

address review: re-anchoring and consume edge cases

Browse Source 
- jumpTo no longer returns early on a stale current position after a
  re-anchor, which could leave the player and snapshot unsynchronised.
- playNext from inside the block inserts after the entry holding the
  current track, not always at index 1.
- expose a layout version in PlaybackState so the queue view reloads
  when a re-anchor shifts the combined positions under it.
- removeAt recomputes the current track's combined position from the
  refreshed snapshot instead of assuming a one-down shift.
- restore keeps the saved resume position when the anchor moves.
- next on the last queued track with repeat off no longer consumes it
  and leaves it playing; repeat wraps instead.
- drop a redundant sort in range; revert a rejected cross-section drag
  by reloading.
This commit is contained in:
austinried
2026-10-04 00:30:00 +00:00
parent 9832f7c217
commit e7a1927ccd
4 changed files with 259 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt
+30 -5
View File
@@ -87,6 +87,18 @@ data class QueueSnapshot(
fun locateContext(contextPlay: Long): Pair<QueueEntry, Long>? = locateIn(entries, flatContext(contextPlay) ?: return null)
}
private fun entryIndexAfter(
entries: List<ResolvedQueueEntry>,
index: Long,
): Int {
var start = 0L
entries.forEachIndexed { position, resolved ->
if (index < start + resolved.length) return position + 1
start += resolved.length
}
return entries.size
}
private fun locateIn(
entries: List<ResolvedQueueEntry>,
index: Long,
@@ -274,6 +286,13 @@ class QueueRepository(
): Long? {
val upIndex = idsFor(snapshot.upNext).indexOf(songId)
if (upIndex >= 0) return snapshot.anchorPlay + 1 + upIndex
return combinedContextIndexOf(snapshot, songId)
}
suspend fun combinedContextIndexOf(
snapshot: QueueSnapshot,
songId: String,
): Long? {
val flat = flatIds(snapshot).indexOf(songId)
if (flat < 0) return null
val play = snapshot.shuffleOrder?.indexOf(flat.toLong())?.toLong() ?: flat.toLong()
@@ -299,7 +318,6 @@ class QueueRepository(
if (blockFirst <= blockLast) items += blockPlayRange(snapshot, blockFirst - anchor - 1, blockLast - anchor - 1)
val afterFirst = maxOf(from, anchor + block + 1)
if (afterFirst <= to) items += contextPlayRange(snapshot, afterFirst - block, to - block)
items.sortBy { it.position }
return items
}
@@ -319,12 +337,19 @@ class QueueRepository(
playNext: Boolean,
): Long {
val list = snapshot.upNext.map { it.entry }.toMutableList()
val active = snapshot.isUpNext(currentPosition)
val insertIndex =
when {
playNext && active -> 1
playNext -> 0
else -> list.size
playNext && snapshot.isUpNext(currentPosition) -> {
entryIndexAfter(snapshot.upNext, snapshot.upNextIndex(currentPosition))
}
playNext -> {
0
}
else -> {
list.size
}
}.coerceIn(0, list.size)
list.add(insertIndex, entry.copy(id = 0, position = 0))
val anchor =
app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt
+41 -12
View File
@@ -52,6 +52,7 @@ data class PlaybackState(
val hasPrevious: Boolean = false,
val shuffle: Boolean = false,
val repeat: RepeatMode = RepeatMode.Off,
val layout: Long = 0,
)
private sealed interface QueueUndo {
@@ -130,6 +131,7 @@ class PlaybackController(
private var windowStart = 0L
private var windowEnd = -1L
private var lastPosition: Long? = null
private var layoutVersion = 0L
private var updating = false
private var lastEdit: QueueUndo? = null
private var shuffleEnabled = false
@@ -280,6 +282,7 @@ class PlaybackController(
if (anchor.changed) {
snapshot = readSnapshot()
this@PlaybackController.snapshot = snapshot
layoutVersion++
}
val target = anchor.position.coerceIn(0, snapshot.size - 1)
windowJob?.cancel()
@@ -315,18 +318,37 @@ class PlaybackController(
return@withLock
}
val current = currentPosition() ?: return@withLock
val target = (if (position < current) current - 1 else current).coerceIn(0, snapshot.size - 2)
val playing = player.playWhenReady
val wasUpNext = snapshot.isUpNext(current)
val removed = if (position == current) null else queueRepository.itemAt(snapshot, position)?.toQueueItem()
queueRepository.removeAt(snapshot, position)
this.snapshot = readSnapshot()
val updated = readSnapshot()
this.snapshot = updated
windowJob?.cancel()
if (updated.size == 0L) {
stopLocked()
lastEdit = ReloadUndo(entries, cursor, snapshot.shuffleOrder, upNext, snapshot.upNextAnchor)
return@withLock
}
val currentId = player.currentItem?.id
val target =
(
currentId?.let {
if (wasUpNext) queueRepository.combinedIndexOf(updated, it) else queueRepository.combinedContextIndexOf(updated, it)
} ?: (if (position < current) current - 1 else current)
).coerceIn(0, updated.size - 1)
lastEdit =
if (removed == null) {
ReloadUndo(entries, cursor, snapshot.shuffleOrder, upNext, snapshot.upNextAnchor)
} else {
RemovedUndo(entries, cursor, target, position, removed, snapshot.shuffleOrder, upNext, snapshot.upNextAnchor)
}
if (snapshot.upNextSize > 0L || updated.upNextSize > 0L) {
layoutVersion++
queueRepository.setCursor(target)
loadWindow(target, autoplay = playing, startPositionMs = if (position == current) 0 else player.currentPositionMs)
return@withLock
}
when {
position == current -> {
loadWindow(target, autoplay = playing, startPositionMs = 0)
@@ -521,17 +543,16 @@ class PlaybackController(
startLock.withLock {
val snapshot = snapshot ?: return@withLock
if (!snapshot.isUpNext(position)) return@withLock
if (position >= snapshot.size - 1 && repeatMode == RepeatMode.Off) return@withLock
queueRepository.removeUpNextAt(snapshot, snapshot.upNextIndex(position))
val updated = readSnapshot()
this.snapshot = updated
layoutVersion++
windowJob?.cancel()
endedHandled = false
queueRepository.setCursor(position)
if (position < updated.size) {
loadWindow(position, autoplay = true)
} else {
refresh(position)
}
val next = if (position < updated.size) position else 0L
queueRepository.setCursor(next)
loadWindow(next, autoplay = true)
}
fun previous() {
@@ -595,6 +616,7 @@ class PlaybackController(
val position = queueRepository.addUpNext(snapshot, current, entry, playNext)
val updated = readSnapshot()
this.snapshot = updated
layoutVersion++
queueRepository.setCursor(position)
windowJob?.cancel()
rebuildWindow(queueRepository.window(updated, position, QUEUE_WINDOW_RADIUS), position)
@@ -644,6 +666,7 @@ class PlaybackController(
}
val anchor = queueRepository.reanchorFor(reordered, position)
val finalSnapshot = if (anchor.changed) readSnapshot() else reordered
if (anchor.changed) layoutVersion++
this@PlaybackController.snapshot = finalSnapshot
queueRepository.setCursor(anchor.position)
rebuildWindow(queueRepository.window(finalSnapshot, anchor.position, QUEUE_WINDOW_RADIUS), anchor.position)
@@ -751,15 +774,17 @@ class PlaybackController(
return@withLock
}
this.queueSourceId = queueSourceId
val savedPositionMs = queueRepository.cursorPositionMs().coerceAtLeast(0)
val cursor = queueRepository.cursor().coerceIn(0, snapshot.size - 1)
val anchor = queueRepository.reanchorFor(snapshot, cursor)
if (anchor.changed) {
snapshot = queueRepository.snapshot()
this.snapshot = snapshot
queueRepository.setCursor(anchor.position)
queueRepository.setPosition(savedPositionMs)
}
lastSavedPositionMs = queueRepository.cursorPositionMs().coerceAtLeast(0)
loadWindow(anchor.position, autoplay = false, startPositionMs = lastSavedPositionMs)
lastSavedPositionMs = savedPositionMs
loadWindow(anchor.position, autoplay = false, startPositionMs = savedPositionMs)
}
private suspend fun stop() = startLock.withLock { stopLocked() }
@@ -829,12 +854,15 @@ class PlaybackController(
var snapshot = readSnapshot()
if (snapshot.size == 0L) return@withLock
val anchor = queueRepository.reanchorFor(snapshot, target)
if (anchor.changed) snapshot = readSnapshot()
if (anchor.changed) {
snapshot = readSnapshot()
layoutVersion++
}
this@PlaybackController.snapshot = snapshot
val current = currentPosition() ?: return@withLock
val dest = anchor.position.coerceIn(0, snapshot.size - 1)
val wasEnded = player.isEnded
if (dest == current && !wasEnded) return@withLock
if (!anchor.changed && dest == current && !wasEnded) return@withLock
windowJob?.cancel()
queueRepository.setCursor(dest)
if (anchor.changed) {
@@ -1056,6 +1084,7 @@ class PlaybackController(
hasPrevious = true,
shuffle = shuffleEnabled,
repeat = repeatMode,
layout = layoutVersion,
)
if (player.isPlaying) startPositionTicker() else stopPositionTicker()
}
app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt
+5 -1
View File
@@ -265,7 +265,7 @@ fun QueueRoute(
LaunchedEffect(queueContext) {
sourceTitle.value = controller.sourceTitle(queueContext)?.takeIf { it.isNotBlank() } ?: fallbackTitle
}
LaunchedEffect(playback.shuffle, open) { if (open) viewModel.open() }
LaunchedEffect(playback.shuffle, playback.layout, open) { if (open) viewModel.open() }
QueueScreen(
rows = viewModel.rows,
ready = viewModel.ready,
@@ -283,6 +283,7 @@ fun QueueRoute(
onLoadOlder = viewModel::loadOlder,
onLoadNewer = viewModel::loadNewer,
onUndo = viewModel::undo,
onRefresh = viewModel::open,
modifier = modifier,
)
}
@@ -307,6 +308,7 @@ fun QueueScreen(
generation: Int = 0,
shuffle: Boolean = false,
contextTitle: String? = null,
onRefresh: () -> Unit = {},
) {
val listState = rememberLazyListState()
val fill = rememberViewportFill(listState)
@@ -415,6 +417,8 @@ fun QueueScreen(
if (to != from && sameSection) {
onMove(from, to)
showUndo("Queue reordered")
} else if (to != from) {
onRefresh()
}
}
dragId = null
app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt
+183
View File
@@ -429,6 +429,189 @@ class PlaybackControllerTest {
}
}
@Test
fun skippingPastTheBlockAfterConsumingKeepsGoing() {
seedAlbum(3, sourceId = 1)
seedSong("x1", "al2")
seedSong("x2", "al2")
controller.playAlbum(1, "al1", 0)
await {
controller.state.value.item
?.id == "s1"
}
controller.addToQueue(1, QueueKind.Song, "x1")
controller.addToQueue(1, QueueKind.Song, "x2")
controller.playAt(2)
await {
controller.state.value.item
?.id == "x2"
}
controller.next()
await {
controller.state.value.item
?.id == "s2"
}
controller.next()
await {
controller.state.value.item
?.id == "s3"
}
}
@Test
fun playNextFromInsideTheBlockGoesAfterTheCurrentTrack() {
seedAlbum(3, sourceId = 1)
seedSong("x1", "al2")
seedSong("x2", "al2")
seedSong("x3", "al2")
controller.playAlbum(1, "al1", 0)
await {
controller.state.value.item
?.id == "s1"
}
controller.addToQueue(1, QueueKind.Song, "x1")
controller.addToQueue(1, QueueKind.Song, "x2")
controller.playAt(2)
await {
controller.state.value.item
?.id == "x2"
}
controller.playNext(1, QueueKind.Song, "x3")
await {
controller.state.value.item
?.id == "x2"
}
await { runBlocking { controller.upcomingItem()?.id } == "x3" }
controller.next()
await {
controller.state.value.item
?.id == "x3"
}
}
@Test
fun removingAContextTrackUnderTheBlockKeepsTheCursorOnThePlayingTrack() {
seedAlbum(3, 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" }
controller.next()
await {
controller.state.value.item
?.id == "x1"
}
runBlocking { controller.removeAt(0) }
await {
controller.state.value.item
?.id == "x1"
}
assertEquals(1L, runBlocking { queues.cursor() })
}
@Test
fun restoringKeepsTheSavedPositionWhenTheAnchorMoves() {
seedAlbum(3, sourceId = 1)
seedSong("x1", "al2")
seedSong("x2", "al2")
controller.playAlbum(1, "al1", 0)
await {
controller.state.value.item
?.id == "s1"
}
controller.addToQueue(1, QueueKind.Song, "x1")
controller.addToQueue(1, QueueKind.Song, "x2")
controller.playAt(2)
await {
controller.state.value.item
?.id == "x2"
}
controller.next()
await {
controller.state.value.item
?.id == "s2"
}
runBlocking { queues.setPosition(12_000L) }
val restoredHandle = FakePlayerHandle()
val restored = PlaybackController(sources, queues, FakePlayerConnection(restoredHandle), dispatcher = dispatcher)
restored.connect()
await {
restored.state.value.item
?.id == "s2"
}
assertEquals(12_000L, restored.positionMs.value)
restored.close()
}
@Test
fun nextOnTheLastQueuedTrackDoesNotStrandIt() {
seedAlbum(3, 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.next()
await {
controller.state.value.item
?.id == "x1"
}
controller.next()
Thread.sleep(200)
assertFalse(runBlocking { queues.snapshot().upNext }.isEmpty())
assertEquals(
"x1",
controller.state.value.item
?.id,
)
}
@Test
fun reanchoringBumpsTheLayoutVersion() {
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" }
val before = controller.state.value.layout
controller.playAt(4)
await {
controller.state.value.item
?.id == "s4"
}
assertTrue(controller.state.value.layout > before)
}
@Test
fun theDurationUsesTheNextTracksMetadataWhileItLoads() {
seedAlbum(3, sourceId = 1)