Modpack compatibility prompt, GameComparator clean-up

Core/ServiceLocator.cs

@@ -37,7 +37,7 @@ private static void Init()
         {
             var builder = new ContainerBuilder();
 
-            builder.RegisterType<GrasGameComparator>()
+            builder.RegisterType<StrictGameComparator>()
                 .As<IGameComparator>();
 
             builder.RegisterType<JsonConfiguration>()

Core/Types/GameComparator/BaseGameComparator.cs

@@ -1,3 +1,5 @@
+using System.Linq;
+
 using CKAN.Versioning;
 
 namespace CKAN
@@ -6,22 +8,13 @@ public abstract class BaseGameComparator : IGameComparator
     {
         public BaseGameComparator() { }
 
-        public virtual bool Compatible(GameVersionCriteria gameVersionCriteria, CkanModule module)
-        {
-            if (gameVersionCriteria.Versions.Count == 0)
-            {
-                return true;
-            }
-            foreach (GameVersion gameVersion in gameVersionCriteria.Versions)
-            {
-                if (SingleVersionsCompatible (gameVersion, module))
-                {
-                    return true;
-                }
-            }
-            return false;
-        }
+        public virtual bool Compatible(GameVersionCriteria gameVersionCriteria,
+                                       CkanModule          module)
+            => gameVersionCriteria.Versions.Count == 0
+                || gameVersionCriteria.Versions
+                                      .Any(gv => SingleVersionsCompatible(gv, module));
 
-        public abstract bool SingleVersionsCompatible(GameVersion gameVersionCriteria, CkanModule module);
+        public abstract bool SingleVersionsCompatible(GameVersion gameVersionCriteria,
+                                                      CkanModule  module);
     }
 }

Core/Versioning/GameVersionRange.cs

@@ -16,26 +16,16 @@ public sealed partial class GameVersionRange
 
         public GameVersionRange(GameVersionBound lower, GameVersionBound upper)
         {
-            if (lower is null)
-            {
-                throw new ArgumentNullException("lower");
-            }
-
-            if (upper is null)
-            {
-                throw new ArgumentNullException("upper");
-            }
-
-            Lower = lower;
-            Upper = upper;
+            Lower = lower ?? GameVersionBound.Unbounded;
+            Upper = upper ?? GameVersionBound.Unbounded;
 
             _string = DeriveString(this);
         }
 
         public GameVersionRange(GameVersion lower, GameVersion upper)
             : this(lower?.ToVersionRange().Lower, upper?.ToVersionRange().Upper) { }
 
-        public override string ToString() =>_string;
+        public override string ToString() => _string;
 
         public GameVersionRange IntersectWith(GameVersionRange other)
         {

GUI/Main/Main.cs

@@ -696,7 +696,7 @@ private void installFromckanToolStripMenuItem_Click(object sender, EventArgs e)
         private void InstallFromCkanFiles(string[] files)
         {
             // We'll need to make some registry changes to do this.
-            RegistryManager registry_manager = RegistryManager.Instance(CurrentInstance, repoData);
+            var registry_manager = RegistryManager.Instance(CurrentInstance, repoData);
             var crit = CurrentInstance.VersionCriteria();
 
             var installed = registry_manager.registry.InstalledModules.Select(inst => inst.Module).ToList();
@@ -743,6 +743,34 @@ private void InstallFromCkanFiles(string[] files)
                     continue;
                 }
             }
