Include the currently-playing episode in the Next Up queue (issue #196) - #210
Merged
Merged
Conversation
The currently-playing episode was fully removed from the queue table on play (issue #171) and only ever shown via the mini/expanded player -- Next Up itself had no idea it existed. It's now a real queue entry like any other, always kept at the front, and shown in the list clearly marked "Now Playing" (a small play icon + label swapped in for the date) rather than hidden. - QueueRepository: moveToFront() replaces the old remove-on-play, peekFront() replaces the old pop-and-remove popNext() (advancing to an episode shouldn't also dequeue it now that it's meant to stay queued while it plays), and moveToEnd() gets a failed episode out of the front slot without discarding it outright. - PlaybackController.play()/restoreLastPlayingItem() move the episode to the queue's front instead of removing it. The old requeuePreviousEpisode() (re-add if unfinished) is replaced by dropFromQueueIfFinished() (remove if it turns out to have finished) -- the previous episode never actually left the queue this time, so there's nothing to add back, only a stale finished row to potentially clean up. - PlaybackService: onPlaybackStateChanged(STATE_ENDED) drops the finished episode's row itself now (previously unnecessary, since play() used to do that dequeue upfront) -- deferred until after playNextQueued() so the issue #82 silence-gap fix isn't undone. onPlayerError moves the failed episode to the back instead, since it never actually finished. Both pass playNextQueued() an itemId to treat as "not really next" so it doesn't just find the very episode it's meant to be skipping past -- needed because a lone queued episode moved to the back is still the only (and therefore front) entry. startPreloadingQueueHead() now skips the currently-playing entry when picking what to preload next, for the same reason. No UI filtering was needed for the queue list itself -- ReorderableQueueList already fully supported an isCurrentlyPlaying row (highlight, disabled long-press-to-play, etc.) from when that state was only ever transient; it just needed the explicit "Now Playing" label added.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The currently-playing episode was fully removed from the queue table when it started playing (issue #171) and was only ever visible via the mini/expanded player -- Next Up itself had no idea it existed. Per discussion on the issue, it's now a real queue entry like any other, always kept at the front, and shown in the list clearly marked "Now Playing" instead of hidden.
QueueRepository:moveToFront()replaces the old remove-on-play;peekFront()replaces the old pop-and-removepopNext()(advancing to an episode shouldn't dequeue it now that it's meant to stay queued while playing);moveToEnd()gets a failed episode out of the front slot without discarding it outright.PlaybackController.play()/restoreLastPlayingItem()move the episode to the queue's front instead of removing it.requeuePreviousEpisode()(re-add if unfinished) becomesdropFromQueueIfFinished()(remove if it turns out to have finished) -- the previous episode never actually left the queue this time.PlaybackService: theSTATE_ENDEDhandler drops the finished episode's row itself now, deferred until afterplayNextQueued()so issue Substantial silence gap between Next Up episodes #82's silence-gap fix isn't undone.onPlayerErrormoves the failed episode to the back instead (it never actually finished, so it's still worth surfacing to retry). Both pass an excluded itemId intoplayNextQueued()so it doesn't just re-select the very episode it's meant to be skipping past -- needed because a lone queued episode moved to the back is still the only (and therefore front) entry.startPreloadingQueueHead()now skips the currently-playing entry when picking what to preload next, for the same reason.ReorderableQueueListalready fully supported anisCurrentlyPlayingrow (highlight, disabled long-press-to-play, etc.) from when that state was only ever transient; it just needed the explicit "Now Playing" label (small play icon + text, swapped in for the date).Test plan
./gradlew assembleDebug testDebugUnitTest lintDebug-- all greenQueueRepositoryTest(peekFront/moveToFront/moveToEnd),PlaybackControllerTest(play moves to front instead of removing),QueueViewModelTest(playNow moves to front instead of shrinking the queue)PlaybackService's advance-on-error/end logic itself -- same pre-existing constraint as issues Podcasts stop playing intermittently #201/Chapter navigation is no longer available #202's PRs (noMediaSessionService/ExoPlayer test harness in this repo).Closes #196