tachiyomi-1.x icon indicating copy to clipboard operation
tachiyomi-1.x copied to clipboard

Rearchitecture wishlist

Open arkon opened this issue 4 years ago • 35 comments

Gathering some feedback from different fork and extension maintainers here as ideas for 1.x, in no particular order:

General

  • No Rx
  • All logic (except UI) is unit testable
  • Improve backup/restore so it doesn't rely on functional sources
  • Improvements to how downloads are handled?

Extensions/sources

  • Way to better handle Base64 images
  • Standardized way to indicate paid/locked chapters
  • Some way to return text in some scenarios (@CarlosEsco might need to clarify what he meant by this)
  • Deeplinking to chapters
  • Better way to handle login for extensions (webview, OAuth, api, etc.)
  • Some API in extensions to make app prompt for login?
  • Flagging entries as manga/manwha/manhua/webtoon/comics/etc., NSFW, etc.
  • Pagination for chapter list methods

arkon avatar May 10 '20 20:05 arkon

Text could be two folds

  1. Xkcd has a text that needs to be displayed along with the image, right we use a 3rd party server to generate an image to display.
  2. This expands scope but could basically start supportig light novels

nonproto avatar May 10 '20 20:05 nonproto

Streaming a chapter should start downloading rest of pages to cache as soon as the initial x pages are downloaded into cache

nonproto avatar May 10 '20 20:05 nonproto

Some sources are completely parsed from JSON from start to finish. When a user tries to "open in webview" they get JSON instead of a webpage. Currently we get around that by overriding the fetch function to call a private API request (and map the response to mangaDetailsParse()) and override mangaDetailsRequest() with the request to an actual webpage. It might be better if we had something like open fun webviewDetailsRequest() = mangaDetailsRequest() that we could override instead of overriding 2 functions and creating a third.

