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.