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

Improve handling of unregistered files at uninstall #3890

Merged
merged 3 commits into from
Aug 30, 2023

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Aug 23, 2023

Problem

#3886 reports that, despite #2962, some users are still leaving mostly-empty game folders in place after uninstalling the corresponding mod, which breaks ModuleManager patches that use :NEEDS[dirname] and expect the mod to be installed in the folder.

Causes

  • The Delete Directories tab apparently is not sufficiently clear that when in doubt, you should delete. The only scenarios in which deletion is not desirable is if the user added the files themselves, either by making their own .cfg file or by manually installing another mod, or if the user intends to re-install the mod and wishes to retain settings files generated at run-time.
  • KSP1 auto-generates @thumbs directories inside directories with names ending in Parts (case sensitive, so parts doesn't get one but PorkjetParts does), which are a cache of images to be used as thumbnail images for parts, and which users may not recognize or understand as deletion-worthy.
    (Note that some mods choose to include these folders in their download ZIPs to save them being recreated on users' PCs in-game; in these cases, CKAN would already be deleting them at uninstall anyway.)

Changes

Updates to module uninstallation:

  • Now when you uninstall a mod, after its files are removed:
    1. The list of leftover files that drives the Delete Directories tab is checked for unregistered auto-deletable directory names, defined per-game in IGame.AutoRemovableDirs:
      • KSP1 has @thumbs and could have others added in the future. To avoid ever deleting something the user wanted to keep, it's important to ensure that it's known to be auto-generated and otherwise not of interest to user manipulation, and @thumbs clears that bar.
      • KSP2 has nothing but could have something added in the future as it develops (@KSP-CKAN/wranglers, please comment if you know of any analogous examples!)
    2. If there are any such files and folders, and they aren't owned by any other installed mods, they're deleted silently.
    3. The remaining unregistered files continue onward to the Delete Directories tab as before
  • The auto-deletability logic has some tests added

This will allow the Delete Directories tab to be skipped safely in many instances where it appears today, with outcomes the same or better than they otherwise would have been.

Updates to the Delete Directories tab:

  • Now the "Delete Checked" button is bold to encourage users to see it as the default
  • Now the "Delete Checked" button is focused when it appears to encourage users to see it as the default
  • Now the explanatory label:
    • ... is shorter to make it easier to read; the removed text was about:
      • The fact that some mods were just uninstalled (duh, user does not need to be told this)
      • Examples of reasons why the files might exist (no need to speculate, the fact that they're there is the point)
      • Discussion of ModuleManager (the same warning could appear for KSP2)
    • ... specifically calls out that keeping folders can break mods, with an exclamation point (!)
    • ... explicitly says "deleting them is recommended"
    • ... is auto-resized to make sure the entire text fits in the allocated height
      • Previously duplicated StringHeight functions are now refactored into the Util class

image

This should help users to end up with clean, functional installs without risk of silently deleting important files that they wanted to keep.

In addition:

  • If a "Failed to roll back" exception is thrown, the full list of inner exceptions is printed to try to generate some useful output
  • Some minor clean-up is done to fix assorted compiler warnings

Fixes #3886.

@HebaruSan HebaruSan added Enhancement New features or functionality GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN Tests Issues affecting the internal tests labels Aug 23, 2023
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

Great QoL improvement, thanks @HebaruSan !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI Tests Issues affecting the internal tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: When uninstalling a mod, generated @thumbs directories should be auto-removed
2 participants