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

Combine recommendation and suggestion screens #2744

Merged
merged 2 commits into from
Apr 29, 2019

Conversation

HebaruSan
Copy link
Member

(a.k.a., "Rewrite recommendations... again")

Problem

If you import a .ckan file that recommends CustomBarnKit, it'll try to install two mods by default:

  • CustomBarnKit
  • CustomBarnKit-RO

These conflict with one another, so if the user simply accepts the recommendation, the install will fail with an error.

Cause

CustomBarnKit-RO provides CustomBarnKit, so recommending one pulls in both.

This is important for something like FerramAerospaceResearchContinued, which we want to match a recommendation for FAR.

Changes

We need the ability to present a recommendation as unchecked. This is impossible within the current code structure, since all recommendations are handled the same way in one big group, and there's no conduit through which we could pass an indication of whether to check the checkbox.

Now the recommendations and suggestions prompts are merged into one big prompt, containing some items that are checked by default (the recommendations) and others that are unchecked by default (the suggestions). This reduces the number of clicks needed to get through an install.

To make this work, the recommendations code is refactored to generate a single sequence of ListViewItems, each of which represents a recommendation or suggestion, and the checked state of which is controlled by more subtle logic. If only one module matches the recommendation, or if there's an exact identifier match, then it'll be checked, otherwise it'll be unchecked. In the original CustomBarnKit scenario, CustomBarnKit will be checked while CustomBarnKit-RO is unchecked. This allows the user to change it if they really want to, while still providing something that will work by default.

Using the new code is somewhat simpler for calling code, since it no longer needs to loop over modules or generate lists of recommendations and suggestions. Instead we can now simply pass the list of source modules and receive back the rows that would go to the dialog as input.

Fixes #2724.

@HebaruSan HebaruSan added Bug Something is not working as intended Enhancement New features or functionality GUI Issues affecting the interactive GUI Pull request Relationships Issues affecting depends, recommends, etc. labels Apr 28, 2019
@Olympic1 Olympic1 removed Bug Something is not working as intended Enhancement New features or functionality labels Apr 28, 2019
@HebaruSan HebaruSan added Bug Something is not working as intended Enhancement New features or functionality labels Apr 28, 2019
@HebaruSan HebaruSan merged commit f265dab into KSP-CKAN:master Apr 29, 2019
@Olympic1 Olympic1 removed Bug Something is not working as intended Enhancement New features or functionality Pull request labels Apr 29, 2019
@HebaruSan HebaruSan deleted the feature/combined-rec-sug branch April 29, 2019 16:26
@HebaruSan
Copy link
Member Author

BTW, submitted mono/mono#14325 to try to fix the group header colors on Linux:

screenshot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Importing .ckan also selects conflicting recommends
3 participants