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

Fix already contains module error at upgrade #3660

Merged

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Sep 8, 2022

Problem

After #3643, upgrades are broken in the dev builds:

  1. Install an old version of KSP-Recall
  2. Click the upgrade checkbox on that mod's row

image

System.ArgumentException: Already contains module: KSP-Recall
   at CKAN.RelationshipResolver.Add(CkanModule module, SelectionReason reason)
   at CKAN.RelationshipResolver.AddModulesToInstall(IEnumerable`1 modules)
   at CKAN.RelationshipResolver..ctor(IEnumerable`1 modulesToInstall, IEnumerable`1 modulesToRemove, RelationshipResolverOptions options, IRegistryQuerier registry, GameVersionCriteria GameVersion)
   at CKAN.IRegistryQuerierHelpers.FindRemovableAutoInstalled(IRegistryQuerier querier, List`1 installedModules, HashSet`1 dlls, IDictionary`2 dlc, GameVersionCriteria crit)
   at CKAN.IRegistryQuerierHelpers.FindRemovableAutoInstalled(IRegistryQuerier querier, List`1 installedModules, GameVersionCriteria crit)
   at CKAN.GUI.ModList.ComputeChangeSetFromModList(IRegistryQuerier registry, HashSet`1 changeSet, ModuleInstaller installer, GameVersionCriteria version)
   at CKAN.GUI.ManageMods.<UpdateChangeSetAndConflicts>d__117.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at CKAN.GUI.ManageMods.<ModList_CellValueChanged>d__74.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

Reported on Discord by @TwiceDaIce, who was not able to submit an issue at the time.

Cause

The exception indicates that the list of modules passed to the resolver contained a duplicate. This was supposed to be prevented by the way FindRemovableAutoInstalled's parameter is constructed, but modules_to_remove doesn't contain the installed version (as I assumed it would by its name since that version would be removed). This duplicate would always have been there for upgrades, but prior to #3643 it didn't matter because we weren't passing the modules to a resolver.

Tried and failed

I tried adding the installed module to modules_to_remove, and this broke switching from one module version to another. There are too many uses of these variables and too many implicit assumptions about how they behave for me to sort it out that way.

Changes

Now a new InstalledAfterChanges function generates exactly the list of modules that I want, without duplicates, so upgrading and switching versions both work.

@HebaruSan HebaruSan added Bug GUI Issues affecting the interactive GUI Pull request labels Sep 8, 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 looks sensible, thanks @HebaruSan !

@HebaruSan HebaruSan merged commit c260526 into KSP-CKAN:master Sep 9, 2022
@HebaruSan HebaruSan deleted the fix/upgrade-already-contains-module branch September 9, 2022 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug GUI Issues affecting the interactive GUI Pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants