ref: e4d05c36d996c3053a66ca29cfe84e9652d501e3
parent: 666c528326b881460f2b677e20a2a9bc4d86898f
author: Simon Tatham <anakin@pobox.com>
date: Wed Sep 20 06:13:36 EDT 2017
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.)
--- a/PuzzleApplet.java
+++ b/PuzzleApplet.java
@@ -61,19 +61,19 @@
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);
@@ -215,10 +215,6 @@
protected void handleResized() {pp.createBackBuffer(pp.getWidth(), pp.getHeight(), colors[0]);
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) {--- a/emcc.c
+++ b/emcc.c
@@ -725,7 +725,7 @@
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 @@
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;
--- a/gtk.c
+++ b/gtk.c
@@ -140,7 +140,7 @@
*/
struct frontend {GtkWidget *window;
- GtkAccelGroup *accelgroup;
+ GtkAccelGroup *dummy_accelgroup;
GtkWidget *area;
GtkWidget *statusbar;
GtkWidget *menubar;
@@ -1160,16 +1160,6 @@
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 @@
#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 @@
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 @@
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 @@
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 @@
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 @@
#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 @@
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);
--- a/midend.c
+++ b/midend.c
@@ -590,19 +590,23 @@
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;
@@ -609,14 +613,17 @@
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
--- a/nestedvm.c
+++ b/nestedvm.c
@@ -305,10 +305,34 @@
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;
}
--- a/osx.m
+++ b/osx.m
@@ -735,7 +735,7 @@
- (void)newGame:(id)sender
{- [self processKey:'n'];
+ [self processKey:UI_NEWGAME];
}
- (void)restartGame:(id)sender
{@@ -809,11 +809,11 @@
}
- (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
--- a/puzzles.h
+++ b/puzzles.h
@@ -47,6 +47,15 @@
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 @@
#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.
--- a/windows.c
+++ b/windows.c
@@ -2993,7 +2993,7 @@
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:
@@ -3000,11 +3000,11 @@
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 @@
}
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: