feature: adds option to prefetch all images when loading a card pool - #7838
feature: adds option to prefetch all images when loading a card pool#7838SethMilliken wants to merge 7 commits into
Conversation
Jetz72
left a comment
There was a problem hiding this comment.
Maybe there should be an sanity cap on how many fetches can be queued? If someone opened a thousand-plus card quest inventory would this queue all of those?
| @FunctionalInterface | ||
| public interface FetchFunction { | ||
| void fetch(String imageKey); | ||
| } | ||
|
|
||
| public FetchFunction fetchWithoutCallbackFunc() { | ||
| return (String key) -> fetchImage(key, () -> {}); | ||
| } | ||
|
|
There was a problem hiding this comment.
Would it work to just have a fetchImageWithoutCallback(String imageKey) that forwards to fetchImage, and use it in the stream as getImageFetcher()::fetchImageWithoutCallback?
There was a problem hiding this comment.
Haha. Yes. This is a vestige of a much earlier approach I had taken. Fixed.
e88696a to
1968ebd
Compare
This would be easy enough with a |
|
If you're looking to expand the scope of this and the logic for queuing image downloads, you might consider a priority queue or similar structure. If you have a hundred images queued and the user mouses over a card with no image, it'd be good for that card to be able to skip to the front of the line. Might also be worth considering a way to cancel pending downloads if they're no longer of interest - namely if user leaves the screen where they're requested. This may be more trouble than it's worth, though - it'd have to be hooked into all the screens that use it. You should also be aware that we're exploring the possibility of changing up image keys from a string to an object. See #7771 and #7757. |
I've been tinkering with some ideas for refactoring |
1cab968 to
d4201c6
Compare
Images are currently downloaded on-demand on mouseover, which can feel sluggish. This change adds a new preference that, when enabled, will prefetch all images from a (non-infinite) card pool when opening it. Primarily for all Sanction Format modes. Disabled by default. Added only to desktop. Although the mobile version also loads images on-demand, it does so as soon as the cards are visible, which is less of an issue. Note: will require adjustment when `imageKey` refactor (Card-Forge#7757) lands.
…tually used Fixes broken build since `GuiMobile` did not implment `IGuiBase::prefetchAllImages`.
d4201c6 to
29e6a0a
Compare
|
Moved prefetch method to desktop |
- make `WorkerThreadFactory` counter threadsafe - 0-pad thread number in thread names
|
Clearly I ought to have made this a "Draft" to begin with. Latest changes:
|
8aecb0d to
d0e7b67
Compare
I think if we want to start utilizing another logger, it's the kind of thing that should be done in a big commit that hits the whole project, not just one area. Not opposed to exploring it, but it'd be good to discuss options and benefits first. |
Ah, looks like I misread the repo tea leaves. All the plumbing was already there (slf4j in the I'll get the ball rolling on the independent conversation about logging facilities, and I'll drop that commit before this lands. |
|
Its come up before (e.g.). But I don't think the matter has gotten its own dedicated discussion, issue, or PR yet. |
|
This PR has not been updated in a while nad has been marked on stale. Stale PRs will be auto closed |
Images are currently downloaded on-demand on mouseover, which can feel sluggish. This change adds a new preference that, when enabled, will prefetch all images from a (non-infinite) card pool when opening it. Primarily for all Sanction Format modes.
Disabled by default.
Added only to desktop. Although the mobile version also loads images on-demand, it does so as soon as the cards are visible, which is less of an issue.
Note: will require adjustment when
imageKeyrefactor (#7757) lands.