From fcb2c42e2aa2055bd22c1536052fac442b6899c2 Mon Sep 17 00:00:00 2001
From: Falko Galperin <falko1@uni-bremen.de>
Date: Sat, 16 Nov 2024 16:44:24 +0100
Subject: [PATCH] Ask user for confirmation before rebinding key mapping (fix
 #683)

---
 Assets/SEE/Controls/KeyActions/KeyMap.cs | 14 +++-
 Assets/SEE/UI/SettingsMenu.cs            | 90 ++++++++++++++----------
 2 files changed, 64 insertions(+), 40 deletions(-)

diff --git a/Assets/SEE/Controls/KeyActions/KeyMap.cs b/Assets/SEE/Controls/KeyActions/KeyMap.cs
index 16a47c4fc0..338487db9e 100644
--- a/Assets/SEE/Controls/KeyActions/KeyMap.cs
+++ b/Assets/SEE/Controls/KeyActions/KeyMap.cs
@@ -134,8 +134,8 @@ internal void ResetKeyCode(KeyActionDescriptor descriptor, KeyCode keyCode)
         {
             if (TryGetKeyAction(keyCode, out KeyAction action))
             {
-                throw new($"Cannot register key {keyCode} for {descriptor.Name}."
-               + $" Key {keyCode} is already bound to {action}.\n");
+                throw new KeyBindingsExistsException($"Cannot register key {keyCode} for {descriptor.Name}."
+                                                     + $" Key {keyCode} is already bound to {action}.\n");
             }
             else
             {
@@ -351,5 +351,13 @@ IEnumerator IEnumerable.GetEnumerator()
             }
         }
         #endregion
+
+        /// <summary>
+        /// An exception indicating that the attempted rebind would result in a duplicate key binding.
+        /// </summary>
+        public class KeyBindingsExistsException : Exception
+        {
+            public KeyBindingsExistsException(string message) : base(message) { }
+        };
     }
-}
\ No newline at end of file
+}
diff --git a/Assets/SEE/UI/SettingsMenu.cs b/Assets/SEE/UI/SettingsMenu.cs
index 8ce5e9c412..6aff99b3e9 100644
--- a/Assets/SEE/UI/SettingsMenu.cs
+++ b/Assets/SEE/UI/SettingsMenu.cs
@@ -1,13 +1,17 @@
-using UnityEngine.UI;
-using UnityEngine;
-using TMPro;
-using SEE.Controls;
-using SEE.Utils;
-using SEE.GO;
 using System;
 using System.Collections.Generic;
-using SEE.UI.Notification;
+using System.Linq;
+using Cysharp.Threading.Tasks;
+using SEE.Controls;
 using SEE.Controls.KeyActions;
