address review simplifications: setShuffle(seed?) and a nullable playContext

- `QueueRepository.setShuffle(seed: Long?)` derives `shuffleEnabled` from the
  seed, so disabled callers drop the meaningless sentinel `0L`.
- `playContext` returns `Long?` for symmetry with `flatContext`; `anchorPlay`
  treats an out-of-range anchor as -1 instead of silently clamping to the last
  track, and `combinedContextIndexOf` returns null.
This commit is contained in:
austinried
2026-10-04 00:30:01 +00:00
parent e6386ebddd
commit 7f674f04be
3 changed files with 23 additions and 27 deletions
@@ -60,8 +60,7 @@ data class QueueSnapshot(
( (
when { when {
shuffleSeed == null -> upNextAnchor shuffleSeed == null -> upNextAnchor
upNextAnchor < 0L -> -1L else -> playContext(upNextAnchor) ?: -1L
else -> playContext(upNextAnchor)
} }
).coerceAtMost(contextSize - 1L) ).coerceAtMost(contextSize - 1L)
@@ -80,8 +79,8 @@ data class QueueSnapshot(
return shuffleSeed?.let { Shuffle.toFlat(it, contextSize, contextPlay) } ?: contextPlay return shuffleSeed?.let { Shuffle.toFlat(it, contextSize, contextPlay) } ?: contextPlay
} }
fun playContext(flat: Long): Long { fun playContext(flat: Long): Long? {
if (flat !in 0 until contextSize) return flat if (flat !in 0 until contextSize) return null
return shuffleSeed?.let { Shuffle.toSequence(it, contextSize, flat) } ?: flat return shuffleSeed?.let { Shuffle.toSequence(it, contextSize, flat) } ?: flat
} }
@@ -208,7 +207,7 @@ class QueueRepository(
) { ) {
write(entries) write(entries)
writeUpNext(upNext) writeUpNext(upNext)
setShuffle(shuffleSeed != null, shuffleSeed ?: 0L) setShuffle(shuffleSeed)
cursorMutex.withLock { cursorMutex.withLock {
dao.setCursor(cursorRow().copy(queuePosition = 0, positionMs = 0, upNextAnchor = upNextAnchor)) dao.setCursor(cursorRow().copy(queuePosition = 0, positionMs = 0, upNextAnchor = upNextAnchor))
} }
@@ -239,16 +238,13 @@ class QueueRepository(
return QueueModes(row.shuffleEnabled, row.repeatMode) return QueueModes(row.shuffleEnabled, row.repeatMode)
} }
suspend fun setShuffle( suspend fun setShuffle(seed: Long?) {
enabled: Boolean, val size = if (seed != null) contextSize() else 0L
seed: Long,
) {
val size = if (enabled) contextSize() else 0L
cursorMutex.withLock { cursorMutex.withLock {
dao.setCursor( dao.setCursor(
cursorRow().copy( cursorRow().copy(
shuffleEnabled = enabled, shuffleEnabled = seed != null,
shuffleSeed = if (enabled) seed else 0L, shuffleSeed = seed ?: 0L,
shuffleSize = size, shuffleSize = size,
), ),
) )
@@ -288,7 +284,7 @@ class QueueRepository(
): Long? { ): Long? {
val flat = flatIds(snapshot).indexOf(songId) val flat = flatIds(snapshot).indexOf(songId)
if (flat < 0) return null if (flat < 0) return null
return snapshot.combined(snapshot.playContext(flat.toLong())) return snapshot.playContext(flat.toLong())?.let { snapshot.combined(it) }
} }
suspend fun range( suspend fun range(
@@ -255,7 +255,7 @@ class PlaybackController(
val start = randomIndex(snap.contextSize, avoid) val start = randomIndex(snap.contextSize, avoid)
val seed = Random.nextLong() val seed = Random.nextLong()
val position = Shuffle.toSequence(seed, snap.contextSize, start) val position = Shuffle.toSequence(seed, snap.contextSize, start)
queueRepository.setShuffle(true, seed) queueRepository.setShuffle(seed)
shuffleEnabled = true shuffleEnabled = true
queueSourceId = entry.sourceId queueSourceId = entry.sourceId
queueRepository.setCursor(position) queueRepository.setCursor(position)
@@ -732,9 +732,9 @@ class PlaybackController(
val enabling = !shuffleEnabled val enabling = !shuffleEnabled
val seed = if (enabling) Random.nextLong() else 0L val seed = if (enabling) Random.nextLong() else 0L
if (enabling) { if (enabling) {
queueRepository.setShuffle(true, seed) queueRepository.setShuffle(seed)
} else { } else {
queueRepository.setShuffle(false, 0L) queueRepository.setShuffle(null)
} }
shuffleEnabled = enabling shuffleEnabled = enabling
val reordered = readSnapshot() val reordered = readSnapshot()
@@ -813,7 +813,7 @@ class PlaybackController(
val position = val position =
if (shuffleEnabled) { if (shuffleEnabled) {
val seed = Random.nextLong() val seed = Random.nextLong()
queueRepository.setShuffle(true, seed) queueRepository.setShuffle(seed)
Shuffle.toSequence(seed, snapshot.contextSize, start) Shuffle.toSequence(seed, snapshot.contextSize, start)
} else { } else {
start start
@@ -837,7 +837,7 @@ class PlaybackController(
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 seed = Random.nextLong()
queueRepository.setShuffle(true, seed) queueRepository.setShuffle(seed)
queueRepository.setCursor(Shuffle.toSequence(seed, snapshot.contextSize, start)) queueRepository.setCursor(Shuffle.toSequence(seed, snapshot.contextSize, start))
snapshot = queueRepository.snapshot() snapshot = queueRepository.snapshot()
} else if (!snapshot.shuffled) { } else if (!snapshot.shuffled) {
@@ -1105,7 +1105,7 @@ class PlaybackController(
private suspend fun readSnapshot(): QueueSnapshot { private suspend fun readSnapshot(): QueueSnapshot {
val snapshot = queueRepository.snapshot() val snapshot = queueRepository.snapshot()
if (!snapshot.shuffled) { if (!snapshot.shuffled) {
if (shuffleEnabled && snapshot.size > 0L) queueRepository.setShuffle(false, 0L) if (shuffleEnabled && snapshot.size > 0L) queueRepository.setShuffle(null)
shuffleEnabled = false shuffleEnabled = false
} }
return snapshot return snapshot
@@ -441,7 +441,7 @@ class QueueRepositoryTest {
runTest { runTest {
seedLibrary() seedLibrary()
val snapshot = repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"))) val snapshot = repository.snapshotAfter(listOf(repository.albumEntry(1, "al1")))
repository.setShuffle(true, 12345L) repository.setShuffle(12345L)
val shuffled = repository.snapshot() val shuffled = repository.snapshot()
val contextBefore = (0 until shuffled.contextSize).mapNotNull { repository.itemAt(shuffled, it)?.song?.id } val contextBefore = (0 until shuffled.contextSize).mapNotNull { repository.itemAt(shuffled, it)?.song?.id }
@@ -545,7 +545,7 @@ class QueueRepositoryTest {
runTest { runTest {
seedLibrary() seedLibrary()
repository.replace(listOf(repository.albumEntry(1, "al1"))) repository.replace(listOf(repository.albumEntry(1, "al1")))
repository.setShuffle(true, 1234L) repository.setShuffle(1234L)
assertTrue(repository.snapshot().shuffled) assertTrue(repository.snapshot().shuffled)
db.libraryDao().upsertSongs(listOf(song("s6", "al1", track = 4, album = "First Album"))) db.libraryDao().upsertSongs(listOf(song("s6", "al1", track = 4, album = "First Album")))
@@ -561,7 +561,7 @@ class QueueRepositoryTest {
repository.replace(listOf(repository.albumEntry(1, "al1"))) repository.replace(listOf(repository.albumEntry(1, "al1")))
val entriesBefore = db.queueDao().entries().size val entriesBefore = db.queueDao().entries().size
repository.setShuffle(true, 5678L) repository.setShuffle(5678L)
val snapshot = repository.snapshot() val snapshot = repository.snapshot()
assertTrue(snapshot.shuffled) assertTrue(snapshot.shuffled)
@@ -584,7 +584,7 @@ class QueueRepositoryTest {
runTest { runTest {
seedLibrary() seedLibrary()
repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"))) repository.snapshotAfter(listOf(repository.albumEntry(1, "al1")))
repository.setShuffle(true, 42L) repository.setShuffle(42L)
val shuffled = repository.snapshot() val shuffled = repository.snapshot()
val resolved = (0 until shuffled.size).mapNotNull { repository.itemAt(shuffled, it)?.song?.id } val resolved = (0 until shuffled.size).mapNotNull { repository.itemAt(shuffled, it)?.song?.id }
@@ -600,10 +600,10 @@ class QueueRepositoryTest {
runTest { runTest {
seedLibrary() seedLibrary()
repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"))) repository.snapshotAfter(listOf(repository.albumEntry(1, "al1")))
repository.setShuffle(true, 99L) repository.setShuffle(99L)
assertTrue(repository.snapshot().shuffled) assertTrue(repository.snapshot().shuffled)
repository.setShuffle(false, 0L) repository.setShuffle(null)
val restored = repository.snapshot() val restored = repository.snapshot()
assertFalse(restored.shuffled) assertFalse(restored.shuffled)
@@ -615,7 +615,7 @@ class QueueRepositoryTest {
runTest { runTest {
seedLibrary() seedLibrary()
repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"), repository.songEntry(1, "s4"))) repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"), repository.songEntry(1, "s4")))
repository.setShuffle(true, 42L) repository.setShuffle(42L)
val snapshot = repository.snapshot() val snapshot = repository.snapshot()
val range = repository.range(snapshot, 0, snapshot.size - 1) val range = repository.range(snapshot, 0, snapshot.size - 1)
@@ -630,7 +630,7 @@ class QueueRepositoryTest {
seedLibrary() seedLibrary()
val snapshot = repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"))) val snapshot = repository.snapshotAfter(listOf(repository.albumEntry(1, "al1")))
repository.removeAt(snapshot, 1) repository.removeAt(snapshot, 1)
repository.setShuffle(true, 7L) repository.setShuffle(7L)
val shuffled = repository.snapshot() val shuffled = repository.snapshot()
val range = repository.range(shuffled, 0, shuffled.size - 1) val range = repository.range(shuffled, 0, shuffled.size - 1)