diff --git a/CHANGELOG.md b/CHANGELOG.md index d6f6b1ff26..77b4b2cd3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ All notable changes to this project will be documented in this file. - [GUI] I18n updates from Crowdin (#4050 by: HebaruSan) - [Multiple] Better version specific relationships at install and upgrade (#4023 by: HebaruSan) - [GUI] Proportional, granular progress updates for installing (#4055 by: HebaruSan) +- [GUI] Modpack compatibility prompt, GameComparator clean-up (#4056 by: HebaruSan) ### Bugfixes diff --git a/Core/ServiceLocator.cs b/Core/ServiceLocator.cs index 1ba9fa817f..1561cb80b3 100644 --- a/Core/ServiceLocator.cs +++ b/Core/ServiceLocator.cs @@ -37,7 +37,7 @@ private static void Init() { var builder = new ContainerBuilder(); - builder.RegisterType() + builder.RegisterType() .As(); builder.RegisterType() diff --git a/Core/Types/GameComparator/BaseGameComparator.cs b/Core/Types/GameComparator/BaseGameComparator.cs index 94870805da..9fe8429c3f 100644 --- a/Core/Types/GameComparator/BaseGameComparator.cs +++ b/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); } } diff --git a/Core/Types/GameComparator/GrasGameComparator.cs b/Core/Types/GameComparator/GrasGameComparator.cs deleted file mode 100644 index 2216037d27..0000000000 --- a/Core/Types/GameComparator/GrasGameComparator.cs +++ /dev/null @@ -1,41 +0,0 @@ -using CKAN.Versioning; - -namespace CKAN -{ - /// - /// Test to see if the user's game is "Generally Recognised As Safe" (GRAS) with a given mod, - /// with extra understanding of which KSP versions are "safe" (ie: 1.0.5 mostly works with 1.0.4 mods). - /// If the mod has `ksp_version_strict` set then this is identical to strict checking. - /// - public class GrasGameComparator : BaseGameComparator - { - private static readonly StrictGameComparator strict = new StrictGameComparator(); - private static readonly GameVersion v103 = GameVersion.Parse("1.0.3"); - private static readonly GameVersion v104 = GameVersion.Parse("1.0.4"); - - public override bool Compatible(GameVersionCriteria gameVersionCriteria, CkanModule module) - { - // If it's strictly compatible, then it's compatible. - if (strict.Compatible(gameVersionCriteria, module)) - { - return true; - } - - // If we're in strict mode, and it's not strictly compatible, then it's - // not compatible. - return !module.ksp_version_strict - && base.Compatible(gameVersionCriteria, module); - } - - public override bool SingleVersionsCompatible (GameVersion gameVersion, CkanModule module) - { - - // Otherwise, check if it's "generally recognise as safe". - - // If we're running KSP 1.0.4, then allow the mod to run if we would have - // considered it compatible under 1.0.3 (as 1.0.4 was "just a hotfix"). - return gameVersion.Equals(v104) - && strict.SingleVersionsCompatible(v103, module); - } - } -} diff --git a/Core/Types/GameComparator/YoyoGameComparator.cs b/Core/Types/GameComparator/YoyoGameComparator.cs deleted file mode 100644 index ac37fa2d7d..0000000000 --- a/Core/Types/GameComparator/YoyoGameComparator.cs +++ /dev/null @@ -1,16 +0,0 @@ -using CKAN.Versioning; - -namespace CKAN -{ - /// - /// You're On Your Own (YOYO) game compatibility comparison. - /// This claims everything is compatible with everything. - /// - public class YoyoGameComparator : IGameComparator - { - public bool Compatible(GameVersionCriteria gameVersion, CkanModule module) - { - return true; - } - } -} \ No newline at end of file diff --git a/Core/Versioning/GameVersionRange.cs b/Core/Versioning/GameVersionRange.cs index 483441706f..648d2a3c9e 100644 --- a/Core/Versioning/GameVersionRange.cs +++ b/Core/Versioning/GameVersionRange.cs @@ -16,18 +16,8 @@ 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); } @@ -35,7 +25,7 @@ public GameVersionRange(GameVersionBound lower, GameVersionBound upper) 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) { diff --git a/GUI/Main/Main.cs b/GUI/Main/Main.cs index 87bda9f2f0..12202dcc87 100644 --- a/GUI/Main/Main.cs +++ b/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(), diff --git a/GUI/Properties/Resources.resx b/GUI/Properties/Resources.resx index 9b4259d5a0..308d301e84 100644 --- a/GUI/Properties/Resources.resx +++ b/GUI/Properties/Resources.resx @@ -200,13 +200,18 @@ This means that CKAN forgot about all your installed mods, but they are still in {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? + Install + Cancel + 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? + Add compatible versions + Keep current compatibility 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. - Install - Cancel Update selected by user to version {0}. Re-install (metadata changed) Re-install (user requested) diff --git a/Tests/Core/Types/GameComparator.cs b/Tests/Core/Types/GameComparator.cs index 20191af256..725eeffc7e 100644 --- a/Tests/Core/Types/GameComparator.cs +++ b/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 }, diff --git a/Tests/Core/Versioning/KspVersionBoundTests.cs b/Tests/Core/Versioning/GameVersionBoundTests.cs similarity index 99% rename from Tests/Core/Versioning/KspVersionBoundTests.cs rename to Tests/Core/Versioning/GameVersionBoundTests.cs index d88087b77f..a9e5409cf7 100644 --- a/Tests/Core/Versioning/KspVersionBoundTests.cs +++ b/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 diff --git a/Tests/Core/Versioning/KspVersionJsonConverterTests.cs b/Tests/Core/Versioning/GameVersionJsonConverterTests.cs similarity index 99% rename from Tests/Core/Versioning/KspVersionJsonConverterTests.cs rename to Tests/Core/Versioning/GameVersionJsonConverterTests.cs index 74639f4a23..9af0a59852 100644 --- a/Tests/Core/Versioning/KspVersionJsonConverterTests.cs +++ b/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 diff --git a/Tests/Core/Versioning/KspVersionRangeTests.cs b/Tests/Core/Versioning/GameVersionRangeTests.cs similarity index 98% rename from Tests/Core/Versioning/KspVersionRangeTests.cs rename to Tests/Core/Versioning/GameVersionRangeTests.cs index b106c70853..4823a47811 100644 --- a/Tests/Core/Versioning/KspVersionRangeTests.cs +++ b/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")] diff --git a/Tests/Core/Versioning/KspVersionTests.cs b/Tests/Core/Versioning/GameVersionTests.cs similarity index 99% rename from Tests/Core/Versioning/KspVersionTests.cs rename to Tests/Core/Versioning/GameVersionTests.cs index 7563e37daa..2db97e0c6f 100644 --- a/Tests/Core/Versioning/KspVersionTests.cs +++ b/Tests/Core/Versioning/GameVersionTests.cs @@ -1,7 +1,9 @@ using System; -using CKAN.Versioning; + using NUnit.Framework; +using CKAN.Versioning; + #pragma warning disable 219, 414 namespace Tests.Core.Versioning diff --git a/Tests/Core/Versioning/Version.cs b/Tests/Core/Versioning/Version.cs index 617c9413e5..6b4c5e0f3a 100644 --- a/Tests/Core/Versioning/Version.cs +++ b/Tests/Core/Versioning/Version.cs @@ -1,6 +1,7 @@ -using CKAN.Versioning; using NUnit.Framework; +using CKAN.Versioning; + namespace Tests.Core.Versioning { [TestFixture]