Merge #3478 Properly determine the game when cloning instances

CHANGELOG.md

@@ -32,6 +32,7 @@ All notable changes to this project will be documented in this file.
 - [GUI] Suppress filter updates for unchanged semantic search meaning (#3435 by: HebaruSan; reviewed: DasSkelett)
 - [GUI] Use CRLF for resx files (#3471 by: HebaruSan; reviewed: DasSkelett)
 - [Core] Case insensitive installed file lookup on Windows (#3479 by: HebaruSan; reviewed: DasSkelett)
+- [Core] Properly determine the game when cloning instances (#3478 by: DasSkelett; reviewed: HebaruSan)
 
 ### Internal
 

Cmdline/ConsoleUser.cs

@@ -78,7 +78,7 @@ public bool RaiseYesNoDialog(string question)
         /// The output is index 0 based.
         /// To supply a default option, make the first option an integer indicating the index of it.
         /// </summary>
-        /// <returns>The selection dialog</returns>
+        /// <returns>The selected index or -1 if cancelled</returns>
         /// <param name="message">Message</param>
         /// <param name="args">Array of available options</param>
         public int RaiseSelectionDialog(string message, params object[] args)

Core/GameInstanceManager.cs

@@ -22,6 +22,11 @@ public class GameInstanceManager : IDisposable
             new KerbalSpaceProgram()
         };
 
+        /// <summary>
+        /// An IUser object for user interaction.
+        /// It is initialized during the startup with a ConsoleUser,
+        /// do not use in functions that could be called by the GUI.
+        /// </summary>
         public IUser User { get; set; }
         public IConfiguration Configuration { get; set; }
         public GameInstance CurrentInstance { get; set; }
@@ -69,7 +74,7 @@ public GameInstanceManager(IUser user, IConfiguration configuration = null)
         }
 
         /// <summary>
-        /// Returns the preferred KSP instance, or null if none can be found.
+        /// Returns the preferred game instance, or null if none can be found.
         ///
         /// This works by checking to see if we're in a KSP dir first, then the
         /// config for an autostart instance, then will try to auto-populate
@@ -170,45 +175,40 @@ public GameInstance FindAndRegisterDefaultInstance()
         }
 
         /// <summary>
-        /// Adds a KSP instance to config.
-        /// Returns the resulting KSP object.
+        /// Adds a game instance to config.
         /// </summary>
-        public GameInstance AddInstance(GameInstance ksp_instance)
+        /// <returns>The resulting GameInstance object</returns>
+        /// <exception cref="NotKSPDirKraken">Thrown if the instance is not a valid game instance.</exception>
+        public GameInstance AddInstance(GameInstance instance)
         {
-            if (ksp_instance.Valid)
+            if (instance.Valid)
             {
-                string name = ksp_instance.Name;
-                instances.Add(name, ksp_instance);
+                string name = instance.Name;
+                instances.Add(name, instance);
                 Configuration.SetRegistryToInstances(instances);
             }
             else
             {
-                throw new NotKSPDirKraken(ksp_instance.GameDir());
+                throw new NotKSPDirKraken(instance.GameDir());
             }
-            return ksp_instance;
+            return instance;
         }
 
+        /// <summary>
+        /// Adds a game instance to config.
+        /// </summary>
+        /// <param name="path">The path of the instance</param>
+        /// <param name="name">The name of the instance</param>
+        /// <param name="user">IUser object for interaction</param>
+        /// <returns>The resulting GameInstance object</returns>
+        /// <exception cref="NotKSPDirKraken">Thrown if the instance is not a valid game instance.</exception>
         public GameInstance AddInstance(string path, string name, IUser user)
         {
-            var matchingGames = knownGames
-                .Where(g => g.GameInFolder(new DirectoryInfo(path)))
-                .ToList();
-            switch (matchingGames.Count)
-            {
-                case 0:
-                    throw new NotKSPDirKraken(path);
-
-                case 1:
-                    return AddInstance(new GameInstance(
-                        matchingGames.First(),
-                        path, name, user
-                    ));
+            var game = DetermineGame(new DirectoryInfo(path), user);
+            if (game == null)
+                return null;
 
-                default:
-                    // TODO: Prompt user to choose
-                    return null;
-
-            }
+            return AddInstance(new GameInstance(game, path, name, user));
         }
 
         /// <summary>
@@ -612,5 +612,32 @@ public static bool IsGameInstanceDir(DirectoryInfo path)
             return knownGames.Any(g => g.GameInFolder(path));
         }
 
+        /// <summary>
+        /// Tries to determine the game that is installed at the given path
+        /// </summary>
+        /// <param name="path">A DirectoryInfo of the path to check</param>
+        /// <param name="user">IUser object for interaction</param>
+        /// <returns>An instance of the matching game or null if the user cancelled</returns>
+        /// <exception cref="NotKSPDirKraken">Thrown when no games found</exception>
+        public IGame DetermineGame(DirectoryInfo path, IUser user)
+        {
+            var matchingGames = knownGames.Where(g => g.GameInFolder(path)).ToList();
+            switch (matchingGames.Count)
+            {
+                case 0:
+                    throw new NotKSPDirKraken(path.FullName);
+
+                case 1:
+                    return matchingGames.First();
+
+                default:
+                    // Prompt user to choose
+                    int selection = user.RaiseSelectionDialog(
+                        $"Please select the game that is installed at {path.FullName.Replace('/', Path.DirectorySeparatorChar)}",
+                        matchingGames.Select(g => g.ShortName).ToArray());
+                    return selection >= 0 ? matchingGames[selection] : null;
+            }
+        }
+
     }
 }

