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

Merging the website advanced options and accounts to TModuleContainer #1020

Closed
riderkick opened this issue Feb 19, 2018 · 16 comments
Closed

Comments

@riderkick
Copy link
Owner

riderkick commented Feb 19, 2018

With the recent changes, which is removing old codes and all website modules now is TModuleContainer.
Now we can begin merging the advanced options to TModuleContainer.
Custom cookies, proxy, task/threads limit, and other possible related http properties that we can set. And we can use TModuleContainer.PrepareHTTP to apply the settings.
Or even merge the accounts to this container. And use single storage for all of them.

First step is to remove the mangalist.ini. Because we have to remove the loading of advanced.ini which depend on the website name to do the lookup.
For anyone who want to help the process can help with filling the category field in the existing module that doesn't have it.

I will update the process here.

@kmvi

kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
kmvi added a commit that referenced this issue Feb 19, 2018
@kmvi
Copy link
Contributor

kmvi commented Feb 19, 2018

mangalist.ini cleaned. There are also EightMuses, Seemh and Shogakukan modules without category. They require rewriting.

@riderkick
Copy link
Owner Author

Those without category doesn't have manga list/can't populate them yet. So they mean to not appear in website selection.

@riderkick
Copy link
Owner Author

Added website settings:

    MaxTaskLimit: Integer;
    MaxConnectionLimit: Integer;
    UpdateListNumberOfThreads: Integer;
    UpdateListDirectoryPageNumber: Integer;
    UserAgent: String;
    Cookies: String;
    ProxyType: TProxyType;
    ProxyHost: String;
    ProxyPort: String;
    ProxyUsername: String;
    ProxyPassword: String; 

Let me know if there is another field need to be added.

@kmvi what do you think about MaxThreadsPerTask and UpdateListNumberOfThreads. They shouldn't be overlap MaxConnectionLimit. UpdateListNumberOfThreads may be useful when creating databases. Do I've to include them? If they're exist, they gonna overlap MaxConnectionLimit and may cause normal operation like download and checking new chapters that respect the limit waiting to long if the active connections doesn't go down because update list threads keep feeding them.

@kmvi
Copy link
Contributor

kmvi commented Feb 20, 2018

These options will be very useful, especially considering #1022 #950
Is it possible to forcefully grab threads from update list process and give them to download process?

@riderkick
Copy link
Owner Author

Issue 1022, 950 can use global max connection limit per website. That's the best use rather than using updatelistnumberofthreads. We talking about limiting the connections to avoid ban, than they should use max connections limit. Which should be respected in download thread and favorite check thread.
Updatelistnumberofthreads is mean to have larger limit than the usual limit, to make update list process faster. Let's say we make connections limit to 8 and can be shared between two task equally. 2 number of task with 4 threads per task. and user can set 8 connections max for certain website. Problem arise when user also wanted to make update process faster by increase the number of threads (updatelistnumberofthreads) to 16 or more. When update list running it will fill the pool and if there is active download thread or favorite check then it may wait forever until update list finished because the pool always full 16, when its expect the pool to be lower than 8.

MaxConnectionLimit, DownloadMaxThreadsPerTask, UpdateListNumberOfThreads.
2 later settings came before MaxConnectionLimit exist. So they mean to not respect MaxConnectionLimit.
Problem arise when we wanted all of that settings active at the same time. I think some settings need to be put down. 2 later settings is useful when we don't have MaxConnectionLimit. While MaxConnectionLimit is useful to avoid ban. User who don't understand the complexity may mess this settings and make the whole instance unstable.

@riderkick
Copy link
Owner Author

@kmvi can you make the ui for website settings? I commit my works with property editor. But they automatically sorted and doesn't look nice. You can use rtti components. Or better create them at run-time so we don't need to deal with them again if we need to add/remove the property.

@kmvi
Copy link
Contributor

kmvi commented Feb 21, 2018

OK, I'll try

riderkick added a commit that referenced this issue Feb 21, 2018
@kmvi
Copy link
Contributor

kmvi commented Feb 21, 2018

Please check 5a29f86

@riderkick
Copy link
Owner Author

I mean you can use RTTI information of TWebsiteModuleSettings to get the properties without hardcoding the field manually.
We can use 2 column layout. You don't need to set the anchors manually just use ChildSizing property.

@kmvi
Copy link
Contributor

kmvi commented Feb 22, 2018

Fixed. I used ChildSizing, but I think it doesn't look nice.

@riderkick
Copy link
Owner Author

riderkick commented Feb 22, 2018

Yup. I think Property Editor looks nicer. It just sorted automatically. There is no way to disable the sort without override the init event. And that event is long.
My suggestion is to make them more narrow. And please put them in scrollbox.
Or maybe we can use virtualstringtree to mimic the property editor.

@kmvi
Copy link
Contributor

kmvi commented Feb 22, 2018

Added scrollbox. But I cannot cope with ChildSizing 😓. Maybe better revert to anchors?

@riderkick
Copy link
Owner Author

Yes we can use anchors. The second column should be anchored to the longest caption. And add checkbox true/false. It's not used now, but maybe later. Also add button to edit the string property. Is there any teditbutton rtti component? if not then bind the button with the ttiedit. This button should show a modal to input the text with tmemo. Just like property editor in IDE.

@riderkick
Copy link
Owner Author

I still prefer the property editor though. With child level and tcollections. Maybe we need them later. We can fold the proxy settings to "Proxy".

@kmvi
Copy link
Contributor

kmvi commented Feb 22, 2018

Please check

@riderkick
Copy link
Owner Author

I still think they're to wide. too many spaces. I don't know how it looks on high dpi screen. But it doesn't look nice on my screen.
I move http settings to child class. We can show them with group box.

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

No branches or pull requests

2 participants