Mod installation progress tracking #1625

VilppeRiskidev · 2025-01-23T14:02:07Z

No description provided.

src/components/mixins/DownloadMixin.vue

src/components/profiles-modals/ImportProfileModal.vue

src/components/views/DownloadModModal.vue

src/pages/DownloadMonitor.vue

src/store/modules/DownloadModule.ts

src/utils/ProfileUtils.ts

…talling and display it on the UI

src/components/mixins/DownloadMixin.vue

src/components/profiles-modals/ImportProfileModal.vue

src/pages/DownloadMonitor.vue

src/utils/ProfileUtils.ts

anttimaki

In addition to these there was some buggy behaviour shared in voice chat.

src/pages/DownloadMonitor.vue

src/r2mm/downloading/BetterThunderstoreDownloader.ts

VilppeRiskidev · 2025-02-03T15:53:59Z

Buggy behaviour should be at least better now, I'll have to test a bit more to see if it still fails in some conditions.

VilppeRiskidev · 2025-02-03T16:21:07Z

https://github.com/ebkr/r2modmanPlus/pull/1617/files#r1939046141 "Any error from .getModList() is silently ignored" has been fixed

VilppeRiskidev · 2025-02-03T16:22:43Z

https://github.com/ebkr/r2modmanPlus/pull/1617/files#r1939046141

Depending on how .resolveConflicts() is used elsewhere, and how it's implemented internally it could make sense to not pass modList as an argument but read it internally (it seems only profile is required to do that). However, keeping ProfileModList and the implemenation of ConflictManagementProvider loosely coupled might be the correct idea too
.resolveConflicts() is called separately for each installed mod in this loop, and it might be unnecessary if calling it only once in the end with the final modList yields the same results. However, if we want to fail early to keep the profile from corrupting somehow, the current approach might be the correct idea

These two seem a bit trickier, I think we shuld create a new ticket for these if it is something we want to do.

anttimaki

I don't consider any of the comments alone a blocker, but given there's a few of them, consider if some should be implemented now.

Ideas for future tasks:

Prevent background mod list updates while there are active downloads
Allow clearing one/all completed downloads via DownloadMonitor
Allow retrying failed downloads via DownloadMonitor
Use the actual file size information in ThunderstoreVersion to give different weights to different size mods when determining the progress

src/components/views/DownloadModModal.vue

anttimaki · 2025-02-05T11:21:28Z

src/pages/DownloadMonitor.vue

+                                        <p v-else>Downloading:</p>
+                                        <p>{{Math.min(Math.floor(downloadObject.downloadProgress), 100)}}% complete</p>


To match with "Download failed", this could also simply say "Download complete" on one row without the percentage, once both download and installation is done.

I separated the "Download complete" case, for the logic to be more easy to read.
I also changed the progress bar's style to be 'is-success' when it is completed. I think it makes sense in this case. I know the old implementation used 'is-info', so please comment if this change feels wrong.

I'm probably not the right person to judge whether the completed progress bar should be green or stay blue. But changing the failed download bar to green is mixed signals and should IMO be avoided.

src/pages/DownloadMonitor.vue

anttimaki · 2025-02-05T11:21:38Z

src/r2mm/downloading/BetterThunderstoreDownloader.ts

                downloadCount += 1;
            } else {
                console.error(`Ignore unknown status code "${status}"`);
                return;
            }
-            totalProgressCallback(totalProgress, combo.getMod().getName(), status, err);
+            totalProgressCallback(Math.round(totalDownloadProgress), modInProgressName, status, err);


I think other places use Math.floor, any reason to use .round instead here? (If changed, should be changed to TSMM sibling PR as well.)

The reason was that the generateProgressPercentage() sometimes returns like 99.9999999 instead of 100 (floating point number calculations are sometimes a tiny bit inaccurate) and therefore the download progress got stuck at 99%. Should the generateProgressPercentage actually only return integers (round the number before returning)? (I implemented that as a part of the changes)

If the change is OK, should/could the rounding of the progress percentage be removed from the UI components or is it a best practice to round just in case?

In hindsight this whole thing should probably be designed so that it's the Vuex module that receives updates of each mods progress and internally keeps tracks how many mods is completed out of how many mods total, and what's the process of currently downloading mod. And it should then be the one that provides a getter or some other mechanic for providing the total progress to UI (rounded in one place and in one consistent way). That would also allow us to show it as percentage for progress bars, or as "Downloading mod 5 out of 6" for other status indicators.

Anyway, I'd be careful in changing generateProgressPercentage() as it's called from elsewhere too. If you think this change improves the overall situation, feel free to go ahead with it, but do test it thoroughly (all functions that call generateProgressPercentage() and related UI parts).

src/utils/ProfileUtils.ts

anttimaki

Bonus: it seems at some point the order of downloads has changed. Develop downloads the target mod first and then the dependencies, while this branch downloads dependencies first. IDK if it matters at all, but inadvertently changing feels like begging for problems.

anttimaki · 2025-02-07T10:09:41Z

src/pages/DownloadMonitor.vue

+                                    />
+                                </div>
+
+                                <div v-else-if="downloadObject.downloadProgress === 100 && downloadObject.installProgress === 100">


Separate task: update the progress object to have status enum instead of single failed boolean flag. Having statuses like "downloading", "installing", "done", and "failed" makes places like this much more readable.

anttimaki · 2025-02-07T10:09:46Z

src/pages/DownloadMonitor.vue

+                                        />
+                                    </div>
+                                    <div v-else class="col">
+                                        <p>Installing: {{ downloadObject.modName }}</p>


Seems modName in installing phase shows "teamName-modName" while downloading part only shows the "modName". The latter is IMO cleaner.

anttimaki · 2025-02-07T10:09:53Z

src/r2mm/downloading/BetterThunderstoreDownloader.ts

            if (status === StatusEnum.PENDING) {
-                totalProgress = this.generateProgressPercentage(progress, downloadCount, allModsToDownload.length);
+                totalDownloadProgress = this.generateProgressPercentage(downloadProgress, downloadCount, allModsToDownload.length);


Not having "download is 80%, extracting 20%" multiplier here like there's in downloadImportedMods leads to situation where downloading e.g. "TV Loader Anime Openings" shows download being completed and installation "waiting for download" for two minutes while the videos are extracted from the zip.

VilppeRiskidev self-assigned this Jan 23, 2025

VilppeRiskidev requested a review from anttimaki January 23, 2025 14:31

VilppeRiskidev force-pushed the download-completed-callback-improvement branch from e433150 to 911baa4 Compare January 23, 2025 14:44

VilppeRiskidev force-pushed the mod-installation-progress-tracking branch 2 times, most recently from 4ef5cbc to f27752a Compare January 23, 2025 15:03

anttimaki requested changes Jan 24, 2025

View reviewed changes

VilppeRiskidev force-pushed the mod-installation-progress-tracking branch from f27752a to 0ca299f Compare January 28, 2025 12:55

VilppeRiskidev force-pushed the download-completed-callback-improvement branch from 5242ff6 to 9ca4173 Compare January 28, 2025 12:59

VilppeRiskidev added 6 commits January 28, 2025 15:00

Simplify download logic by omitting callback

2d0cc91

Move download progress callbacks to DownloadMixin

09fd3bf

Show failed download status unambiguously in DownloadMonitor

7680c4b

Rename progress to downloadProgress

fb6ad5d

Add installationProgress to the Vuex store, update the value when ins…

1c37df5

…talling and display it on the UI

Show installation progress in the DownloadMonitor correclty

a7b06bd

VilppeRiskidev force-pushed the mod-installation-progress-tracking branch 2 times, most recently from dfb6b56 to 03af2b9 Compare January 28, 2025 14:33

VilppeRiskidev requested a review from anttimaki January 28, 2025 14:35

anttimaki requested changes Jan 29, 2025

View reviewed changes

VilppeRiskidev force-pushed the download-completed-callback-improvement branch from 9ca4173 to db6a4f9 Compare January 29, 2025 13:31

VilppeRiskidev force-pushed the mod-installation-progress-tracking branch 3 times, most recently from 61bd4bb to 17bfa4f Compare January 30, 2025 13:54

VilppeRiskidev force-pushed the download-completed-callback-improvement branch from db6a4f9 to 7ea20ca Compare January 30, 2025 14:05

VilppeRiskidev force-pushed the mod-installation-progress-tracking branch from 17bfa4f to 0845cb9 Compare January 30, 2025 14:07

VilppeRiskidev requested a review from anttimaki January 30, 2025 14:08

anttimaki mentioned this pull request Feb 3, 2025

Download completed callback improvement #1617

Open

anttimaki reviewed Feb 3, 2025

View reviewed changes

src/pages/DownloadMonitor.vue Outdated Show resolved Hide resolved

src/r2mm/downloading/BetterThunderstoreDownloader.ts Outdated Show resolved Hide resolved

VilppeRiskidev force-pushed the download-completed-callback-improvement branch from 7ea20ca to 9ca4173 Compare February 3, 2025 13:48

VilppeRiskidev force-pushed the mod-installation-progress-tracking branch from 0845cb9 to 03af2b9 Compare February 3, 2025 13:51

Further improve installation progress tracking

c359b28

VilppeRiskidev force-pushed the mod-installation-progress-tracking branch from 03af2b9 to c359b28 Compare February 3, 2025 14:01

Fix issues with mod download/install progress tracking

d03f65a

VilppeRiskidev requested a review from anttimaki February 3, 2025 16:23

anttimaki reviewed Feb 5, 2025

View reviewed changes

Fix getModList errors being silently ignored

c868b98

VilppeRiskidev force-pushed the mod-installation-progress-tracking branch from 628656b to c868b98 Compare February 5, 2025 15:12

VilppeRiskidev requested a review from anttimaki February 5, 2025 15:17

anttimaki reviewed Feb 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mod installation progress tracking #1625

Mod installation progress tracking #1625

VilppeRiskidev commented Jan 23, 2025

anttimaki left a comment

VilppeRiskidev commented Feb 3, 2025

VilppeRiskidev commented Feb 3, 2025

VilppeRiskidev commented Feb 3, 2025

anttimaki left a comment

anttimaki Feb 5, 2025

VilppeRiskidev Feb 5, 2025

anttimaki Feb 7, 2025

anttimaki Feb 5, 2025

VilppeRiskidev Feb 5, 2025 •

edited

Loading

VilppeRiskidev Feb 5, 2025

anttimaki Feb 7, 2025

anttimaki left a comment

anttimaki Feb 7, 2025

anttimaki Feb 7, 2025

anttimaki Feb 7, 2025

		<p v-else>Downloading:</p>
		<p>{{Math.min(Math.floor(downloadObject.downloadProgress), 100)}}% complete</p>

Mod installation progress tracking #1625

Are you sure you want to change the base?

Mod installation progress tracking #1625

Conversation

VilppeRiskidev commented Jan 23, 2025

anttimaki left a comment

Choose a reason for hiding this comment

VilppeRiskidev commented Feb 3, 2025

VilppeRiskidev commented Feb 3, 2025

VilppeRiskidev commented Feb 3, 2025

anttimaki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VilppeRiskidev Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anttimaki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VilppeRiskidev Feb 5, 2025 •

edited

Loading