Timing of enabling/disabling menu items?

[Warty]

Hi All,

I'm working on a roguelike game. This is an "OS-friendly" game, or at least that's my goal.

Now that almost all the features are in place, I'm looking to do some optimization to speed it up on 68000 machines. My first profile pass shows that the OnMenu() and OffMenu() OS functions are called a lot, and consuming a fair chunk of time.

	Hit Count	MS Per Call	MS Overall	Min MS	Max MS

BltBitMapRastPort	6781	0.313	2125.241	0.169	0.922

Text	1773	0.499	884.653	0.187	2.460

__stack_check	42808	0.005	218.430	0.000	0.300

OnMenu	6435	0.031	199.362	0.000	0.569

OpenWindow	2	74.830	149.660	54.873	94.787

CloseWindow	2	69.876	139.751	49.256	90.495

Graphics_BlitTileToXY	6751	0.020	134.801	0.000	0.221

rtEZRequest	2	62.103	124.206	46.150	78.056

vfprintf	1325	0.090	119.621	0.000	0.846

Menu_EnableDisableMenuItem	8865	0.013	110.967	0.000	0.229

fputc	14657	0.007	102.109	0.000	0.504

DrawImage	1	87.563	87.563	87.563	87.563

OffMenu	2430	0.030	73.150	0.000	0.187

I have a menu system. Some menu items need to be disabled under certain conditions. The way I set that up is to, as soon as any signal is received, call a function that first enables all menu items, then runs through a few conditions and potentially disables some specific menu items. I never disable entire menus, just items within a menu.

Ideally, this wouldn't get called until any one menu is actually opened. And wouldn't get called for key input signals at all. but then how would keyboard shortcuts work (AMIGA-C, etc.). so...

1. I copied this system from another more complicated-menu app. I can re-write a bit so that I don't actually re-enable every menu every time. fine.
2. is there a smarter way to know if I should actually bother with the menu enable/disable code? I assumed from documentation that I have to do it before a user could possibly select a menu item, so I'm doing it basically all the time, but a better way?

event hander: wait until a signal is received, then call the enable/disable code:

		Wait( 1 << parentWindow->UserPort->mp_SigBit );

		Menu_EnableDisable();

(then process the message, and at the end of that, call the enable/disable again. I honestly don't remember why I have that doing that, that code is from another app a few years back. I think maybe I needed to make sure keyboard shortcuts were disabled if the user's menu action would have effect of disabling some menu items that had been enabled).

The enable/disable code looks like this:

	// enable all menu items
	Menu_EnableAllMenuItems(the_window);
	
	// selectively disable specific menu items based on conditions
	if (global_active_window == global_splash_window)
	{
		Menu_EnableDisableMenuItem(the_window, MENU_APP, MENU_APP_HELP, false);
		Menu_EnableDisableMenuItem(the_window, MENU_APP, MENU_APP_ABOUT, false);
	
		Menu_EnableDisableMenuItem(the_window, MENU_GAME, MENU_GAME_ABANDON, false);
	
		Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_CYCLE_WEAPONS, false);
		Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_CYCLE_MAGIC, false);
		Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_RANGED_ATTACK, false);
		Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_TOGGLE_LANTERN, false);
		Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_DROP_LAST_ITEM, false);
		Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_REST, false);
		Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_INVENTORY, false);
	}
	else
	{
		Menu_EnableDisableMenuItem(the_window, MENU_GAME, MENU_GAME_NEW, false);
		Menu_EnableDisableMenuItem(the_window, MENU_GAME, MENU_GAME_RESTORE, false);

		if (global_gameover_active == true || 
			global_inventory_active == true || 
			global_dialog_active == true || 
			global_noticeboard_active == true)
		{
			Menu_EnableDisableMenuItem(the_window, MENU_APP, MENU_APP_HELP, false);

			Menu_EnableDisableMenuItem(the_window, MENU_GAME, MENU_GAME_ABANDON, false);
			Menu_EnableDisableMenuItem(the_window, MENU_GAME, MENU_GAME_NEW, false);
			Menu_EnableDisableMenuItem(the_window, MENU_GAME, MENU_GAME_RESTORE, false);

			Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_CYCLE_WEAPONS, false);
			Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_CYCLE_MAGIC, false);
			Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_RANGED_ATTACK, false);
			Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_TOGGLE_LANTERN, false);
			Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_DROP_LAST_ITEM, false);
			Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_REST, false);
			Menu_EnableDisableMenuItem(the_window, MENU_ACTIONS, MENU_ACTIONS_INVENTORY, false);
		}
	}

[Minuous]

Calling OnMenu()/OffMenu() every time you get a signal is overkill, better to just call them when strictly necessary.

A ghosted menu item will ignore its Amiga-key keyboard shortcut automatically, no need to worry about that.

