Generate special fake keypresses from menu options.

This fixes an amusing UI bug that I think can currently only come up
in the unpublished puzzle 'Group', but there's no reason why other
puzzles _couldn't_ do the thing that triggers the bug, if they wanted
to.

Group has unusual keyboard handling, in that sometimes (when a cell is
selected for input and the key in question is valid for the current
puzzle size) the game's interpret_move function will eat keystrokes
like 'n' and 'u' that would otherwise trigger special UI events like
New Game or Undo.

The bug is that fake keypress events generated from the GUI menus
looked enough like those keystrokes that interpret_move would eat
those too. So if you start, say, a 16x16 Group puzzle, select an empty
cell, and then choose 'new game' from the menu, Group will enter 'n'
into the cell instead of starting a new game!

I've fixed this by inventing a new set of special keystroke values
called things like UI_NEWGAME and UI_UNDO, and having the GUI menus in
all my front ends generate those in place of 'n' and 'u'. So now the
midend can tell the difference between 'n' on the keyboard and New
Game from the menu, and so Group can treat them differently too. In
fact, out of sheer overcaution, midend.c will spot keystrokes in this
range and not even _pass_ them to the game back end, so Group
shouldn't be able to override these special events even by mistake.

One fiddly consequence is that in gtk.c I've had to rethink the menu
accelerator system. I was adding visible menu accelerators to a few
menu items, so that (for example) 'U' and 'R' showed up to the right
of Undo and Redo in the menu. Of course this had the side effect of
making them real functioning accelerators from GTK's point of view,
which activate the menu item in the same way as usual, causing it to
send whatever keystroke the menu item generates. In other words,
whenever I entered 'n' into a cell in a large Group game, this was the
route followed by even a normal 'n' originated from a real keystroke -
it activated the New Game menu item by mistake, which would then send
'n' by mistake instead of starting a new game!

Those mistakes cancelled each other out, but now I've fixed the
latter, I've had to fix the former too or else the GTK front end would
now undo all of this good work, by _always_ translating 'n' on the
keyboard to UI_NEWGAME, even if the puzzle would have wanted to treat
a real press of 'n' differently. So I've fixed _that_ in turn by
putting those menu accelerators in a GtkAccelGroup that is never
actually enabled on the main window, so the accelerator keys will be
displayed in the menu but not processed by GTK's keyboard handling.

(Also, while I was redoing this code, I've removed the logic in
add_menu_item_with_key that reverse-engineered an ASCII value into
Control and Shift modifiers plus a base key, because the only
arguments to that function were fixed at compile time anyway so it's
easier to just write the results of that conversion directly into the
call sites; and I've added the GTK_ACCEL_LOCKED flag, in recognition
of the fact that _because_ these accelerators are processed by a weird
mechanism, they cannot be dynamically reconfigured by users and
actually work afterwards.)

