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

Support depends on any_of lists #2660

Merged
merged 3 commits into from
Feb 9, 2019
Merged

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Jan 16, 2019

Background

Sometimes a mod needs some functionality to be provided by a third party dependency, and there are several choices available, but they have different APIs, so the dependent mod has to implement support for each one separately.

For example, TextureReplacer, TextureReplacerReplaced, SigmaReplacements-SkyBox, and DiRT can all be used to replace the stock SkyBox with a custom one, but they're all mutually incompatible and so a dependent mod has to implement support for them separately one by one.

The provides property can't handle this (well), since there is no baseline abstract concept that they could sensibly provide. Consider if we had all four of the above mentioned mods provide something like SkyBoxReplacer; then we would not have a solution for a dependent mod that only implemented compatibility with 2 or 3 of them. We would need one provides tag for each possible permutation of supports that a mod could implement, which would be unworkable.

Changes

NOTE: This is a high risk change. Core pieces of RelationshipResolver are modified, which risks breaking common tasks. Please review with care!

Now a new any_of format for relationships is allowed:

    "depends": [
        {
            "any_of": [
                { "name": "SigmaReplacements-Heads" },
                { "name": "TextureReplacerReplaced" }
            ]
        },
        { "name": "ModuleManager" }
    ],

This creates a requirement that can be satisfied by any of the specified modules. If the user installs a module with the above syntax, they are prompted to choose which dependency to use:

$ ~/Downloads/KSP/CKAN/_build/ckan.exe install -c ScottManleyHeadPack-1.0.0.ckan 
Too many mods provide SigmaReplacements-Heads OR TextureReplacerReplaced. Please pick from the following:

1) SigmaReplacements-Heads (Sigma Replacements: Heads)
2) TextureReplacerReplaced (TextureReplacerReplaced)
Enter a number between 1 and 2 (To cancel press "c" or "n".): 
1
About to install...

 * Scott Manley Head Pack 1.0.0 (cached)
 * Sigma Replacements: Heads H_v0.2.4 (cached)

Continue? [Y/n] 


Installing mod "ScottManleyHeadPack 1.0.0"
Installing mod "SigmaReplacements-Heads H_v0.2.4"
Updating registry
Committing filesystem changes
Rescanning GameData
Done!

(It works in GUI too.)

The schema and spec are updated to incorporate this syntax.

RelationshipDescriptor is now an abstract base class (let me know if an interface would be better somehow) with two child classes, ModuleRelationshipDescriptor (formerly known as RelationshipDescriptor) and AnyOfRelationshipDescriptor which models the new syntax and mainly holds a List<RelationshipDescriptor> and multiplexes access to it.

Various tests are updated to instantiate ModuleRelationshipDescriptor now that RelationshipDescriptor is abstract.

A new JsonRelationshipConverter class handles parsing .ckan files, populating lists of RelationshipDescriptor with instances of the appropriate child classes based on the contained properties.

Parts of RelationshipResolver are refactored to handle the new syntax, mainly by calling virtual members of RelationshipDescriptor.

Fixes #2522.

Reverse dependencies fix

Previously if you installed a mod that depended on another mod, but the latest of version of the dependency conflicts with or depends on a different version of the original mod, then you'd get an error.

Now older versions of the dependency will be checked to see whether they're compatible.

Includes tests to make sure this works.

Fixes #1693.

Tests fix

I was still getting the error from #2628, I think because the cache objects in KSPManager weren't being disposed. Now this is added to the affected tests.

@HebaruSan HebaruSan added Enhancement Core (ckan.dll) Issues affecting the core part of CKAN Pull request Spec Issues affecting the spec Discussion needed Schema Issues affecting the schema Relationships Issues affecting depends, recommends, etc. labels Jan 16, 2019
@HebaruSan HebaruSan changed the title Support depends on any_of lists Support depends on any-of lists Jan 16, 2019
@HebaruSan HebaruSan added the Tests Issues affecting the internal tests label Jan 22, 2019
@HebaruSan HebaruSan changed the title Support depends on any-of lists Support depends on any_of lists Feb 3, 2019
@politas
Copy link
Member

politas commented Feb 9, 2019

I'm testing this PR, and I'm not sure when things changed, but if I get a failure applying a changeset and click retry, The ChangeSet tab reappears, and is selected, but the actual tab contents remain as the Status page, so to re-appy the changeset, I need to switch off and back to the Changeset tab.

Also, the Retry button is not snapping to the bottom right.

None of that is relevant to this PR, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Discussion needed Enhancement Pull request Relationships Issues affecting depends, recommends, etc. Schema Issues affecting the schema Spec Issues affecting the spec Tests Issues affecting the internal tests
Projects
None yet
2 participants