Core/User.cs

@@ -10,6 +10,13 @@ public interface IUser
         bool Headless { get; }
 
         bool RaiseYesNoDialog(string question);
+
+        /// <summary>
+        /// Ask the user to select one of the elements of the array.
+        /// The output is index 0 based.
+        /// To supply a default option, make the first option an integer indicating the index of it.
+        /// </summary>
+        /// <returns>The index of the item selected from the array or -1 if cancelled</returns>
         int  RaiseSelectionDialog(string message, params object[] args);
         void RaiseError(string message, params object[] args);
 

GUI/Dialogs/CloneFakeGameDialog.cs

@@ -53,7 +53,7 @@ private void comboBoxKnownInstance_SelectedIndexChanged(object sender, EventArgs
             string sel = comboBoxKnownInstance.SelectedItem as string;
             textBoxClonePath.Text = string.IsNullOrEmpty(sel)
                 ? ""
-                : manager.Instances[sel].GameDir();
+                : manager.Instances[sel].GameDir().Replace('/', Path.DirectorySeparatorChar);
         }
 
         /// <summary>
@@ -113,6 +113,7 @@ private void radioButton_CheckedChanged(object sender, EventArgs e)
         /// </summary>
         private async void buttonOK_Click(object sender, EventArgs e)
         {
+            string existingPath = textBoxClonePath.Text;
             string newName = textBoxNewName.Text;
             string newPath = textBoxNewPath.Text;
 
@@ -144,17 +145,26 @@ private async void buttonOK_Click(object sender, EventArgs e)
 
                 try
                 {
-                    await Task.Run(() =>
+                    GameInstance instanceToClone = null;
+                    if (!manager.Instances.TryGetValue(comboBoxKnownInstance.SelectedItem as string, out instanceToClone)
+                        || existingPath != instanceToClone.GameDir().Replace('/', Path.DirectorySeparatorChar))
                     {
-                        GameInstance sourceInstance = manager.Instances.Values
-                            .FirstOrDefault(i => i.GameDir() == textBoxClonePath.Text);
-                        GameInstance instanceToClone = new GameInstance(
-                            sourceInstance.game,
-                            textBoxClonePath.Text,
+                        IGame sourceGame = manager.DetermineGame(new DirectoryInfo(existingPath), user);
+                        if (sourceGame == null)
+                        {
+                            // User cancelled, let them try again
+                            reactivateDialog();
+                            return;
+                        }
+                        instanceToClone = new GameInstance(
+                            sourceGame,
+                            existingPath,
                             "irrelevant",
                             user
                         );
-
+                    }
+                    await Task.Run(() =>
+                    {
                         if (instanceToClone.Valid)
                         {
                             manager.CloneInstance(instanceToClone, newName, newPath);
@@ -173,13 +183,13 @@ await Task.Run(() =>
                 }
                 catch (NotKSPDirKraken kraken)
                 {
-                    user.RaiseError(string.Format(Properties.Resources.CloneFakeKspDialogInstanceNotValid, kraken.path));
+                    user.RaiseError(string.Format(Properties.Resources.CloneFakeKspDialogInstanceNotValid, kraken.path.Replace('/', Path.DirectorySeparatorChar)));
                     reactivateDialog();
                     return;
                 }
                 catch (PathErrorKraken kraken)
                 {
-                    user.RaiseError(string.Format(Properties.Resources.CloneFakeKspDialogDestinationNotEmpty, kraken.path));
+                    user.RaiseError(string.Format(Properties.Resources.CloneFakeKspDialogDestinationNotEmpty, kraken.path.Replace('/', Path.DirectorySeparatorChar)));
                     reactivateDialog();
                     return;
                 }

GUI/Dialogs/SelectionDialog.cs

@@ -80,7 +80,7 @@ public int ShowSelectionDialog (string message, params object[] args)
                 if (defaultSelection == i)
                 {
                     Util.Invoke(OptionsList, () => OptionsList.Items.Add(String.Concat(args[i].ToString(), "  -- Default")));
-                    
+
                 }
                 else
                 {

GUI/Main/Main.cs

@@ -83,7 +83,16 @@ public Main(string[] cmdlineArgs, GameInstanceManager mgr, bool showConsole)
             Instance = this;
 
             currentUser = new GUIUser(this, this.Wait);
-            manager = mgr ?? new GameInstanceManager(currentUser);
+            if (mgr != null)
+            {
+                // With a working GUI, assign a GUIUser to the GameInstanceManager to replace the ConsoleUser
+                mgr.User = currentUser;
+                manager = mgr;
+            }
+            else
+            {
+                manager = new GameInstanceManager(currentUser);
+            }
 
             controlFactory = new ControlFactory();