From 8ad727d87023fa0455aa95249ba973e9d8a14191 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 19 Sep 2026 10:36:11 +0200 Subject: [PATCH] Fix watched-state sync so members' Next Up and Continue Watching follow Synced rows only ever had the Played flag set. Jellyfin computes Next Up from LastPlayedDate on the member's own row and Continue Watching from the resume position, so a member watching through the shared account got the tick on each episode but their Next Up never advanced. Members are now written the way BaseItem.MarkPlayed/MarkUnplayed write: date and position included. Rows the old version ticked without a date are repaired the next time the item syncs. PlaybackFinished was also treated as an unwatched toggle. Jellyfin raises it on every stop, not just completion (and on 10.11 PlaybackStart resets Played to false first), so a stop halfway through on the shared account cleared members' own watched state whenever Sync unwatched was on. Playback-derived reasons (PlaybackFinished, PlaybackProgress, UpdateUserData) now only ever mirror "watched"; TogglePlayed and Import remain explicit and mirror either way. PlaybackProgress is acted on so the tick lands as soon as the completion threshold is crossed, including for clients that never report a stop. Co-Authored-By: Claude Opus 5 (1M context) --- .../WatchedStateSyncTests.cs | 139 ++++++++++++++++-- .../Services/WatchedStateSyncService.cs | 84 +++++++++-- README.md | 18 ++- 3 files changed, 214 insertions(+), 27 deletions(-) diff --git a/Jellyfin.Plugin.WatchedTogether.Tests/WatchedStateSyncTests.cs b/Jellyfin.Plugin.WatchedTogether.Tests/WatchedStateSyncTests.cs index e6589ec..fe74412 100644 --- a/Jellyfin.Plugin.WatchedTogether.Tests/WatchedStateSyncTests.cs +++ b/Jellyfin.Plugin.WatchedTogether.Tests/WatchedStateSyncTests.cs @@ -26,7 +26,7 @@ public class WatchedStateSyncTests public Mock Groups { get; } = new(); - public List<(User Member, bool Played, int PlayCount)> Saves { get; } = new(); + public List Saves { get; } = new(); public WatchedStateSyncService Service { get; private set; } = null!; @@ -34,7 +34,9 @@ public class WatchedStateSyncTests SharedGroup? group, IReadOnlyList members, bool memberAlreadyPlayed = false, - int memberPlayCount = 0) + int memberPlayCount = 0, + DateTime? memberLastPlayedDate = null, + long memberPositionTicks = 0) { var h = new Harness(); @@ -46,7 +48,9 @@ public class WatchedStateSyncTests { Key = "k", Played = memberAlreadyPlayed, - PlayCount = memberPlayCount + PlayCount = memberPlayCount, + LastPlayedDate = memberLastPlayedDate, + PlaybackPositionTicks = memberPositionTicks }); h.UserData.Setup(m => m.SaveUserData( @@ -56,7 +60,8 @@ public class WatchedStateSyncTests It.IsAny(), It.IsAny())) .Callback( - (u, _, d, _, _) => h.Saves.Add((u, d.Played, d.PlayCount))); + (u, _, d, _, _) => h.Saves.Add( + new Save(u, d.Played, d.PlayCount, d.LastPlayedDate, d.PlaybackPositionTicks))); h.Service = new WatchedStateSyncService( h.UserData.Object, @@ -70,7 +75,7 @@ public class WatchedStateSyncTests /// /// Raises UserDataSaved as the server would, by starting the service so it subscribes. /// - public void Raise(Guid userId, bool played, UserDataSaveReason reason) + public void Raise(Guid userId, bool played, UserDataSaveReason reason, DateTime? lastPlayedDate = null) { Service.StartAsync(CancellationToken.None).GetAwaiter().GetResult(); @@ -80,7 +85,7 @@ public class WatchedStateSyncTests { UserId = userId, Item = new Folder { Name = "Some Item" }, - UserData = new UserItemData { Key = "k", Played = played }, + UserData = new UserItemData { Key = "k", Played = played, LastPlayedDate = lastPlayedDate }, SaveReason = reason }); @@ -88,6 +93,14 @@ public class WatchedStateSyncTests } } + /// + /// A snapshot of one member row as it was handed to SaveUserData. The service reuses the + /// object it got from GetUserData, so the fields are copied rather than the reference kept. + /// + private sealed record Save(User Member, bool Played, int PlayCount, DateTime? LastPlayedDate, long PositionTicks); + + private static readonly DateTime SharedWatchedAt = new(2026, 9, 18, 21, 30, 0, DateTimeKind.Utc); + private static User MakeUser(string name) => new(name, "Prov", "ResetProv"); private static SharedGroup MakeGroup(bool syncUnwatched = true, bool syncPlayCount = false) @@ -126,11 +139,10 @@ public class WatchedStateSyncTests [Theory] [InlineData(UserDataSaveReason.PlaybackStart)] - [InlineData(UserDataSaveReason.PlaybackProgress)] [InlineData(UserDataSaveReason.UpdateUserRating)] public void IrrelevantSaveReasons_AreIgnored(UserDataSaveReason reason) { - // UserDataSaved fires constantly during playback; only watched-state changes matter. + // Neither of these ever carries a watched-state change, whatever the flag says. var h = Harness.Create(MakeGroup(), [MakeUser("alice")]); h.Raise(SharedId, true, reason); @@ -138,6 +150,100 @@ public class WatchedStateSyncTests Assert.Empty(h.Saves); } + [Theory] + [InlineData(UserDataSaveReason.PlaybackFinished)] + [InlineData(UserDataSaveReason.PlaybackProgress)] + [InlineData(UserDataSaveReason.UpdateUserData)] + public void PlaybackDerivedReasons_SyncWatched(UserDataSaveReason reason) + { + // Progress crosses the completion threshold before the stop arrives, and some clients + // never send a stop at all, so the tick has to land on the progress tick too. + var h = Harness.Create(MakeGroup(), [MakeUser("alice")]); + + h.Raise(SharedId, true, reason); + + Assert.Single(h.Saves); + Assert.True(h.Saves[0].Played); + } + + [Theory] + [InlineData(UserDataSaveReason.PlaybackFinished)] + [InlineData(UserDataSaveReason.PlaybackProgress)] + [InlineData(UserDataSaveReason.UpdateUserData)] + public void PlaybackDerivedReasons_NeverSyncUnwatched(UserDataSaveReason reason) + { + // PlaybackFinished fires on every stop, not just on completion, and on 10.11 PlaybackStart + // resets Played to false first. Stopping halfway through on the shared account must not + // clear what a member watched on their own - even with Sync unwatched on. + var h = Harness.Create(MakeGroup(syncUnwatched: true), [MakeUser("alice")], memberAlreadyPlayed: true); + + h.Raise(SharedId, false, reason); + + Assert.Empty(h.Saves); + } + + [Fact] + public void Played_WritesWhatMarkPlayedWrites() + { + // Next Up is driven by LastPlayedDate on the member's own row, and a stale resume position + // would keep the item in Continue Watching, so the tick alone is not enough. + var h = Harness.Create(MakeGroup(), [MakeUser("alice")], memberPositionTicks: 12_345); + + h.Raise(SharedId, true, UserDataSaveReason.PlaybackFinished, SharedWatchedAt); + + var save = Assert.Single(h.Saves); + Assert.True(save.Played); + Assert.Equal(SharedWatchedAt, save.LastPlayedDate); + Assert.Equal(0, save.PositionTicks); + } + + [Fact] + public void Played_FallsBackToNowWhenSharedAccountHasNoDate() + { + var before = DateTime.UtcNow; + var h = Harness.Create(MakeGroup(), [MakeUser("alice")]); + + h.Raise(SharedId, true, UserDataSaveReason.TogglePlayed); + + var save = Assert.Single(h.Saves); + Assert.NotNull(save.LastPlayedDate); + Assert.InRange(save.LastPlayedDate!.Value, before, DateTime.UtcNow); + } + + [Fact] + public void Played_RepairsAnAlreadyTickedRowThatHasNoDate() + { + // Rows synced by earlier versions have the tick but no date. They are not "already in + // sync": without a date the member's Next Up never moves. + var h = Harness.Create(MakeGroup(), [MakeUser("alice")], memberAlreadyPlayed: true, memberLastPlayedDate: null); + + h.Raise(SharedId, true, UserDataSaveReason.PlaybackFinished, SharedWatchedAt); + + var save = Assert.Single(h.Saves); + Assert.True(save.Played); + Assert.Equal(SharedWatchedAt, save.LastPlayedDate); + } + + [Fact] + public void Unwatched_WritesWhatMarkUnplayedWrites() + { + var h = Harness.Create( + MakeGroup(syncUnwatched: true), + [MakeUser("alice")], + memberAlreadyPlayed: true, + memberPlayCount: 3, + memberLastPlayedDate: SharedWatchedAt, + memberPositionTicks: 999); + + h.Raise(SharedId, false, UserDataSaveReason.TogglePlayed); + + var save = Assert.Single(h.Saves); + Assert.False(save.Played); + Assert.Null(save.LastPlayedDate); + Assert.Equal(0, save.PositionTicks); + Assert.Equal(3, save.PlayCount); + } + [Fact] public void Unwatched_PropagatesWhenSyncUnwatchedEnabled() { @@ -159,13 +265,20 @@ public class WatchedStateSyncTests Assert.Empty(h.Saves); } - [Fact] - public void RedundantWrites_AreSuppressed() + [Theory] + [InlineData(UserDataSaveReason.PlaybackFinished)] + [InlineData(UserDataSaveReason.PlaybackProgress)] + public void RedundantWrites_AreSuppressed(UserDataSaveReason reason) { - // The member already matches the shared account, so there is nothing to write. - var h = Harness.Create(MakeGroup(), [MakeUser("alice")], memberAlreadyPlayed: true); + // The member already matches the shared account, so there is nothing to write. This is + // what keeps the stream of progress ticks after the completion threshold write-free. + var h = Harness.Create( + MakeGroup(), + [MakeUser("alice")], + memberAlreadyPlayed: true, + memberLastPlayedDate: SharedWatchedAt); - h.Raise(SharedId, true, UserDataSaveReason.PlaybackFinished); + h.Raise(SharedId, true, reason); Assert.Empty(h.Saves); } diff --git a/Jellyfin.Plugin.WatchedTogether/Services/WatchedStateSyncService.cs b/Jellyfin.Plugin.WatchedTogether/Services/WatchedStateSyncService.cs index 7e3108e..33e4d24 100644 --- a/Jellyfin.Plugin.WatchedTogether/Services/WatchedStateSyncService.cs +++ b/Jellyfin.Plugin.WatchedTogether/Services/WatchedStateSyncService.cs @@ -70,9 +70,17 @@ public sealed class WatchedStateSyncService : IHostedService, IDisposable /// Mirrors a shared account's played state onto its members. /// /// + /// /// No loop guard is needed. Writing to a member raises this event again with that member's id, /// which is not a shared account id, so the handler returns immediately. The /// Played equality check below suppresses redundant writes on top of that. + /// + /// + /// A member is written the same way Jellyfin's own BaseItem.MarkPlayed and + /// MarkUnplayed write, not just the Played flag. Next Up is driven entirely by + /// LastPlayedDate on the member's own row, so a tick without a date leaves the member's + /// Next Up stuck; and a stale resume position would keep the item in Continue Watching. + /// /// private void OnUserDataSaved(object? sender, UserDataSaveEventArgs e) { @@ -81,11 +89,8 @@ public sealed class WatchedStateSyncService : IHostedService, IDisposable return; } - // UserDataSaved fires constantly during playback (progress ticks); only act on the reasons - // that actually represent a change in watched state. - if (e.SaveReason is not (UserDataSaveReason.PlaybackFinished - or UserDataSaveReason.TogglePlayed - or UserDataSaveReason.Import)) + var played = e.UserData.Played; + if (!IsWatchedStateChange(e.SaveReason, played)) { return; } @@ -96,7 +101,6 @@ public sealed class WatchedStateSyncService : IHostedService, IDisposable return; } - var played = e.UserData.Played; if (!played && !group.SyncUnwatched) { return; @@ -107,16 +111,36 @@ public sealed class WatchedStateSyncService : IHostedService, IDisposable try { var data = _userDataManager.GetUserData(member, e.Item); - if (data is null || data.Played == played) + if (data is null) { continue; } - data.Played = played; - - if (group.SyncPlayCount && played && data.PlayCount < 1) + // Rows written by earlier versions carry the tick but no date; give those a date + // the next time the item syncs rather than skipping them as already in sync. + if (data.Played == played && (!played || data.LastPlayedDate.HasValue)) { - data.PlayCount = 1; + continue; + } + + if (played) + { + data.Played = true; + data.PlaybackPositionTicks = 0; + data.LastPlayedDate = e.UserData.LastPlayedDate ?? DateTime.UtcNow; + + if (group.SyncPlayCount && data.PlayCount < 1) + { + data.PlayCount = 1; + } + } + else + { + // Same as Jellyfin's MarkUnplayed, except the play count is left alone: it is + // documented as never decreasing. + data.Played = false; + data.PlaybackPositionTicks = 0; + data.LastPlayedDate = null; } _userDataManager.SaveUserData( @@ -144,4 +168,42 @@ public sealed class WatchedStateSyncService : IHostedService, IDisposable } } } + + /// + /// Decides whether a save represents a change in watched state worth mirroring. + /// + /// + /// + /// and + /// are explicit: someone set the flag, so whatever it says is mirrored, unwatched included. + /// + /// + /// The rest only ever mean "watched" when is true. Jellyfin raises + /// on every stop, not just on completion, so + /// a stop before the completion threshold leaves Played false without anybody having + /// marked anything unwatched - and on 10.11 PlaybackStart resets it to false as well. + /// Mirroring that would clear members' own watched state. Likewise + /// is a partial update (a favourite toggle + /// arrives with the same reason) where a false flag need not mean a change at all. + /// + /// + /// is included so the tick lands as soon as + /// the completion threshold is crossed, and still lands for clients that never report a stop. + /// The equality check in the handler keeps the remaining progress ticks write-free. + /// + /// + /// Why the user data was saved. + /// The played flag on the saved user data. + /// true if the save should be mirrored to members. + private static bool IsWatchedStateChange(UserDataSaveReason reason, bool played) + { + return reason switch + { + UserDataSaveReason.TogglePlayed or UserDataSaveReason.Import => true, + UserDataSaveReason.PlaybackFinished + or UserDataSaveReason.PlaybackProgress + or UserDataSaveReason.UpdateUserData => played, + _ => false, + }; + } } diff --git a/README.md b/README.md index 72deef2..cc036dd 100644 --- a/README.md +++ b/README.md @@ -141,9 +141,21 @@ Two consequences worth knowing: ### Watched-state sync -The plugin subscribes to `UserDataSaved` and filters tightly: only `PlaybackFinished`, -`TogglePlayed` and `Import` are acted on, so the constant stream of progress updates during playback -is ignored. +The plugin subscribes to `UserDataSaved` and decides per save reason what it means: + +- `TogglePlayed` and `Import` are explicit - someone set the flag - so whatever it says is mirrored, + unwatched included (subject to *Sync unwatched*). +- `PlaybackFinished`, `PlaybackProgress` and `UpdateUserData` only ever mirror *watched*. Jellyfin + raises `PlaybackFinished` on every stop, not just on completion, so a stop halfway through leaves + `Played` false without anyone having marked anything unwatched; propagating that would wipe what a + member watched on their own. Acting on progress too means the tick lands the moment the completion + threshold is crossed, and still lands for clients that never report a stop. +- Everything else (`PlaybackStart`, `UpdateUserRating`) is ignored. + +A member is written the way Jellyfin's own *mark played* writes - `Played`, `LastPlayedDate` and a +cleared resume position - not just the flag. *Next Up* is computed from `LastPlayedDate` on the +member's own row, so a bare tick would leave their Next Up stuck on the wrong episode. Members whose +row already matches are skipped, which keeps the progress ticks after the threshold write-free. No feedback loop is possible: writing to a member raises the event again with *that member's* ID, which is not a shared account, so the handler stops immediately.