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

Fix legendary download progress bar percentage for updates #4240

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RoguedBear
Copy link

closes #4190

Issue:

as discussed in #4190, the current method of calculating progress percentage for updates provides a misleading output.

This PR attempts to fix this by:

  • preserving the existing functionality of using already downloaded files into progress bar calculation for installs
  • and ignoring download cache for updates to better represent the game update progress.

For more context and clarity, a detailed discussion is present in the linked issue

Testing:

I am using Rocket League as an example here. I redownloaded the game from an old manifest that legendary had saved locally, then triggered an update from Heroic.

Before (Heroic 2.15.2)

  • image_2025-01-05_01-14-15

logs:

  • image_2025-01-05_01-16-52

After (my dev branch)

  • image_2025-01-05_01-25-34

logs:

  • image_2025-01-05_01-25-52

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)

@flavioislima
Copy link
Member

flavioislima commented Jan 4, 2025

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@RoguedBear
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

Copy link
Collaborator

@arielj arielj left a comment

Choose a reason for hiding this comment

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

To be sure I understand, this will match the % displayed by legendary in its output in the different cases of downloading, updating, and pausing/resuming? or it's still there something different?

because if that's the case I guess we could just change this to read the % from the output

something like:

progress.percent = data.match(/Progress: (\d+\.\d\d)%/m)

and replace all the calculation?

@RoguedBear
Copy link
Author

RoguedBear commented Jan 9, 2025

To be sure I understand, this will match the % displayed by legendary in its output in the different cases of downloading, updating, and pausing/resuming? or it's still there something different?

From what ive seen experimenting with legendary by default all downloads are resumable regardless of it being installation or update. However for updates i noticed with each download session the "GBs to download" decreases as it takes into account what is already downloaded, and the progress starts from 0%.

In the beginning I was thinking of adding progress.percent = data.match(/Progress: (\d+\.\d\d)%/m) too but I found that the reason this calculation was introduced (in this commit 223b915 ) was as a way to include already downloaded files as part of the progress percentage.

This approach works really well for new installs as the progress bar correctly reflects how much is downloaded in each pause/resume session.
But when updating a game you can see from the screenshots i posted here and in the linked issue that we get a very misleading percentage bar (as the calculation is including the already downloaded game size as part of the update the moment actual update beings to download)

I figured a middle ground approach could work fine here where the existing behavior is preserved for new installs, but for updates do what legendary does -> current downloaded in this session / total to download in this session %


I still think that for users not familiar with legendary it could cause confusion even after this PR is merged as for updates the progress starts from 0% each session, but I think that can be resolved if we display total size to download on the client side.
Then the users would notice that the remaining download size decreases each time they pause/resume, and get reassurance it is actually resuming.

Also this doesn't "exactly" match legendary's percentage, but is very close to it. I am doing size downloaded / total size to download, whereas legendary does number of files downloaded / number of files to download.
You can verify this from the legendary logs screenshot in PR description

@RoguedBear
Copy link
Author

RoguedBear commented Jan 9, 2025

looking at #2239, Although pause/resume has worked well in all the time ive used heroic & legendary, If there is more experimentation we can do regarding new installs, updates and pause/resuming, I would be open to help with that.

@arielj
Copy link
Collaborator

arielj commented Jan 9, 2025

Also this doesn't "exactly" match legendary's percentage, but is very close to it. I am doing size downloaded / total size to download, whereas legendary does number of files downloaded / number of files to download. You can verify this from the legendary logs screenshot in PR description

Yeah, I didn't mean exactly, but wanted to understand is: if we are showing a similar % than legendary in the 3 different scenarios (start from beginning, resume paused/canceled, update), then we don't need the calculation, if we have some consideration to do something different I guess it's fine to keep the calculation.

One thing that I know is a problem with the calculation from Heroic's side is that since it's based on size downloaded and not in files downloaded, stopping/cancelling a download will actually discard the current files that didn't finish downloading (at least for epic games), which can be a problem with games that come with big files (I understand that's the main issue in #2239), so a user will see a % (based on size) when they want to pause, legendary will discard whatever it is downloading at this point, and when resumed the % will be different. With small files downloads it's not that much of a difference, but it's a problem in many cases with big files. So size % is what most people want to know in general, but file % is what people should care when pause/resume/crash. Showing just one is a bit misleading to the user (as reported in that other ticket).

