tachiyomi
tachiyomi copied to clipboard
tachiyomiorg
Remove unnecessary type guards to enable better tracker updating, addresses tracking offline reading concerns
Closes #2905 Closes #2397 Closes #1626
Rather than doing something like queuing tracker changes when offline, being able to sync the library so that trackers are updated based on locally read chapters is even better.
This feature is basically already implemented, but there's some unnecessary checks going on that prevent it from actually working.
The in-app setting is:
Settings => Library => Automatically update trackers
The syncChapersWitTrackServiceTwoWay method doesn't require an EnhancedTrackService parameter, so there's no reason to be guarding.
https://github.com/tachiyomiorg/tachiyomi/blob/ff2a4e69526a2edfa5f3bd7570c09204e5ebb9f9/app/src/main/java/eu/kanade/tachiyomi/util/chapter/ChapterTrackSync.kt#L10-L18
I added a PR(#5568) with option to update tracker read progress with library but Arkon wished for offline queue.
Rather than doing something like queuing tracker changes when offline, being able to sync the library so that trackers are updated based on locally read chapters is even better.
Those are 2 separate issues. This PR updates local chapter read statuses based on what's in the tracker(s). What you described is the need to update trackers based on local chapter read statuses while offline.
I do think that this probably makes sense though, and should supersede #3705. I'll review it later.
I do think that this probably makes sense though, and should supersede #3705. I'll review it later.
main thing i see with this, the function as it exists is a guaranteed 1 to 1 mapping, since its the komga extension connecting to the user's komga server.
Using this method for trackers should probably be opt-in (if it is accepted) because it's probably gonna cause false positives and either mark things read, or completely ignore some chapters.
The way most trackers and Tachi works is i read chapters 1 in tachi, it updates tracker for chapters 1. If i read chapter 1.1 in tachi it updates tracker for chapter 1. The concept of partial chapters doesn't really exist on the trackers.
So the below code: https://github.com/tachiyomiorg/tachiyomi/blob/ff2a4e69526a2edfa5f3bd7570c09204e5ebb9f9/app/src/main/java/eu/kanade/tachiyomi/util/chapter/ChapterTrackSync.kt#L20-L23
would not work at all for 3rd party tracker.
number -> index
1.1 -> 0
1.2 -> 1
1.3 -> 2
2 -> 3
3.1 -> 4
for the remote tracker lets say has last_chapter_read as 2.
1.1, 1.2 will be marked read and 1.3, and 2 will be unread.
This also completey ignores specials, and extra chapters that would also throw off the indexes
This PR updates local chapter read statuses based on what's in the tracker(s). What you described is the need to update trackers based on local chapter read statuses while offline.
True since it's omnidirectional, but the reason I want it is the other direction: updating trackers based on locally read chapters.
This also completey ignores specials, and extra chapters that would also throw off the indexes
Took me a few rereads to understand, but I believe you're right.
Will try and update the logic to match existing behaviour.
https://github.com/tachiyomiorg/tachiyomi/blob/c0647c311002e66c54c3f85d1d075cd216124c86/app/src/main/java/eu/kanade/tachiyomi/ui/reader/ReaderPresenter.kt#L693-L728
updating trackers based on locally read chapters.
This already happens for all trackers and this change doesn't affect that though?
This already happens for all trackers and this change doesn't affect that though?
If I read a chapter offline, then the remote tracker can't be updated. Even if the local record for the track is updated, it won't be pushed until I go to that manga and read another chapter. However, with this change, once I go online and refresh my library then the remote tracker will be updated.
Maybe I'm wrong on some technicalities, but this is the general behaviour I've observed.
If the internal remote track models are getting updated despite the calls failing, that sounds like a bug. We shouldn't be drifting between what we think we've sent to trackers and what was actually sent.
Probably will add a toggle for preventing it from marking local chapters as read, since sometimes special chapters get posted later. E.g., read 0-24, then 12.5 comes out. Without the toggle, 12.5 would get marked as read.
Maybe a 3-way switch: always mark read, never mark read, never mark special as read
Demo of current PR behaviour.
https://user-images.githubusercontent.com/9356891/128424076-4ef49a55-087d-4995-9a9c-0234e5a4aff0.mp4
Edit: reencoded to fix embed
How does it handle these
https://mangadex.org/title/85b51b37-0ce6-4144-a19b-6b064bc2c2ae
https://mangadex.org/title/bd38c075-8b12-450a-aeee-125e3ac5da4b
These are the cases I was concerned with.
The sync method gets called when:
- refreshing the library
- opening the tracker menu
- associating trackers
- auto-associating trackers when manga is added to your library
- only for enhanced trackers.
It will only sync if the update trackers toggle is enabled.
The sync will update the tracker to match the highest locally read chapter number.
The sync will mark local chapters read according to a new preference.
Preference values:
-
always- marks local chapters read if local.ch# <= remote.ch#
- this truncates with
.toInt()so should not mark ch4.5 as read if you read ch4.4
-
never- never marks local chapters read, current behaviour before this PR
-
not special- same as
alwaysbut doesn't mark chapters with a decimal as read - prevents bonus chapters from being marked read since they usually come out later
- same as
Demos
I used a manga that has decimal chapters, hopefully that's good for the example
mode=never
https://user-images.githubusercontent.com/9356891/128451433-80300898-b71e-48ce-9c68-75dc10af85d4.mp4
mode=always
https://user-images.githubusercontent.com/9356891/128450958-a8d0aec6-b1b6-463f-983b-142c7f48a839.mp4
mode=not_special
https://user-images.githubusercontent.com/9356891/128450954-d1dc3157-3963-4d0a-b7e3-989e02f77187.mp4
reading offline with mode=never then refreshing library to update trackers
The cropping on the video is wack because I cut out some dead time in a video editor.
https://user-images.githubusercontent.com/9356891/128452055-bb0c1246-2495-4315-9e38-540b5986f5b2.mp4
One suggestion I have for this is
When auto-sync is disabled and a manga is not synced with tracker, it could change tracker icon (in manga info) to "sync problem" icon that has a different color. (orange or red idk, maybe theme specific)
And then have a "Sync now" button in tracker sheet, kinda like #3705. (Edit: "Sync now" could also be triggered by long pressing tracker icon maybe?)
This change would probably break Komga tracker which is for now using ordinal position instead of chapter number.
See #5699 for more details.
The concept of partial chapters doesn't really exist on the trackers
Is this something we could change, ie have the Track model to handle Float instead of Int for the last read chapter, which would be in-line with Tachiyomi's handling of chapter_number, and let the tracker services handle the conversion to Int if they need it?
Looking a bit more, seems like a less good idea. I misunderstood the quote
The concept of partial chapters doesn't really exist on the trackers
I thought it meant that the tracker impl in Tachiyomi only supported whole chapters, but it really means that the remote tracking services themselves all use int for ch numbers. Keeping it as-is is probably best
Looking a bit more, seems like a less good idea. I misunderstood the quote
The concept of partial chapters doesn't really exist on the trackers
I thought it meant that the tracker impl in Tachiyomi only supported whole chapters, but it really means that the remote tracking services themselves all use
intfor ch numbers. Keeping it as-is is probably best
That was the case before, but typically it's not the case for Komga. That's why I had to use the ordinal index of chapters instead of chapter number.
We're now in a bit of catch 22, where this PR will break Komga tracker because of the use of chapter number and the fact that they are coerced to int. But komga cannot switch to chapter number because they are int.
A reasonable course of action would be:
- For me to release a v2 of the api in Komga that would use chapter number (number sort) for the tracking for tachiyomi
- Have a PR in Tachiyomi to change the last read chapter to Float, and update existing tracker services to coerce to Int.
- Do the change for Komga to use chapter number (#5699 basically)
- Your PR could go ahead
Ah, thanks that helps. So on Komga, ch60 would be 60 individual chapters, but on anilist it could be ch6, with a bunch of decimal chapters. Having it pass a float to Komga still doesn't solve it then, since it's indeterminable how many decimal chapters are in the float. I have an idea on an easy fix, lemme try.
Edit: my idea didn't work
Ah, thanks that helps. So on Komga, ch60 would be 60 individual chapters, but on anilist it could be ch6, with a bunch of decimal chapters. Having it pass a float to Komga still doesn't solve it then, since it's indeterminable how many decimal chapters are in the float. I have an idea on an easy fix, lemme try.
On Komga, it's up to you. Everything can be edited.
@CarlosEsco @arkon how do you feel about changing the internal track model to use float instead of int, and let the tracker services handle the conversion?
Sorry, I haven't really been following the conversation recently.
how do you feel about changing the internal track model to use float instead of int, and let the tracker services handle the conversion?
I'm fine with that if there's a need for it for some trackers. I'd prefer that to be a separate, isolated PR though.
Agree with arkon. A separate PR that is merged and ran for a couple weeks to verify no issues, then this PR can be readdressed
Of course, i'll work on a first PR to make the Track last read a Float, and change the Track services to do the conversion. Once that's done, i'll tackle #5699, as it will have some impacts on ChapterTrackSync.
@TeamDman all of the work i needed to do has been done and has been merged. Komga tracker in Tachiyomi doesn't rely any more on ordinal position of chapter, and there is no more limitation of using Int chapters. The UI is also functional for Komga tracker now, even though we could make it better.
I think you could update your PR to fix the merge conflict 👍🏻
The syncChapersWitTrackServiceTwoWay method doesn't require an EnhancedTrackService parameter, so there's no reason to be guarding.
I have tested this and enabled the two-way sync for Kitsu tracker. The results were successfully being able to sync progress across devices online via tracking sync on update. I don't see why this was not implemented yet, the part of allowing two-way sync for all trackers.
Let me know if you think I should create a PR for this change to supersede this one, although maybe my implementation definitely can be more optimized.
Giving the user the option to choose a source would be nice. Example, only update when the chapter changes in MAL and ignore the rest (if you have MAL obviously). And any update on when it can be added?
And any update on when it can be added?
Just fixed the merge conflicts, I have NOT tested it again recently though.
Giving the user the option to choose a source would be nice
Not sure why this would be necessary, as the proposed code should not cause backtracking when updating trackers based on locally read chapters