From d0b34a14e4f7a4c6c460c9e7d6e942c293f6c2c4 Mon Sep 17 00:00:00 2001
From: Alexander Capehart <alex@oxycblt.org>
Date: Thu, 17 Aug 2023 20:42:12 -0600
Subject: [PATCH] playback: fix broken item navigation

Caused yet again by sharing StateFlows leading to a strange
out-of-order collector notification, which then allows detail
fragments to consume item navigation requests before the playback
panel can even get them. SharedFlow doesn't help here, so we are
just forced to move this to MainFragment which does not have this
issue for some reason.
---
 .../java/org/oxycblt/auxio/MainFragment.kt    | 21 ++++++++++++++++
 .../auxio/playback/PlaybackPanelFragment.kt   | 24 +------------------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt
index c86037eb1..297bad95c 100644
--- a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt
@@ -38,6 +38,7 @@ import kotlin.math.max
 import kotlin.math.min
 import org.oxycblt.auxio.databinding.FragmentMainBinding
 import org.oxycblt.auxio.detail.DetailViewModel
+import org.oxycblt.auxio.detail.Show
 import org.oxycblt.auxio.home.HomeViewModel
 import org.oxycblt.auxio.home.Outer
 import org.oxycblt.auxio.list.ListViewModel
@@ -49,6 +50,7 @@ import org.oxycblt.auxio.playback.PlaybackViewModel
 import org.oxycblt.auxio.playback.queue.QueueBottomSheetBehavior
 import org.oxycblt.auxio.ui.DialogAwareNavigationListener
 import org.oxycblt.auxio.ui.ViewBindingFragment
+import org.oxycblt.auxio.util.collect
 import org.oxycblt.auxio.util.collectImmediately
 import org.oxycblt.auxio.util.context
 import org.oxycblt.auxio.util.coordinatorLayoutBehavior
@@ -149,6 +151,11 @@ class MainFragment :
         }
 
         // --- VIEWMODEL SETUP ---
+        // This has to be done here instead of the playback panel to make sure that it's prioritized
+        // by StateFlow over any detail fragment.
+        // FIXME: This is a consequence of sharing events across several consumers. There has to be
+        //  a better way of doing this.
+        collect(detailModel.toShow.flow, ::handleShow)
         collectImmediately(detailModel.editedPlaylist, detailBackCallback::invalidateEnabled)
         collectImmediately(homeModel.showOuter.flow, ::handleShowOuter)
         collectImmediately(listModel.selected, selectionBackCallback::invalidateEnabled)
@@ -287,6 +294,20 @@ class MainFragment :
         return true
     }
 
+    private fun handleShow(show: Show?) {
+        when (show) {
+            is Show.SongAlbumDetails,
+            is Show.ArtistDetails,
+            is Show.AlbumDetails -> playbackModel.openMain()
+            is Show.SongDetails,
+            is Show.SongArtistDecision,
+            is Show.AlbumArtistDecision,
+            is Show.GenreDetails,
+            is Show.PlaylistDetails,
+            null -> {}
+        }
+    }
+
     private fun handleShowOuter(outer: Outer?) {
         val directions =
             when (outer) {
diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt
index b43b3330e..b7228d508 100644
--- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt
@@ -33,7 +33,6 @@ import dagger.hilt.android.AndroidEntryPoint
 import org.oxycblt.auxio.R
 import org.oxycblt.auxio.databinding.FragmentPlaybackPanelBinding
 import org.oxycblt.auxio.detail.DetailViewModel
-import org.oxycblt.auxio.detail.Show
 import org.oxycblt.auxio.list.ListViewModel
 import org.oxycblt.auxio.music.MusicParent
 import org.oxycblt.auxio.music.Song
@@ -41,7 +40,6 @@ import org.oxycblt.auxio.music.resolveNames
 import org.oxycblt.auxio.playback.state.RepeatMode
 import org.oxycblt.auxio.playback.ui.StyledSeekBar
 import org.oxycblt.auxio.ui.ViewBindingFragment
-import org.oxycblt.auxio.util.collect
 import org.oxycblt.auxio.util.collectImmediately
 import org.oxycblt.auxio.util.logD
 import org.oxycblt.auxio.util.overrideOnOverflowMenuClick
@@ -105,12 +103,7 @@ class PlaybackPanelFragment :
         // respective item.
         binding.playbackSong.apply {
             isSelected = true
-            setOnClickListener {
-                playbackModel.song.value?.let {
-                    detailModel.showAlbum(it)
-                    playbackModel.openMain()
-                }
-            }
+            setOnClickListener { playbackModel.song.value?.let(detailModel::showAlbum) }
         }
         binding.playbackArtist.apply {
             isSelected = true
@@ -138,7 +131,6 @@ class PlaybackPanelFragment :
         collectImmediately(playbackModel.repeatMode, ::updateRepeat)
         collectImmediately(playbackModel.isPlaying, ::updatePlaying)
         collectImmediately(playbackModel.isShuffled, ::updateShuffled)
-        collect(detailModel.toShow.flow, ::handleShow)
     }
 
     override fun onDestroyBinding(binding: FragmentPlaybackPanelBinding) {
@@ -220,20 +212,6 @@ class PlaybackPanelFragment :
         requireBinding().playbackShuffle.isActivated = isShuffled
     }
 
-    private fun handleShow(show: Show?) {
-        when (show) {
-            is Show.SongAlbumDetails,
-            is Show.ArtistDetails,
-            is Show.AlbumDetails -> playbackModel.openMain()
-            is Show.SongDetails,
-            is Show.SongArtistDecision,
-            is Show.AlbumArtistDecision,
-            is Show.GenreDetails,
-            is Show.PlaylistDetails,
-            null -> {}
-        }
-    }
-
     private fun navigateToCurrentArtist() {
         playbackModel.song.value?.let(detailModel::showArtist)
     }