[hatari-devel] Keyboard shortcut definition dialog

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

Thomas, I noticed that you added SDL GUI support for
defining keyboard shortcuts.  I don't think the current
logic works yet as it should.


If you browse through the shortcuts, you see that there's
no shortcut key shown for "Pause".  This is because the code
assumes that initially all shortcuts are with modifier, and
it unconditionally sets that checkbox.

Instead, it should check whether that shortcut is defined
with modifier or not, and only then set the checkbox, when
user changes the shortcut.

Then, only if user changes the modifier setting, that should
be changed from the configuration.   Mostly untested fix
patch for these is attached.


Another issue is that you check for duplicate shortcut
keys only when key is defined.  However, toggling modifier
can also cause key to become duplicate.

What if the code would prevent user from exiting the dialog
if there are duplicates, and telling about this with alert?


	- Eero

diff -r c70bfb005c69 doc/emutos.txt
--- a/doc/emutos.txt	Sun Dec 27 10:32:43 2015 +0100
+++ b/doc/emutos.txt	Sun Dec 27 18:09:52 2015 +0200
@@ -816,6 +816,8 @@
 - UFO (by Dune & Sector1)
 - Unbeatable (by Masters of Electric City)
 - Vision (by POV)
+- We Were @Â STNICCC 2015 (by Oxygene)
+  - Reset screen colors are wrong
 - XiTEC Presentation (by Omega)
 
 Color games:
diff -r c70bfb005c69 src/gui-sdl/dlgKeyboard.c
--- a/src/gui-sdl/dlgKeyboard.c	Sun Dec 27 10:32:43 2015 +0100
+++ b/src/gui-sdl/dlgKeyboard.c	Sun Dec 27 18:09:52 2015 +0200
@@ -176,21 +176,46 @@
 
 
 /**
- * Refresh the shortcut texts in the dialog
+ * Switch modifier in given shortcut
  */
-static void DlgKbd_RefreshShortcut(int sc)
+static void DlgKbd_SwitchModifier(int sc)
 {
 	int keysym;
 
 	if (keyboarddlg[DLGKEY_SCWITHMOD].state & SG_SELECTED)
+	{
 		keysym = ConfigureParams.Shortcut.withModifier[sc];
+		ConfigureParams.Shortcut.withoutModifier[sc] = keysym;
+		ConfigureParams.Shortcut.withModifier[sc] = 0;
+	}
 	else
+	{
 		keysym = ConfigureParams.Shortcut.withoutModifier[sc];
+		ConfigureParams.Shortcut.withModifier[sc] = keysym;
+		ConfigureParams.Shortcut.withoutModifier[sc] = 0;
+	}
+}
 
+/**
+ * Switch to given shortcut in the dialog
+ */
+static void DlgKbd_SwitchShortcut(int sc)
+{
+	int keysym;
+
+	keysym = ConfigureParams.Shortcut.withModifier[sc];
+	if (keysym)
+		keyboarddlg[DLGKEY_SCWITHMOD].state |= SG_SELECTED;
+	else
+	{
+		keysym = ConfigureParams.Shortcut.withoutModifier[sc];
+		keyboarddlg[DLGKEY_SCWITHMOD].state &= ~SG_SELECTED;
+	}
 	strlcpy(sc_curval, Keymap_GetKeyName(keysym), sizeof(sc_curval));
 	keyboarddlg[DLGKEY_SCNAME].txt = sc_names[sc];
 }
 
+
 /**
  * Show and process the "Keyboard" dialog.
  */
@@ -213,8 +238,7 @@
 	                keyboarddlg[DLGKEY_MAPNAME].w);
 	keyboarddlg[DLGKEY_MAPNAME].txt = dlgmapfile;
 
-	keyboarddlg[DLGKEY_SCWITHMOD].state = SG_SELECTED;
-	DlgKbd_RefreshShortcut(cur_sc);
+	DlgKbd_SwitchShortcut(cur_sc);
 
 	if (ConfigureParams.Keyboard.bDisableKeyRepeat)
 		keyboarddlg[DLGKEY_DISREPEAT].state |= SG_SELECTED;
@@ -237,22 +261,22 @@
 			if (cur_sc > 0)
 			{
 				--cur_sc;
-				DlgKbd_RefreshShortcut(cur_sc);
+				DlgKbd_SwitchShortcut(cur_sc);
 			}
 			break;
 		 case DLGKEY_SCNEXT:
 			if (cur_sc < SHORTCUT_KEYS-1)
 			{
 				++cur_sc;
-				DlgKbd_RefreshShortcut(cur_sc);
+				DlgKbd_SwitchShortcut(cur_sc);
 			}
 			break;
 		 case DLGKEY_SCDEFINE:
 			DlgKbd_DefineShortcutKey(cur_sc);
-			DlgKbd_RefreshShortcut(cur_sc);
+			DlgKbd_SwitchShortcut(cur_sc);
 			break;
 		 case DLGKEY_SCWITHMOD:
-			DlgKbd_RefreshShortcut(cur_sc);
+			DlgKbd_SwitchModifier(cur_sc);
 			break;
 		}
 	}