If the example code you used does things like that, maybe it would be a good idea to base it on some better example code :-D Not sure what documentation you are using but maybe it is not the RKRM Libraries; I would suggest a read of the "Intuition: Menus" chapter, it is very informative.

[Warty]

An example of dynamically enabling and disabling menu items would be great!

This is the entire sum of information in the RKRM about enabling/disabling menus:

ENABLING AND DISABLING MENUS AND MENU ITEMS
Disabling menu items makes them unavailable for selection by the user.
Disabled menus and menu items are displayed in a ghosted fashion; that is, their imagery is overlaid with a faint pattern of dots, making it less distinct.
Enabling or disabling a menu or menu item is always a safe procedure, whether or not the user is currently using the menus. Of course, by the time you have disabled the item, the user may have already selected it. Thus, the program may receive a IDCMP_MENUPICK message for that item, even though it considers the item disabled. The program should be prepared to handle this case and ignore items that it knows are already disabled. This implies that the program must track internally which items are enabled and which are disabled.
The OffMenu() and OnMenu() functions may be used to enable or disable items while a menu strip is attached to the window.
void OffMenu ( struct Wlndow *window, unsigned long menuNumber );
void OnMenu ( struct Wlndow *wlndow, unsigned long menuNumber );
These routines check if the user is currently using the menus and whether the menus need to be redrawn to reflect the new states. If the menus are currently in use, these routines wait for the user to finish before proceeding.
If the item component referenced by menuNumber equals NOITEM, the entire menu will be disabled or enabled. If the item component equates to an actual component number, then that item will be disabled or enabled. Use the macros defined below for the construction of menu numbers from their component parts.
The program can enable or disable whole menus, just the menu items, or just single sub-items.
• To enable or disable a whole menu, set the item component of the menu number to NOITEM. This will enable or disable all items and any sub-items for that menu.
• To enable or disable a single item and all sub-items attached to that item, set the item component of the menu number to the item’s ordinal number. If the item has a sub-item list, set the sub-item component of the menu number to NOSUB. If the item has no sub-item list, the sub-item component of the menu number is ignored.
• To enable or disable a single sub-item, set the item and sub-item components appropriately.
It is also legal to remove the menu strip from each window that it is attached to (with ClearMenuStrip()) change the ITEMENABLED or MENUENABLED flag of one or more Menu or MenuItem structures and add the menu back using ResetMenuStrip() (in V36 or higher) or SetMenuStrip() (in any version of the OS).

The example program in that chapter does not use OnMenu() or OffMenu().

I may have overreacted to the notes, but they do imply, at least to me, that it's too late to try and disable a menu once a user has the right mouse button down:
• Of course, by the time you have disabled the item, the user may have already selected it. Thus, the program may receive a IDCMP_MENUPICK message for that item, even though it considers the item disabled.
and
• These routines [OnMenu, OffMenu] check if the user is currently using the menus and whether the menus need to be redrawn to reflect the new states. If the menus are currently in use, these routines wait for the user to finish before proceeding.

Having an internal structure to check which menus should be enabled or not is easy of course. But I don't want a menu to show as active, and the user thinks they can select it, but then it's not really active and I skip their input anyway.

I'll see if I can pick up some better example menu code on google.

[Minuous]

Of course there won't be much if you are using the 1986 edition, you can get the later editions here: https://commodore.bombjack.org/amiga/amiga-books.htm

[Warty]

Originally Posted by Minuous

Of course there won't be much if you are using the 1986 edition, you can get the later editions here: https://commodore.bombjack.org/amiga/amiga-books.htm

I'm using the 3rd edition, from 1992. Which one are you talking about?

[Minuous]

Sorry, I had read it too quickly as "this is the entire sum of information in the RKMs about menus" for some reason :-/

Certainly by the time the menu is being displayed it is too late to be making changes, maybe it could be done at the time an IDCMP_MENUVERIFY is received but that's not really how it's meant to be done. Better to just call your "function that first enables all menu items, then runs through a few conditions and potentially disables some specific menu items" (ie. OnMenu()/OffMenu() calls) when one of those conditions actually changes. If this is still too slow then the optimization is to not call On/OffMenu() for every menu item, only the ones whose on/off status actually needs to be changed.

[Warty]

Originally Posted by Minuous

Certainly by the time the menu is being displayed it is too late to be making changes, maybe it could be done at the time an IDCMP_MENUVERIFY is received but that's not really how it's meant to be done.

Yeah, I was thinking about doing menu verify, but I agree, it doesn't seem like that was it was designed for at all.

Originally Posted by Minuous

Better to just call your "function that first enables all menu items, then runs through a few conditions and potentially disables some specific menu items" (ie. OnMenu()/OffMenu() calls) when one of those conditions actually changes. If this is still too slow then the optimization is to not call On/OffMenu() for every menu item, only the ones whose on/off status actually needs to be changed.

Yeah, sounds like the next thing to try. Should be straightforward.

Thanks for helping me think through it.