Skip to content

[General] Images offline cache #1732

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

Merged
merged 3 commits into from
Aug 17, 2022
Merged

[General] Images offline cache #1732

merged 3 commits into from
Aug 17, 2022

Conversation

arielj
Copy link
Collaborator

@arielj arielj commented Aug 15, 2022

This PR adds a mechanism to cache game images in the file system to be overcome the issue with the chrome's cache having a limit in size.

How it works:

  • added a custom protocol imagecache:// intercepted by electron to respond with a local file
  • added a CachedImage component that first tries to use the cache, and, if the request fails, it fallback to the original url
  • added an images_cache module with a function that will convert a imagecache:// url into a local file response and, if the file is not already cached, cache it in the background

this was suprisingly way easier than I expected xD


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@arielj arielj added the pr:ready-for-review Feature-complete, ready for the grind! :P label Aug 15, 2022
Copy link
Collaborator

@Nocccer Nocccer left a comment

Choose a reason for hiding this comment

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

LGTM. I would love to get rid of placing files directly into electron folder.

@flavioislima
Copy link
Member

Testing here and it works fine for the library but not for the game page.
Also, would not be better to use the appNames for the images instead of a hash? Not sure if that is important, just a thought.

image

@arielj
Copy link
Collaborator Author

arielj commented Aug 15, 2022

Testing here and it works fine for the library but not for the game page. Also, would not be better to use the appNames for the images instead of a hash? Not sure if that is important, just a thought.

image

I'll check the game image!

I thought about using the app name, but then we have 2 different sizes for the main image plus the logo, so I would have to come up with a pattern to generate file names and also I would have to pass the app name and image type around in the url (I guess I could get the requested size from the url), but it sounds like too complicated for something that is just a cache, the digest ensures it's unique and it's really simple

@arielj
Copy link
Collaborator Author

arielj commented Aug 15, 2022

@flavioislima I fixed the issue with the game image (added a fallback src to use the smaller one if it fails)

I couldn't use heroic/Cache/images, electron clears that folder at boot

@flavioislima
Copy link
Member

NIce, Looks good now! Ready to merge! 👍🏽

@flavioislima flavioislima added the pr:ready-to-merge This PR is fully ready for merge. label Aug 17, 2022
@flavioislima flavioislima changed the title Images offline cache [General] Images offline cache Aug 17, 2022
@flavioislima flavioislima merged commit f505f3f into main Aug 17, 2022
@flavioislima flavioislima deleted the feature/images-cache branch August 17, 2022 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P pr:ready-to-merge This PR is fully ready for merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants