From a4d65afa679b4848ebfa20d83cf558029f8e9878 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Sun, 1 Aug 2021 19:55:38 -0500 Subject: [PATCH 1/4] Check first recommends in any_of list --- Core/ModuleInstaller.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Core/ModuleInstaller.cs b/Core/ModuleInstaller.cs index 862d09aac3..99dcb9ae7c 100644 --- a/Core/ModuleInstaller.cs +++ b/Core/ModuleInstaller.cs @@ -1183,6 +1183,7 @@ out Dictionary> supporters ) { Dictionary> dependersIndex = getDependersIndex(sourceModules, registry, toInstall); + var instList = toInstall.ToList(); recommendations = new Dictionary>>(); suggestions = new Dictionary>(); supporters = new Dictionary>(); @@ -1194,21 +1195,23 @@ out Dictionary> supporters registry, ksp.VersionCriteria() ); + int i = 0; foreach (CkanModule provider in providers) { if (!registry.IsInstalled(provider.identifier) && !toInstall.Any(m => m.identifier == provider.identifier) && dependersIndex.TryGetValue(provider, out List dependers) && (provider.IsDLC || CanInstall(RelationshipResolver.DependsOnlyOpts(), - toInstall.ToList().Concat(new List() { provider }).ToList(), registry))) + instList.Concat(new List() { provider }).ToList(), registry))) { dependersIndex.Remove(provider); recommendations.Add( provider, new Tuple>( - !provider.IsDLC && (providers.Count <= 1 || provider.identifier == (rel as ModuleRelationshipDescriptor)?.name), + !provider.IsDLC && (i == 0 || provider.identifier == (rel as ModuleRelationshipDescriptor)?.name), dependers) ); + ++i; } } } @@ -1227,7 +1230,7 @@ out Dictionary> supporters && !toInstall.Any(m => m.identifier == provider.identifier) && dependersIndex.TryGetValue(provider, out List dependers) && (provider.IsDLC || CanInstall(RelationshipResolver.DependsOnlyOpts(), - toInstall.ToList().Concat(new List() { provider }).ToList(), registry))) + instList.Concat(new List() { provider }).ToList(), registry))) { dependersIndex.Remove(provider); suggestions.Add(provider, dependers); @@ -1267,7 +1270,7 @@ out Dictionary> supporters } supporters.RemoveWhere(kvp => !CanInstall( RelationshipResolver.DependsOnlyOpts(), - toInstall.ToList().Concat(new List() { kvp.Key }).ToList(), + instList.Concat(new List() { kvp.Key }).ToList(), registry )); From 0b04b5f7aa2898440723b869dc3d955bc8184841 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Sun, 1 Aug 2021 17:58:45 -0500 Subject: [PATCH 2/4] Show relationship comment in provides prompts --- CKAN.schema | 8 ++++++++ Cmdline/Action/Install.cs | 22 +++++++--------------- Cmdline/Action/Upgrade.cs | 2 +- ConsoleUI/InstallScreen.cs | 2 +- Core/Relationships/RelationshipResolver.cs | 4 ++-- Core/Types/Kraken.cs | 22 +++++++++++++--------- Core/Types/RelationshipDescriptor.cs | 5 +++-- GUI/Controls/ChooseProvidedMods.cs | 7 ++----- GUI/Main/MainInstall.cs | 2 +- 9 files changed, 38 insertions(+), 36 deletions(-) diff --git a/CKAN.schema b/CKAN.schema index 3f3b753c16..474db9dac4 100644 --- a/CKAN.schema +++ b/CKAN.schema @@ -384,6 +384,10 @@ "max_version" : { "description" : "Optional maximum version", "$ref" : "#/definitions/version" + }, + "choice_help_text": { + "description" : "Optional help text shown when user has to choose among multiple modules", + "type" : "string" } }, "required" : [ "name" ] @@ -394,6 +398,10 @@ "any_of": { "description": "List of modules that can satisfy this relationship", "$ref": "#/definitions/relationship" + }, + "choice_help_text": { + "description" : "Optional help text shown when user has to choose among multiple modules", + "type" : "string" } }, "required": [ "any_of" ] diff --git a/Cmdline/Action/Install.cs b/Cmdline/Action/Install.cs index 4603cd8d4f..0f20276b20 100644 --- a/Cmdline/Action/Install.cs +++ b/Cmdline/Action/Install.cs @@ -174,33 +174,25 @@ public int RunCommand(CKAN.GameInstance ksp, object raw_options) } catch (TooManyModsProvideKraken ex) { - // Request the user selects one of the mods. - string[] mods = new string[ex.modules.Count]; - - for (int i = 0; i < ex.modules.Count; i++) - { - mods[i] = String.Format("{0} ({1})", ex.modules[i].identifier, ex.modules[i].name); - } - - string message = String.Format("Too many mods provide {0}. Please pick from the following:\r\n", ex.requested); - + // Request the user selects one of the mods int result; - try { - result = user.RaiseSelectionDialog(message, mods); + result = user.RaiseSelectionDialog( + ex.Message, + ex.modules + .Select(m => string.Format("{0} ({1})", m.identifier, m.name)) + .ToArray()); } catch (Kraken e) { user.RaiseMessage(e.Message); - return Exit.ERROR; } if (result < 0) { - user.RaiseMessage(String.Empty); // Looks tidier. - + user.RaiseMessage(""); return Exit.ERROR; } diff --git a/Cmdline/Action/Upgrade.cs b/Cmdline/Action/Upgrade.cs index b0bac76861..246a65ecd0 100644 --- a/Cmdline/Action/Upgrade.cs +++ b/Cmdline/Action/Upgrade.cs @@ -230,7 +230,7 @@ private static void UpgradeModules( catch (TooManyModsProvideKraken k) { int choice = user.RaiseSelectionDialog( - $"Choose a module to provide {k.requested}:", + k.Message, k.modules.Select(m => $"{m.identifier} ({m.name})").ToArray()); if (choice < 0) { diff --git a/ConsoleUI/InstallScreen.cs b/ConsoleUI/InstallScreen.cs index 5707b30310..d8233a2230 100644 --- a/ConsoleUI/InstallScreen.cs +++ b/ConsoleUI/InstallScreen.cs @@ -118,7 +118,7 @@ public override void Run(ConsoleTheme theme, Action process = null } catch (TooManyModsProvideKraken ex) { ConsoleChoiceDialog ch = new ConsoleChoiceDialog( - $"Module {ex.requested} is provided by multiple modules. Which would you like to install?", + ex.Message, "Name", ex.modules, (CkanModule mod) => mod.ToString() diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index 8b10b0febf..4253df8162 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -477,13 +477,13 @@ private void ResolveStanza(IEnumerable stanza, Selection { //We still have either nothing, or too many to pick from //Just throw the TMP now - throw new TooManyModsProvideKraken(descriptor.ToString(), candidates); + throw new TooManyModsProvideKraken(descriptor.ToString(), candidates, descriptor.choice_help_text); } candidates[0] = provide.First(); } else { - throw new TooManyModsProvideKraken(descriptor.ToString(), candidates); + throw new TooManyModsProvideKraken(descriptor.ToString(), candidates, descriptor.choice_help_text); } } diff --git a/Core/Types/Kraken.cs b/Core/Types/Kraken.cs index 7ba2b8f5c9..2c8b2140a4 100644 --- a/Core/Types/Kraken.cs +++ b/Core/Types/Kraken.cs @@ -149,20 +149,24 @@ public RegistryVersionNotSupportedKraken(int v, string reason = null, Exception public class TooManyModsProvideKraken : Kraken { - public List modules; - public string requested; + public readonly List modules; + public readonly string requested; + public readonly string choice_help_text; - public TooManyModsProvideKraken(string requested, List modules, Exception innerException = null) - : base(FormatMessage(requested, modules), innerException) + public TooManyModsProvideKraken(string requested, List modules, string choice_help_text = null, Exception innerException = null) + : base(FormatMessage(requested, modules, choice_help_text), innerException) { - this.modules = modules; - this.requested = requested; + this.requested = requested; + this.modules = modules; + this.choice_help_text = choice_help_text; } - internal static string FormatMessage(string requested, List modules) + private static string FormatMessage(string requested, List modules, string choice_help_text = null) { - string oops = string.Format("Too many mods provide {0}:\r\n", requested); - return oops + String.Join("\r\n", modules.Select(m => $"* {m}")); + return choice_help_text + ?? string.Format( + "Module {0} is provided by more than one available module. Please choose one of the following:", + requested); } } diff --git a/Core/Types/RelationshipDescriptor.cs b/Core/Types/RelationshipDescriptor.cs index f4eb06a2d9..e69395cc47 100644 --- a/Core/Types/RelationshipDescriptor.cs +++ b/Core/Types/RelationshipDescriptor.cs @@ -28,6 +28,9 @@ public abstract List LatestAvailableWithProvides( public abstract bool StartsWith(string prefix); + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + public string choice_help_text; + // virtual ToString() already present in 'object' } @@ -42,7 +45,6 @@ public class ModuleRelationshipDescriptor : RelationshipDescriptor [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public ModuleVersion version; - public override bool WithinBounds(CkanModule otherModule) { return otherModule.ProvidesList.Contains(name) @@ -169,7 +171,6 @@ public override bool StartsWith(string prefix) return name.IndexOf(prefix, StringComparison.CurrentCultureIgnoreCase) == 0; } - /// /// A user friendly message for what versions satisfies this descriptor. /// diff --git a/GUI/Controls/ChooseProvidedMods.cs b/GUI/Controls/ChooseProvidedMods.cs index ed38845137..33c38de114 100644 --- a/GUI/Controls/ChooseProvidedMods.cs +++ b/GUI/Controls/ChooseProvidedMods.cs @@ -13,14 +13,11 @@ public ChooseProvidedMods() InitializeComponent(); } - public void LoadProviders(string requested, List modules, NetModuleCache cache) + public void LoadProviders(string message, List modules, NetModuleCache cache) { Util.Invoke(this, () => { - ChooseProvidedModsLabel.Text = String.Format( - Properties.Resources.MainInstallProvidedBy, - requested - ); + ChooseProvidedModsLabel.Text = message; ChooseProvidedModsListView.Items.Clear(); ChooseProvidedModsListView.Items.AddRange(modules diff --git a/GUI/Main/MainInstall.cs b/GUI/Main/MainInstall.cs index f115842c06..344c89236b 100644 --- a/GUI/Main/MainInstall.cs +++ b/GUI/Main/MainInstall.cs @@ -224,7 +224,7 @@ out Dictionary> supporters { // Prompt user to choose which mod to use tabController.ShowTab("ChooseProvidedModsTabPage", 3); - ChooseProvidedMods.LoadProviders(k.requested, k.modules, Manager.Cache); + ChooseProvidedMods.LoadProviders(k.Message, k.modules, Manager.Cache); tabController.SetTabLock(true); CkanModule chosen = ChooseProvidedMods.Wait(); // Close the selection prompt From 36a49f0bb70a8a815943331ea0a3656be58be015 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Sun, 8 Aug 2021 08:27:56 -0500 Subject: [PATCH 3/4] Don't ban all other properties with any_of --- Core/Converters/JsonRelationshipConverter.cs | 7 +++++-- Core/Types/RelationshipDescriptor.cs | 8 ++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Core/Converters/JsonRelationshipConverter.cs b/Core/Converters/JsonRelationshipConverter.cs index d7ea89deb0..e66a5180e6 100644 --- a/Core/Converters/JsonRelationshipConverter.cs +++ b/Core/Converters/JsonRelationshipConverter.cs @@ -26,9 +26,12 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist if (child["any_of"] != null) { // Catch confused/invalid metadata - if (child.Properties().Count() > 1) + foreach (string forbiddenPropertyName in AnyOfRelationshipDescriptor.ForbiddenPropertyNames) { - throw new Kraken("`any_of` should not be combined with other properties"); + if (child.Property(forbiddenPropertyName) != null) + { + throw new Kraken($"`any_of` should not be combined with `{forbiddenPropertyName}`"); + } } rels.Add(child.ToObject()); } diff --git a/Core/Types/RelationshipDescriptor.cs b/Core/Types/RelationshipDescriptor.cs index e69395cc47..142c422efc 100644 --- a/Core/Types/RelationshipDescriptor.cs +++ b/Core/Types/RelationshipDescriptor.cs @@ -216,6 +216,14 @@ public class AnyOfRelationshipDescriptor : RelationshipDescriptor [JsonConverter(typeof(JsonRelationshipConverter))] public List any_of; + public static readonly List ForbiddenPropertyNames = new List() + { + "name", + "version", + "min_version", + "max_version" + }; + public override bool WithinBounds(CkanModule otherModule) { return any_of?.Any(r => r.WithinBounds(otherModule)) From 7dd593cd4e79b4170936b62dbd7fa1ea3c4edcc5 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Sun, 8 Aug 2021 10:00:08 -0500 Subject: [PATCH 4/4] Deduplicate modules in any_of --- Core/Types/RelationshipDescriptor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Core/Types/RelationshipDescriptor.cs b/Core/Types/RelationshipDescriptor.cs index 142c422efc..a53c72c44b 100644 --- a/Core/Types/RelationshipDescriptor.cs +++ b/Core/Types/RelationshipDescriptor.cs @@ -245,7 +245,7 @@ public override List LatestAvailableWithProvides( IEnumerable toInstall = null ) { - return any_of?.SelectMany(r => r.LatestAvailableWithProvides(registry, crit, installed, toInstall)).ToList(); + return any_of?.SelectMany(r => r.LatestAvailableWithProvides(registry, crit, installed, toInstall)).Distinct().ToList(); } public override bool Equals(RelationshipDescriptor other)