+
+            CkanModule.GetMinMaxVersions(toInstall.Where(m => m.IsMetapackage),
+                                         out _, out _,
+                                         out GameVersion minGame, out GameVersion maxGame);
+            var filesRange = new GameVersionRange(minGame, maxGame);
+            var instRanges = crit.Versions.Select(gv => gv.ToVersionRange())
+                                          .ToList();
+            var missing = CurrentInstance.game
+                                         .KnownVersions
+                                         .Where(gv => filesRange.Contains(gv)
+                                                      && !instRanges.Any(ir => ir.Contains(gv)))
+                                         // Use broad Major.Minor group for each specific version
+                                         .Select(gv => new GameVersion(gv.Major, gv.Minor))
+                                         .Distinct()
+                                         .ToList();
+            if (missing.Any()
+                && YesNoDialog(string.Format(Properties.Resources.MetapackageAddCompatibilityPrompt,
+                                             filesRange.ToSummaryString(CurrentInstance.game),
+                                             crit.ToSummaryString(CurrentInstance.game)),
+                               Properties.Resources.MetapackageAddCompatibilityYes,
+                               Properties.Resources.MetapackageAddCompatibilityNo))
+            {
+                CurrentInstance.SetCompatibleVersions(crit.Versions
+                                                          .Concat(missing)
+                                                          .ToList());
+                crit = CurrentInstance.VersionCriteria();
+            }
+
             // Get all recursively incompatible module identifiers (quickly)
             var allIncompat = registry_manager.registry.IncompatibleModules(crit)
                 .Select(mod => mod.identifier)
