From 56e84758eb4ffb2810d2a3d965832bc440bd7fcd Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sat, 17 Oct 2026 19:34:29 +0200 Subject: [PATCH] fix(syncplay): move on to the next episode through the group Next Up, the end of an episode and the episode browser started the next episode on this TV alone. The group's commands for the previous entry then landed on it, so Next Up kept coming back and played on through the series. Moving on now goes through the group, as on Moonfin-Core, and commands for another queue entry are dropped. Queue edits and repeat or shuffle changes also no longer start the group's item on a TV that has nothing playing. Co-Authored-By: Claude Opus 5.5 --- packages/app/src/App/App.js | 13 ++- packages/app/src/context/SyncPlayContext.js | 18 +++- .../app/src/context/SyncPlayContext.test.js | 88 +++++++++++++++++++ packages/app/src/services/syncPlay.js | 9 ++ packages/app/src/services/syncPlay.test.js | 11 +++ 5 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 packages/app/src/context/SyncPlayContext.test.js diff --git a/packages/app/src/App/App.js b/packages/app/src/App/App.js index bed2aecc2..4d6f1cd5b 100644 --- a/packages/app/src/App/App.js +++ b/packages/app/src/App/App.js @@ -38,7 +38,7 @@ import {AchievementsProvider, useAchievements} from '../context/AchievementsCont import {achievementIconPath} from '../views/Settings/achievements/achievementIcons'; import {threadIsPhoto} from '../utils/achievementsModel'; import {ServerMessagesProvider, useServerMessages} from '../context/ServerMessagesContext'; -import {SyncPlayProvider, useSyncPlay} from '../context/SyncPlayContext'; +import {SyncPlayProvider, followsQueueUpdate, useSyncPlay} from '../context/SyncPlayContext'; import {useVersionCheck} from '../hooks/useVersionCheck'; import UpdateNotification from '../components/UpdateNotification'; import {ItemMenuProvider} from '../components/ItemContextMenu'; @@ -168,7 +168,7 @@ const AppContent = (props) => { const {streamNotification, dismissStreamNotification, pluginInfo} = useSeerr(); const {pendingPopups, markPopupsRead} = useServerMessages(); const themeMusic = useThemeMusic(); - const {openDialog: openSyncPlay, closeDialog: closeSyncPlay, isDialogOpen: syncPlayDialogOpen, playQueueUpdate: syncPlayQueueUpdate, isInGroup: isSyncPlayInGroup, setNewQueue: syncPlaySetNewQueue, getGroupPositionTicks: getSyncPlayPositionTicks} = useSyncPlay(); + const {openDialog: openSyncPlay, closeDialog: closeSyncPlay, isDialogOpen: syncPlayDialogOpen, playQueueUpdate: syncPlayQueueUpdate, isInGroup: isSyncPlayInGroup, setNewQueue: syncPlaySetNewQueue, moveGroupTo: syncPlayMoveGroupTo, getGroupPositionTicks: getSyncPlayPositionTicks} = useSyncPlay(); const handledSyncPlayQueueRef = useRef(null); const [remoteMessage, setRemoteMessage] = useState(null); @@ -994,7 +994,7 @@ const AppContent = (props) => { const update = syncPlayQueueUpdate; if (!update || update === handledSyncPlayQueueRef.current) return; handledSyncPlayQueueRef.current = update; - if (!update.startsPlayback && playingItem?.Id === update.item.Id) return; + if (!followsQueueUpdate(update, playingItem?.Id)) return; playSyncPlayItem(update.item); }, [syncPlayQueueUpdate, playingItem, playSyncPlayItem]); @@ -1010,6 +1010,11 @@ const AppContent = (props) => { // one the viewer picked by hand from the episode browser passes `resume` so an episode they // were part way through carries on from where it stopped. const handlePlayNext = useCallback((item, options) => { + // In a group the group moves on, and its queue update brings this player along. + if (isSyncPlayInGroup && item.MediaType !== 'Audio' && item.Type !== 'TvChannel') { + syncPlayMoveGroupTo(item.Id); + return; + } setPlayingItem(item); setPlaybackOptions(prev => { if (prev?.audioPlaylist?.some(t => t.Id === item.Id)) { @@ -1024,7 +1029,7 @@ const AppContent = (props) => { return null; }); setIsResume(options?.resume === true); - }, []); + }, [isSyncPlayInGroup, syncPlayMoveGroupTo]); const handlePlayerEnd = useCallback(() => { // Episodes played on from inside the player (next episode, the episode browser) leave the diff --git a/packages/app/src/context/SyncPlayContext.js b/packages/app/src/context/SyncPlayContext.js index edd519cdf..68aeead5d 100644 --- a/packages/app/src/context/SyncPlayContext.js +++ b/packages/app/src/context/SyncPlayContext.js @@ -7,7 +7,11 @@ import {api} from '../services/jellyfinApi'; const SyncPlayContext = createContext(null); // Queue entries carry the GUID with dashes, item DTOs without them. -const sameItemId = (a, b) => !!a && !!b && String(a).replace(/-/g, '').toLowerCase() === String(b).replace(/-/g, '').toLowerCase(); +const sameItemId = (a, b) => !!a && !!b && syncPlayService.itemIdKey(a) === syncPlayService.itemIdKey(b); + +// Only a new item, or one other than what's playing, moves the player. With nothing playing it stays put. +export const followsQueueUpdate = (update, playingItemId) => + update.startsPlayback || (Boolean(playingItemId) && !sameItemId(playingItemId, update.item.Id)); export const useSyncPlay = () => useContext(SyncPlayContext); @@ -156,6 +160,17 @@ export const SyncPlayProvider = ({children}) => { // time since. const getGroupPositionTicks = useCallback(() => syncPlayService.getGroupPositionTicks(), []); + // Moves the whole group to an item, by next or previous when it's the neighbouring entry. + const moveGroupTo = useCallback((itemId) => { + const entries = playQueue?.Playlist || []; + const index = playQueue?.PlayingItemIndex ?? -1; + const entry = entries.find((e) => sameItemId(e.ItemId, itemId)); + if (!entry) syncPlayService.setNewQueue([itemId]); + else if (index >= 0 && entry === entries[index + 1]) syncPlayService.nextItem(); + else if (index > 0 && entry === entries[index - 1]) syncPlayService.previousItem(); + else syncPlayService.setPlaylistItem(entry.PlaylistItemId); + }, [playQueue]); + const openDialog = useCallback(() => { if (settings.syncplayEnabled === false) return; setIsDialogOpen(true); @@ -177,6 +192,7 @@ export const SyncPlayProvider = ({children}) => { playQueueUpdate, refreshGroups, getGroupPositionTicks, + moveGroupTo, getGroup: syncPlayService.getGroup, createGroup: handleCreateGroup, joinGroup: handleJoinGroup, diff --git a/packages/app/src/context/SyncPlayContext.test.js b/packages/app/src/context/SyncPlayContext.test.js new file mode 100644 index 000000000..94f05df74 --- /dev/null +++ b/packages/app/src/context/SyncPlayContext.test.js @@ -0,0 +1,88 @@ +import {act, renderHook} from '@testing-library/react'; + +import {SyncPlayProvider, followsQueueUpdate, useSyncPlay} from './SyncPlayContext'; + +// The CLI ships a second copy of React, so the components' JSX goes through the copy under test. +jest.mock('react/jsx-dev-runtime', () => { + const React = require('react'); + return { + jsxDEV: (type, {children, ...props}, key) => { + const config = key === undefined ? props : {...props, key}; + return children === undefined ? React.createElement(type, config) : React.createElement(type, config, children); + } + }; +}); + +let mockEmit; +const mockQueueRequests = {nextItem: jest.fn(), previousItem: jest.fn(), setPlaylistItem: jest.fn(), setNewQueue: jest.fn()}; +jest.mock('../services/syncPlay', () => ({ + itemIdKey: jest.requireActual('../services/syncPlay').itemIdKey, + start: () => {}, + stop: () => {}, + addListener: (listener) => { + mockEmit = listener; + return () => {}; + }, + nextItem: (...args) => mockQueueRequests.nextItem(...args), + previousItem: (...args) => mockQueueRequests.previousItem(...args), + setPlaylistItem: (...args) => mockQueueRequests.setPlaylistItem(...args), + setNewQueue: (...args) => mockQueueRequests.setNewQueue(...args) +})); +jest.mock('./AuthContext', () => ({useAuth: () => ({isAuthenticated: true, serverType: 'jellyfin'})})); +jest.mock('./SettingsContext', () => ({useSettings: () => ({settings: {}})})); +jest.mock('../services/jellyfinApi', () => ({api: {getItem: () => new Promise(() => {})}})); + +const renderSyncPlay = () => renderHook(() => useSyncPlay(), {wrapper: SyncPlayProvider}); + +describe('followsQueueUpdate', () => { + const update = (startsPlayback) => ({item: {Id: 'aaaaaaaa-0000-0000-0000-000000000001'}, startsPlayback}); + + test('plays a new item whatever this device is doing', () => { + expect(followsQueueUpdate(update(true), null)).toBe(true); + expect(followsQueueUpdate(update(true), 'aaaaaaaa000000000000000000000001')).toBe(true); + }); + + test('a reorder or a removal leaves this device alone when nothing is playing', () => { + expect(followsQueueUpdate(update(false), null)).toBe(false); + expect(followsQueueUpdate(update(false), undefined)).toBe(false); + }); + + test('nor when the player already shows that item, however its id is spelled', () => { + expect(followsQueueUpdate(update(false), 'AAAAAAAA000000000000000000000001')).toBe(false); + }); + + test('but moves a player that shows something else', () => { + expect(followsQueueUpdate(update(false), 'bbbbbbbb000000000000000000000002')).toBe(true); + }); +}); + +describe('moveGroupTo', () => { + const entry = (n) => ({PlaylistItemId: `p${n}`, ItemId: `aaaaaaaa00000000000000000000000${n}`}); + const queue = (playing) => ({PlayingItemIndex: playing, Playlist: [entry(1), entry(2), entry(3), entry(4)]}); + const inGroupAt = (playing) => { + const {result} = renderSyncPlay(); + act(() => mockEmit('playQueue', queue(playing))); + return result; + }; + + beforeEach(() => Object.values(mockQueueRequests).forEach((fn) => fn.mockReset())); + + test('asks the group for its next or previous entry, however the id is spelled', () => { + const result = inGroupAt(1); + result.current.moveGroupTo('aaaaaaaa-0000-0000-0000-000000000003'); + expect(mockQueueRequests.nextItem).toHaveBeenCalledTimes(1); + result.current.moveGroupTo('aaaaaaaa000000000000000000000001'); + expect(mockQueueRequests.previousItem).toHaveBeenCalledTimes(1); + }); + + test('jumps to an entry further along', () => { + inGroupAt(1).current.moveGroupTo('aaaaaaaa000000000000000000000004'); + expect(mockQueueRequests.setPlaylistItem).toHaveBeenCalledWith('p4'); + }); + + test('starts a new queue for anything not in it, even from the last entry', () => { + inGroupAt(3).current.moveGroupTo('bbbbbbbb000000000000000000000009'); + expect(mockQueueRequests.setNewQueue).toHaveBeenCalledWith(['bbbbbbbb000000000000000000000009']); + expect(mockQueueRequests.nextItem).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app/src/services/syncPlay.js b/packages/app/src/services/syncPlay.js index 5d6b4669c..689f79b32 100644 --- a/packages/app/src/services/syncPlay.js +++ b/packages/app/src/services/syncPlay.js @@ -443,8 +443,17 @@ const handleGroupUpdate = (data) => { } }; +// Ids come with and without dashes. +export const itemIdKey = (id) => String(id || '').replace(/-/g, '').toLowerCase(); + const handlePlaybackCommand = (data) => { if (!data) return; + // A command for another queue entry would land on the wrong item. An all-zero id names none. + const commandItem = itemIdKey(data.PlaylistItemId); + if (/[1-9a-f]/.test(commandItem) && currentPlaylistItemId && commandItem !== itemIdKey(currentPlaylistItemId)) { + syncLog('[SyncPlay] dropped', data.Command, 'for item', data.PlaylistItemId); + return; + } syncLog('[SyncPlay] command', data.Command, 'at', data.PositionTicks != null ? data.PositionTicks / 10000000 : null, 's, when', data.When, 'delay', getDelayToWhen(data.When), 'ms'); // eslint-disable-line no-use-before-define // A command says where the group was at its own time, which for a late // one or an echo is not now. diff --git a/packages/app/src/services/syncPlay.test.js b/packages/app/src/services/syncPlay.test.js index fe3f4c1ab..cdb604f53 100644 --- a/packages/app/src/services/syncPlay.test.js +++ b/packages/app/src/services/syncPlay.test.js @@ -222,6 +222,17 @@ describe('SyncPlay group position', () => { expect(service.getGroupPositionTicks()).toBe(frozen); }); + test('a command for another queue entry is dropped', () => { + const itemCommand = (PlaylistItemId) => socket.onmessage({ + data: JSON.stringify({MessageType: 'SyncPlayCommand', Data: {Command: 'Seek', PositionTicks: 300 * SECOND, When: new Date().toISOString(), PlaylistItemId}}) + }); + groupUpdate('PlayQueue', {Playlist: [{PlaylistItemId: 'bbbb-0002'}], PlayingItemIndex: 0, StartPositionTicks: 0, IsPlaying: false}); + itemCommand('aaaa-0001'); + expect(service.getGroupPositionTicks()).toBe(0); + itemCommand('BBBB0002'); + expect(service.getGroupPositionTicks()).toBe(300 * SECOND); + }); + test('is forgotten on leaving', () => { command('Unpause', 100 * SECOND, Date.now()); groupUpdate('GroupLeft', {GroupId: 'g1'});