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

Use current metadata for installed module compatibility #2886

Merged
merged 1 commit into from
Oct 19, 2019

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Oct 9, 2019

Problem

If a module's compatibility metadata changes after you install it, the GUI filters will always reflect the compatibility at time of install. For example, AblativeAirbrakes 0.3.0 only supported KSP 1.3.1 in March 2018, but now supports 1.2.2–1.7.3. If you installed it back then and kept it installed to the present, it would only be counted as compatible in GUI if you have 1.3.1 marked as compatible, but if you uninstall and/or reinstall, it will be considered compatible with all of 1.2.2–1.7.3.

Cause

GUIMod has multiple layered constructors; at the bottom level you can create a GUIMod if you only know the identifier, above that you can create a GUIMod from a CkanModule, and at the top you can create a GUIMod using an InstalledModule. Each layer calls the level below it and then fills in extra info; the InstalledModule layer passes its source module (metadata from time of install), and the CkanModule layer passes its identifier.

The handling of compatibility in this hierarchy had some flaws. By default, everything was assumed to be compatible, which could be overridden by a constructor parameter or based on the CkanModule parameter's compatibility info. In the case of an InstalledModule, this meant that an incompatible source module would mark the overall module as incompatible, even if its current available module had subsequently been updated in CKAN-meta to be compatible.

Changes

Now the incompatible constructor parameters are nullable and null by default. If set to true or false, we use that value. If not passed in (i.e., still null by the time we get to the identifier level), then we use the identifier to look up the compatibility from the registry based on the current metadata for all versions of the module.

To avoid paying the cost of this computation for compatible modules, MainModList now passes incompatible=false for known-compatible modules. This saves a few operations per compatible module. This function was already passing true for incompatible modules, and it will leave the parameter null for installed modules to force a check of the current metadata.

The CkanModule-level GUIMod constructor no longer checks compatibility because it was not strictly correct and is now redundant. The identifier-level constructor that it calls will set the correct value before the CkanModule-level constructor runs.

Several functions are changed to return IEnumerable instead of List for possible performance improvements when a search stops early (e.g., if the new .All call finds a false value early on, the rest of the list does not need to be generated).

Fixes #2883.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Pull request Registry Issues affecting the registry labels Oct 9, 2019
@HebaruSan HebaruSan requested a review from DasSkelett October 9, 2019 16:59
@DasSkelett
Copy link
Member

Let me try if I can get the merging right...

@DasSkelett DasSkelett merged commit f58be31 into KSP-CKAN:master Oct 19, 2019
@HebaruSan HebaruSan deleted the fix/incomp-inst branch October 19, 2019 18:18
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 Registry Issues affecting the registry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Recently-compatible installed mods missing from compatible filter
2 participants