From 552c3b550088e0c3905b19e9028109dd15ab7954 Mon Sep 17 00:00:00 2001 From: Damontecres Date: Tue, 23 Jun 2026 17:23:47 -0400 Subject: [PATCH] Fix inconsistent playback end behavior issues (#1616) ## Description The exact behavior of what happens when playback ends based on the user's settings would change between versions. This PR defines the behaviors better and includes unit tests to verify future changes won't break it again. ### Related issues Fixes #1548 Related to https://github.com/damontecres/Wholphin/pull/1257 ### Testing Emulator & unit tests ## Screenshots N/A ## AI or LLM usage None --- .../wholphin/ui/playback/PlaybackViewModel.kt | 28 +++-- .../ui/playback/PlaybackViewModelTests.kt | 105 +++++++++++++++++- 2 files changed, 124 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt index 6a531119..a6ae07d5 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt @@ -1034,14 +1034,26 @@ class PlaybackViewModel } is PlaylistItem.Media -> { - if (currentItem is PlaylistItem.Intro) { - Timber.v("Current item is intro, so playing next up immediately") - playNextUp() - } else if (preferences.appPreferences.playbackPreferences.showNextUpWhen != ShowNextUpWhen.NEXT_UP_NEVER) { - Timber.v("Setting next up to ${nextItem.id}") - _state.update { it.copy(nextUp = nextItem.item) } - } else { - controllerViewState.showControls() + val prefs = preferences.appPreferences.playbackPreferences + when { + currentItem is PlaylistItem.Intro -> { + Timber.v("Current item is intro, so playing next up immediately") + playNextUp() + } + + prefs.showNextUpWhen == ShowNextUpWhen.NEXT_UP_NEVER && !prefs.autoPlayNext -> { + Timber.v("Never show or auto play next up, returning") + navigationManager.goBack() + } + + prefs.showNextUpWhen != ShowNextUpWhen.NEXT_UP_NEVER -> { + Timber.v("Setting next up to ${nextItem.id}") + _state.update { it.copy(nextUp = nextItem.item) } + } + + else -> { + controllerViewState.showControls() + } } } diff --git a/app/src/test/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModelTests.kt b/app/src/test/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModelTests.kt index ad4d7a43..d2d824db 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModelTests.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModelTests.kt @@ -10,6 +10,7 @@ import com.github.damontecres.wholphin.data.model.BaseItem import com.github.damontecres.wholphin.data.model.JellyfinServer import com.github.damontecres.wholphin.data.model.JellyfinUser import com.github.damontecres.wholphin.data.model.Playlist +import com.github.damontecres.wholphin.data.model.PlaylistItem import com.github.damontecres.wholphin.preferences.AppPreferences import com.github.damontecres.wholphin.preferences.PlaybackPreferences import com.github.damontecres.wholphin.preferences.ShowNextUpWhen @@ -456,11 +457,12 @@ class PlaybackViewModelTests { } @Test - fun `Show next up never`() = + fun `Show next up never with automatic play`() = runTest(testDispatcher) { setupForPlaylist() setupPreferences { showNextUpWhen = ShowNextUpWhen.NEXT_UP_NEVER + autoPlayNext = true } val viewModel = createViewModel(Destination.PlaybackList(playlist.id)) @@ -476,7 +478,108 @@ class PlaybackViewModelTests { viewModel.state.value.also { state -> Assert.assertNull(state.nextUp) + Assert.assertTrue(state.hasNext) } + verify(exactly = 0) { mockNavigationManager.goBack() } + } + + @Test + fun `Show next up never without automatic play`() = + runTest(testDispatcher) { + setupForPlaylist() + setupPreferences { + showNextUpWhen = ShowNextUpWhen.NEXT_UP_NEVER + autoPlayNext = false + } + + val viewModel = createViewModel(Destination.PlaybackList(playlist.id)) + + viewModel.state.value.also { state -> + Assert.assertEquals(playlistItems.items, state.playlist.items) + Assert.assertEquals(0, state.playlistIndex) + Assert.assertNull(state.nextUp) + } + + viewModel.onPlaybackStateChanged(Player.STATE_ENDED) + testScheduler.advanceUntilIdle() + + viewModel.state.value.also { state -> + Assert.assertNull(state.nextUp) + Assert.assertTrue(state.hasNext) + } + verify(exactly = 1) { mockNavigationManager.goBack() } + } + + @Test + fun `Playback ends with no next up`() = + runTest(testDispatcher) { + setupForPlaylist() + setupPreferences { + showNextUpWhen = ShowNextUpWhen.END_OF_PLAYBACK + autoPlayNext = false + } + + val viewModel = createViewModel(Destination.PlaybackList(playlist.id)) + + viewModel.state.value.also { state -> + Assert.assertEquals(playlistItems.items, state.playlist.items) + Assert.assertEquals(0, state.playlistIndex) + Assert.assertNull(state.nextUp) + } + viewModel.playNextUp() + viewModel.playNextUp() + testScheduler.advanceUntilIdle() + + viewModel.onPlaybackStateChanged(Player.STATE_ENDED) + testScheduler.advanceUntilIdle() + + viewModel.state.value.also { state -> + Assert.assertNull(state.nextUp) + Assert.assertFalse(state.hasNext) + } + verify(exactly = 1) { mockNavigationManager.goBack() } + } + + @Test + fun `Intro playback ends always play next`() = + runTest(testDispatcher) { + setupPreferences { + cinemaMode = true + showNextUpWhen = ShowNextUpWhen.NEXT_UP_NEVER + autoPlayNext = false + } + setupForPlaylist() + coEvery { mockUserLibraryApi.getItem(movie.id) } returns successResponse(movie) + coEvery { mockUserLibraryApi.getIntros(movie.id, any()) } returns + successQueryResult(listOf(intro)) + + val viewModel = createViewModel(Destination.PlaybackList(playlist.id)) + + viewModel.state.value.also { state -> + Assert.assertEquals( + listOf(PlaylistItem.Intro(BaseItem(intro))) + playlistItems.items, + state.playlist.items, + ) + Assert.assertEquals(0, state.playlistIndex) + Assert.assertNull(state.nextUp) + } + + viewModel.onPlaybackStateChanged(Player.STATE_ENDED) + testScheduler.advanceUntilIdle() + + viewModel.state.value.also { state -> + Assert.assertEquals(1, state.playlistIndex) + Assert.assertNull(state.nextUp) + } + verify(exactly = 0) { mockNavigationManager.goBack() } + + val mediaItems = mutableListOf() + verify(exactly = 2) { + mockPlayer.setMediaItem(withArg { mediaItems.add(it) }, any()) + } + Assert.assertEquals(2, mediaItems.size) + Assert.assertEquals(intro.id.toString(), mediaItems[0].mediaId) + Assert.assertEquals(movie.id.toString(), mediaItems[1].mediaId) } @Test