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

Treat mods with missing files as upgradeable/reinstallable #4067

Merged
merged 12 commits into from
Mar 25, 2024

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Mar 25, 2024

Motivation

Users supposedly sometimes install a mod in CKAN, then delete the files manually in GameData, then install depending mods in CKAN and complain that they don't work. We need to detect missing files and guide the user into restoring them.

It was suggested to create some kind of "clean-up" button, but I don't think users who expect to be able to delete CKAN-installed mods manually without causing problems would know to use something like that. We need something automatic and pro-active.

Problems

While working on the above, I discovered that (as expected) the major data model refactoring in #4023 broke a lot of things:

  • Checking checkboxes on Mono wasn't updating the changeset or enabling the Apply button
  • The changeset always displays mod info
  • Metadata changes were no longer causing the upgrade checkbox to appear, and/or if you checked it, the changeset wasn't updated and the Apply button wasn't enabled
  • Manually installed mods no longer appeared as upgradeable
  • If you add an upgradeable mod to a Held label, its upgrade checkbox remains, the upgrade column remains visible, and the Update all button stays enabled
  • If your changeset included reinstall actions, the X icon from Ability to clear auto-installed flag from changeset tab #4033 to remove these steps from your changeset appeared but didn't work

Causes

  • I think we were trying to perform GUI updates in a background thread via guiModule_PropertyChanged, and ModGrid.RefreshEdit was freezing on Mono.
  • The new grid always auto-selects its first row, which is treated as if the user clicked on it to see mod info
  • As of the transition to tracking the user's changes solely via GUIMod.SelectedMod, there's no longer a way to represent reinstallation in GUIMod.
  • ModList.GetGUIMods left manually installed mods to be handled by the registry.CompatibleModules check, which has no mechanism for setting GUIMod.HasUpdate, which drives the checkbox
  • I wrote ModList.ResetHasUpdate to re-run the CheckUpgradeable logic and update the UI appropriately, but I think the usage of it got lost in the shuffle of rebasing and conflict resolution.
  • The logic for removing things from the changeset had special handling for the replace checkbox but not the upgrade checkbox

Changes

  • Now if you install a mod with CKAN and delete any of its files, the upgrade checkbox will appear for that mod, and checking it will add a reinstall action to the changeset for that mod, similar to how changed metadata is handled after Treat reinstalling a changed module as an update #3828.
    Fixes Add a “clean GameData” button to the toolbar #4017.
  • Now guiModule_PropertyChanged uses Util.Invoke to access GUI properties and only calls ModGrid.RefreshEdit on Windows. Some adjustments are also made to ModGrid_CellValueChanged to address a problem where multiple checkboxes on the Version tab could become checked if you tried to install historical versions.
  • Now we try to clear the selection more aggressively to suit Mono's quirks. It's not quite working 100%, but it's closer and sometimes seems to work.
  • Now the state of the upgrade checkbox is passed through to GUIMod.GetModChanges, which uses it to add reinstallation actions to the changeset as appropriate
  • Upgradeable manually installed mods are now handled in the registry.CheckUpgradeable section and have GUIMod.HasUpdate set appropriately. Some adjustments are also made to ensure the checkbox actually works when checked.
  • Now ModList.ResetHasUpdate is called when there are changes related to Held labels, which make the checkboxes appear and disappear appropriately, show/hide the upgrade column, and enable/disable the Upgrade all button.
  • Now reinstalls can be removed from the changeset as expected

Other small stuff:

  • ./build on Linux was emitting a message about using the -m parameter to MSBuild to enable parallelization. Now we do that.
  • GUI on Linux was throwing an exception at exit related to failure to dispose the tray icon due to closed file handles or somesuch.
    Now it's fixed by hiding and disposing the tray icon ourselves in the main form's OnClosing rather than letting this.components handle it for us.
  • When we install files, we calculate the SHA1sum of each one and save it to the registry and then never do anything with it (this goes back to the earliest commits that I was able to find). This is colossally wasteful of CPU time, since some files we install are quite large and therefore costly to hash.
    Now this is removed. We can put it back if we decide to add actual functionality that needs it.
  • The "always uncheck all" button from Recommendations usability improvements #4025 was being added redundantly to both the "uncheck all" button's dropdown and the main toolbar.
    Now it's only in the dropdown.

@HebaruSan HebaruSan added Bug Something is not working as intended Enhancement New features or functionality GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN Build Issues affecting the build system Linux Issues specific for Linux Mono Issues specific for Mono Cake Issues affecting Cake Registry Issues affecting the registry Performance Something's slower than it should be labels Mar 25, 2024
@HebaruSan HebaruSan merged commit 76318a5 into KSP-CKAN:master Mar 25, 2024
8 checks passed
@HebaruSan HebaruSan deleted the feature/missing-file-reinstall branch March 25, 2024 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Build Issues affecting the build system Cake Issues affecting Cake Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI Linux Issues specific for Linux Mono Issues specific for Mono Performance Something's slower than it should be Registry Issues affecting the registry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a “clean GameData” button to the toolbar
1 participant