Account for decimals in unread chapter count

[crxssed7]

Fixes #298

I was hesitating on whether I should open a PR for this as I think the behaviour is intentional (however I don't really agree with the current implementation).

I tracked it down to the markChapter method which at first seemed like it was over engineering something really simple, but I think the original dev who wrote this intended for chapters with decimal points to be consolidated as one chapter (e.g 41.1, 41.2, 41.3, 41.4 is just chapter 41). I think this is a pretty odd way of looking at it and is very confusing at first glance - it took me a second to figure out why this was implemented the way that it is. As it relies heavily on chapter numbers being present, it means that any chapter with no chapter number is completely ignored (as seen in the Moon Knight screenshot in the original issue).

Feel free to close this PR if you think the current implementation is correct.

[xgi]

So the current logic you see is mainly for 3 reasons:

• The chapterList passed to getNumberUnreadChapters can contain chapters from multiple languages (depending on the user's settings). This is the main reason for the "highestRead" logic -- sometimes only one language will have recent chapters, and oftentimes a chapter will be available in both languages. I think showing the user a "2" if there's one new chapter available in 2 languages is almost certainly wrong.
• The chapterList can also contain multiple chapter objects with the same number and language (e.g. from different scanlation groups). Showing "2" in this case would probably also be unexpected.
• Dealing with unmarked chapters. If there are chapters 1-100 available, and the user has only read the 90th, I would expect to show "10". The client makes it easy to mark previous chapters as read, but I'm not sure many people do that, especially when adding a series they started elsewhere. I think it makes sense to only consider chapters above the highest read one.

This is why I chose to work with chapterNumbers instead of chapter objects. Of course, it introduces 2 other issues:

• Dealing with gaps. If chapter 1 is read and the only other chapter is 10, currently we would show the user "9". That's probably wrong.
• Rounding. If chapter 40 is read and there are also 40.1, 40.2, 40.3, we currently show "1". That's probably wrong too.

I think it's worth addressing those points, but it's not trivial to consider all of these (and wouldn't be as simple as either of our implementations). Let me know if you disagree!

[crxssed7]

So the current logic you see is mainly for 3 reasons:

* The `chapterList` passed to `getNumberUnreadChapters` can contain chapters from multiple languages (depending on the user's settings). This is the main reason for the "highestRead" logic -- sometimes only one language will have recent chapters, and oftentimes a chapter will be available in both languages. I think showing the user a "2" if there's one new chapter available in 2 languages is almost certainly wrong.

* The `chapterList` can also contain multiple chapter objects with the same number and language (e.g. from different scanlation groups). Showing "2" in this case would probably also be unexpected.

* Dealing with unmarked chapters. If there are chapters 1-100 available, and the user has only read the 90th, I would expect to show "10". The client makes it easy to mark previous chapters as read, but I'm not sure many people do that, especially when adding a series they started elsewhere. I think it makes sense to only consider chapters above the highest read one.

This is why I chose to work with chapterNumbers instead of chapter objects. Of course, it introduces 2 other issues:

* Dealing with gaps. If chapter 1 is read and the only other chapter is 10, currently we would show the user "9". That's probably wrong.

* Rounding. If chapter 40 is read and there are also 40.1, 40.2, 40.3, we currently show "1". That's probably wrong too.

I think it's worth addressing those points, but it's not trivial to consider all of these (and wouldn't be as simple as either of our implementations). Let me know if you disagree!

That makes a lot more sense now with context 👍🏾

I agree with all your considerations - sounds like a tricky situation whichever way you look at it. When I get a chance (most likely this weekend) I'll have a go at updating the implementation.

[crxssed7]

This should be ready for a re-review. I've tried my best to follow all the considerations but with there being so many it isn't perfect. The code has also become a lot more complicated.

Trade paperback comics on ReadComicOnline will show the unread count inverted (basically the read count) which is completely wrong. This is because there are no chapter numbers present so we can't exactly sort it to get the absolute chapter number. I'm planning on opening a PR on Tiyo that'll hopefully address this.

[xgi]

Left minor comments but I'm OK with merging. Thank you!

[xgi]

nit: you could avoid needing to increment both here and only set absoluteNumber after this block (since it's not used yet)

[crxssed7]

Ahh yes, didn't catch that

[xgi]

The [chapter] assignment here looks pretty weird, it seems to work but I don't really understand why. I think this section is equivalent to just:

let chapter = groupedChapters.find(chap => chap.read) || groupedChapters[0];

[crxssed7]

The reason I did this was because the linter kept complaining at me, so I assumed it was because of the project level linter config 😅

In hindsight it was probably just my VSCode extension 🤷🏾