Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/episode groups #20

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Tyr3al
Copy link
Contributor

@Tyr3al Tyr3al commented Jun 30, 2023

I recently came across the issue, that the standard TMDB episode order for a particular show was not matching the DVD/Bluray release one. Fortunately in many cases, there are additional "Episode Groups" available, that contain custom orders and can be accessed by via TMDBs API.

To resolve my issue, I added a new flag called "--episode-group" or short "-g". When this flag is enabled, the app first behaves like if the "--manual" flag was set - the user is asked for the right tv show. After that, a second dialog is started where the user can select the wanted episode group.

Next a mapping dictionary is created, to be able to fetch the right episode information with the groups numbers, e.g. 1x47 in an episode group is equivalent to 2x03 in TMDB's standard order. For this to work, the groups within the episode group have to have a parsable name like "Season 1" , "Volume 1" or Special (which is handles as Season 0).

I tried to work with the existing code and only modify the management of the episode numbers for the API lookups.

Changelog:

Updated

  • System.Text.Json 7.0.2 -> 7.0.3

Added

  • Episode Group Support (--episode-group / -g)

Changed

  • Updated codebase with new .NET Features
  • Updated codebase according to sonarqube recommendations

jamerst
jamerst previously approved these changes Jul 8, 2023
@jamerst jamerst dismissed their stale review July 8, 2023 13:32

Clicking wrong thing accidentally

var sanitizedGroupName = tvGroup.Name.Trim().ToLower();
int? seasonNumber = null;

var seasonMatch = EpisodeRegex().Match(sanitizedGroupName);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how reliable it will be to do the parsing this way, but is there really an alternative? This breaks in some cases (e.g. Season 04 Part 1 and 2 for this due to the duplicate season 4). It also breaks for any episode groups that aren't in English of course.

I don't think much can be done about this though, the subgroups don't have a set numerical season number so it all just depends on how the subgroups are named.

I'm not sure how common this is for episode groups on TMDB, I guess it would be fine to just add a disclaimer that it may not work in every case.

Handle cases where shows do not have any episode groups
Fix chosen episode group not being selected
Fix error when finding cover filename
Increment config version
@jamerst
Copy link
Owner

jamerst commented Jul 8, 2023

Thanks for the pull request, apologies for the delay in reviewing it. This is a bit of a complicated feature that I had considered in the past, but just couldn't be bothered to implement 😆. Props for doing it for me!

Just fixed a few minor bugs found during testing, I think I'm happy for it to be merged if you're happy with the episode group parsing caveats mentioned in a previous comment? It would probably be a good idea to handle errors with duplicate seasons more gracefully if possible, currently it throws an exception due to the duplicate dictionary key.

@Tyr3al
Copy link
Contributor Author

Tyr3al commented Jul 13, 2023

Thanks for looking into it!

Yes you are right, the error handling could be better. I’ll get to it, as soon as i have time ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants