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

Download various asset files on startup if necessary. #16088

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

warmenhoven
Copy link
Contributor

tvOS has two problems that interact with each other:

  1. The OS randomly decides to delete "cache", which is everything on disk, including all of the assets, databases, playlists, etc., except for the retroarch.cfg file. The current implementation of the bundled assets.zip file only extracts the zip on first install.
  2. One of the most common ways of installing on tvOS is AltStore, which on some platforms has limits on the size of the IPA. Also we can't reduce size by removing cores as there is no core downloader; cores need to be signed with the IPA. We currently work around this by deliberately stripping things out of the bundled assets.zip with the expectation that the user will be forced to download them later if necessary.

So this PR on startup will look to see if the assets are where they are supposed to be, and use the online updater to download them if not.

@LibretroAdmin
Copy link
Contributor

I think we need to look for the defines that state whether the online updater/downloader is even available. For instance, for Steam builds we avoid downloading anything from our CDN as per Steam/Valve rules, everything has to come from their CDN instead.

@warmenhoven
Copy link
Contributor Author

Thanks for the prompt, I thought I had covered all the bases, but did miss HAVE_ONLINE_UPDATER, which Steam builds do not set. I've updated the PR.

@zoltanvb
Copy link
Contributor

zoltanvb commented Jan 9, 2024

Assets and core info files are fine and will help to reduce some new user troubles. (Although a streamlined asset package would be nice, currently it is 90 MB and most of it is XMB which is neither the default nor is available everywhere, but that is beside this PR.)
Overlays and database, though, are an extra ~50 MB and are not always wanted, I'd suggest to download overlays only on mobile (if there is a suitable condition).

@warmenhoven
Copy link
Contributor Author

Yeah, before I submitted this PR, I had originally put #if defined(RARCH_MOBILE) && defined(HAVE_OVERLAY). I had also considered throwing in the cheats and shaders for completeness. In the end I did back off a bit from including everything that's in the bundled_assets zip, to just the things I thought would be absolutely necessary for scanning a ROM, loading a core, and playing a game, so I did want to leave the databases in there. But I think you're right, overlays should be limited to just mobile. I've updated the PR.

@warmenhoven warmenhoven marked this pull request as draft January 11, 2024 19:45
@RobLoach
Copy link
Member

A few questions I have around this...

  1. This sounds like an issue with the tvOS distribution? Could we instead fix packaging the assets there instead? If size is an issue, possibly distribute only the required assets?
  2. If you do not have an internet connection, this will just always break. Getting a pop up of failure on every application run would be quite annoying.
  3. I don't want Overlays, yet this forces that decision on me
  4. On some platforms, writing to these directories is not an option
  5. There can be cases where the assets are RetroArch-version specific

Overall, I would much prefer fixing the distributions rather than relying on an internet connection being there and auto-updating.

@warmenhoven
Copy link
Contributor Author

warmenhoven commented Jan 23, 2024

Yeah, I've become increasingly unhappy with this particular PR as it's sat, but it's a reasonable straw man to use to try to figure out what it should look like.

To answer your first question first, there some problems that are specific either to tvOS or both iOS/tvOS that interact:

  1. The size of the IPA is limited. The IPA is just the bundled app, with any cores and assets included, and zipped. There actually isn't a strict limit that's imposed by Apple; instead one of the primary ways people install the IPA is using AltStore/AltServer, and on Windows has a limit on IPA sizes of 310MB. (Right now we're at 328MB.)
  2. Binaries (including cores) have to be bunded in the IPA. Apple signing rules make the core downloader impossible (without jailbreaking; AltStore allows installation without jailbreaking). There are ways of overcoming this without compiling everything yourself (that aren't even hard) but most people are reticent to do this.
  3. The assets bundle is already trimmed. Right now the assets zip excludes shaders, sounds, and xmb. There are possibly more things that could be pared down but it's already getting pretty close to being only the required assets (though it does include overlays, which are arguably required on iOS). Removing the assets entirely should not be necessary, but for reference, removing it entirely brings the IPA size down to 250.3MB. I believe most cores are already compiled optimized for size on iOS/tvOS so most of the IPA size reduction would need to come from the assets bundle.
  4. tvOS only gets 500kb of persistent disk space. Everything else is "cache" and can be deleted more or less at random (which makes the device pretty useless without a persistent internet connection). The only thing stored in the persistent store right now is retroarch.cfg.
  5. The decision about whether to extract the bundled assets.zip is based on flags in the config file, ensuring that it only happens once. However, this fails on tvOS, where the config file is persisted but the extracted assets are unpredictably deleted.

I agree that in general we should be bundling assets and not relying on auto-updating, but iOS/tvOS present unique challenges. I wouldn't propose this as a general solution for macOS, for example. I'm happy to make this iOS/tvOS specific. That said, it might be nice if there are other edge/outlier cases that this could cover (like incorrect packaging, or the user changing the location of certain directories and then not realizing the directory needs to be repopulated somehow).

Addressing your other questions...

  1. Answered above, but to restate, there are two issues: an iOS/tvOS packaging/distribution issue, and a tvOS-deletes-the-world issue. For packaging, we're already encroaching on not including all required assets and we're still 18MB over our target.
  2. The current version of the PR only tries if it can't find specific directories, so it wouldn't be every run. Also tvOS pretty much always has an internet connection; it's not really a very usable device without one.
  3. The current packaged assets bundle also forces that on you. Also the current PR only tries to download overlays if you've compiled with online updater support, overlay support, and the overlays are missing. But yes, if you delete all the overlays after the assets bundle is unzipped and you don't use the online updater to bring them back, this would put them back against your wishes. I could fix this by making it more iOS/tvOS specific and only putting stuff back when we detect that everything has gone missing, which is easy to do.
  4. This is only if the online updater is compiled in, and writes to the directories that the online updater would. Also it's only writing to the directories the assets bundle would write to. Also RetroArch is the one responsible for creating those directories anyway (see dir_check_defaults()).
  5. Yes, and, that hasn't happened for years (most changes are additive), and if it did, it would also break people who tried to use the online updater in general. I've tried to target this change just at the times where the assets are completely missing, which hopefully is an edge case (except for tvOS).

I'm going to sit on this a while longer and think more about exactly where to go from here.

@matheuswillder
Copy link

This could also fix the issue with the current builds for F-Droid, which currently has almost all assets missing due to the F-Droid reproducibility requirements, see: #16126

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.

None yet

5 participants