diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 4b06f659..716a800c 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -990,19 +990,28 @@ class PlaybackController( connect() return } - scope.launch { start(entry, startPosition, behavior ?: preferences.songTapBehavior().first()) } + scope.launch { + startLock.withLock { + startLocked(entry, startPosition, behavior ?: preferences.songTapBehavior().first()) + } + } } private suspend fun startPending(pending: PendingPlay) { - val behavior = pending.behavior ?: preferences.songTapBehavior().first() - start(pending.entry, pending.position, behavior) + start(pending.entry, pending.position, pending.behavior ?: preferences.songTapBehavior().first()) } private suspend fun start( entry: QueueEntry, startPosition: Long, behavior: SongTapBehavior, - ) = startLock.withLock { + ) = startLock.withLock { startLocked(entry, startPosition, behavior) } + + private suspend fun startLocked( + entry: QueueEntry, + startPosition: Long, + behavior: SongTapBehavior, + ) { val modes = queueRepository.modes() val shuffle = when (behavior) { @@ -1017,7 +1026,7 @@ class PlaybackController( queueSourceId = entry.sourceId if (snapshot.size == 0L) { stopLocked() - return@withLock + return } val start = startPosition.coerceIn(0, snapshot.contextSize - 1) val position = diff --git a/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt b/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt index 53b6cd6f..c08af789 100644 --- a/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt @@ -849,8 +849,6 @@ private fun SortNameModeDialog( ) } -private val songTapBehaviorOptions = SongTapBehavior.entries - private fun songTapBehaviorLabel( resources: Resources, behavior: SongTapBehavior, @@ -875,7 +873,7 @@ private fun SongTapBehaviorDialog( text = { ChoiceGroup( header = stringResource(R.string.settings_music_options_song_tap_prompt), - options = songTapBehaviorOptions.map { it to songTapBehaviorLabel(resources, it) }, + options = SongTapBehavior.entries.map { it to songTapBehaviorLabel(resources, it) }, selected = draft, onSelect = { draft = it }, ) diff --git a/app/src/test/kotlin/com/subtracks/data/prefs/UserPreferencesTest.kt b/app/src/test/kotlin/com/subtracks/data/prefs/UserPreferencesTest.kt index 5495f695..26b4cc80 100644 --- a/app/src/test/kotlin/com/subtracks/data/prefs/UserPreferencesTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/prefs/UserPreferencesTest.kt @@ -67,6 +67,26 @@ class UserPreferencesTest { file.delete() } + @Test + fun songTapBehaviorDefaultsToInOrderAndRoundTrips() = + runTest { + val file = File.createTempFile("user-prefs", ".preferences_pb").apply { delete() } + val store = PreferenceDataStoreFactory.create(scope = backgroundScope) { file } + val prefs = UserPreferences(store) + + assertEquals(SongTapBehavior.InOrder, prefs.songTapBehavior().first()) + + prefs.setSongTapBehavior(SongTapBehavior.Shuffle) + + assertEquals(SongTapBehavior.Shuffle, prefs.songTapBehavior().first()) + + store.edit { it[stringPreferencesKey("song_tap_behavior")] = "Bogus" } + + assertEquals(SongTapBehavior.InOrder, prefs.songTapBehavior().first()) + + file.delete() + } + @Test fun theSelectedLibraryTabRoundTrips() = runTest { diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index dedc5bae..4ce8bca1 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -2,6 +2,9 @@ package com.subtracks.playback import android.content.Context import android.content.res.Resources +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.emptyPreferences import androidx.media3.common.PlaybackException import androidx.room3.useWriterConnection import androidx.test.core.app.ApplicationProvider @@ -33,14 +36,17 @@ import com.subtracks.data.prefs.fakeUserPreferences import com.subtracks.data.repo.DownloadRepository import com.subtracks.data.repo.QueueRepository import com.subtracks.data.repo.SourceRepository +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext @@ -2565,6 +2571,8 @@ class PlaybackControllerTest { fun playAllStartsInOrderRegardlessOfTheSongTapBehavior() { seedAlbum(3, sourceId = 1) runBlocking { prefs.setSongTapBehavior(SongTapBehavior.Shuffle) } + controller.shuffleAlbum(1, "al1") + await { controller.state.value.shuffle } controller.playAlbumInOrder(1, "al1") await { @@ -2575,6 +2583,36 @@ class PlaybackControllerTest { assertEquals(0L, controller.state.value.position) } + @Test + fun twoTapsKeepTheirOrderWhileThePreferenceReadIsSlow() { + seedAlbum(3, sourceId = 1) + val (slowPrefs, entered, gate) = gatedSongTapPreferences() + val slowController = + PlaybackController( + sources, + queues, + FakePlayerConnection(handle), + downloads, + slowPrefs, + scope = CoroutineScope(SupervisorJob() + dispatcher).also { extraScopes += it }, + ) + slowController.connect() + await { slowController.ready.value } + + slowController.playAlbum(1, "al1", 0) + runBlocking { entered.await() } + slowController.playAlbum(1, "al1", 1) + gate.complete(Unit) + runBlocking { delay(1_000) } + + assertEquals( + "s2", + slowController.state.value.item + ?.id, + ) + assertEquals(1L, slowController.state.value.position) + } + @Test fun jumpingInTheQueueWhileShuffledKeepsTheOrder() { seedAlbum(5, sourceId = 1) @@ -3694,6 +3732,30 @@ class PlaybackControllerTest { (0 until snapshot.size).mapNotNull { queues.itemAt(snapshot, it)?.song?.id } } + private fun gatedSongTapPreferences(): Triple, CompletableDeferred> { + val entered = CompletableDeferred() + val gate = CompletableDeferred() + var reads = 0 + val prefs = + UserPreferences( + object : DataStore { + private val state = MutableStateFlow(emptyPreferences()) + + override val data: Flow = + state.onStart { + if (reads++ == 0) { + entered.complete(Unit) + gate.await() + } + } + + override suspend fun updateData(transform: suspend (Preferences) -> Preferences): Preferences = + transform(state.value).also { state.value = it } + }, + ) + return Triple(prefs, entered, gate) + } + private fun controllerOn( dispatcher: CoroutineDispatcher, handle: FakePlayerHandle,