I really don't know what's the best solution, but I do think that screen is missing some information in general:

  • progress based on files downloaded (and making it clear that this is the % to reference for pause/resume)
  • total to be downloaded (now we have a % and amount downloaded, but it's not great UX)
  • disk split into write/read

I honestly don't know what's the right answer, personally I would err on the side of showing as much information as possible (the things I listed + the things we already show), and maybe improve the pause message to say "THIS is the % where we will restart the download, are you sure?????" with a (?) to explain why it's that % (based on files) and not the other % (based on size).

@RoguedBear
Copy link
Author

RoguedBear commented Jan 10, 2025

Yeah, I didn't mean exactly, but wanted to understand is: if we are showing a similar % than legendary in the 3 different scenarios (start from beginning, resume paused/canceled, update), then we don't need the calculation, if we have some consideration to do something different I guess it's fine to keep the calculation.

Yep, there is different calculation in the case of update. existing logic still remains for installs, and new logic is only for updates


So size % is what most people want to know in general, but file % is what people should care when pause/resume/crash. Showing just one is a bit misleading to the user (as reported in that other ticket).

I was doing some exploring of my own, and i couldnt find any way to know how many files are already downloaded for a resume.
if we want to switch to using file count based % then we lose including download cache in the total progress we display on resume of an install.

However, if for updates we want to switch to file based %, that I can try and implement that

Regardless, I too would like to see as much information as is relevant. Adding to your points maybe we can even have a second % bar to keep track for number of files downloaded separate from the size % and utilise that on the warning message

I was also thinking of maybe estimating how much data could be lost by tracking the size for the file currently being downloaded and displaying that in the pause/resume warning dialogue but it could be prone to errors as legendary might use multiple workers to download files parallely.

But until a clear idea of solving this comes up or if legendary can export in detail about the download cache, i think a simple solution such as:

  • communicating to the user in a warning as you mentioned, something like "Warning: some download data might be lost which will be redownloaded next time"
  • and displaying total size to download as mentioned in my comment and yours.

...could work well in conveying this meaning

@arielj
Copy link
Collaborator

arielj commented Jan 10, 2025

I was doing some exploring of my own, and i couldnt find any way to know how many files are already downloaded for a resume. if we want to switch to using file count based % then we lose including download cache in the total progress we display on resume of an install.

The issue here is that we track the wrong % when resuming an install, because we track size % while legendary will actually throw away the current file being downloaded and start from that file, so if the file is big the resume % is actually lower.

However, if for updates we want to switch to file based %, that I can try and implement that

Sorry, I honestly don't know what's better to say which one we want, maybe we just need to show all the %s we have with relevant detail of what they mean instead of picking one

Regardless, I too would like to see as much information as is relevant. Adding to your points maybe we can even have a second % bar to keep track for number of files downloaded separate from the size % and utilise that on the warning message

Yeah, I it would be better to show show both.

I was also thinking of maybe estimating how much data could be lost by tracking the size for the file currently being downloaded and displaying that in the pause/resume warning dialogue but it could be prone to errors as legendary might use multiple workers to download files parallely.

But until a clear idea of solving this comes up or if legendary can export in detail about the download cache, i think a simple solution such as:

  • communicating to the user in a warning as you mentioned, something like "Warning: some download data might be lost which will be redownloaded next time"
  • and displaying total size to download as mentioned in my comment and yours.

...could work well in conveying this meaning

I don't think we have that information (about the current file being downloaded -because we can be downloading multiple files at once- and about the size of the file -I don't think legendary exposes that in the output-) so we can't really do that

I'm sure there's always room for improvement after we get the UI to show all the information, but having that displayed is a good start (also, one thing to consider is GOG/Amazon games report based on size I think, so might be weird that some downloads have more info than others, not sure if that's great)

Maybe that's for another PR, I really don't know what's the best answer. Maybe this is just to improve the update % and then something else is about showing more information in general.

@RoguedBear
Copy link
Author

I am also confused. Initially my scope of the PR was correcting only the misleading update % value. But now it feels like the entire progress bar itself is a bit misleading for legendary only for the case if pause/resume happens.

I guess until there is a proper idea and proper metrics available, we can a display warning in the UI.

I am not sure if this PR will be merged or not, but I'd be willing to work on improving progress bar in the future.

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.

Heroic update progress bar shows incorrect percentage which doesnt match with legendary's percentage
3 participants