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

Upgrade AD mods with mismatched version in filename #3287

Merged
merged 1 commit into from
Feb 3, 2021

Conversation

HebaruSan
Copy link
Member

Problem

If you manually install a version of ModuleManager different from the one CKAN would install for you (e.g., I install MM 4.1.0 but CKAN knows that 4.1.4 is current), trying to convert it to CKAN control by upgrading as per #3043 fails with this error:

Module Manager 4.1.4 (cached)
DLL for module ModuleManager found at GameData/ModuleManager.4.1.0.dll, but it's not where CKAN would install it. Aborting to prevent multiple copies of the same mod being installed. To install this module, uninstall it manually and try again.
Error during installation!"

Cause

We created that warning to catch cases where the manual install is incorrect, as it says. But the logic of the check is too strict; for a smooth automatic upgrade, we require that the old and new file names match exactly. That won't be the case for some valid manual installs for a mod like ModuleManager, because it puts its version in the file name, and there are multiple different ModuleManager versions compatible with a given KSP version.

Changes

  • Now we still try to catch bad manual installs, but we no longer require an exact file name match; instead, we just require:

    • A file starting with the mod identifier
    • Ending with .dll (case insensitive)
    • Installed to the same folder where the manually installed DLL is

    This will still complain if the folders don't match, but will now allow different ModuleManager DLLs to replace one another

  • Now we delete a manually installed DLL we're replacing as part of the install/upgrade transaction (previously would have happened in DeleteConflictingFiles before, but only because the file names had to match exactly, which we no longer require)

Side changes:

  • A too-simple regular expression is replaced with a call to string.EndsWith because that's probably simpler and faster
  • Registry.RegisterDll now no longer uses a hard coded "GameData" prefix but instead checks the game's primary mod directory for multi-game support
  • I think it was wrong to create a CreateTransactionScope for deleting overwritten files in Prompt user to overwrite manually installed files #3043, because one is already created in calling code, so that's gone now

Fixes #3221.

@HebaruSan HebaruSan added Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Pull request labels Feb 1, 2021
@HebaruSan HebaruSan requested a review from DasSkelett February 1, 2021 03:35
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.

Works excellently, thanks!

@HebaruSan HebaruSan merged commit cd33a56 into KSP-CKAN:master Feb 3, 2021
@HebaruSan HebaruSan deleted the fix/upgrade-ad-other-ver branch February 3, 2021 18:28
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 Core (ckan.dll) Issues affecting the core part of CKAN
Projects
None yet
2 participants