jellyfin / jellyfin-plugin-trakt

https://jellyfin.org
MIT License
179 stars 32 forks source link

Wrong episodes marked as watched during sync from Trakt when series not ordered by Air Date #204

Open SenorSmartyPants opened 1 year ago

SenorSmartyPants commented 1 year ago

I have some series ordered by DVD or Production order on TMDB. I added another season to one of these series that I had partially watched history in Trakt.

2x01 DVD order is 2x04 aired in this example. Trakt correctly has 2x04 marked as watched on the website. But when JF pulls history from Trakt 2x04 DVD is marked as watched, not 2x01 DVD. JF has episode provider IDs, so I think the solution is the same as in #191 . Just check if the series is not aired order, and if so, do not use watched API, only history API.

pinging @h3llrais3r for opinion.

Thanks!

h3llrais3r commented 1 year ago

Hi, not all issues are solved with my fix. I still have other cases where not everything is perfectly synced (f.e. La case de papel).

I think the problem is still located here: image

It uses the history to find a match on episode and then reads the season number from it. Then it checks if it finds and season/episode in jellyfin that matches that. So in your case, it sees it as 2x04 on trakt and marks the 2x04 episode in jellyfin as watched. (Is there a possibility then to change another ordering? I'm using TVDB all the time, so I'm don't know the TMDB ordering in jellyfin)

Can you try to enable debug logging and run the import from trakt? Logs are important to understand what's happening. Please also share which show you are talking about.

SenorSmartyPants commented 1 year ago

I knew your work wouldn't already have solved my problem, but I think it is almost there. I will work on getting a debug log later.

So in your case, it sees it as 2x04 on trakt and marks the 2x04 episode in jellyfin as watched.

Yes

(Is there a possibility then to change another ordering? I'm using TVDB all the time, so I'm don't know the TMDB ordering in jellyfin)

Trakt uses the default TMDB order, which is aired order. I'm using a TMDB episode group (which is what they call alternate ordering) already. But the number of seasons and the number of episodes in the seasons are the same for most of my series. For example.

Star Trek Season 1 aired order Star Trek Season 1 production order

What I would like the plugin to do, is first check

if (Series.DisplayOrder != "originalAirDate" || Series.DisplayOrder != String.Empty) // not sure what the default value is
{ 
    // don't even call watched API, some SxE numbers won't match
    // use history API and match only using episode IDs, not season/episode numbers
}
h3llrais3r commented 1 year ago

I'm starting to think that it should even be better to only use the history api and no longer the watched api for matching the episodes. And when a match is found by indexer id, there is no need to check the season and episode number... But there should still be a fallback to season and number, because not everyone has the indexer id's linked to the episodes in jellyfin...

I tried with my fix to not touch too much of the existing code, but maybe we'll have to review the complete import from trakt for the episodes (but also keep the existing logic with the season and number as fallback)

SenorSmartyPants commented 1 year ago

IMO ids should be preferred over SxE. But perhaps history API is a fast call and easier on Trakt? For most users that have seasons/episodes that match Trakt it's not an issue.

But there should definitely be a way (or several) to say I prefer ids. Either an option in the plugin, or based on metadata about the series displayorder (or other fields?)

h3llrais3r commented 1 year ago

Yeah, that's why I tried to only include the fallback to history api when it could not find a match with regular season/episode. I'll think about it... because I still want to have my non matched episodes also imported correctly. But this probably requires some rewrite of the logic.

SenorSmartyPants commented 1 year ago

I'm starting to think that it should even be better to only use the history api and no longer the watched api for matching the episodes.

I think you might be right. I was just looking over the Trakt API and it does seem like History is the better data set for what the plugin is trying to do.

h3llrais3r commented 1 year ago

I have already been rewriting the logic a bit. I'm now using the history by default, and fallback to the old logic by season/number if no history could be found (but I actually think that this will never happen) So if the maintainer of this plugin agrees, we might switch completely to history api for episode import from trakt.

h3llrais3r commented 1 year ago

If you want, I can share the updated plugin so you can test? But I would advise to test it on a small test library... not your full library yet. 😉 Still testing myself as well.

SenorSmartyPants commented 1 year ago

I can test. I have a subset of my library copied for testing.

h3llrais3r commented 1 year ago

You want the DLL? Or I can already make the PR so you can compile yourself?

SenorSmartyPants commented 1 year ago

I've compiled it for my PRs before, but it's been months since I have.

h3llrais3r commented 1 year ago

It's like you want, PR is already made. If you want the DLL, here you go: Trakt.zip This is the compiled version of the PR I made: https://github.com/jellyfin/jellyfin-plugin-trakt/pull/205