TV-Rename / tvrename

Organise your TV & Movie videos with ease
http://www.tvrename.com
Other
300 stars 36 forks source link

v3 Issues #326

Closed SirSparkles closed 6 years ago

SirSparkles commented 6 years ago

Hi @JoeBiellik

Just testing / reviewing the v3 branch. looks good, but wanted to capture some observations/bugs/things to do. You probably know about them all, but just in case:

JoeBiellik commented 6 years ago

Thanks for having a look.

  1. You shouldn't be prompted to download/generate media center files for missing episodes, do you have a way for me to reproduce this?
  2. Renaming isn't implemented
  3. I can't reproduce this either.

I think overriding season options (split/merge/join ep etc) is the last major part v3 is missing, then it's just a lot of little features and niceties. If you want to help with code or patches it's appreciated.

SirSparkles commented 6 years ago

image

Just trying to add one show and see what it does to the existing library

Will step through the code and let you know what I find

JoeBiellik commented 6 years ago

Huh, that's strange. If I wipe all settings, add a show with the location set to an empty folder and scan, it correctly skips all the episode metadata files as the video files are missing.

If I then add a couple of video files (correctly named) into the directory and rescan, it picks up those files and prompts to create NFO/JPG for the found files:

image

After generating those files a second scan doesn't list anything to do for those files:

image

Maybe its a bug with some of the options you have set (custom name template)?

SirSparkles commented 6 years ago

Hi, Worked through a few things now. Mainly user error/github misunderstandings.

I'm happy to work on doing fixes as PRs to v3 for you to review - just not sure if you wan to keep working on the core architecture before I wade in?

SirSparkles commented 6 years ago

Just compared the series xml: v2-S02E01 - The North Remembers.nfo.txt v3-S02E01 - The North Remembers.nfo.txt

Many bug-fixes/improvements, but the only issue I see is tvshow.nfo is missing:

JoeBiellik commented 6 years ago

@MarkSummerville As I mentioned before the last big missing piece in v3 is overriding episode details - this includes multipart episodes and all the logic around processing episodes. So far I've been implementing features as needed hence ProcessedEpisode is still pretty basic.

I'll review your PR, my plan was to always split up the Kodi action for show and episode so thanks for that.

My plan moving forward was to get that episode processing in and then do a small refactor of the core models - I'm sure it can be cleaned up now more is implemented - and improve the linking between show, season and episode like you mention. After that, I think it's a good idea to try and get functional parity with v2 - making sure core processing behaves the same when it comes to moving/renaming etc. Hopefully, this can be done with unit tests so we quickly know if we break something going forwards. Finally, the code can be reviewed and optimised to fully take advantage of parallelism.

I welcome any code you can help with, I keep getting distracted by various little features but am working on the episode processing. If you wanted to start writing tests for the metadata files that would be great or if you just want to keep reviewing and fixing things it speeds up development.