From 63346a8ceac9ac1061e59af2a99ab1a44c851392 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 3 May 2023 12:57:27 +0100 Subject: [PATCH] Windows: reorganise menu ids. A user pointed out today that IDM_PREFS overlaps the second preset, because I forgot that IDM_PRESETS was not a single id but the base for an open-ended series. Looking more closely, there are several other problems with the IDM_* constants. IDM_GAMES (used in COMBINED mode) shouldn't be at an arbitrary distance _above_ IDM_PRESETS, because that risks a collision; instead, the games' and presets' ids should interleave. Also, the ids above IDM_GAMES were going up in steps of 1, which should have been 0x10, for the usual reason that the bottom four bits of the id aren't guaranteed. And IDM_KEYEMUL was completely unused (I suspect it was part of the discarded WinCE support). Now the #defines that are the bases of series are labelled as IDM_FOO_BASE; they interleave as they should; and there's a clear comment. --- windows.c | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/windows.c b/windows.c index d616a71..52b2ac3 100644 --- a/windows.c +++ b/windows.c @@ -35,11 +35,14 @@ #define IDM_SAVE 0x00E0 #define IDM_LOAD 0x00F0 #define IDM_PRINT 0x0100 -#define IDM_PRESETS 0x0110 -#define IDM_PREFS 0x0120 -#define IDM_GAMES 0x0300 +#define IDM_PREFS 0x0110 -#define IDM_KEYEMUL 0x0400 +/* Menu items for preset game_params go up from IDM_PRESET_BASE in + * steps of MENUITEM_STEP = 0x20. Menu items for selecting different + * games (in -DCOMBINED mode) go up from IDM_GAME_BASE similarly. */ +#define IDM_PRESET_BASE 0x0120 +#define IDM_GAME_BASE 0x0130 +#define MENUITEM_STEP 0x0020 #define HELP_FILE_NAME "puzzles.hlp" #define HELP_CNT_NAME "puzzles.cnt" @@ -1540,7 +1543,8 @@ static void populate_preset_menu(frontend *fe, UINT flags = MF_ENABLED; if (entry->params) { - id_or_sub = (UINT_PTR)(IDM_PRESETS + 0x10 * entry->id); + id_or_sub = (UINT_PTR)( + IDM_PRESET_BASE + MENUITEM_STEP * entry->id); fe->preset_menuitems[entry->id].which_menu = winmenu; fe->preset_menuitems[entry->id].item_index = @@ -1702,7 +1706,9 @@ static int fe_set_midend(frontend *fe, midend *me) if (strcmp(gamelist[i]->name, fe->game->name) != 0) { /* only include those games that aren't the same as the * game we're currently playing. */ - AppendMenu(games, MF_ENABLED, IDM_GAMES + i, gamelist[i]->name); + AppendMenu(games, MF_ENABLED, + IDM_GAME_BASE + MENUITEM_STEP * i, + gamelist[i]->name); } } } @@ -2863,27 +2869,34 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, start_help(fe, help_type == CHM ? fe->game->htmlhelp_topic : fe->game->winhelp_topic); break; - default: + default: { + unsigned n; + #ifdef COMBINED - if (wParam >= IDM_GAMES && wParam < (IDM_GAMES + (WPARAM)gamecount)) { - int p = wParam - IDM_GAMES; + n = (wParam - IDM_GAME_BASE) / MENUITEM_STEP; + if (n < gamecount && wParam == IDM_GAME_BASE + MENUITEM_STEP * n) { char *error = NULL; - fe_set_midend(fe, midend_for_new_game(fe, gamelist[p], NULL, + fe_set_midend(fe, midend_for_new_game(fe, gamelist[n], NULL, false, false, &error)); sfree(error); - } else + break; + } #endif - { + + n = (wParam - IDM_PRESET_BASE) / MENUITEM_STEP; + if (wParam == IDM_PRESET_BASE + MENUITEM_STEP * n) { game_params *preset = preset_menu_lookup_by_id( - fe->preset_menu, - ((wParam &~ 0xF) - IDM_PRESETS) / 0x10); + fe->preset_menu, n); if (preset) { midend_set_params(fe->me, preset); new_game_type(fe); + break; } } - break; + + break; + } } break; case WM_DESTROY: