Merge #3426 Override provides prompt with relationship property, chec…

…k first recommendation in any_of group
KSP-CKAN · Aug 8, 2021 · 241741f · 241741f
2 parents a833159 + 7dd593c
commit 241741f
Show file tree

Hide file tree

Showing 12 changed files with 60 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ All notable changes to this project will be documented in this file.
 - [GUI] Japanese Localization (#3394 by: utah239; reviewed: HebaruSan)
 - [Multiple] Match underscore in DLL to dash in identifier (#3412 by: HebaruSan; reviewed: DasSkelett)
 - [CLI] Add versions table option to ckan show command (#3414 by: HebaruSan; reviewed: DasSkelett)
+- [Multiple] Override provides prompt with relationship property, check first recommendation in any_of group (#3426 by: HebaruSan; reviewed: DasSkelett)
 
 ### Bugfixes
 

diff --git 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
@@ -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
@@ -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
@@ -118,7 +118,7 @@ public override void Run(ConsoleTheme theme, Action<ConsoleTheme> process = null
                     } catch (TooManyModsProvideKraken ex) {
 
                         ConsoleChoiceDialog<CkanModule> ch = new ConsoleChoiceDialog<CkanModule>(
-                            $"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/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<AnyOfRelationshipDescriptor>());
                     }

diff --git a/Core/ModuleInstaller.cs b/Core/ModuleInstaller.cs
@@ -1183,6 +1183,7 @@ out Dictionary<CkanModule, HashSet<string>> supporters
         )
         {
             Dictionary<CkanModule, List<string>> dependersIndex = getDependersIndex(sourceModules, registry, toInstall);
+            var instList = toInstall.ToList();
             recommendations = new Dictionary<CkanModule, Tuple<bool, List<string>>>();
             suggestions = new Dictionary<CkanModule, List<string>>();
             supporters = new Dictionary<CkanModule, HashSet<string>>();
@@ -1194,21 +1195,23 @@ out Dictionary<CkanModule, HashSet<string>> 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<string> dependers)
                             && (provider.IsDLC || CanInstall(RelationshipResolver.DependsOnlyOpts(),
-                                toInstall.ToList().Concat(new List<CkanModule>() { provider }).ToList(), registry)))
+                                instList.Concat(new List<CkanModule>() { provider }).ToList(), registry)))
                         {
                             dependersIndex.Remove(provider);
                             recommendations.Add(
                                 provider,
                                 new Tuple<bool, List<string>>(
-                                    !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<CkanModule, HashSet<string>> supporters
                             && !toInstall.Any(m => m.identifier == provider.identifier)
                             && dependersIndex.TryGetValue(provider, out List<string> dependers)
                             && (provider.IsDLC || CanInstall(RelationshipResolver.DependsOnlyOpts(),
-                                toInstall.ToList().Concat(new List<CkanModule>() { provider }).ToList(), registry)))
+                                instList.Concat(new List<CkanModule>() { provider }).ToList(), registry)))
                         {
                             dependersIndex.Remove(provider);
                             suggestions.Add(provider, dependers);
@@ -1267,7 +1270,7 @@ out Dictionary<CkanModule, HashSet<string>> supporters
             }
             supporters.RemoveWhere(kvp => !CanInstall(
                 RelationshipResolver.DependsOnlyOpts(),
-                toInstall.ToList().Concat(new List<CkanModule>() { kvp.Key }).ToList(),
+                instList.Concat(new List<CkanModule>() { kvp.Key }).ToList(),
                 registry
             ));
 

diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs
@@ -477,13 +477,13 @@ private void ResolveStanza(IEnumerable<RelationshipDescriptor> 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
@@ -149,20 +149,24 @@ public RegistryVersionNotSupportedKraken(int v, string reason = null, Exception
 
     public class TooManyModsProvideKraken : Kraken
     {
-        public List<CkanModule> modules;
-        public string requested;
+        public readonly List<CkanModule> modules;
+        public readonly string requested;
+        public readonly string choice_help_text;
 
-        public TooManyModsProvideKraken(string requested, List<CkanModule> modules, Exception innerException = null)
-            : base(FormatMessage(requested, modules), innerException)
+        public TooManyModsProvideKraken(string requested, List<CkanModule> 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<CkanModule> modules)
+        private static string FormatMessage(string requested, List<CkanModule> 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
@@ -28,6 +28,9 @@ public abstract List<CkanModule> 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;
         }
 
-
         /// <summary>
         /// A user friendly message for what versions satisfies this descriptor.
         /// </summary>
@@ -215,6 +216,14 @@ public class AnyOfRelationshipDescriptor : RelationshipDescriptor
         [JsonConverter(typeof(JsonRelationshipConverter))]
         public List<RelationshipDescriptor> any_of;
 
+        public static readonly List<string> ForbiddenPropertyNames = new List<string>()
+        {
+            "name",
+            "version",
+            "min_version",
+            "max_version"
+        };
+
         public override bool WithinBounds(CkanModule otherModule)
         {
             return any_of?.Any(r => r.WithinBounds(otherModule))
@@ -236,7 +245,7 @@ public override List<CkanModule> LatestAvailableWithProvides(
             IEnumerable<CkanModule> 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)

diff --git a/GUI/Controls/ChooseProvidedMods.cs b/GUI/Controls/ChooseProvidedMods.cs
@@ -13,14 +13,11 @@ public ChooseProvidedMods()
             InitializeComponent();
         }
 
-        public void LoadProviders(string requested, List<CkanModule> modules, NetModuleCache cache)
+        public void LoadProviders(string message, List<CkanModule> 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
@@ -224,7 +224,7 @@ out Dictionary<CkanModule, HashSet<string>> 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