@@ -751,12 +779,11 @@ private void InstallFromCkanFiles(string[] files)
             var myIncompat = toInstall.Where(mod => allIncompat.Contains(mod.identifier)).ToList();
             if (!myIncompat.Any()
                 // Confirm installation of incompatible like the Versions tab does
-                || Instance.YesNoDialog(
-                    string.Format(Properties.Resources.ModpackInstallIncompatiblePrompt,
-                        string.Join(Environment.NewLine, myIncompat),
-                        crit.ToSummaryString(CurrentInstance.game)),
-                    Properties.Resources.AllModVersionsInstallYes,
-                    Properties.Resources.AllModVersionsInstallNo))
+                || YesNoDialog(string.Format(Properties.Resources.ModpackInstallIncompatiblePrompt,
+                                             string.Join(Environment.NewLine, myIncompat),
+                                             crit.ToSummaryString(CurrentInstance.game)),
+                               Properties.Resources.AllModVersionsInstallYes,
+                               Properties.Resources.AllModVersionsInstallNo))
             {
                 UpdateChangesDialog(toInstall.Select(m => new ModChange(m, GUIModChangeType.Install))
                                              .ToList(),

GUI/Properties/Resources.resx

@@ -200,13 +200,18 @@ This means that CKAN forgot about all your installed mods, but they are still in
   <data name="AllModVersionsInstallPrompt" xml:space="preserve"><value>{0} is not supported on your current compatible game versions ({1}) and may not work at all. If you have any problems with it, you should NOT ask its maintainers for help.
 
 Do you really want to install it?</value></data>
+  <data name="AllModVersionsInstallYes" xml:space="preserve"><value>Install</value></data>
+  <data name="AllModVersionsInstallNo" xml:space="preserve"><value>Cancel</value></data>
+  <data name="MetapackageAddCompatibilityPrompt" xml:space="preserve"><value>The selected modpacks are compatible with {0}, but your game instance is only compatible with {1}. This may cause some dependencies to fail to install.
+
+Do you want to add the additional versions to this game instance's compatibility list?</value></data>
+  <data name="MetapackageAddCompatibilityYes" xml:space="preserve"><value>Add compatible versions</value></data>
+  <data name="MetapackageAddCompatibilityNo" xml:space="preserve"><value>Keep current compatibility</value></data>
   <data name="ModpackInstallIncompatiblePrompt" xml:space="preserve"><value>Some of the selected mods (or their dependencies) are not supported on your current compatible game versions ({1}) and may not work at all. If you have any problems with them, you should NOT ask their maintainers for help.
 
 {0}
 
 Are you sure you want to install them? Cancelling will abort the entire installation.</value></data>
-  <data name="AllModVersionsInstallYes" xml:space="preserve"><value>Install</value></data>
-  <data name="AllModVersionsInstallNo" xml:space="preserve"><value>Cancel</value></data>
   <data name="MainChangesetUpdateSelected" xml:space="preserve"><value>Update selected by user to version {0}.</value></data>
   <data name="MainChangesetReinstall" xml:space="preserve"><value>Re-install (metadata changed)</value></data>
   <data name="MainChangesetUserReinstall" xml:space="preserve"><value>Re-install (user requested)</value></data>

Tests/Core/Types/GameComparator.cs

@@ -1,7 +1,9 @@
 using System;
+
+using NUnit.Framework;
+
 using CKAN;
 using CKAN.Versioning;
-using NUnit.Framework;
 using Tests.Data;
 
 namespace Tests.Core.Types
@@ -19,10 +21,7 @@ public void Setup()
             gameMod = TestData.kOS_014_module();
         }
 
-        [Test]
-        [TestCase(typeof(StrictGameComparator), true)]
-        [TestCase(typeof(GrasGameComparator), true)]
-        [TestCase(typeof(YoyoGameComparator), true)]
+        [Test, TestCase(typeof(StrictGameComparator), true)]
         public void TotallyCompatible(Type type, bool expected)
         {
             var comparator = (IGameComparator) Activator.CreateInstance(type);
@@ -35,10 +34,7 @@ public void TotallyCompatible(Type type, bool expected)
             Assert.AreEqual(expected, comparator.Compatible(new GameVersionCriteria (gameVersion), gameMod));
         }
 
-        [Test]
-        [TestCase(typeof(StrictGameComparator), false)]
-        [TestCase(typeof(GrasGameComparator), true)]
-        [TestCase(typeof(YoyoGameComparator), true)]
+        [Test, TestCase(typeof(StrictGameComparator), false)]
         public void GenerallySafeLax(Type type, bool expected)
         {
             var comparator = (IGameComparator) Activator.CreateInstance(type);
@@ -51,10 +47,7 @@ public void GenerallySafeLax(Type type, bool expected)
             Assert.AreEqual(expected, comparator.Compatible(new GameVersionCriteria (gameVersion), gameMod));
         }
 
-        [Test]
-        [TestCase(typeof(StrictGameComparator), false)]
-        [TestCase(typeof(GrasGameComparator), false)]
-        [TestCase(typeof(YoyoGameComparator), true)]
+        [Test, TestCase(typeof(StrictGameComparator), false)]
         public void GenerallySafeStrict(Type type, bool expected)
         {
             var comparator = (IGameComparator) Activator.CreateInstance(type);
@@ -69,10 +62,7 @@ public void GenerallySafeStrict(Type type, bool expected)
             Assert.AreEqual(expected, comparator.Compatible(new GameVersionCriteria (gameVersion), gameMod));
         }
 
-        [Test]
-        [TestCase(typeof(StrictGameComparator), false)]
-        [TestCase(typeof(GrasGameComparator), false)]
-        [TestCase(typeof(YoyoGameComparator), true)]
+        [Test, TestCase(typeof(StrictGameComparator), false)]
         public void Incompatible(Type type, bool expected)
         {
             var comparator = (IGameComparator) Activator.CreateInstance(type);
@@ -83,7 +73,7 @@ public void Incompatible(Type type, bool expected)
 
         public static readonly object[] TestStrictGameComparatorCases =
         {
-                        //MOD comapat.      //KSP           //expected
+            //             Mod compat.      KSP             expected
             new object[] { "1.0",           "1.0.4",        true },
             new object[] { "1.1",           "1.0.4",        false },
 
@@ -131,26 +121,26 @@ public void TestStrictGameComparator(string modVersion, string gameVersion, bool
 
         public static readonly object[] TestStrictGameComparatorMinMaxCases =
         {
-                        //Min comapat     //Max comapat     //KSP           //expected
-            new object[] { "1.0.4",           null,           "1.0.3",       false },
-            new object[] { "1.0.4",           null,           "1.0.4",       true },
-            new object[] { "1.0.4",           null,           "1.0.5",       true },
-            new object[] { "1.0.4",           null,           "1.1",         true },
-
-            new object[] { "1.0",           null,           "0.9",          false },
-            new object[] { "1.0",           null,           "1.0",          true },
-            new object[] { "1.0",           null,           "1.0.4",        true },
-            new object[] { "1.0",           null,           "1.1",          true },
-
-            new object[] { "1.1",           null,           "1.0.4",        false },
-            new object[] { "1.1",           null,           "1.1",          true },
-            new object[] { "1.1",           null,           "1.1.1",        true },
-            new object[] { "1.1",           null,           "1.2",          true },
-
-            new object[] { null,            "1.0.4",        "1.0.5",        false },
-            new object[] { null,            "1.0.4",        "1.0.4",        true },
-            new object[] { null,            "1.0.4",        "1.0.3",        true },
-            new object[] { null,            "1.0.4",        "1.0",          true },
+            //             Min comapat      Max comapat     KSP           expected
+            new object[] { "1.0.4",         null,           "1.0.3",      false },
+            new object[] { "1.0.4",         null,           "1.0.4",      true },
+            new object[] { "1.0.4",         null,           "1.0.5",      true },
+            new object[] { "1.0.4",         null,           "1.1",        true },
+
+            new object[] { "1.0",           null,           "0.9",        false },
+            new object[] { "1.0",           null,           "1.0",        true },
+            new object[] { "1.0",           null,           "1.0.4",      true },
+            new object[] { "1.0",           null,           "1.1",        true },
+
+            new object[] { "1.1",           null,           "1.0.4",      false },
+            new object[] { "1.1",           null,           "1.1",        true },
+            new object[] { "1.1",           null,           "1.1.1",      true },
+            new object[] { "1.1",           null,           "1.2",        true },
+
+            new object[] { null,            "1.0.4",        "1.0.5",      false },
+            new object[] { null,            "1.0.4",        "1.0.4",      true },
+            new object[] { null,            "1.0.4",        "1.0.3",      true },
+            new object[] { null,            "1.0.4",        "1.0",        true },
 
             new object[] { null,            "1.0",          "0.9",        true },
             new object[] { null,            "1.0",          "1.0",        true },

Tests/Core/Versioning/KspVersionBoundTests.cs → Tests/Core/Versioning/GameVersionBoundTests.cs

@@ -1,7 +1,9 @@
 using System;
-using CKAN.Versioning;
+
 using NUnit.Framework;
 
+using CKAN.Versioning;
+
 #pragma warning disable 414
 
 namespace Tests.Core.Versioning

Tests/Core/Versioning/KspVersionJsonConverterTests.cs → Tests/Core/Versioning/GameVersionJsonConverterTests.cs

@@ -1,8 +1,10 @@
 using System;
-using CKAN.Versioning;
+
 using Newtonsoft.Json;
 using NUnit.Framework;
 
+using CKAN.Versioning;
+
 #pragma warning disable 414
 
 namespace Tests.Core.Versioning

Tests/Core/Versioning/KspVersionRangeTests.cs → Tests/Core/Versioning/GameVersionRangeTests.cs

@@ -463,27 +463,25 @@ public void RangeFromVersionsEqualsRangeFromBounds()
         }
 
         [Test]
-        public void CtorThrowsOnNullLowerParameter()
+        public void Ctor_NullLowerParameter_Unbounded()
         {
             // Act
             // ReSharper disable once ObjectCreationAsStatement
-            TestDelegate act =
-                () => new GameVersionRange(null, new GameVersionBound(new GameVersion(1, 2, 3, 4), false));
+            var range = new GameVersionRange(null, new GameVersionBound(new GameVersion(1, 2, 3, 4), false));
 
             // Assert
-            Assert.That(act, Throws.Exception);
+            Assert.AreEqual(GameVersionBound.Unbounded, range.Lower);
         }
 
         [Test]
-        public void CtorThrowsOnNullUpperParameter()
+        public void Ctor_NullUpperParameter_Unbounded()
         {
             // Act
             // ReSharper disable once ObjectCreationAsStatement
-            TestDelegate act =
-                () => new GameVersionRange(new GameVersionBound(new GameVersion(1, 2, 3, 4), false), null);
+            var range = new GameVersionRange(new GameVersionBound(new GameVersion(1, 2, 3, 4), false), null);
 
             // Assert
-            Assert.That(act, Throws.Exception);
+            Assert.AreEqual(GameVersionBound.Unbounded, range.Upper);
         }
 
         [TestCaseSource("ToStringCases")]