mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 07:29:14 +01:00
react to active source changes
The active source becomes one immutable id+source pair, so cover art and stream/download URLs can no longer read a torn state mid-switch. The root navigation watches the active source id and, on a real switch, closes the overlays and pops an open album/artist/playlist detail back to the library.
This commit is contained in:
@@ -40,11 +40,13 @@ class SourceRepository(
|
|||||||
) {
|
) {
|
||||||
private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
|
private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
|
||||||
|
|
||||||
@Volatile
|
private data class ActiveSource(
|
||||||
private var active: SubsonicMusicSource? = null
|
val id: Long,
|
||||||
|
val source: SubsonicMusicSource,
|
||||||
|
)
|
||||||
|
|
||||||
@Volatile
|
@Volatile
|
||||||
private var activeSourceId: Long? = null
|
private var active: ActiveSource? = null
|
||||||
|
|
||||||
private val _quality = MutableStateFlow(StreamQuality())
|
private val _quality = MutableStateFlow(StreamQuality())
|
||||||
val quality: StateFlow<StreamQuality> = _quality
|
val quality: StateFlow<StreamQuality> = _quality
|
||||||
@@ -87,10 +89,9 @@ class SourceRepository(
|
|||||||
) { config, mode, wifi, mobile, concurrency ->
|
) { config, mode, wifi, mobile, concurrency ->
|
||||||
Triple(config, if (mode == NetworkMode.Wifi) wifi else mobile, concurrency)
|
Triple(config, if (mode == NetworkMode.Wifi) wifi else mobile, concurrency)
|
||||||
}.collect { (config, quality, concurrency) ->
|
}.collect { (config, quality, concurrency) ->
|
||||||
active = config?.toMusicSource(quality, concurrency)
|
val sourceChanged = config?.id != active?.id
|
||||||
|
active = config?.let { ActiveSource(it.id, it.toMusicSource(quality, concurrency)) }
|
||||||
_quality.value = quality
|
_quality.value = quality
|
||||||
val sourceChanged = config?.id != activeSourceId
|
|
||||||
activeSourceId = config?.id
|
|
||||||
if (sourceChanged && config != null && config.useTokenAuth && !_offline.value) {
|
if (sourceChanged && config != null && config.useTokenAuth && !_offline.value) {
|
||||||
scope.launch { runCatching { config.toClient().check("ping") } }
|
scope.launch { runCatching { config.toClient().check("ping") } }
|
||||||
}
|
}
|
||||||
@@ -116,12 +117,12 @@ class SourceRepository(
|
|||||||
coverArt: String?,
|
coverArt: String?,
|
||||||
thumbnail: Boolean = false,
|
thumbnail: Boolean = false,
|
||||||
): CoverArtRef? {
|
): CoverArtRef? {
|
||||||
val source = active ?: return null
|
val current = active ?: return null
|
||||||
if (coverArt == null) return null
|
if (coverArt == null) return null
|
||||||
val cacheKey = coverArtKey(source.id, coverArt, thumbnail)
|
val cacheKey = coverArtKey(current.id, coverArt, thumbnail)
|
||||||
artworkStore.uri(source.id, cacheKey)?.let { return CoverArtRef(url = it, cacheKey = cacheKey) }
|
artworkStore.uri(current.id, cacheKey)?.let { return CoverArtRef(url = it, cacheKey = cacheKey) }
|
||||||
if (_offline.value) return null
|
if (_offline.value) return null
|
||||||
val url = source.coverArtUri(coverArt, thumbnail)?.toString() ?: return null
|
val url = current.source.coverArtUri(coverArt, thumbnail)?.toString() ?: return null
|
||||||
return CoverArtRef(url = url, cacheKey = cacheKey)
|
return CoverArtRef(url = url, cacheKey = cacheKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -131,9 +132,8 @@ class SourceRepository(
|
|||||||
thumbnail: Boolean,
|
thumbnail: Boolean,
|
||||||
): String? {
|
): String? {
|
||||||
if (_offline.value) return null
|
if (_offline.value) return null
|
||||||
val source = active ?: return null
|
val current = active?.takeIf { it.id == sourceId } ?: return null
|
||||||
if (source.id != sourceId) return null
|
return current.source.coverArtUri(coverArt, thumbnail)?.toString()
|
||||||
return source.coverArtUri(coverArt, thumbnail)?.toString()
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fun streamUri(
|
fun streamUri(
|
||||||
@@ -142,7 +142,7 @@ class SourceRepository(
|
|||||||
quality: StreamQuality,
|
quality: StreamQuality,
|
||||||
): String? {
|
): String? {
|
||||||
if (_offline.value) return null
|
if (_offline.value) return null
|
||||||
val uri = active?.streamUri(songId)?.toString() ?: return null
|
val uri = active?.source?.streamUri(songId)?.toString() ?: return null
|
||||||
val length = declaredStreamLength(durationMs, quality) ?: return uri
|
val length = declaredStreamLength(durationMs, quality) ?: return uri
|
||||||
return uri + streamLengthSuffix(length)
|
return uri + streamLengthSuffix(length)
|
||||||
}
|
}
|
||||||
@@ -152,14 +152,14 @@ class SourceRepository(
|
|||||||
songId: String,
|
songId: String,
|
||||||
): String? {
|
): String? {
|
||||||
if (_offline.value) return null
|
if (_offline.value) return null
|
||||||
val source = active?.takeIf { activeSourceId == sourceId } ?: return null
|
val current = active?.takeIf { it.id == sourceId } ?: return null
|
||||||
// A transcode preference downloads the stream, so the saved file is already the wanted
|
// A transcode preference downloads the stream, so the saved file is already the wanted
|
||||||
// quality rather than the original that would then have to be transcoded on every play.
|
// quality rather than the original that would then have to be transcoded on every play.
|
||||||
val quality = downloadQuality
|
val quality = downloadQuality
|
||||||
return if (quality.transcodes) {
|
return if (quality.transcodes) {
|
||||||
source.streamUri(songId, quality.maxBitrate, quality.format).toString()
|
current.source.streamUri(songId, quality.maxBitrate, quality.format).toString()
|
||||||
} else {
|
} else {
|
||||||
source.downloadUri(songId).toString()
|
current.source.downloadUri(songId).toString()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -131,6 +131,23 @@ private object Routes {
|
|||||||
fun editServer(id: Long) = "edit-server/$id"
|
fun editServer(id: Long) = "edit-server/$id"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private val DETAIL_ROUTES =
|
||||||
|
setOf(Routes.ALBUM_DETAIL, Routes.ARTIST_DETAIL, Routes.PLAYLIST_DETAIL)
|
||||||
|
|
||||||
|
internal fun resetOnSourceSwitch(
|
||||||
|
previousSourceId: Long?,
|
||||||
|
currentSourceId: Long?,
|
||||||
|
route: String?,
|
||||||
|
closeNowPlaying: () -> Unit,
|
||||||
|
closeQueue: () -> Unit,
|
||||||
|
popDetail: () -> Unit,
|
||||||
|
) {
|
||||||
|
if (previousSourceId == null || currentSourceId == null || previousSourceId == currentSourceId) return
|
||||||
|
closeNowPlaying()
|
||||||
|
closeQueue()
|
||||||
|
if (route in DETAIL_ROUTES) popDetail()
|
||||||
|
}
|
||||||
|
|
||||||
internal data class BackStackKey(
|
internal data class BackStackKey(
|
||||||
val route: String?,
|
val route: String?,
|
||||||
val argument: String?,
|
val argument: String?,
|
||||||
@@ -191,6 +208,25 @@ private fun MainNavigation() {
|
|||||||
val contextMenuHost = remember { ContextMenuHost() }
|
val contextMenuHost = remember { ContextMenuHost() }
|
||||||
var infoSong by remember { mutableStateOf<Song?>(null) }
|
var infoSong by remember { mutableStateOf<Song?>(null) }
|
||||||
|
|
||||||
|
val activeSourceId by libraryRepository.activeSourceId.collectAsStateWithLifecycle(initialValue = null)
|
||||||
|
var previousSourceId by rememberSaveable { mutableStateOf<Long?>(null) }
|
||||||
|
LaunchedEffect(activeSourceId) {
|
||||||
|
val id = activeSourceId
|
||||||
|
resetOnSourceSwitch(
|
||||||
|
previousSourceId = previousSourceId,
|
||||||
|
currentSourceId = id,
|
||||||
|
route = currentRoute,
|
||||||
|
closeNowPlaying = {
|
||||||
|
settleJob?.cancel()
|
||||||
|
nowPlayingOpen = false
|
||||||
|
nowPlayingProgress = 0f
|
||||||
|
},
|
||||||
|
closeQueue = { showingQueue = false },
|
||||||
|
popDetail = { navController.popBackStack(Routes.LIBRARY, inclusive = false) },
|
||||||
|
)
|
||||||
|
if (id != null) previousSourceId = id
|
||||||
|
}
|
||||||
|
|
||||||
fun settleNowPlaying(open: Boolean) {
|
fun settleNowPlaying(open: Boolean) {
|
||||||
if (open) {
|
if (open) {
|
||||||
nowPlayingOpen = true
|
nowPlayingOpen = true
|
||||||
|
|||||||
@@ -0,0 +1,86 @@
|
|||||||
|
package com.subtracks.ui
|
||||||
|
|
||||||
|
import org.junit.Assert.assertFalse
|
||||||
|
import org.junit.Assert.assertTrue
|
||||||
|
import org.junit.Test
|
||||||
|
|
||||||
|
class SourceSwitchResetTest {
|
||||||
|
private var nowPlayingClosed = false
|
||||||
|
private var queueClosed = false
|
||||||
|
private var detailPopped = false
|
||||||
|
|
||||||
|
private fun reset(
|
||||||
|
previous: Long?,
|
||||||
|
current: Long?,
|
||||||
|
route: String?,
|
||||||
|
) {
|
||||||
|
nowPlayingClosed = false
|
||||||
|
queueClosed = false
|
||||||
|
detailPopped = false
|
||||||
|
resetOnSourceSwitch(
|
||||||
|
previousSourceId = previous,
|
||||||
|
currentSourceId = current,
|
||||||
|
route = route,
|
||||||
|
closeNowPlaying = { nowPlayingClosed = true },
|
||||||
|
closeQueue = { queueClosed = true },
|
||||||
|
popDetail = { detailPopped = true },
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun theFirstSourceResetsNothing() {
|
||||||
|
reset(previous = null, current = 1L, route = "album/{albumId}?coverArt={coverArt}")
|
||||||
|
|
||||||
|
assertFalse(nowPlayingClosed)
|
||||||
|
assertFalse(queueClosed)
|
||||||
|
assertFalse(detailPopped)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun reselectingTheSameSourceResetsNothing() {
|
||||||
|
reset(previous = 1L, current = 1L, route = "settings")
|
||||||
|
|
||||||
|
assertFalse(nowPlayingClosed)
|
||||||
|
assertFalse(queueClosed)
|
||||||
|
assertFalse(detailPopped)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun losingTheSourceResetsNothing() {
|
||||||
|
reset(previous = 1L, current = null, route = "settings")
|
||||||
|
|
||||||
|
assertFalse(nowPlayingClosed)
|
||||||
|
assertFalse(queueClosed)
|
||||||
|
assertFalse(detailPopped)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun aSwitchClosesTheOverlays() {
|
||||||
|
reset(previous = 1L, current = 2L, route = "settings")
|
||||||
|
|
||||||
|
assertTrue(nowPlayingClosed)
|
||||||
|
assertTrue(queueClosed)
|
||||||
|
assertFalse(detailPopped)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun aSwitchOnADetailPopsIt() {
|
||||||
|
reset(previous = 1L, current = 2L, route = "album/{albumId}?coverArt={coverArt}")
|
||||||
|
|
||||||
|
assertTrue(nowPlayingClosed)
|
||||||
|
assertTrue(queueClosed)
|
||||||
|
assertTrue(detailPopped)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun everyDetailRoutePopsOnASwitch() {
|
||||||
|
listOf(
|
||||||
|
"album/{albumId}?coverArt={coverArt}",
|
||||||
|
"artist/{artistId}?coverArt={coverArt}",
|
||||||
|
"playlist/{playlistId}",
|
||||||
|
).forEach { route ->
|
||||||
|
reset(previous = 1L, current = 2L, route = route)
|
||||||
|
assertTrue("$route should be dismissed", detailPopped)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user