search

jellyfin / jellyfin

The Free Software Media System
https://jellyfin.org
GNU General Public License v2.0
30.97k stars 2.86k forks source link

fix: keep current season name if no override exists #11674

Closed revam closed 2 weeks ago

revam commented 2 weeks ago

Changes TL;DR: Keep the season name if the no override exists for the season.

Longer version: So my plugin uses custom season names — which have worked fine throughout all of 10.8's lifecycle — but after the new NFO named season parsing was added in 10.9 (21 months ago no less), then the support for custom season names provided by remote metadata plugins partially broke as a result. This simple change fixes the issue by keeping the current season name for the existing seasons but allows the NFO season names to override the name if needed/desired, thus not breaking the support for custom season names provided by remote metadata plugins that we've loved Jellyfin for so far.

Also, I did the digging and debugging to fix this before checking for existing issues, and also doesn't think the other PR addressed the root issue here, so I decided to submit this PR since I believe it fixes it.

Issues https://github.com/jellyfin/jellyfin/issues/11655 ← Fixes this. https://github.com/jellyfin/jellyfin/issues/11656 ← Fixes this. https://github.com/jellyfin/jellyfin/pull/11647 ← I felt this didn't address the issue I was having, as I've been debugging Jellyfin to find out why it changed it back, and saw that PR didn't address that issue.

gnattu commented 2 weeks ago

I don't think your change will fix the problem here either, as you are not addressing the root cause. #11647 is doing the correct thing.

Your code will pass in a null or empty string and still get the season name reset because you are not addressing the root cause: the current season names are not present in the series.SeasonNames at the time of scanning.

In fact, the line you changed won't be executed if that season has a custom season name in the first place. If that line is ever executed, it means the season name is lost and needs to be fixed by #11647.

revam commented 2 weeks ago

image image

revam commented 2 weeks ago

The fact that they're not in the dict is the issue. it's resetting it because they're not in the dict.

revam commented 2 weeks ago

In fact, the line you changed won't be executed if that season has a custom season name in the first place. If that line is ever executed, it means the season name is lost and needs to be fixed by https://github.com/jellyfin/jellyfin/pull/11647.

I don't know if this is intended behavior or not, but it will be reset to "Season X" if a series NFO doesn't contain a name for the season even if a custom name was previously set by a remote metadata provider. If that is the intended behavior, then I will rest my case here and close this PR.

Shadowghost commented 2 weeks ago

No that's not intended, I'm still debugging this use case. Maybe we just need to combine both PRs, need to check the implications

Shadowghost commented 2 weeks ago

@revam since I can't debug it with the shoko plugin, would you mind checking my updated PR?

revam commented 2 weeks ago

I'll test the other pr later today when i have some free time. @Shadowghost

revam commented 2 weeks ago

@Shadowghost That seems to have done the trick for my use case at least. So should I close this PR then?

image

revam commented 2 weeks ago

I'll just close it.