diff --git a/PuzzleApplet.java b/PuzzleApplet.java
index 512aede580592e5a8b079ad9b17d4e6fa27fcece..8f8bec1c0cdd83896c26c18d45aafdf498881152 100644 (file)
--- a/PuzzleApplet.java
+++ b/PuzzleApplet.java
@@ -61,19 +61,19 @@ public class PuzzleApplet extends JApplet implements Runtime.CallJavaCB {
             JMenuBar menubar = new JMenuBar();
             JMenu jm;
             menubar.add(jm = new JMenu("Game"));
-            addMenuItemWithKey(jm, "New", 'n');
+            addMenuItemCallback(jm, "New", "jcallback_newgame_event");
             addMenuItemCallback(jm, "Restart", "jcallback_restart_event");
             addMenuItemCallback(jm, "Specific...", "jcallback_config_event", CFG_DESC);
             addMenuItemCallback(jm, "Random Seed...", "jcallback_config_event", CFG_SEED);
             jm.addSeparator();
-            addMenuItemWithKey(jm, "Undo", 'u');
-            addMenuItemWithKey(jm, "Redo", 'r');
+            addMenuItemCallback(jm, "Undo", "jcallback_undo_event");
+            addMenuItemCallback(jm, "Redo", "jcallback_redo_event");
             jm.addSeparator();
             solveCommand = addMenuItemCallback(jm, "Solve", "jcallback_solve_event");
             solveCommand.setEnabled(false);
             if (mainWindow != null) {
                 jm.addSeparator();
-                addMenuItemWithKey(jm, "Exit", 'q');
+                addMenuItemCallback(jm, "Exit", "jcallback_quit_event");
             }
             menubar.add(typeMenu = new JMenu("Type"));
             typeMenu.setVisible(false);
@@ -217,10 +217,6 @@ public class PuzzleApplet extends JApplet implements Runtime.CallJavaCB {
         runtimeCall("jcallback_resize", new int[] {pp.getWidth(), pp.getHeight()});
     }
 
-    private void addMenuItemWithKey(JMenu jm, String name, int key) {
-        addMenuItemCallback(jm, name, "jcallback_menu_key_event", key);
-    }
-
     private JMenuItem addMenuItemCallback(JMenu jm, String name, final String callback, final int arg) {
         return addMenuItemCallback(jm, name, callback, new int[] {arg}, false);
     }

diff --git a/emcc.c b/emcc.c
index e25fdf86dd633bc9e0d69179b0e136b52209c134..a55f0a69b387b21e12ccfadddd4e81bc02ee8b08 100644 (file)
--- a/emcc.c
+++ b/emcc.c
@@ -725,7 +725,7 @@ void command(int n)
         update_undo_redo();
         break;
       case 5:                          /* New Game */
-        midend_process_key(me, 0, 0, 'n');
+        midend_process_key(me, 0, 0, UI_NEWGAME);
         update_undo_redo();
         js_focus_canvas();
         break;
@@ -735,12 +735,12 @@ void command(int n)
         js_focus_canvas();
         break;
       case 7:                          /* Undo */
-        midend_process_key(me, 0, 0, 'u');
+        midend_process_key(me, 0, 0, UI_UNDO);
         update_undo_redo();
         js_focus_canvas();
         break;
       case 8:                          /* Redo */
-        midend_process_key(me, 0, 0, 'r');
+        midend_process_key(me, 0, 0, UI_REDO);
         update_undo_redo();
         js_focus_canvas();
         break;

diff --git a/gtk.c b/gtk.c
index c5e3d1c9972de48c4f68a8cbaf176b87ce0aea7d..9ce5c8da7284251dec1a6bd8881d6c3ccb888d25 100644 (file)
--- a/gtk.c
+++ b/gtk.c
@@ -140,7 +140,7 @@ struct font {
  */
 struct frontend {
     GtkWidget *window;
-    GtkAccelGroup *accelgroup;
+    GtkAccelGroup *dummy_accelgroup;
     GtkWidget *area;
     GtkWidget *statusbar;
     GtkWidget *menubar;
@@ -1160,16 +1160,6 @@ static gint key_event(GtkWidget *widget, GdkEventKey *event, gpointer data)
     if (!backing_store_ok(fe))
         return TRUE;
 
-#if !GTK_CHECK_VERSION(2,0,0)
-    /* Gtk 1.2 passes a key event to this function even if it's also
-     * defined as an accelerator.
-     * Gtk 2 doesn't do this, and this function appears not to exist there. */
-    if (fe->accelgroup &&
-        gtk_accel_group_get_entry(fe->accelgroup,
-        event->keyval, event->state))
-        return TRUE;
-#endif
-
     /* Handle mnemonics. */
     if (gtk_window_activate_key(GTK_WINDOW(fe->window), event))
         return TRUE;
@@ -2348,32 +2338,34 @@ static void menu_about_event(GtkMenuItem *menuitem, gpointer data)
 #endif
 }
 
-static GtkWidget *add_menu_item_with_key(frontend *fe, GtkContainer *cont,
-                                         char *text, int key)
+static GtkWidget *add_menu_ui_item(
+    frontend *fe, GtkContainer *cont, char *text, int action,
+    int accel_key, int accel_keyqual)
 {
     GtkWidget *menuitem = gtk_menu_item_new_with_label(text);
-    int keyqual;
     gtk_container_add(cont, menuitem);
-    g_object_set_data(G_OBJECT(menuitem), "user-data", GINT_TO_POINTER(key));
+    g_object_set_data(G_OBJECT(menuitem), "user-data",
+                      GINT_TO_POINTER(action));
     g_signal_connect(G_OBJECT(menuitem), "activate",
                      G_CALLBACK(menu_key_event), fe);
-    switch (key & ~0x1F) {
-      case 0x00:
-       key += 0x60;
-       keyqual = GDK_CONTROL_MASK;
-       break;
-      case 0x40:
-       key += 0x20;
-       keyqual = GDK_SHIFT_MASK;
-       break;
-      default:
-       keyqual = 0;
-       break;
+
+    if (accel_key) {
+        /*
+         * Display a keyboard accelerator alongside this menu item.
+         * Actually this won't be processed via the usual GTK
+         * accelerator system, because we add it to a dummy
+         * accelerator group which is never actually activated on the
+         * main window; this permits back ends to override special
+         * keys like 'n' and 'r' and 'u' in some UI states. So
+         * whatever keystroke we display here will still go to
+         * key_event and be handled in the normal way.
+         */
+        gtk_widget_add_accelerator(menuitem,
+                                   "activate", fe->dummy_accelgroup,
+                                   accel_key, accel_keyqual,
+                                   GTK_ACCEL_VISIBLE | GTK_ACCEL_LOCKED);
     }
-    gtk_widget_add_accelerator(menuitem,
-                              "activate", fe->accelgroup,
-                              key, keyqual,
-                              GTK_ACCEL_VISIBLE);
+
     gtk_widget_show(menuitem);
     return menuitem;
 }
@@ -2535,8 +2527,11 @@ static frontend *new_window(char *arg, int argtype, char **error)
     gtk_container_add(GTK_CONTAINER(fe->window), GTK_WIDGET(vbox));
     gtk_widget_show(GTK_WIDGET(vbox));
 
-    fe->accelgroup = gtk_accel_group_new();
-    gtk_window_add_accel_group(GTK_WINDOW(fe->window), fe->accelgroup);
+    fe->dummy_accelgroup = gtk_accel_group_new();
+    /*
+     * Intentionally _not_ added to the window via
+     * gtk_window_add_accel_group; see menu_key_event
+     */
 
     hbox = GTK_BOX(gtk_hbox_new(FALSE, 0));
     gtk_box_pack_start(vbox, GTK_WIDGET(hbox), FALSE, FALSE, 0);
@@ -2553,7 +2548,7 @@ static frontend *new_window(char *arg, int argtype, char **error)
     menu = gtk_menu_new();
     gtk_menu_item_set_submenu(GTK_MENU_ITEM(menuitem), menu);
 
-    add_menu_item_with_key(fe, GTK_CONTAINER(menu), "New", 'n');
+    add_menu_ui_item(fe, GTK_CONTAINER(menu), "New", UI_NEWGAME, 'n', 0);
 
     menuitem = gtk_menu_item_new_with_label("Restart");
     gtk_container_add(GTK_CONTAINER(menu), menuitem);
@@ -2623,8 +2618,8 @@ static frontend *new_window(char *arg, int argtype, char **error)
     gtk_widget_show(menuitem);
 #ifndef STYLUS_BASED
     add_menu_separator(GTK_CONTAINER(menu));
-    add_menu_item_with_key(fe, GTK_CONTAINER(menu), "Undo", 'u');
-    add_menu_item_with_key(fe, GTK_CONTAINER(menu), "Redo", 'r');
+    add_menu_ui_item(fe, GTK_CONTAINER(menu), "Undo", UI_UNDO, 'u', 0);
+    add_menu_ui_item(fe, GTK_CONTAINER(menu), "Redo", UI_REDO, 'r', 0);
 #endif
     if (thegame.can_format_as_text_ever) {
        add_menu_separator(GTK_CONTAINER(menu));
@@ -2646,7 +2641,7 @@ static frontend *new_window(char *arg, int argtype, char **error)
        gtk_widget_show(menuitem);
     }
     add_menu_separator(GTK_CONTAINER(menu));
-    add_menu_item_with_key(fe, GTK_CONTAINER(menu), "Exit", 'q');
+    add_menu_ui_item(fe, GTK_CONTAINER(menu), "Exit", UI_QUIT, 'q', 0);
 
     menuitem = gtk_menu_item_new_with_mnemonic("_Help");
     gtk_container_add(GTK_CONTAINER(fe->menubar), menuitem);
@@ -2664,7 +2659,7 @@ static frontend *new_window(char *arg, int argtype, char **error)
 #ifdef STYLUS_BASED
     menuitem=gtk_button_new_with_mnemonic("_Redo");
     g_object_set_data(G_OBJECT(menuitem), "user-data",
-                      GINT_TO_POINTER((int)('r')));
+                      GINT_TO_POINTER(UI_REDO));
     g_signal_connect(G_OBJECT(menuitem), "clicked",
                      G_CALLBACK(menu_key_event), fe);
     gtk_box_pack_end(hbox, menuitem, FALSE, FALSE, 0);
@@ -2672,7 +2667,7 @@ static frontend *new_window(char *arg, int argtype, char **error)
 
     menuitem=gtk_button_new_with_mnemonic("_Undo");
     g_object_set_data(G_OBJECT(menuitem), "user-data",
-                      GINT_TO_POINTER((int)('u')));
+                      GINT_TO_POINTER(UI_UNDO));
     g_signal_connect(G_OBJECT(menuitem), "clicked",
                      G_CALLBACK(menu_key_event), fe);
     gtk_box_pack_end(hbox, menuitem, FALSE, FALSE, 0);

diff --git a/midend.c b/midend.c
index 3059d6b81ea95c3f8020bd5a54919c0f0721849a..2edc1ec6ef4286a4124f7d2f440a0b2ed472dee2 100644 (file)
--- a/midend.c
+++ b/midend.c
@@ -590,33 +590,40 @@ static int midend_really_process_key(midend *me, int x, int y, int button)
     int type = MOVE, gottype = FALSE, ret = 1;
     float anim_time;
     game_state *s;
-    char *movestr;
-       
-    movestr =
-       me->ourgame->interpret_move(me->states[me->statepos-1].state,
-                                   me->ui, me->drawstate, x, y, button);
+    char *movestr = NULL;
+
+    if (!IS_UI_FAKE_KEY(button)) {
+        movestr = me->ourgame->interpret_move(
+            me->states[me->statepos-1].state,
+            me->ui, me->drawstate, x, y, button);
+    }
 
     if (!movestr) {
-       if (button == 'n' || button == 'N' || button == '\x0E') {
+       if (button == 'n' || button == 'N' || button == '\x0E' ||
+            button == UI_NEWGAME) {
            midend_new_game(me);
            midend_redraw(me);
            goto done;                 /* never animate */
        } else if (button == 'u' || button == 'U' ||
-                  button == '\x1A' || button == '\x1F') {
+                  button == '\x1A' || button == '\x1F' ||
+                   button == UI_UNDO) {
            midend_stop_anim(me);
            type = me->states[me->statepos-1].movetype;
            gottype = TRUE;
            if (!midend_undo(me))
                goto done;
        } else if (button == 'r' || button == 'R' ||
-                  button == '\x12' || button == '\x19') {
+                  button == '\x12' || button == '\x19' ||
+                   button == UI_REDO) {
            midend_stop_anim(me);
            if (!midend_redo(me))
                goto done;
-       } else if (button == '\x13' && me->ourgame->can_solve) {
+       } else if ((button == '\x13' || button == UI_SOLVE) &&
+                   me->ourgame->can_solve) {
            if (midend_solve(me))
                goto done;
-       } else if (button == 'q' || button == 'Q' || button == '\x11') {
+       } else if (button == 'q' || button == 'Q' || button == '\x11' ||
+                   button == UI_QUIT) {
            ret = 0;
            goto done;
        } else

diff --git a/nestedvm.c b/nestedvm.c
index 79b797116f6d7bd8384b366e7ba40fed5c221d99..f7a2ae8ec58217e50c4769489936697e7bb4683d 100644 (file)
--- a/nestedvm.c
+++ b/nestedvm.c
@@ -305,10 +305,34 @@ static int get_config(frontend *fe, int which)
     return fe->cfgret;
 }
 
-int jcallback_menu_key_event(int key)
+int jcallback_newgame_event(void)
 {
     frontend *fe = (frontend *)_fe;
-    if (!midend_process_key(fe->me, 0, 0, key))
+    if (!midend_process_key(fe->me, 0, 0, UI_NEWGAME))
+       return 42;
+    return 0;
+}
+
+int jcallback_undo_event(void)
+{
+    frontend *fe = (frontend *)_fe;
+    if (!midend_process_key(fe->me, 0, 0, UI_UNDO))
+       return 42;
+    return 0;
+}
+
+int jcallback_redo_event(void)
+{
+    frontend *fe = (frontend *)_fe;
+    if (!midend_process_key(fe->me, 0, 0, UI_REDO))
+       return 42;
+    return 0;
+}
+
+int jcallback_quit_event(void)
+{
+    frontend *fe = (frontend *)_fe;
+    if (!midend_process_key(fe->me, 0, 0, UI_QUIT))
        return 42;
     return 0;
 }

diff --git a/osx.m b/osx.m
index 9d74da15742f7f4f43fc1fde4d78780fdd47cc73..fae98c73e5d87ba3ab30e59d4ade4e8d35996ee0 100644 (file)
--- a/osx.m
+++ b/osx.m
@@ -735,7 +735,7 @@ struct frontend {
 
 - (void)newGame:(id)sender
 {
-    [self processKey:'n'];
+    [self processKey:UI_NEWGAME];
 }
 - (void)restartGame:(id)sender
 {
@@ -809,11 +809,11 @@ struct frontend {
 }
 - (void)undoMove:(id)sender
 {
-    [self processKey:'u'];
+    [self processKey:UI_UNDO];
 }
 - (void)redoMove:(id)sender
 {
-    [self processKey:'r'&0x1F];
+    [self processKey:UI_REDO];
 }
 
 - (void)copy:(id)sender

diff --git a/puzzles.h b/puzzles.h
index 03af2ca18671183233049537b9384022796476e0..47c164c163b6fc6220a6a5f0fbf8eb06de7b05ed 100644 (file)
--- a/puzzles.h
+++ b/puzzles.h
@@ -47,6 +47,15 @@ enum {
     CURSOR_RIGHT,
     CURSOR_SELECT,
     CURSOR_SELECT2,
+    /* UI_* are special keystrokes generated by front ends in response
+     * to menu actions, never passed to back ends */
+    UI_LOWER_BOUND,
+    UI_QUIT,
+    UI_NEWGAME,
+    UI_SOLVE,
+    UI_UNDO,
+    UI_REDO,
+    UI_UPPER_BOUND,
     
     /* made smaller because of 'limited range of datatype' errors. */
     MOD_CTRL       = 0x1000,
@@ -64,6 +73,7 @@ enum {
 #define IS_CURSOR_MOVE(m) ( (m) == CURSOR_UP || (m) == CURSOR_DOWN || \
                             (m) == CURSOR_RIGHT || (m) == CURSOR_LEFT )
 #define IS_CURSOR_SELECT(m) ( (m) == CURSOR_SELECT || (m) == CURSOR_SELECT2)
+#define IS_UI_FAKE_KEY(m) ( (m) > UI_LOWER_BOUND && (m) < UI_UPPER_BOUND )
 
 /*
  * Flags in the back end's `flags' word.

diff --git a/windows.c b/windows.c
index 6b0a7debe0b0916dafb15f736d2e5b82f984035c..076e713d7ad5590f954c187413a094855fb6af81 100644 (file)
--- a/windows.c
+++ b/windows.c
@@ -2993,18 +2993,18 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
        cmd = wParam & ~0xF;           /* low 4 bits reserved to Windows */
        switch (cmd) {
          case IDM_NEW:
-           if (!midend_process_key(fe->me, 0, 0, 'n'))
+           if (!midend_process_key(fe->me, 0, 0, UI_NEWGAME))
                PostQuitMessage(0);
            break;
          case IDM_RESTART:
            midend_restart_game(fe->me);
            break;
          case IDM_UNDO:
-           if (!midend_process_key(fe->me, 0, 0, 'u'))
+           if (!midend_process_key(fe->me, 0, 0, UI_UNDO))
                PostQuitMessage(0);
            break;
          case IDM_REDO:
-           if (!midend_process_key(fe->me, 0, 0, '\x12'))
+           if (!midend_process_key(fe->me, 0, 0, UI_REDO))
                PostQuitMessage(0);
            break;
          case IDM_COPY:
@@ -3026,7 +3026,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
            }
            break;
          case IDM_QUIT:
-           if (!midend_process_key(fe->me, 0, 0, 'q'))
+           if (!midend_process_key(fe->me, 0, 0, UI_QUIT))
                PostQuitMessage(0);
            break;
          case IDM_CONFIG: