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

Installation message usability improvements #3989

Merged

Conversation

HebaruSan
Copy link
Member

Problems

  • When a download of a module with multiple explicit download URLs or an implicit archive.org fallback URL fails, the downloader does not begin downloading that mod's next URL (but it does start the next queued download from the host that just failed). When the last download succeeds or fails, the installation does not proceed if there were any failed downloads.
  • If you use the Preferred Hosts window to make spacedock.info lower priority than <ALL OTHER HOSTS>, downloads of redistributable mods will succeed but not be stored to the cache, so the installation will fail
    • Related to this, archive.org isn't shown in the host list of that window for KSP2, even though there are redistributable mods archived there. This means that this host can't be prioritized above or below <ALL OTHER HOSTS>, and it's easy to forget it exists.

Causes

  • NetAsyncDownloader.shouldQueue can return true when it shouldn't, specifically for a download that has just failed and moved on to its next URL, because the same download itself is seen as an active download from the same host, even though it hasn't started yet. When this happens, we put that download in the pending queue instead of starting it, and we will probably never return to it.
  • When a download completes, we search the installing modules for the completed URL to figure out which one it was so we can validate its hashes and store it into the cache. This fails for implicit archive.org URLs because they're not present in the CkanModule.download list, but rather in a separate CkanModule.InternetArchiveDownload property. Without a module, we can't validate or store to the cache.
  • Registry.GetAllHosts also examines only CkanModule.download and not CkanModule.InternetArchiveDownload

Motivations

  • As of Search by supports relationship and other search improvements #3939, the GUI progress screen's text box no longer includes blank lines. This was done to suppress an annoying extraneous blank line in the middle of updating the mod list, but now we can't use blank lines for visual hierarchy when it is appropriate. The purpose of that extraneous blank line was to clear any error messages from the status bar, which should be a distinct operation from adding a log message.
  • The message log during installation is somewhat inconsistent and quirky
    • For some steps, the completion message appears in the log but the starting message only shows up in the progress bar label, so it can be difficult to tell what's happening or what happened previously and in what sequence
    • Quotes are put around URLs and mod names sometimes, a bit at random
    • For uninstalling mods, the identifier is shown rather than the name and version
    • The successfully-installed message prints the mod name without the version and puts it in the middle of the string rather than the end where it would be simpler to parse
  • GUIUser's message and progress functions are tightly coupled with code in MainDialogs.cs (which isn't really about dialogs), and there is ambiguity about which piece of that structure should own a given bit of functionality

Changes

  • Now NetAsyncDownloader.shouldQueue skips the same URL that it's being asked to assess, so it will return false if there are no other active downloads from the same host, as intended
    Fixes [Bug]: Failed downloads popup doesn't appear when downloads time out #3983.
  • Now we also check CkanModule.InternetArchiveDownload to find the module, so downloads from these URLs will be properly stored to cache
  • Now instead of calling IUser.RaiseMessage with an empty string to clear the status bar, a new MangageMods.ClearStatusBar event is raised, in response to which Main clears the status bar
  • Several calls to Main.Instance.AddStatusMessage now raise a ManageMods.RaiseMessage event instead because global variables are bad
  • Now Registry.GetAllHosts examines CkanModule.InternetArchiveDownload in addition to CkanModule.download, so archive.org will appear for any game instance containing redistributable mods that have been uploaded to archive.org
  • Now those message quirks are cleaned up
    • The strings on the progress bar are also added to the message log, with some duplicate filtering to avoid spamming a lot of messages. Progress updates specifically about downloads with byte counts are moved to a new separate IUser function to make it easier to exclude their spammy output from the log.
    • The quotes are removed
    • For uninstalling mods, the name and version are shown
    • The successfully-installed message prints the mod name and version at than the end
  • The message and progress functions are removed from MainDialogs.cs and that functionality is now owned exclusively by GUIUser, which accepts new parameters related to the status bar in order to do its job
  • The [ForbidGUICalls] attribute is added to every member of GUIUser to ensure thread safety. I think this wasn't done previously because polymorphism via the IUser interface inhibited Add test to check GUI thread safety #3914's test from analyzing this code fully. Something to return to.

@HebaruSan HebaruSan added Bug Enhancement GUI Issues affecting the interactive GUI Cmdline Issues affecting the command line Core (ckan.dll) Issues affecting the core part of CKAN Network Issues affecting internet connections of CKAN labels Dec 31, 2023
@HebaruSan HebaruSan merged commit 7cf9d0b into KSP-CKAN:master Dec 31, 2023
8 checks passed
@HebaruSan HebaruSan deleted the fix/gui-progress-bar-consistency branch December 31, 2023 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Cmdline Issues affecting the command line Core (ckan.dll) Issues affecting the core part of CKAN Enhancement GUI Issues affecting the interactive GUI Network Issues affecting internet connections of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Failed downloads popup doesn't appear when downloads time out
1 participant