Another quirk with some of these API sources (and possibly some regular ones) is needing to store information for later requests. As an example, you might not be able to build a chapterListRequest() until certain information has been parsed from fetchMangaDetails(). Only problem is what to do with that information. You can "hide" the information at the end of manga.description during mangaDetailsParse() and then use it in chapterListRequest() or you could chain calls, but neither way is exactly pretty. Having an extra field in SManga and SChapter where useful information (that won't be presented to the user) can be set might be useful.

SnakeDoc83 avatar May 10 '20 22:05 SnakeDoc83

This is all more extension stuff:

  • fetchSearchManga() should map query along with response to searchMangaParse()
  • allow for paginated chapter lists
  • being able to mark a manga as licensed during pageListParse() -- sometimes you can't know a manga is licensed until after trying to request a chapter.
  • a way to parse filters stuff like genre tags and load them asynchronously (or store them for later use) instead of having to hardcode everything.
  • something better than Calendar to parse relative dates (e.g. "3 days ago") with
  • fix setUrlWithoutDomain() (can't remember what the exact issue with it is, somehow it doesn't work with URLs that have spaces or special characters in them).
  • there are some sources (usually Korean) that regularly change their domain (to avoid censorship) -- allow the extension to fetch and parse a new baseUrl before continuing on with normal functions instead of having to update the baseuUrl manually every week
  • a WebcomicsSource that's even simpler than HttpSource
  • there are some sources that are a mix of manga and webnovels, having an SManga.WEBNOVEL that worked like SManga.LICENSED could be useful
  • built-in logic that removes duplicate manga while doing fetchLatestUpdates()
  • val id by lazy { createId(name, lang, versionCode) } where createId() has the current logic; that way it's easier to keep the same id if we need to change name

SnakeDoc83 avatar May 10 '20 22:05 SnakeDoc83

  • [x] No rx
  • [x] All business logic testable
  • [x] Deep linking to chapters (opens reader) and manga page (although the UI has been removed due to the Jetpack Compose migration but the backend supports it)
  • [x] Install extensions as a local package so that they can be installed and updated automatically without user prompt (not yet pushed, needs some minor updates) (system installations are still supported, so that people can create and distribute their own apk, although they won't be updatable from the app)
  • [ ] Create a deep link handler apk that is optionally installed in the system and collects each extension deeplinks on a single apk to redirect to the main app. Probably updated from the same screen as the app's updates checker.

About the source API changes, please take a look at the "source-api" package. Everything exposed to the extensions will be in that module and the "common" one. I'd like to have explicit intents on the API, like give me the chapter list, give me a link to open in browser, etc... Instead of adding abstract functions like the manga details request that led to issues when adding the open link feature.

Edit: will follow up tomorrow on some of the comments

inorichi avatar May 11 '20 21:05 inorichi

Improve backup/restore so it doesn't rely on functional sources

Yup, we need an offline backup with the smallest possible size (protobuf maybe?)

Improvements to how downloads are handled?

Yep, I want to rewrite that with Actor, Channel or Flow. Also support optionally compressing the chapter at the end. There will be more limitations on the available downloads directory though, I won't be using SAF as it's very slow and has many limitations.

Way to better handle Base64 images

I need some example for this

Standardized way to indicate paid/locked chapters

Is this really needed? I'm not against but it might be unnecessary complexity

Some way to return text in some scenarios

I guess we could have a page list of a sealed class Representable or something like that. Then the reader displays that information, however this complicates the downloader as it has to download .txt files

Better way to handle login for extensions (webview, OAuth, api, etc.)

Sounds good, but I'm not sure about the design, probably implementing the LoginSource and adding the required intents

Some` API in extensions to make app prompt for login?

I think the app should handle this, extensions should have work passively IMO

Flagging entries as manga/manwha/manhua/webtoon/comics/etc., NSFW, etc.

I added a flag to the entire catalog, but it could be expanded to each manga. What happens if the source doesn't provide this data? Create a default value Unspecified so the UI could ignore this field?

Streaming a chapter should start downloading rest of pages to cache as soon as the initial x pages are downloaded into cache

I did that before and led to the first pages being removed on long chapters. There should be a max number of cached pages. I changed that on tachi long ago to 5 or so but I wouldn't push it any further than 10

Some sources are completely parsed from JSON from start to finish. When a user tries to "open in webview" they get JSON instead of a webpage. Currently we get around that by overriding the fetch function to call a private API request (and map the response to mangaDetailsParse()) and override mangaDetailsRequest() with the request to an actual webpage. It might be better if we had something like open fun webviewDetailsRequest() = mangaDetailsRequest() that we could override instead of overriding 2 functions and creating a third.

Replied yesterday: there should be more explicit intents now, there won't be generic anythingRequest but explicit getMangaDetailsPage() that returns the url tachiyomi can use to open in webview/browser. If you want to reuse code you can always create a private function that would be called by that method and fetchMangaDetails() for example.

Another quirk with some of these API sources (and possibly some regular ones) is needing to store information for later requests. As an example, you might not be able to build a chapterListRequest() until certain information has been parsed from fetchMangaDetails(). Only problem is what to do with that information. You can "hide" the information at the end of manga.description during mangaDetailsParse() and then use it in chapterListRequest() or you could chain calls, but neither way is exactly pretty. Having an extra field in SManga and SChapter where useful information (that won't be presented to the user) can be set might be useful.

I'm not sure I understand the issue but since the source is a single instance you could create a LRUMap and save anything you want there for later calls.

allow for paginated chapter lists

I'm not sure this will be possible as we need to know if any of the chapters was deleted and maintain the order of the chapters.

a way to parse filters stuff like genre tags and load them asynchronously (or store them for later use) instead of having to hardcode everything.

Each extension receives a SharedPreferences instance so you can persist and load anything you want there, even invalidating the data whenever you need.

something better than Calendar to parse relative dates (e.g. "3 days ago") with

I'm open to include another time API (preferably one that works on JVM not just Android) (maybe java.time but I don't know if the desugaring will work fine with extensions as they modify the bytecode since this API is Java 8)

fix setUrlWithoutDomain() (can't remember what the exact issue with it is, somehow it doesn't work with URLs that have spaces or special characters in them).

That function is removed. The source is now responsible to provide a key: String that tachiyomi will save as is. That key should contain the minimum information that allows querying the site (for mangadex for example just the title id).

there are some sources (usually Korean) that regularly change their domain (to avoid censorship) -- allow the extension to fetch and parse a new baseUrl before continuing on with normal functions instead of having to update the baseuUrl manually every week

You should be able to do this with the preferences file too

a WebcomicsSource that's even simpler than HttpSource

What does this do?

there are some sources that are a mix of manga and webnovels, having an SManga.WEBNOVEL that worked like SManga.LICENSED could be useful

But we can't support webnovels either way, right? At least while tachiyomi is just an image reader app.

built-in logic that removes duplicate manga while doing fetchLatestUpdates()

Sounds right. I guess this is because when you query the next page of a site the same results are returned (they don't properly implement that). I could add some logic that if all the returned elements are already in the list, we stop querying for more

val id by lazy { createId(name, lang, versionCode) } where createId() has the current logic; that way it's easier to keep the same id if we need to change name

The source id is now created during build time. You can let it create one for you, or you provide one here. This allows to generate an index (stupid example here) that includes the source id so that the app can know which catalogs it has to install if the user restored a backup. This also allowed me to check for duplicate source ids and duplicate package names, throwing an error if any of these happen.

inorichi avatar May 12 '20 11:05 inorichi

how about letting users migrate manga from the manga details page?

hXtreme avatar May 12 '20 11:05 hXtreme

Standardized way to indicate paid/locked chapters

There are definitely a few extensions that have paid/locked chapters. It could be simple as a flag added on the chapter that can be showed in UI

I did that before and led to the first pages being removed on long chapters. There should be a max number of cached pages. I changed that on tachi long ago to 5 or so but I wouldn't push it any further than 10

I don't mean the amount loaded in memory but the amount downloaded. I believe currently if on page 1 it makes a request to source for 1->2 ->3 ->4 if i go to page to 2 it then makes a request to page 5 to begin downloading. What i am suggesting is when i click into page 1 it download 1,2,3,4 and once they finish they continue to download the rest of the chapter. The reader should then just be reading the images from the temp download folder.

Another suggestion chapter updates from sites should NOT wipe out the chapter list by default if the user has the chapters downloaded, it should keep them in the list and denote they are removed from the site but allow the user to continue to read them as long as the download exists

nonproto avatar May 12 '20 11:05 nonproto

Cloud sync of backups and reading progress. I feel even if we don't want to host something providing the infrastructure for a user to add their own firebase username/password/url. Then left them pay for sync/backup

nonproto avatar May 12 '20 11:05 nonproto

Standardized way to indicate paid/locked chapters

There are definitely a few extensions that have paid/locked chapters. It could be simple as a flag added on the chapter that can be showed in UI

For example the Webtoons.com extension which is pretty popular, the three latest chapters are always locked unless you use their app.

Soitora avatar May 12 '20 11:05 Soitora

Another suggestion: Custom covers are urls now. A different variable on the manga. If its empty then load the default cover url. If it is not load the custom cover.

if you set a page to a custom cover it sets the pages url to the cover. If you want to use your own image you have to upload it to a image hosting site like imgur.

This would allow users to set covers for sources from trackers, dex, wherever. Plus it would allow the custom covers to be backed up without actually saving the cover

nonproto avatar May 12 '20 12:05 nonproto

Another suggestion: Custom covers are urls now. A different variable on the manga. If its empty then load the default cover url. If it is not load the custom cover.

Fine. I had some work on custom covers but allowing to backup convinced me. I'll probably keep the Manga model with a single cover, and the DB query selects the custom cover if present or fallbacks to the source cover.

inorichi avatar May 12 '20 13:05 inorichi

If you want to use your own image you have to upload it to a image hosting site like imgur.

Is it not possible to use file:///path/to/local/file.png URL to use local files as cover?

hXtreme avatar May 12 '20 13:05 hXtreme

but then you lose the ability to backup and restore on new device. Since file:///path/to/local/file.png doesnt exist

nonproto avatar May 12 '20 13:05 nonproto

I'm just pointing out that there's no compulsion to upload covers online, the feature is still useable locally.

hXtreme avatar May 12 '20 13:05 hXtreme

yes but you would need to prompt the user and let them know that because they chose a local file they would not be able to backup the image.

nonproto avatar May 12 '20 13:05 nonproto

Can possibly integrate Imgur API like some have, then when it uploads it actually loads them to Imgur and saves as URL for Tachiyomi

Soitora avatar May 12 '20 14:05 Soitora

Few more: Tagging source in general for extensions. Allowing you to add NSFW tags to hentai sources etc Extensions have a description field and provide a description in the initial json to allow description of sources to show in app or when you want to install

nonproto avatar May 12 '20 14:05 nonproto

Tagging source in general for extensions. Allowing you to add NSFW tags to hentai sources etc Extensions have a description field and provide a description in the initial json to allow description of sources to show in app or when you want to install

Both are already implemented. Description would be for something simple like the base url or a basic text, or a message saying the extension doesn't work. There's no need to describe a list of sources because each extension can implement only one site for a language (the gradle file generates as many apks as needed, so source code can be shared)

inorichi avatar May 12 '20 15:05 inorichi

Standardized way to indicate paid/locked chapters

Is this really needed? I'm not against but it might be unnecessary complexity

In 0.x if you don't show the complete chapter list, including locked chapters, users ask why the chapter lists differ from the source. If you do show the full chapter list there can be an error trying to view/download locked chapters and the error message is probably opaque to them. Ideally, flagging chapters as locked would come with an ability to filter/not filter them as the user chooses or at the very least would provide better error messaging for those chapters. I think we've tried a few approaches with 0.x extensions--always filtering locked chapters out, showing them but with a locked icon in the chapter name, and adding a choice using preferences. It'd be nice to have one consistent method that was clear to users.

allow for paginated chapter lists

I'm not sure this will be possible as we need to know if any of the chapters was deleted and maintain the order of the chapters.

To clarify, I meant allow for parsing chapter lists that are paginated on the source's website. I'm suggesting the function that parses a chapter list should have a page variable and a hasNextPage check like the other parse functions.

I'm open to include another time API

If not java.time maybe ThreeTenBP which backports it to Java 6/7 or Joda-Time. They're a little older but could still be an improvement.

a WebcomicsSource that's even simpler than HttpSource

What does this do?

Would cut down on unused functions for sources that are a webcomic. Example webcomic source.

there are some sources that are a mix of manga and webnovels, having an SManga.WEBNOVEL that worked like SManga.LICENSED could be useful

But we can't support webnovels either way, right? At least while tachiyomi is just an image reader app.

In some instances while parsing a source's catalog that has a mix of manga and novel's, the novels can't be filtered out. So it's possible for the user to go in to a manga, see a chapter list, try to open it, and get an error (because it was actually a novel). With manga that are statused licensed the user gets a message informing them that they can't read that manga and they don't see a chapter list. I'm suggesting that if a user comes across a novel in their catalog (and it's statused as such), they get an appropriate message and don't see a chapter list.

built-in logic that removes duplicate manga while doing fetchLatestUpdates()

Sounds right. I guess this is because when you query the next page of a site the same results are returned (they don't properly implement that). I could add some logic that if all the returned elements are already in the list, we stop querying for more

The issue is that some sites on their latest updates page will show chapter+manga instead of just manga for each entry. Example: page 1 could have Manga A3 (title is A, chapter is 3), Manga A2, Manga B3, Manga C3 and then on page 2 is Manga A1, Manga B2, Manga B1, Manga C2, Manga C1, and Manga D1. Normally that would show as 10 manga in latest if you don't add any deduplication code. You could add a .distinctBy { it.title } to cut that down, but it only takes you down to 7 (A, B, and C twice each because they're on page 1 and 2, plus D). You end up having to track titles over pages and check against that too, finally it's down to 4 unique manga instead of seeing 6 other duplicates mixed in. Example site and see latestUpdatesParse() from here.

SnakeDoc83 avatar May 13 '20 02:05 SnakeDoc83

@inorichi how do you plan to handle scoped storage changes?

nonproto avatar May 14 '20 12:05 nonproto

I'll probably only use the app's specific directory (Android/data/<package_name>), which isn't affected by scoped storage or anything else.

inorichi avatar May 14 '20 20:05 inorichi

Better way to handle login for extensions (webview, OAuth, api, etc.)

HTTP auth would be nice to support Madokami.

ObserverOfTime avatar May 17 '20 11:05 ObserverOfTime

Regarding the base64 images issue, here's a chapter for a site that uses data URLs for images. In making an extension for that source I assigned the images as fake URLs and then used an interceptor to decode the image and build it in to a Response. Although it works, it doesn't seem ideal.

SnakeDoc83 avatar May 23 '20 08:05 SnakeDoc83

Do you have any suggestions for that? I have in mind a sealed class:

sealed class PageSource { // suggestions accepted for a better naming
  data class ImageUrl(val url: String) : PageSource()
  data class ImageBase64(val data: String) : PageSource()
  data class Text(val text: String) : PageSource() // Requested by @CarlosEsco
}

Then we'd have a suspend fun getPageList(chapter) that returns a list of Page(val url: String, var source: PageSource? = null)

And for the cases where the page list request can't return the source for all pages we'd have suspend fun getPageSource(page) that returns the PageSource.

Thoughts?

PS: Feel free to send PR/suggestions to the source-api module.

inorichi avatar May 23 '20 09:05 inorichi

Catalog/Source/Extensions could be in a interpreted language(i.e. lua, js, kotilin/js) which will help with needing to install apks and extension-lib updating and backwards compatibility, debugging.

AriaMoradi avatar Nov 09 '20 00:11 AriaMoradi

dynamic Filter(Genre) list

AriaMoradi avatar Nov 28 '20 08:11 AriaMoradi

Have dipped my hand into maintaining a few extensions, and here's a couple things that would be great

Dynamic Filters

This is technically already possible, see wpmangareader:

This implementation is faced with the big caveat that filters are one of the first things fetched, so the user has to intervene by pressing reset.

Possible implementation strategies:

  • A poor man's implementation could just be to instantiate the filters after receiving the first page of {latest, popular, search} results.
  • Alternatively, allowing for some form of networking to occur when invoking getFilterList would also work.

Better support for non-numerical page based pagination

fetch{search,popular,latest} manga all assume that the source uses numbers starting from 1 for pagination.

This isn't always the case, and leads to awkward implementations to get around this.

Sequences provide a nice abstraction which reduces down to just "fetch a list"

e.g fetching from a paginated json api


fun fetchPopularManga() = sequence {
    val hasMore = true
    while (hasMore) {
         response = // send request
         yieldAll(parseManga(response.body!!.string())) 
         hasMore = hasNextPage(response)
    }
    
}

The source api could provide some convenience functions to facilitate common pagination strategies

e.g:

fun HttpSource.paginateByNumber (f: (page: Int) -> Iterable<T?>): Sequence<T> {
   return sequence { 
      generateSequence(1) { it + 1 }.map(::f).flatten().forEach {
         it?.let { yield(it) } ?: return@forEach
      }
   }
}

fun fetchPopularManga() = HttpSource.paginateByNumber { page ->
   // do the usual fetchPopularManga(page: Int) stuff
}

h-hyuuga avatar May 19 '21 01:05 h-hyuuga

Analyze/Process layer for images

Currently, in Tachiyomi all analyzing/processing of images is done right when an image is about to be presented to the user. This is not ideal because some of it runs on the main thread and others need tons of checks to be kept in check.

This layer would be able to analyze the boundaries of the image to determine if the image is a PAGE, SPREAD, or WEBTOON. And with the boundaries analyzed be able to determine how to process it if it should split the SPREAD/WEBTOON into a list of streams or if it should combine pages to get dual page views. But also do color analysis to automatically pick the background color. Before it gets passed down to the reader

ghostbear avatar May 21 '21 10:05 ghostbear

Support for multi image pages used when pages are split in half and consist of 2+ images. (Madar sites started using such pages)

jopejoe1 avatar Jun 01 '21 06:06 jopejoe1