jellyfin / jellyfin-plugin-trakt

https://jellyfin.org
MIT License
173 stars 33 forks source link

Show disappearing from Up Next when next episode is marked as watched by plugin #193

Closed foux closed 1 year ago

foux commented 1 year ago

Issue

If we have an episode of a show on Up Next, and that episode is marked as watched by the Trakt Plugin, the show will disapear completely from the Up Next section, instead of the next episode of the show to appear.

Expected result

When an episode in Up Next is marked as watched by the plugin, the next episode should appear in its place in the Up Next section

Possible workaround

None found. The only solution is to find the next episode in the library and view it, then the one after this one will appear again in Up Next.

blackwind commented 1 year ago

I believe this relates to what we've been talking about here:

https://github.com/shemanaev/jellyfin-plugin-media-cleaner/issues/6

Currently, the Trakt plugin isn't setting LastPlayedDate on items it pulls from Trakt, and in combination with Jellyfin's Settings > Display > Max days in "Next Up" setting, Jellyfin sees that the last watch of your show wasn't within the past 365 days (or whichever value you've set), so it disappears from Next Up. Since the value currently being set is null, no modification to that option will give you the result you desire. Only by the plugin setting LastPlayedDate when plays are downloaded from Trakt can this be resolved.

blackwind commented 1 year ago

I think I've tracked down the issue. The movie sync block contains the following code to set a LastPlayedDate, but the TV sync block doesn't:

https://github.com/jellyfin/jellyfin-plugin-trakt/blob/ad6e2ab54a44b9d0fbe7b33a4ac3e3fe21721db3/Trakt/ScheduledTasks/SyncFromTraktTask.cs#L255-L261

Adding a similar block at line 428 should resolve it. Can you confirm, @Shadowghost?

Shadowghost commented 1 year ago

The same logic is already applied to episodes:

https://github.com/jellyfin/jellyfin-plugin-trakt/blob/ad6e2ab54a44b9d0fbe7b33a4ac3e3fe21721db3/Trakt/ScheduledTasks/SyncFromTraktTask.cs#L234-L241

blackwind commented 1 year ago

The portion you quoted is from the movies block. I'm assuming that was a mistake. In any case, for some reason, the portion I quoted exists in the movies block but not the TV block, while the portion you quoted exists in both. I believe mine not being in the TV block as well is an oversight and the cause of this issue.

I can confirm that 100% of the TV plays in my library downloaded from Trakt using this plugin don't have LastPlayedDate, while 100% of the ones played through Jellyfin do. This is causing issues with other plugins and, as OP notes, Up Next. Mark the next episode of a show currently in your Up Next as watched on Trakt, run the sync task, and the show will disappear from Up Next.

blackwind commented 1 year ago

Tested the build artifact, and it appears I was wrong. Still broken.

I assumed since the portion I quoted was in the movie block, null < tLastPlayed would evaluate to true in C#, which I'm not familiar with. Upon further investigation, though, movies in my library marked with this plugin also don't contain a LastPlayedDate, so I guess it's entirely broken and always has been. Indeed, MSDN confirms null comparisons always return false, so there's never been a code path to set LastPlayedDate. It gets set if both the local and remote values are null, it gets set if both the local and remote values aren't null, but if local is null and remote isn't -- the most important scenario -- nothing happens. This needs to be corrected.

blackwind commented 1 year ago

Confirmed fixed now.