music: allow editing past queue items

Allow past and currently playing queue items to be edited, instead of
just future queue items.

This was a somewhat requested feature that was impossible with the
prior queue system. With some fixes, the new queue system can now be
used to do this.

This even works with edge cases like removing the currently playing
song. Albeit, it's likely that more bug fixes and testing will be
needed.

Resolves #223.

CHANGELOG.md

@@ -2,10 +2,16 @@
 
 ## dev
 
+#### What's Improved
+- Added ability to edit previously played or currently playing items in the queue
+
 #### What's Fixed
 - Fixed crash that would occur in music folders dialog when user does not have a working
 file manager
 
+#### What's Changed
+- Implemented new queue system
+
 ## 3.0.1
 
 #### What's New

app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt

@@ -98,16 +98,23 @@ class PlaybackViewModel(application: Application) :
     }
 
     override fun onIndexMoved(queue: Queue) {
-        _song.value = playbackManager.queue.currentSong
+        _song.value = queue.currentSong
     }
 
-    override fun onQueueReworked(queue: Queue) {
+    override fun onQueueChanged(queue: Queue, change: Queue.ChangeResult) {
+        // Other types of queue changes preserve the current song.
+        if (change == Queue.ChangeResult.SONG) {
+            _song.value = queue.currentSong
+        }
+    }
+
+    override fun onQueueReordered(queue: Queue) {
         _isShuffled.value = queue.isShuffled
     }
 
     override fun onNewPlayback(queue: Queue, parent: MusicParent?) {
-        _song.value = playbackManager.queue.currentSong
+        _song.value = queue.currentSong
-        _parent.value = playbackManager.parent
+        _parent.value = parent
         _isShuffled.value = queue.isShuffled
     }
 

app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt

@@ -31,10 +31,7 @@ import org.oxycblt.auxio.list.recycler.PlayingIndicatorAdapter
 import org.oxycblt.auxio.list.recycler.SongViewHolder
 import org.oxycblt.auxio.list.recycler.SyncListDiffer
 import org.oxycblt.auxio.music.Song
-import org.oxycblt.auxio.util.context
+import org.oxycblt.auxio.util.*
-import org.oxycblt.auxio.util.getAttrColorCompat
-import org.oxycblt.auxio.util.getDimen
-import org.oxycblt.auxio.util.inflater
 
 /**
  * A [RecyclerView.Adapter] that shows an editable list of queue items.
@@ -96,30 +93,19 @@ class QueueAdapter(private val listener: EditableListListener<Song>) :
      * @param isPlaying Whether playback is ongoing or paused.
      */
     fun setPosition(index: Int, isPlaying: Boolean) {
-        var updatedIndex = false
+        logD("Updating index")
+        val lastIndex = currentIndex
+        currentIndex = index
 
-        if (index != currentIndex) {
+        // Have to update not only the currently playing item, but also all items marked
-            val lastIndex = currentIndex
+        // as playing.
-            currentIndex = index
+        if (currentIndex < lastIndex) {
-            updatedIndex = true
+            notifyItemRangeChanged(0, lastIndex + 1, PAYLOAD_UPDATE_POSITION)
-
+        } else {
-            // Have to update not only the currently playing item, but also all items marked
+            notifyItemRangeChanged(0, currentIndex + 1, PAYLOAD_UPDATE_POSITION)
-            // as playing.
-            if (currentIndex < lastIndex) {
-                notifyItemRangeChanged(0, lastIndex + 1, PAYLOAD_UPDATE_POSITION)
-            } else {
-                notifyItemRangeChanged(0, currentIndex + 1, PAYLOAD_UPDATE_POSITION)
-            }
         }
 
-        if (this.isPlaying != isPlaying) {
+        this.isPlaying = isPlaying
-            this.isPlaying = isPlaying
-            // Don't need to do anything if we've already sent an update from changing the
-            // index.
-            if (!updatedIndex) {
-                notifyItemChanged(index, PAYLOAD_UPDATE_POSITION)
-            }
-        }
     }
 
     private companion object {
@@ -158,7 +144,6 @@ class QueueSongViewHolder private constructor(private val binding: ItemQueueSong
             binding.songAlbumCover.isEnabled = value
             binding.songName.isEnabled = value
             binding.songInfo.isEnabled = value
-            binding.songDragHandle.isEnabled = value
         }
 
     init {

app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt

@@ -35,21 +35,9 @@ import org.oxycblt.auxio.util.logD
 class QueueDragCallback(private val playbackModel: QueueViewModel) : ItemTouchHelper.Callback() {
     private var shouldLift = true
 
-    override fun getMovementFlags(
+    override fun getMovementFlags(recyclerView: RecyclerView, viewHolder: RecyclerView.ViewHolder) =
-        recyclerView: RecyclerView,
+        makeFlag(ItemTouchHelper.ACTION_STATE_DRAG, ItemTouchHelper.UP or ItemTouchHelper.DOWN) or
-        viewHolder: RecyclerView.ViewHolder
+            makeFlag(ItemTouchHelper.ACTION_STATE_SWIPE, ItemTouchHelper.START)
-    ): Int {
-        val queueHolder = viewHolder as QueueSongViewHolder
-        return if (queueHolder.isFuture) {
-            makeFlag(
-                ItemTouchHelper.ACTION_STATE_DRAG, ItemTouchHelper.UP or ItemTouchHelper.DOWN) or
-                makeFlag(ItemTouchHelper.ACTION_STATE_SWIPE, ItemTouchHelper.START)
-        } else {
-            // Avoid allowing any touch actions for already-played queue items, as the playback
-            // system does not currently allow for this.
-            0
-        }
-    }
 
     override fun onChildDraw(
         c: Canvas,

app/src/main/java/org/oxycblt/auxio/playback/queue/QueueViewModel.kt

@@ -24,6 +24,7 @@ import org.oxycblt.auxio.music.MusicParent
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.playback.state.PlaybackStateManager
 import org.oxycblt.auxio.playback.state.Queue
+import org.oxycblt.auxio.util.logD
 
 /**
  * A [ViewModel] that manages the current queue state and allows navigation through the queue.
@@ -79,9 +80,7 @@ class QueueViewModel : ViewModel(), PlaybackStateManager.Listener {
      * @return true if the items were moved, false otherwise.
      */
     fun moveQueueDataItems(adapterFrom: Int, adapterTo: Int): Boolean {
-        if (adapterFrom <= playbackManager.queue.index ||
+        if (adapterFrom !in queue.value.indices || adapterTo !in queue.value.indices) {
-            adapterTo <= playbackManager.queue.index) {
-            // Invalid input. Nothing to do.
             return false
         }
         playbackManager.moveQueueItem(adapterFrom, adapterTo)
@@ -105,14 +104,18 @@ class QueueViewModel : ViewModel(), PlaybackStateManager.Listener {
         _index.value = queue.index
     }
 
-    override fun onQueueChanged(queue: Queue) {
+    override fun onQueueChanged(queue: Queue, change: Queue.ChangeResult) {
-        // Queue changed trivially due to item move -> Diff queue, stay at current index.
+        // Queue changed trivially due to item mo -> Diff queue, stay at current index.
         replaceQueue = false
         scrollTo = null
         _queue.value = queue.resolve()
+        if (change != Queue.ChangeResult.MAPPING) {
+            // Index changed, make sure it remains updated without actually scrolling to it.
+            _index.value = queue.index
+        }
     }
 
-    override fun onQueueReworked(queue: Queue) {
+    override fun onQueueReordered(queue: Queue) {
         // Queue changed completely -> Replace queue, update index
         replaceQueue = true
         scrollTo = queue.index

app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt

@@ -226,11 +226,7 @@ class PlaybackStateManager private constructor() {
      * Add a [Song] to the top of the queue.
      * @param song The [Song] to add.
      */
-    @Synchronized
+    @Synchronized fun playNext(song: Song) = playNext(listOf(song))
-    fun playNext(song: Song) {
-        queue.playNext(listOf(song))
-        notifyQueueChanged()
-    }
 
     /**
      * Add [Song]s to the top of the queue.
@@ -238,19 +234,22 @@ class PlaybackStateManager private constructor() {
      */
     @Synchronized
     fun playNext(songs: List<Song>) {
-        queue.playNext(songs)
+        val internalPlayer = internalPlayer ?: return
-        notifyQueueChanged()
+        when (queue.playNext(songs)) {
+            Queue.ChangeResult.MAPPING -> notifyQueueChanged(Queue.ChangeResult.MAPPING)
+            Queue.ChangeResult.SONG -> {
+                internalPlayer.loadSong(queue.currentSong, true)
+                notifyNewPlayback()
+            }
+            Queue.ChangeResult.INDEX -> error("Unreachable")
+        }
     }
 
     /**
      * Add a [Song] to the end of the queue.
      * @param song The [Song] to add.
      */
-    @Synchronized
+    @Synchronized fun addToQueue(song: Song) = addToQueue(listOf(song))
-    fun addToQueue(song: Song) {
-        queue.addToQueue(listOf(song))
-        notifyQueueChanged()
-    }
 
     /**
      * Add [Song]s to the end of the queue.
@@ -258,8 +257,15 @@ class PlaybackStateManager private constructor() {
      */
     @Synchronized
     fun addToQueue(songs: List<Song>) {
-        queue.addToQueue(songs)
+        val internalPlayer = internalPlayer ?: return
-        notifyQueueChanged()
+        when (queue.addToQueue(songs)) {
+            Queue.ChangeResult.MAPPING -> notifyQueueChanged(Queue.ChangeResult.MAPPING)
+            Queue.ChangeResult.SONG -> {
+                internalPlayer.loadSong(queue.currentSong, true)
+                notifyNewPlayback()
+            }
+            Queue.ChangeResult.INDEX -> error("Unreachable")
+        }
     }
 
     /**
@@ -270,8 +276,7 @@ class PlaybackStateManager private constructor() {
     @Synchronized
     fun moveQueueItem(src: Int, dst: Int) {
         logD("Moving item $src to position $dst")
-        queue.move(src, dst)
+        notifyQueueChanged(queue.move(src, dst))
-        notifyQueueChanged()
     }
 
     /**
@@ -280,9 +285,13 @@ class PlaybackStateManager private constructor() {
      */
     @Synchronized
     fun removeQueueItem(at: Int) {
+        val internalPlayer = internalPlayer ?: return
         logD("Removing item at $at")
-        queue.remove(at)
+        val change = queue.remove(at)
-        notifyQueueChanged()
+        if (change == Queue.ChangeResult.SONG) {
+            internalPlayer.loadSong(queue.currentSong, playerState.isPlaying)
+        }
+        notifyQueueChanged(change)
     }
 
     /**
@@ -292,7 +301,7 @@ class PlaybackStateManager private constructor() {
     @Synchronized
     fun reorder(shuffled: Boolean) {
         queue.reorder(shuffled)
-        notifyQueueReworked()
+        notifyQueueReordered()
     }
 
     // --- INTERNAL PLAYER FUNCTIONS ---
@@ -532,15 +541,15 @@ class PlaybackStateManager private constructor() {
         }
     }
 
-    private fun notifyQueueChanged() {
+    private fun notifyQueueChanged(change: Queue.ChangeResult) {
         for (callback in listeners) {
-            callback.onQueueChanged(queue)
+            callback.onQueueChanged(queue, change)
         }
     }
 
-    private fun notifyQueueReworked() {
+    private fun notifyQueueReordered() {
         for (callback in listeners) {
-            callback.onQueueReworked(queue)
+            callback.onQueueReordered(queue)
         }
     }
 
@@ -575,17 +584,18 @@ class PlaybackStateManager private constructor() {
         fun onIndexMoved(queue: Queue) {}
 
         /**
-         * Called when the [Queue] changed in a trivial manner, such as a move.
+         * Called when the [Queue] changed in a manner outlined by the given [Queue.ChangeResult].
          * @param queue The new [Queue].
+         * @param change The type of [Queue.ChangeResult] that occurred.
          */
-        fun onQueueChanged(queue: Queue) {}
+        fun onQueueChanged(queue: Queue, change: Queue.ChangeResult) {}
 
         /**
          * Called when the [Queue] has changed in a non-trivial manner (such as re-shuffling), but
          * the currently playing [Song] has not.
          * @param queue The new [Queue].
          */
-        fun onQueueReworked(queue: Queue) {}
+        fun onQueueReordered(queue: Queue) {}
 
         /**
          * Called when a new playback configuration was created.

app/src/main/java/org/oxycblt/auxio/playback/state/Queue.kt

@@ -115,12 +115,14 @@ class Queue {
     /**
      * Add [Song]s to the top of the queue. Will start playback if nothing is playing.
      * @param songs The [Song]s to add.
+     * @return [ChangeResult.MAPPING] if added to an existing queue, or [ChangeResult.SONG] if there
+     * was no prior playback and these enqueued [Song]s start new playback.
      */
-    fun playNext(songs: List<Song>) {
+    fun playNext(songs: List<Song>): ChangeResult {
         if (orderedMapping.isEmpty()) {
             // No playback, start playing these songs.
             start(songs[0], songs, false)
-            return
+            return ChangeResult.SONG
         }
 
         val heapIndices = songs.map(::addSongToHeap)
@@ -134,17 +136,20 @@ class Queue {
             // Add the new song in front of the current index in the ordered mapping.
             orderedMapping.addAll(index, heapIndices)
         }
+        return ChangeResult.MAPPING
     }
 
     /**
      * Add [Song]s to the end of the queue. Will start playback if nothing is playing.
      * @param songs The [Song]s to add.
+     * @return [ChangeResult.MAPPING] if added to an existing queue, or [ChangeResult.SONG] if there
+     * was no prior playback and these enqueued [Song]s start new playback.
      */
-    fun addToQueue(songs: List<Song>) {
+    fun addToQueue(songs: List<Song>): ChangeResult {
         if (orderedMapping.isEmpty()) {
             // No playback, start playing these songs.
             start(songs[0], songs, false)
-            return
+            return ChangeResult.SONG
         }
 
         val heapIndices = songs.map(::addSongToHeap)
@@ -153,14 +158,18 @@ class Queue {
         if (shuffledMapping.isNotEmpty()) {
             shuffledMapping.addAll(heapIndices)
         }
+        return ChangeResult.MAPPING
     }
 
     /**
      * Move a [Song] at the given position to a new position.
      * @param src The position of the [Song] to move.
      * @param dst The destination position of the [Song].
+     * @return [ChangeResult.MAPPING] if the move occurred after the current index,
+     * [ChangeResult.INDEX] if the move occurred before or at the current index, requiring it to be
+     * mutated.
      */
-    fun move(src: Int, dst: Int) {
+    fun move(src: Int, dst: Int): ChangeResult {
         if (shuffledMapping.isNotEmpty()) {
             // Move songs only in the shuffled mapping. There is no sane analogous form of
             // this for the ordered mapping.
@@ -170,21 +179,31 @@ class Queue {
             orderedMapping.add(dst, orderedMapping.removeAt(src))
         }
 
-        if (index in (src + 1) until dst) {
+        return when (index) {
+            // Moving the currently playing song.
+            src -> {
+                index = dst
+                ChangeResult.INDEX
+            }
             // Index was ahead of moved song but not ahead of it's destination position.
             // This makes it functionally a removal, so update the index to preserve consistency.
-            index -= 1
+            in (src + 1)..dst -> {
-        } else if (index == src) {
+                index -= 1
-            // Moving the currently playing song.
+                ChangeResult.INDEX
-            index = dst
+            }
+            // Nothing to do.
+            else -> ChangeResult.MAPPING
         }
     }
 
     /**
      * Remove a [Song] at the given position.
      * @param at The position of the [Song] to remove.
+     * @return [ChangeResult.MAPPING] if the removed [Song] was after the current index,
+     * [ChangeResult.INDEX] if the removed [Song] was before the current index, and
+     * [ChangeResult.SONG] if the currently playing [Song] was removed.
      */
-    fun remove(at: Int) {
+    fun remove(at: Int): ChangeResult {
         if (shuffledMapping.isNotEmpty()) {
             // Remove the specified index in the shuffled mapping and the analogous song in the
             // ordered mapping.
@@ -199,9 +218,16 @@ class Queue {
         // of the player to be completely invalidated. It's generally easier to not remove the
         // song and retain player state consistency.
 
-        if (index > at) {
+        return when {
+            // We just removed the currently playing song.
+            index == at -> ChangeResult.SONG
             // Index was ahead of removed song, shift back to preserve consistency.
-            index -= 1
+            index > at -> {
+                index -= 1
+                ChangeResult.INDEX
+            }
+            // Nothing to do
+            else -> ChangeResult.MAPPING
         }
     }
 
@@ -232,4 +258,20 @@ class Queue {
         heap.add(song)
         return heap.lastIndex
     }
+
+    /**
+     * Represents the possible changes that can occur during certain queue mutation events. The
+     * precise meanings of these differ somewhat depending on the type of mutation done.
+     */
+    enum class ChangeResult {
+        /** Only the mapping has changed. */
+        MAPPING,
+        /** The mapping has changed, and the index also changed to align with it. */
+        INDEX,
+        /**
+         * The current song has changed, possibly alongside the mapping and index depending on the
+         * context.
+         */
+        SONG
+    }
 }

app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt

@@ -94,15 +94,24 @@ class MediaSessionComponent(private val context: Context, private val listener:
     // --- PLAYBACKSTATEMANAGER OVERRIDES ---
 
     override fun onIndexMoved(queue: Queue) {
-        updateMediaMetadata(playbackManager.queue.currentSong, playbackManager.parent)
+        updateMediaMetadata(queue.currentSong, playbackManager.parent)
         invalidateSessionState()
     }
 
-    override fun onQueueChanged(queue: Queue) {
+    override fun onQueueChanged(queue: Queue, change: Queue.ChangeResult) {
         updateQueue(queue)
+        when (change) {
+            // Nothing special to do with mapping changes.
+            Queue.ChangeResult.MAPPING -> {}
+            // Index changed, ensure playback state's index changes.
+            Queue.ChangeResult.INDEX -> invalidateSessionState()
+            // Song changed, ensure metadata changes.
+            Queue.ChangeResult.SONG ->
+                updateMediaMetadata(queue.currentSong, playbackManager.parent)
+        }
     }
 
-    override fun onQueueReworked(queue: Queue) {
+    override fun onQueueReordered(queue: Queue) {
         updateQueue(queue)
         invalidateSessionState()
         mediaSession.setShuffleMode(
@@ -115,7 +124,7 @@ class MediaSessionComponent(private val context: Context, private val listener:
     }
 
     override fun onNewPlayback(queue: Queue, parent: MusicParent?) {
-        updateMediaMetadata(playbackManager.queue.currentSong, parent)
+        updateMediaMetadata(queue.currentSong, parent)
         updateQueue(queue)
         invalidateSessionState()
     }

app/src/main/java/org/oxycblt/auxio/widgets/WidgetComponent.kt

@@ -117,7 +117,7 @@ class WidgetComponent(private val context: Context) :
     // Hook all the major song-changing updates + the major player state updates
     // to updating the "Now Playing" widget.
     override fun onIndexMoved(queue: Queue) = update()
-    override fun onQueueReworked(queue: Queue) = update()
+    override fun onQueueReordered(queue: Queue) = update()
     override fun onNewPlayback(queue: Queue, parent: MusicParent?) = update()
     override fun onStateChanged(state: InternalPlayer.State) = update()
     override fun onRepeatChanged(repeatMode: RepeatMode) = update()