xgi / tiyo

MIT License
10 stars 3 forks source link

Account for TPB chapter numbers in ReadComicOnline #7

Closed crxssed7 closed 4 months ago

crxssed7 commented 5 months ago

Without these chapter numbers, certain functionality breaks in the Houdoku app (e.g. unread chapter count). It's not the best solution but I wasn't sure what else to do. Let me know if there could be a better solution.

I've not been able to test it properly as I weren't sure how to get it loaded into Houdoku (couldn't find any instructions unless I've missed something).

xgi commented 5 months ago

I've not been able to test it properly as I weren't sure how to get it loaded into Houdoku (couldn't find any instructions unless I've missed something).

There's a really hacky way:

pnpm exec nx run core:build
./tools/scripts/manual-install.sh /path/to/houdoku # e.g. /home/user/.config/Electron
# might require restarting the client to reload the extension properly

I had unit tests in the old repo but haven't gotten around to adding the setup here yet.

Regardless this seems to work as the code describes, but I had one question: you're always prefixing the "TPB" chapters with 0, but I think they can have multiple?

I don't know much about this site or western comic numbering, so correct me if I'm wrong!

crxssed7 commented 5 months ago

I've not been able to test it properly as I weren't sure how to get it loaded into Houdoku (couldn't find any instructions unless I've missed something).

There's a really hacky way:

pnpm exec nx run core:build
./tools/scripts/manual-install.sh /path/to/houdoku # e.g. /home/user/.config/Electron
# might require restarting the client to reload the extension properly

I had unit tests in the old repo but haven't gotten around to adding the setup here yet.

Regardless this seems to work as the code describes, but I had one question: you're always prefixing the "TPB" chapters with 0, but I think they can have multiple?

* In this one - https://readcomiconline.li/Comic/Talli-Daughter-of-the-Moon - I think it would make more sense to be 1.1, 1.2, 2.1, 2.2, but your code gives 0.1, 0.1, 0.2, 0.2

* Same in https://readcomiconline.li/Comic/Moon-Knight-Epic-Collection

I don't know much about this site or western comic numbering, so correct me if I'm wrong!

Thanks, I'll give this a test when i get the chance.

I originally had it working the way you described with it accounting for each "Part" of the TPB. But considering that each part is basically the same chapter (I'm not sure why ReadComicOnline splits them up, they probably have their reasons) I settled on treating them as the same chapter. Let me know if you disagree with this, I'm happy to revert back to accounting for each part.

As for prefixing with a "0", in the ideal world it would just be 1.1, 1.2, 2.1, 2.2 (or without the decimals according to my solution) but annoyingly some comics have a mix of actual issues and TPB's: https://readcomiconline.li/Comic/Invincible - so I'm having to ensure they appear before the actual chapter numbers. It isn't perfect but webscraping rarely is 😅

I don't think there is a set standard for western comic numbering and is often all over the place. I've been having issues on a personal project with issue numbers and it's always a pain to deal with lol.

xgi commented 4 months ago

The main practical issue I think is that we can't refer to 2 subsequent chapters with the same chapterNumber. Houdoku wouldn't know whether it's the same chapter from a different scan, or a second part. So when users click "next chapter" it would get skipped.

I think my preference would be:

This would cause an overlap if e.g. Issue 7 and TPB 7 both exist, but the groupName should allow users to filter those versions, and when they click "next chapter" it should stay with the same group.

crxssed7 commented 4 months ago

The main practical issue I think is that we can't refer to 2 subsequent chapters with the same chapterNumber. Houdoku wouldn't know whether it's the same chapter from a different scan, or a second part. So when users click "next chapter" it would get skipped.

I think my preference would be:

  • Issue #7 -> chapterNumber=7 groupName=Issue
  • TPB 3 -> volumeNumber=3 chapterNumber=3 groupName=TPB
  • TPB 3 (part 2) -> volumeNumber=3 chapterNumber=3.2 groupName=TPB

This would cause an overlap if e.g. Issue 7 and TPB 7 both exist, but the groupName should allow users to filter those versions, and when they click "next chapter" it should stay with the same group.

Right yes this should work - I wasn't actually aware of the groupName attribute, I'll update the PR to account for this.

crxssed7 commented 4 months ago

This should be ready for a review now, I've tested it locally and it works: image image Thanks for the help on this.