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

Prompt user to overwrite manually installed files #3043

Merged
merged 1 commit into from May 15, 2020

Conversation

HebaruSan
Copy link
Member

Motivation

Most new KSP users start out by installing mods manually and find out about CKAN later, at which point they already have an install with some mods and would like to have CKAN take over.

Currently this user's only option is to manually clean out their game folders and start from scratch with CKAN. Some of the manually installed mods will show up in the GUI as "AD", but CKAN can't remove/install/upgrade them. Other manually installed mods will simply show up as not installed, but error out if you try to install them.

Changes

Now you can:

  1. Select an AD mod
  2. Go to the Versions tab
  3. Check the box for one of the versions (usually you'd want the most recent)
    (Or instead of steps 1-3, select a manually installed mod that doesn't show up as AD and add it to be installed normally)
    The apply button becomes enabled
  4. Click the apply button and continue through the installation
  5. Get a new prompt explaining that some files are already present and manually installed:
    image
    Files that are different are highlighted at the top of the list.
  6. Click No to cancel and abort the install; or
    Click Yes to replace the files
  7. Finish the installation and enjoy your freshly CKAN-installed mod

Similarly, CmdLine's install command can overwrite AD mods' files with the same prompt.

The Versions tab is involved because I don't know how to turn the current "AD" text in the checkbox column into an installable checkbox in a clean and clear way. This is kind of a pilot program for some delicate functionality so I'd like to take a cautious approach. If this goes well we can try to build upon it and make the UI better.

Fixes #949.
Fixes #2186.
Fixes #3029.

@HebaruSan HebaruSan added Enhancement Core (ckan.dll) Issues affecting the core part of CKAN Pull request labels Apr 30, 2020
@techman83
Copy link
Member

I don't have the time to familiarise myself to review, but this is really really cool! Excellent work @HebaruSan !

@DasSkelett
Copy link
Member

Looks like the YesNoDialog needs scrollbars for bigger mods: 😅
YesNoDialog too big for screen

@DasSkelett
Copy link
Member

Encountered this through accident:
If you have a mod with a DLL installed manually, but in a different directory than CKAN would do (e.g. ActionGroupsExtended in GameData/AGExt instead of GameData/Diazo/AGExt), you now can "re-"install it with CKAN. Before, you got an error during installation (AGExt already installed, skipped). However, since no files are really conflicting, the existing files stay in GameData, so the mod is now effectively installed twice.

But I know that removing the original files, if they are not conflicting, is basically impossible currently, since we don't know them, we only know the DLL.
For now we could check whether the DLL, if there already is one for this module, is in conflicting, and if not, well, either error out or remove only this DLL file.

Got an idea?


Somewhat related: Assuming a correct manual installation, there might still be additional files in the subfolders that won't be overwritten by the CKAN-installed version (for example if the AD mod is an older version, or if there are dynamically generated cache/save/config files).
Some of them theoretically should be deleted by CKAN in the replacing process, some of the maybe shouldn't.
Should we try to squeeze in a prompt similar to #2962, with the highest folder containing a file that got overwritten as "root"?

This would make the process much more complicated than it currently is, and it's going into rabbit holes we wanted to avoid, so I am fine with letting it as is.

@HebaruSan
Copy link
Member Author

Scrollbar coming up shortly...

image

And a new error for your mismatched install path scenario...

image

For the extra-files concern, let's keep it simple at first and respond based on whether anyone actually has this problem. We might end up making things complicated unnecessarily.

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Yep, works very well now. Nice work!

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 Pull request
Projects
None yet
3 participants