+using SEE.GO;
+using SEE.UI.Menu;
+using SEE.UI.Notification;
+using SEE.Utils;
+using TMPro;
+using UnityEditor;
+using UnityEngine;
+using UnityEngine.UI;
 
 namespace SEE.UI
 {
@@ -19,20 +23,20 @@ public class SettingsMenu : PlatformDependentComponent
         /// <summary>
         /// Prefab for the <see cref="SettingsMenu"/>.
         /// </summary>
-        private string SettingsPrefab => UIPrefabFolder + "SettingsMenu";
+        private const string settingsPrefab = UIPrefabFolder + "SettingsMenu";
 
         /// <summary>
-        /// Prefab for the KeyBindingContent.
+        /// Prefab for the keyBindingContent.
         /// </summary>
-        private string KeyBindingContent => UIPrefabFolder + "KeyBindingContent";
+        private const string keyBindingContent = UIPrefabFolder + "KeyBindingContent";
 
         /// <summary>
         /// Prefab for the ScrollView.
         /// </summary>
-        private string ScrollPrefab => UIPrefabFolder + "ScrollPrefab";
+        private const string scrollPrefab = UIPrefabFolder + "ScrollPrefab";
 
         /// <summary>
-        /// The game object instantiated for the <see cref="SettingsPrefab"/>.
+        /// The game object instantiated for the <see cref="settingsPrefab"/>.
         /// </summary>
         private GameObject settingsMenuGameObject;
 
@@ -44,14 +48,14 @@ public class SettingsMenu : PlatformDependentComponent
         private readonly Dictionary<string, TextMeshProUGUI> shortNameOfBindingToLabel = new();
 
         /// <summary>
-        /// Sets the <see cref="KeyBindingContent"/> and adds the onClick event
+        /// Sets the <see cref="keyBindingContent"/> and adds the onClick event
         /// <see cref="ExitGame"/> to the ExitButton.
         /// </summary>
         protected override void StartDesktop()
         {
             KeyBindings.LoadKeyBindings();
             // instantiates the SettingsMenu
-            settingsMenuGameObject = PrefabInstantiator.InstantiatePrefab(SettingsPrefab, Canvas.transform, false);
+            settingsMenuGameObject = PrefabInstantiator.InstantiatePrefab(settingsPrefab, Canvas.transform, false);
             // adds the ExitGame method to the exit button
             settingsMenuGameObject.transform.Find("ExitPanel/Buttons/Content/Exit").gameObject.MustGetComponent<Button>()
                                   .onClick.AddListener(ExitGame);
@@ -60,7 +64,7 @@ protected override void StartDesktop()
             foreach (var group in KeyBindings.AllBindings())
             {
                 // Creates a list of keybinding descriptions for the given category.
-                GameObject scrollView = PrefabInstantiator.InstantiatePrefab(ScrollPrefab, Canvas.transform, false).transform.gameObject;
+                GameObject scrollView = PrefabInstantiator.InstantiatePrefab(scrollPrefab, Canvas.transform, false).transform.gameObject;
                 scrollView.transform.SetParent(settingsMenuGameObject.transform.Find("KeybindingsPanel/KeybindingsText/Viewport/Content"));
                 // set the titles of the scrollViews to the scopes
                 TextMeshProUGUI groupTitle = scrollView.transform.Find("Group").gameObject.MustGetComponent<TextMeshProUGUI>();
@@ -68,7 +72,7 @@ protected override void StartDesktop()
 
                 foreach ((_, KeyActionDescriptor descriptor) in group)
                 {
-                    GameObject keyBindingContent = PrefabInstantiator.InstantiatePrefab(KeyBindingContent, Canvas.transform, false).transform.gameObject;
+                    GameObject keyBindingContent = PrefabInstantiator.InstantiatePrefab(SettingsMenu.keyBindingContent, Canvas.transform, false).transform.gameObject;
                     keyBindingContent.transform.SetParent(scrollView.transform.Find("Scroll View/Viewport/Content"));
 
                     // set the text to the short name of the binding
@@ -99,26 +103,11 @@ protected override void UpdateDesktop()
                 // the next button that gets pressed will be the new keyBind.
                 if (Input.anyKeyDown)
                 {
-                    foreach (KeyCode key in Enum.GetValues(typeof(KeyCode)))
+                    KeyCode newKey = Enum.GetValues(typeof(KeyCode)).Cast<KeyCode>()
+                                         .FirstOrDefault(key => Input.GetKeyDown(key) && KeyBindings.AssignableKeyCode(key));
+                    if (newKey != KeyCode.None)
                     {
-                        if (Input.GetKeyDown(key) && KeyBindings.AssignableKeyCode(key))
-                        {
-                            try
-                            {
-                                KeyBindings.SetBindingForKey(bindingToRebind, key);
-                                // TODO (#683): We need to open a modal dialog and ask the user
-                                // whether he/she really wants to change the binding.
-                                shortNameOfBindingToLabel[bindingToRebind.Name].text = key.ToString();
-                            }
-                            catch (Exception ex)
-                            {
-                                ShowNotification.Error("Key code already bound", ex.Message);
-                            }
-
-                            bindingToRebind = null;
-                            SEEInput.KeyboardShortcutsEnabled = true;
-                            break;
-                        }
+                        ReassignKeyAsync(bindingToRebind, newKey).Forget();
                     }
                 }
             }
@@ -140,7 +129,34 @@ protected override void UpdateDesktop()
         }
 
         /// <summary>
-        /// The keyBinding which gets updated.
+        /// Reassigns the binding of the given <paramref name="descriptor"/> to the new key <paramref name="newKey"/>
+        /// after confirming the action with the user.
+        /// </summary>
+        /// <param name="descriptor">The key binding to reassign.</param>
+        /// <param name="newKey">The new key code to assign to the key binding.</param>
+        private async UniTaskVoid ReassignKeyAsync(KeyActionDescriptor descriptor, KeyCode newKey)
+        {
+            string question = $"Do you really want to reassign action \"{descriptor.Name}\" to key {newKey}?";
+            if (!await ConfirmDialog.ConfirmAsync(new(question, title: "Change Key?")))
+            {
+                return;
+            }
+            try
+            {
+                KeyBindings.SetBindingForKey(descriptor, newKey);
+                shortNameOfBindingToLabel[descriptor.Name].text = newKey.ToString();
+            }
+            catch (KeyMap.KeyBindingsExistsException ex)
+            {
+                ShowNotification.Error("Key code already bound", ex.Message, log: false);
+            }
+
+            bindingToRebind = null;
+            SEEInput.KeyboardShortcutsEnabled = true;
+        }
+
+        /// <summary>
+        /// The key binding that gets updated.
         /// </summary>
         private KeyActionDescriptor bindingToRebind = null;
 
@@ -159,7 +175,7 @@ private void StartRebindFor(KeyActionDescriptor binding)
         private static void ExitGame()
         {
 #if UNITY_EDITOR
-            UnityEditor.EditorApplication.isPlaying = false;
+            EditorApplication.isPlaying = false;
 #else
             Application.Quit();
 #endif