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

feat: Komga page-based sync #1032

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

RandomNamer
Copy link

@RandomNamer RandomNamer commented Jul 22, 2024

As mentioned in issue #236
Use book APIs to achieve chapter-level read progress sync for Komga with the Komga tracker. Every page turn would be reported to Komga just like the Komga web app; to sync page progress from Komga, just trigger refresh tracker manually and all read progress, with page-level granularity would be merged with local. Other trackers should also be able to reuse this feature by implementing PageTracker interface. Additionally, there are 3 sync strategies implemented but currently no UI to configure them.

Screenshot_20240721_220342_Mihon

In case the CI is not running [app-dev-arm64-v8a-preview.apk.zip](https://github.com/user-attachments/files/16327306/app-dev-arm64-v8a-preview.apk.zip)

feat: chapter tracker overhaul: add switch, replace existing tracker logic if enabled

fix: sync page progress regardless of chapter index

chore: change log level

feat: Komga page-based sync
@RandomNamer RandomNamer force-pushed the features/komga_book_specific_tracker branch from ad834c9 to acfb276 Compare July 22, 2024 03:18
@AntsyLich
Copy link
Member

Good stuff. I'll give it a review soon. Meanwhile please rebase.

@AntsyLich AntsyLich added this to the v0.17.0 milestone Aug 10, 2024
Comment on lines +32 to +54
companion object {
//Equal compare
private const val SYNC_STRATEGY_DEFAULT = 1
private fun syncStrategyDefault(local: PageTracker.ChapterReadProgress, remote: PageTracker.ChapterReadProgress): RemoteProgressResolution {
return when {
local > remote -> RemoteProgressResolution.REJECT
local < remote -> RemoteProgressResolution.ACCEPT
else -> RemoteProgressResolution.SAME
}
}

//Flush local with remote
private const val SYNC_STRATEGY_ACCEPT_ALL = 2
private fun syncStrategyAcceptAll(local: PageTracker.ChapterReadProgress, remote: PageTracker.ChapterReadProgress): RemoteProgressResolution {
return if (local.completed && remote.completed || local.page == remote.page) RemoteProgressResolution.SAME else RemoteProgressResolution.ACCEPT
}

//Update remote only when both local and remote are not completed and local page index gt remote
private const val SYNC_STRATEGY_ALLOW_REREAD = 3

private fun syncStrategyAllowReread(local: PageTracker.ChapterReadProgress, remote: PageTracker.ChapterReadProgress): RemoteProgressResolution {
return if (local.completed && !remote.completed && remote.page > 1) RemoteProgressResolution.ACCEPT else syncStrategyDefault(local, remote)
}
Copy link
Member

@AntsyLich AntsyLich Aug 21, 2024

Choose a reason for hiding this comment

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

What's the point of all this?

Copy link
Author

Choose a reason for hiding this comment

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

When I'm developing the feature I come up with several sync strategies that tailored to my needs, e.g. for some books I want to flush all states from komga to Mihon and for some other just update every chapter to latest page from both sources (like CRDT).

Copy link
Member

Choose a reason for hiding this comment

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

They're not being used anywhere except test so why do they exist in this file? Are they even being used anywhere aside from test? I haven't reviewed the whole thing so pardon me if i'm missing something.

@AntsyLich AntsyLich modified the milestones: v0.17.0, v0.18.0 Oct 26, 2024
@kanjieater
Copy link

I've been using this fork for a while, and noticed that read chapters do not get deleted automatically if they get synced as read from the tracker. The normal use case of reading a chapter then going does delete as expected still. Might be an additional feature worth adding to this at some point.

@AntsyLich
Copy link
Member

A tracker rework is under plan I'll look into this PR further after we get that merged

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.

3 participants