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

Show all recommendations of metapackages in GUI #2653

Merged
merged 1 commit into from
Jan 14, 2019

Conversation

HebaruSan
Copy link
Member

Problem

If you use the "Install from .ckan..." menu option in GUI, as of #2577 some modules may be missing.

Cause

CanInstall decides whether a list of modules can be installed by creating a RelationshipResolver and checking whether its mod list has at least as many elements as it's trying to install:

if (resolver.ModList().Count() >= toInstall.Count)

The list on the right includes the .ckan file itself, which has IsMetapackage = true.

Unfortunately RelationshipResolver excludes metapackages from ModList(), so such modules would not be included on the left:

private void Add(CkanModule module, SelectionReason reason)
{
if (module.IsMetapackage)
return;

So CanInstall's check will fail when installing a metapackage, unless it has a bunch of extra dependencies.

Changes

Now we exclude metapackages from the right side of that comparison, and all recommendations from the .ckan file are shown in GUI.

Fixes #2652.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Pull request Relationships Issues affecting depends, recommends, etc. labels Jan 10, 2019
@politas politas merged commit 4f8473f into KSP-CKAN:master Jan 14, 2019
@HebaruSan HebaruSan deleted the fix/gui-recs-from-ckan branch January 14, 2019 02:58
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 GUI Issues affecting the interactive GUI Relationships Issues affecting depends, recommends, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants