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

Only show DLL location error if installing DLL #3647

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Aug 26, 2022

Problem

If you manually install NearFutureElectrical, CKAN detects it and shows it as "AD". If you then tell CKAN to take over management of it via the "upgrade" checkbox, it always fails with the error from #3043:

About to install:

  • Near Future Electrical 1.2.3 (cached)
  • B9 Part Switch v2.20.0 (cached)
  • Module Manager 4.2.2 (cached)
  • Community Resource Pack v112.0.1 (cached)
  • Near Future Electrical Core 1.2.3 (cached)
  • Dynamic Battery Storage 2:2.2.5.0 (cached)

DLL for module NearFutureElectrical found at GameData/NearFutureElectrical/Plugins/NearFutureElectrical.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!
If the above message indicates a download error, please try again. Otherwise, please open an issue for us to investigate.
If you suspect a metadata problem: https://github.com/KSP-CKAN/NetKAN/issues/new/choose
If you suspect a bug in the client: https://github.com/KSP-CKAN/CKAN/issues/new/choose

Note that the plain meaning of the text is false; GameData/NearFutureElectrical/Plugins/NearFutureElectrical.dll is exactly where CKAN would install it. (In fact I captured this error by installing with CKAN and then purging the installed_dlls and installed_files collections in my registry to make it look like a manual install).

Reported by Discord user Jreg:

image

Cause

NearFutureElectrical doesn't install NearFutureElectrical.dll! That honor is reserved for NearFutureElectrical-Core, for reasons that were not documented (see KSP-CKAN/NetKAN#1536 and KSP-CKAN/NetKAN#1539 for when this was done).

The first consequence of this is that if you install just NearFutureElectrical-Core and then scrub your registry, CKAN would detect that as NearFutureElectrical being installed, not -Core. Which frankly is probably fine, since you most likely do have it installed and would want to have CKAN take it over when upgrading.

The second consequence is that when CKAN tries to install NearFutureElectrical when it is already AD status, it looks in the ZIP for where the DLL is supposed to be and doesn't find it, which triggers the install error.

Changes

Now we only raise that error if the module actually installs the DLL in question. This allows the upgrade of NFE to succeed.

@HebaruSan HebaruSan added Bug Something is not working as intended Easy This is easy to fix Core (ckan.dll) Issues affecting the core part of CKAN Pull request labels Aug 26, 2022
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.

That's quite an edge case, good find!

@HebaruSan HebaruSan merged commit eb1b5ea into KSP-CKAN:master Aug 30, 2022
@HebaruSan HebaruSan deleted the fix/core-ad-upgrades branch August 30, 2022 00:20
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 Easy This is easy to fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants