keep the shuffle order stable when removing a track

Removing a track re-derived the permutation from (seed, size), so every remaining track moved to a new position and the queue visibly reshuffled (in an album-heavy playlist, the removed row and its neighbours were replaced by other tracks from the same album). Keep the permutation fixed over the domain it was created with and subtract removals from it instead: `ShuffleState` holds the seed, the domain size and the removed domain slots, kept in memory only. `flatContext`/`playContext` skip those slots so the surviving tracks keep their positions; the DB still stores just the seed and domain, so the old per-track `shuffle_order` rows stay gone. A new shuffle or a library change clears the removal list; on restart the removals are gone, so a queue that was edited falls back to unshuffled rather than guessing an order.
2026-11-10 15:11:58 +01:00 · 2026-10-04 00:30:01 +00:00
parent 6d25b31a9a
commit 736e59fec6
2 changed files with 140 additions and 15 deletions
@@ -32,7 +32,7 @@ private data class EntryLengthKey(
 private data class LoadedSnapshot(
    val entries: List<ResolvedQueueEntry>,
    val upNext: List<ResolvedQueueEntry>,
-    val shuffleSeed: Long?,
+    val shuffle: ShuffleState?,
    val anchor: Long,
 )

@@ -41,10 +41,64 @@ data class ResolvedQueueEntry(
    val length: Long,
 )

+/**
+ * A shuffled order over a fixed domain of [domain] tracks, plus the domain positions removed since
+ * the order was created. The permutation stays fixed and removals are subtracted (sparsely, in
+ * memory) rather than re-derived, so editing a shuffled queue keeps the remaining tracks in place.
+ * Only the seed and domain are persisted; on restart the removals are gone, so a queue with edits
+ * falls back to unshuffled rather than guessing.
+ */
+class ShuffleState(
+    val seed: Long,
+    val domain: Long,
+    private val removedFlats: LongArray = LongArray(0),
+    private val removedSlots: LongArray = LongArray(0),
+) {
+    fun flatFor(play: Long): Long {
+        val domainFlat = Shuffle.toFlat(seed, domain, liveSlot(play))
+        return domainFlat - removedFlats.count { it < domainFlat }
+    }
+
+    fun playFor(canonical: Long): Long {
+        val slot = Shuffle.toSequence(seed, domain, liveFlat(canonical))
+        return slot - removedSlots.count { it < slot }
+    }
+
+    fun domainFor(canonical: Long): Long = liveFlat(canonical)
+
+    val liveSize: Long get() = domain - removedFlats.size.toLong()
+
+    fun withRemoved(domainFlat: Long): ShuffleState =
+        ShuffleState(
+            seed,
+            domain,
+            (removedFlats + domainFlat).sortedArray(),
+            (removedSlots + Shuffle.toSequence(seed, domain, domainFlat)).sortedArray(),
+        )
+
+    private fun liveSlot(play: Long): Long {
+        var slot = play
+        while (true) {
+            val next = play + removedSlots.count { it <= slot }
+            if (next <= slot) return slot
+            slot = next
+        }
+    }
+
+    private fun liveFlat(canonical: Long): Long {
+        var flat = canonical
+        while (true) {
+            val next = canonical + removedFlats.count { it <= flat }
+            if (next <= flat) return flat
+            flat = next
+        }
+    }
+}
+
 data class QueueSnapshot(
    val entries: List<ResolvedQueueEntry>,
    val upNext: List<ResolvedQueueEntry> = emptyList(),
-    val shuffleSeed: Long? = null,
+    val shuffle: ShuffleState? = null,
    val upNextAnchor: Long = 0,
    val version: Long = 0,
 ) {
@@ -54,12 +108,14 @@ data class QueueSnapshot(

    val size: Long = contextSize + upNextSize

-    val shuffled: Boolean get() = shuffleSeed != null
+    val shuffled: Boolean get() = shuffle != null
+
+    val shuffleSeed: Long? get() = shuffle?.seed

    val anchorPlay: Long =
        (
            when {
-                shuffleSeed == null -> upNextAnchor
+                shuffle == null -> upNextAnchor
                else -> playContext(upNextAnchor) ?: -1L
            }
        ).coerceAtMost(contextSize - 1L)
@@ -76,12 +132,12 @@ data class QueueSnapshot(

    fun flatContext(contextPlay: Long): Long? {
        if (contextPlay !in 0 until contextSize) return null
-        return shuffleSeed?.let { Shuffle.toFlat(it, contextSize, contextPlay) } ?: contextPlay
+        return shuffle?.flatFor(contextPlay) ?: contextPlay
    }

    fun playContext(flat: Long): Long? {
        if (flat !in 0 until contextSize) return null
-        return shuffleSeed?.let { Shuffle.toSequence(it, contextSize, flat) } ?: flat
+        return shuffle?.playFor(flat) ?: flat
    }

    fun locate(position: Long): Pair<QueueEntry, Long>? {
@@ -145,6 +201,9 @@ class QueueRepository(
    @Volatile
    private var cachedFlatVersion = -1L

+    @Volatile
+    private var shuffleRemoved: ShuffleState? = null
+
    private val lengthCache = ConcurrentHashMap<EntryLengthKey, Long>()

    private val cursorMutex = Mutex()
@@ -226,11 +285,24 @@ class QueueRepository(
                        }
                    val row = dao.cursor()
                    val size = entries.sumOf { it.length }
-                    val seed = row?.takeIf { it.shuffleEnabled && size > 0L && it.shuffleSize == size }?.shuffleSeed
-                    LoadedSnapshot(entries, upNext, seed, row?.upNextAnchor ?: 0L)
+                    val state = shuffleRemoved
+                    val shuffle =
+                        if (row != null && row.shuffleEnabled && size > 0L) {
+                            when {
+                                state != null && state.seed == row.shuffleSeed && state.domain == row.shuffleSize &&
+                                    state.liveSize == size -> state
+
+                                state == null && row.shuffleSize == size -> ShuffleState(row.shuffleSeed, row.shuffleSize)
+
+                                else -> null
+                            }
+                        } else {
+                            null
+                        }
+                    LoadedSnapshot(entries, upNext, shuffle, row?.upNextAnchor ?: 0L)
                }
            }
-        return QueueSnapshot(loaded.entries, loaded.upNext, loaded.shuffleSeed, loaded.anchor, version)
+        return QueueSnapshot(loaded.entries, loaded.upNext, loaded.shuffle, loaded.anchor, version)
    }

    suspend fun modes(): QueueModes {
@@ -249,13 +321,13 @@ class QueueRepository(
                ),
            )
        }
+        shuffleRemoved = seed?.let { ShuffleState(it, size) }
    }

-    private suspend fun setShuffleSize(size: Long) = cursorMutex.withLock { dao.setCursor(cursorRow().copy(shuffleSize = size)) }
-
    fun invalidateLibraryCache() {
        cachedFlatIds = null
        lengthCache.clear()
+        shuffleRemoved = null
        queueVersion.incrementAndGet()
    }

@@ -387,13 +459,13 @@ class QueueRepository(
        }
        val play = snapshot.contextPlay(position)
        if (snapshot.shuffled) {
-            // The permutation is derived from (seed, size), so removing a track re-derives it at
-            // the smaller size; keep the same seed and store the new size so the queue stays
-            // shuffled. There is no stable "next" to preserve here.
+            // Subtract the track from the fixed shuffle order instead of re-deriving it, so the
+            // tracks that remain keep their positions.
+            val order = snapshot.shuffle ?: return
            val flat = snapshot.flatContext(play) ?: return
+            shuffleRemoved = order.withRemoved(order.domainFor(flat))
            write(compact(removeEntry(snapshot.entries, flat)).map { it.entry })
            adjustAnchorOnRemove(flat)
-            setShuffleSize(contextSize())
        } else {
            val removed = compact(removeEntry(snapshot.entries, play))
            write(removed.map { it.entry })
@@ -639,6 +639,59 @@ class QueueRepositoryTest {
            assertEquals(setOf("s1", "s3"), range.map { it.item.song.id }.toSet())
        }

+    @Test
+    fun removingAContextTrackWhileShuffledKeepsTheRemainingOrder() =
+        runTest {
+            seedLibrary()
+            val snapshot = repository.snapshotAfter(listOf(repository.albumEntry(1, "al1")))
+            repository.setShuffle(42L)
+            val before = repository.snapshot()
+            val orderBefore = (0 until before.contextSize).mapNotNull { repository.itemAt(before, it)?.song?.id }
+
+            repository.removeAt(before, 1)
+
+            val after = repository.snapshot()
+            assertTrue(after.shuffled)
+            assertEquals(2L, after.contextSize)
+            val orderAfter = (0 until after.contextSize).mapNotNull { repository.itemAt(after, it)?.song?.id }
+            assertEquals(orderBefore.filterIndexed { index, _ -> index != 1 }, orderAfter)
+        }
+
+    @Test
+    fun severalRemovalsWhileShuffledKeepTheRemainingOrder() =
+        runTest {
+            seedLibrary()
+            val snapshot = repository.snapshotAfter(listOf(repository.albumEntry(1, "al1")))
+            repository.setShuffle(42L)
+            val before = repository.snapshot()
+            val orderBefore = (0 until before.contextSize).mapNotNull { repository.itemAt(before, it)?.song?.id }
+
+            repository.removeAt(before, 0)
+            repository.removeAt(repository.snapshot(), 0)
+
+            val after = repository.snapshot()
+            assertTrue(after.shuffled)
+            assertEquals(1L, after.contextSize)
+            val orderAfter = (0 until after.contextSize).mapNotNull { repository.itemAt(after, it)?.song?.id }
+            assertEquals(orderBefore.drop(2), orderAfter)
+        }
+
+    @Test
+    fun removingWhileShuffledKeepsTheOrderInverse() =
+        runTest {
+            seedLibrary()
+            val snapshot = repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"), repository.songEntry(1, "s4")))
+            repository.setShuffle(9L)
+            repository.removeAt(repository.snapshot(), 2)
+
+            val after = repository.snapshot()
+            assertEquals(3L, after.contextSize)
+            for (play in 0 until after.contextSize) {
+                assertEquals(play, after.playContext(after.flatContext(play)!!))
+            }
+            assertEquals((0L until after.contextSize).toList(), (0 until after.contextSize).map { after.flatContext(it)!! }.sorted())
+        }
+
    private suspend fun seedLibrary() {
        db.sourcesDao().upsertSource(
            Source(id = 1, name = "test", address = "http://localhost", isActive = true, createdAt = 0),