Skip to content

MENUPICK: pointer-based selection for shortcut-less menu items (#63) #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sidick merged 1 commit into from
Aug 9, 2026
Merged

MENUPICK: pointer-based selection for shortcut-less menu items (#63) #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,38 @@ genuinely standard, ordinary ARexx port -- nothing dedicated-port-only
about this fix, any third-party implementer (e.g. AmiAuth) needs only
the same one-condition change.

`MENUPICK`'s pointer-based fallback for shortcut-less items is real
too now (issue #63, not yet in a tagged release): previously,
`MENUPICK` only worked for items with a real keyboard shortcut
(`AmipMenuPickByShortcut()`); an item with none was rejected outright.
`AmipMenuPickByPointer()` (`server/src/action.c`) now drives the
pointer path automatically for such items -- a genuine synthesized
RMB-down/move/move/RMB-up sequence, the same "real input.device
events, not a shortcut" principle `CLICK`/`DRAG` already follow, not a
forged `IDCMP_MENUPICK` message (considered and rejected: it would
bypass real IDCMP/activation/`WFLG_RMBTRAP`/`IDCMP_MENUVERIFY`
handling entirely, a false-positive risk this project's own design
principle exists to prevent). Two real, non-obvious things had to be
found live (2026-08-09) before this worked: (1) RMB-down alone only
switches the screen's own title bar into menu mode -- the pulldown
itself doesn't open until the pointer is actually moved onto the
target menu's own title text, confirmed only after a plain RMB-down
produced zero observable effect (no pulldown render, no fallback
`IDCMP_MOUSEBUTTONS`) even with the pointer already positioned inside
the window and a full second of `Delay()`; and (2) the RKRM documents
no formula at all for a submenu's own screen position (only "overlaps
its parent item's own select box somewhere") -- the real placement
(`parentLeft + parentWidth` for X, ignoring the sub-item's own
`LeftEdge` entirely; `parentTop + item->TopEdge` for Y) was measured
pixel-for-pixel against a real screenshot captured mid-pick under
Copperline (`amipilot.screenshot`'s own PNG conversion, cropped and
scanned for the popup's actual border pixels) after an initial guess
(adding `item->LeftEdge` on top of the parent's right edge) overshot
the box entirely. See `server/README.md`'s Menus section for the full
mechanism and `fixtures/gadtools-app`'s new shortcut-less "Toggle" and
"Sub NoShortcut" items, verified via `run_menu_check`'s
`MENUPICK-TOGGLE-POINTER`/`MENUPICK-SUBITEM-POINTER` checks.

Phase 0.5 (reliability and reach into the wider ecosystem)
before it: `WAITFOR` (including its
`TEXT=` condition) and `CLICK`'s `EXPECT=` (wait/expectation
Expand Down Expand Up @@ -141,10 +173,10 @@ public default password, no rate-limiting; LAN/trusted-network use
only, see `server/README.md`'s TCP section. Phase 0.4 additions
beyond TCP: `LAUNCH` (start a test subject over the wire), the
allowlist-scoped file API (`FSLIST`/`FSSTAT`/`FSMKDIR`/`FSDELETE`/
`FSGET`, `server/src/fs.c`), menu walking + shortcut-based selection
(`MENU`/`MENUPICK`, `intuition-model`'s `AmipWalkMenuStrip()` —
pointer-based selection for shortcut-less items not yet built),
multi-screen support (`SCREENS`/`SCREEN=`, keyed off
`FSGET`, `server/src/fs.c`), menu walking + selection (`MENU`/
`MENUPICK`, `intuition-model`'s `AmipWalkMenuStrip()` — shortcut-based
and, since issue #63, genuine pointer-based selection too for items
with no shortcut), multi-screen support (`SCREENS`/`SCREEN=`, keyed off
`Screen->DefaultTitle`, not the live `Title` field), tier-2 semantic
locators (`ROLE=`/`LABEL=`/`INDEX=` on CLICK/TYPE/GETTEXT's classic
form, resolved via a fresh `AmipWalkWindow()` walk — proximity-to-
Expand Down
43 changes: 37 additions & 6 deletions fixtures/gadtools-app/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,19 @@
* string gadget's text so a MENUPICK-by-shortcut round trip is
* observable via the existing GETTEXT path), a "Toggle" checkmark
* item (CHECKIT|MENUTOGGLE, starts checked -- exercises the walker's
* checkit/checked fields), a permanently "Disabled" item (no
* shortcut, ITEMENABLED off), a separator bar, and a "More" item with
* one submenu entry "Sub Item" (shortcut S, also sets the Host text)
* -- exercises the one-level-deep submenu walk and its own shortcut
* addressing (menu 0, item 4, sub 0).
* checkit/checked fields, and is also the fixture's only enabled,
* shortcut-less leaf item, so it doubles as the MENUPICK
* pointer-based-fallback target, issue #63 -- its own IDCMP_MENUPICK
* marker write proves a real RMB-down/move/RMB-up round trip reached
* Intuition's own menu tracking, not just that a shortcut keystroke
* was struck), a permanently "Disabled" item (no shortcut,
* ITEMENABLED off), a separator bar, and a "More" item with two
* submenu entries: "Sub Item" (shortcut S, also sets the Host text --
* exercises the one-level-deep submenu walk and its own shortcut
* addressing, menu 0, item 4, sub 0) and "Sub NoShortcut" (no
* shortcut, sub 1 -- the fixture's only shortcut-less sub-item,
* proving MENUPICK's pointer-based fallback reaches a genuine
* sub-item, not just a top-level one).
*/

#include <exec/types.h>
Expand Down Expand Up @@ -81,17 +89,20 @@ struct Library *GadToolsBase;

#define MENUNUM_PROJECT 0
#define ITEMNUM_ABOUT 0
#define ITEMNUM_TOGGLE 1
#define ITEMNUM_MORE 4
#define SUBNUM_SUBITEM 0
#define SUBNUM_SUBNOSHORTCUT 1

static struct NewMenu g_newMenu[] = {
{ NM_TITLE, (STRPTR)"Project", NULL, 0, 0, NULL },
{ NM_ITEM, (STRPTR)"About", (STRPTR)"A", 0, 0, NULL },
{ NM_ITEM, (STRPTR)"Toggle", (STRPTR)"T", CHECKIT | MENUTOGGLE | CHECKED, 0, NULL },
{ NM_ITEM, (STRPTR)"Toggle", NULL, CHECKIT | MENUTOGGLE | CHECKED, 0, NULL },
{ NM_ITEM, (STRPTR)"Disabled", NULL, NM_ITEMDISABLED, 0, NULL },
{ NM_ITEM, NM_BARLABEL, NULL, 0, 0, NULL },
{ NM_ITEM, (STRPTR)"More", NULL, 0, 0, NULL },
{ NM_SUB, (STRPTR)"Sub Item", (STRPTR)"S", 0, 0, NULL },
{ NM_SUB, (STRPTR)"Sub NoShortcut", NULL, 0, 0, NULL },
{ NM_END, NULL, NULL, 0, 0, NULL },
};

Expand Down Expand Up @@ -397,11 +408,31 @@ int main(void)
&& SUBNUM(msg->Code) == NOSUB) {
GT_SetGadgetAttrs(hostGad, window, NULL,
GTST_String, (ULONG)"about picked", TAG_DONE);
} else if (MENUNUM(msg->Code) == MENUNUM_PROJECT
&& ITEMNUM(msg->Code) == ITEMNUM_TOGGLE
&& SUBNUM(msg->Code) == NOSUB) {
/* Enabled, shortcut-less -- the fixture's only
* candidate for proving MENUPICK's pointer-based
* fallback (issue #63) genuinely reaches Intuition's
* own IDCMP_MENUPICK, not just that a shortcut
* keystroke was struck. */
GT_SetGadgetAttrs(hostGad, window, NULL,
GTST_String, (ULONG)"toggle picked", TAG_DONE);
} else if (MENUNUM(msg->Code) == MENUNUM_PROJECT
&& ITEMNUM(msg->Code) == ITEMNUM_MORE
&& SUBNUM(msg->Code) == SUBNUM_SUBITEM) {
GT_SetGadgetAttrs(hostGad, window, NULL,
GTST_String, (ULONG)"subitem picked", TAG_DONE);
} else if (MENUNUM(msg->Code) == MENUNUM_PROJECT
&& ITEMNUM(msg->Code) == ITEMNUM_MORE
&& SUBNUM(msg->Code) == SUBNUM_SUBNOSHORTCUT) {
/* Enabled, shortcut-less, one level deep -- the
* fixture's only candidate for proving
* MENUPICK's pointer-based fallback (issue #63)
* reaches a genuine sub-item, not just a
* top-level one (Toggle, above). */
GT_SetGadgetAttrs(hostGad, window, NULL,
GTST_String, (ULONG)"subitem noshortcut picked", TAG_DONE);
}
break;
default:
Expand Down
81 changes: 57 additions & 24 deletions server/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,38 +537,71 @@ Lands in phase 0.2 onward -- see
reports (and what Intuition itself decodes an `IDCMP_MENUPICK`
`Code` into via `MENUNUM()`/`ITEMNUM()`/`SUBNUM()`).

**Selection is keyboard-shortcut only for now.** `AmipMenuPickByShortcut()`
(`server/src/action.c`) activates the window, then strikes the
item's `Command` byte (inverted through the live keymap via
`MapANSI()`, same technique `AmipTypeString()` uses per character)
with the right-Amiga qualifier held -- the same input.device path a
human pressing Right-Amiga+key produces. Intuition resolves that
combination against the window's own live menu strip on its own;
this deliberately does not synthesize `IDCMP_MENUPICK` directly, so
a successful `RC 0` is genuine evidence the pick reached the app
through the real menu-shortcut path, not a shortcut around it.
**Honest limit:** an item with no keyboard shortcut (`COMMSEQ`
unset) can't be picked yet -- `RC 20` names this explicitly
("pointer-based menu selection isn't built yet") rather than
silently failing or guessing a fallback. Pointer-based navigation
(open the menu, move across items/submenus, release over the target
-- `LayoutMenusA()` already precomputes every item's screen-absolute
geometry before the menu is ever opened, so this is buildable) is
real follow-up work, not invented here ahead of it. A disabled item
(`ITEMENABLED` unset) is rejected client-side, before any keystroke
is sent at all -- `RC 20`, distinct from the no-shortcut case.
**Selection has two paths, chosen automatically per item.** When
the target has a real keyboard shortcut (`COMMSEQ` set),
`AmipMenuPickByShortcut()` (`server/src/action.c`) activates the
window, then strikes the item's `Command` byte (inverted through the
live keymap via `MapANSI()`, same technique `AmipTypeString()` uses
per character) with the right-Amiga qualifier held -- the same
input.device path a human pressing Right-Amiga+key produces.
Intuition resolves that combination against the window's own live
menu strip on its own; this deliberately does not synthesize
`IDCMP_MENUPICK` directly, so a successful `RC 0` is genuine evidence
the pick reached the app through the real menu-shortcut path, not a
shortcut around it.

**For an item with no shortcut** (issue #63), `AmipMenuPickByPointer()`
drives the exact sequence a human would: a genuine synthesized
RMB-down, then a move onto the target menu's own title text in the
screen's title bar (this is the part that isn't obvious from the
autodocs -- RMB-down alone only switches the title bar into menu
mode, showing menu titles instead of the window title; the pulldown
itself doesn't open until the pointer is actually moved over the
title, confirmed live), then a move onto the target item (which
highlights it and, for a sub-item, auto-opens the one-level submenu
purely as an Intuition-internal reaction to pointer position -- no
separate event exists to synthesize for either), then a final move
into the submenu if picking a sub-item, then RMB-up over the target
-- exactly what Intuition turns into `IDCMP_MENUPICK` (only the
most-subordinate item under the pointer is selectable; a top-level
item with a submenu can't itself be picked). Every box is resolved
live off the actual `struct Menu`/`MenuItem` fields immediately
before each move (`ResolveMenuItemBox()`), never cached or computed
from assumed pixel metrics -- menu layout depends on the user's own
screen/menu font. Confirmed live (2026-08-09) against real geometry:
`Menu->LeftEdge` is genuinely screen-absolute; a top-level item's own
box is `menu->LeftEdge + item->LeftEdge`,
`screen->TopEdge + screen->BarHeight + 1 + item->TopEdge` (the
screen's own live, font-derived `BarHeight` -- "Bar sizes for this
Screen... BarHeight is one less than the actual menu bar height",
`intuition/screens.h`); a sub-item's box left-anchors flush against
its parent item's own right edge (`parentLeft + parentWidth` --
`item->LeftEdge` plays no part in a sub-item's X placement and must
NOT be added on top of that, confirmed the hard way: doing so
overshot the submenu's own box entirely), with `item->TopEdge`
genuinely its own per-item stacking offset from the submenu box's
top. The RKRM documents none of the sub-item placement -- this was
measured pixel-for-pixel against a real screenshot captured mid-pick
under Copperline (`amipilot.screenshot`'s own PNG conversion), not
guessed. **Honest limit:** a window with `WFLG_RMBTRAP` set opts
entirely out of Intuition's own menu-button handling -- no
synthesized RMB-down can ever open its menu strip, reported as its
own distinct `RC 20` reason rather than a confusing timeout. A
disabled item (`ITEMENABLED` unset) is rejected client-side, before
any input is sent at all, on either path.

Verified end-to-end by `make test-target`'s MENU/MENUPICK check
(`tests/copperline/menu-test.py`) against a menu strip added to
`fixtures/gadtools-app` for this purpose: walks the menu and asserts
every field the walker read live (including a `CHECKIT|MENUTOGGLE`
item's starting `checked` state and a separator bar's blank,
disabled entry), `MENUPICK`s a top-level item and a submenu item by
their shortcuts and confirms each pick genuinely reached the
fixture's own `IDCMP_MENUPICK` handler (which writes a distinct
marker into its Host string gadget, read back via the
their shortcuts, a shortcut-less top-level item and a shortcut-less
sub-item by the pointer path, and confirms each pick genuinely
reached the fixture's own `IDCMP_MENUPICK` handler (which writes a
distinct marker into its Host string gadget, read back via the
already-verified `GETTEXT` path -- proof of real delivery through
Intuition, not just that a keystroke was injected), and confirms the
Intuition, not just that input was injected), and confirms the
fixture's permanently-disabled item is rejected without ever sending
a keystroke.

Expand Down
58 changes: 49 additions & 9 deletions server/include/action_engine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,16 +259,41 @@ BOOL AmipIsWindowOpen(struct Window *target);
* if window has no menu strip or any index is out of range. */
struct MenuItem *AmipFindMenuItem(struct Window *window, LONG menuNum, LONG itemNum, LONG subNum);

/* Same resolution as AmipFindMenuItem, but also hands back the
* resolved top-level struct Menu* (menuOut) and, when subNum >= 0,
* the top-level struct MenuItem* the sub-item hangs off of
* (parentItemOut) -- both optional (pass NULL to ignore). Needed for
* pointer-based picking (issue #63): a sub-item's on-screen box has no
* meaning without knowing which top-level menu/item it belongs to,
* which the plain GA_ID-shaped AmipFindMenuItem() doesn't expose.
* AmipFindMenuItem() itself is a thin wrapper over this with both outs
* NULL. */
struct MenuItem *AmipFindMenuItemWithParents(struct Window *window,
LONG menuNum, LONG itemNum, LONG subNum,
struct Menu **menuOut,
struct MenuItem **parentItemOut);

typedef enum {
AMIP_MENUPICK_OK = 0,
AMIP_MENUPICK_DISABLED, /* item->Flags lacks ITEMENABLED */
AMIP_MENUPICK_NO_SHORTCUT, /* no COMMSEQ/Command, or Command isn't a
* single keystroke under the active
* keymap -- pointer-based menu
* navigation isn't built yet (see
* server/README.md) */
AMIP_MENUPICK_INJECT_FAILED /* keymap.library unavailable, or
AMIP_MENUPICK_NO_SHORTCUT, /* AmipMenuPickByShortcut only: no COMMSEQ/
* Command, or Command isn't a single
* keystroke under the active keymap --
* the caller should fall back to
* AmipMenuPickByPointer() instead of
* treating this as a final failure */
AMIP_MENUPICK_INJECT_FAILED, /* keymap.library unavailable, or
* input.device event injection failed */
AMIP_MENUPICK_RMB_TRAPPED, /* AmipMenuPickByPointer only: window has
* WFLG_RMBTRAP set -- it opts out of
* Intuition's own menu-button handling
* entirely, so no synthesized RMB-down
* can ever open its menu strip. A real,
* permanent limit for that window, not
* a transient injection failure. */
AMIP_MENUPICK_GEOMETRY_FAILED /* AmipMenuPickByPointer only: couldn't
* resolve a sane screen-absolute box
* for the target item (or its parent) */
} AmipMenuPickResult;

/* Selects `item` via its keyboard shortcut: activates window, then
Expand All @@ -277,9 +302,24 @@ typedef enum {
* produce -- Intuition itself resolves that combination against the
* window's live MenuStrip, so this doesn't need to (and doesn't)
* synthesize IDCMP_MENUPICK directly. Does NOT open the menu or move
* the pointer -- pointer-based selection (for items without a
* shortcut) is planned but not built (docs/implementation-plan.md,
* "menu-pick"). */
* the pointer -- for an item with no shortcut, use
* AmipMenuPickByPointer() instead (issue #63). */
AmipMenuPickResult AmipMenuPickByShortcut(struct Window *window, struct MenuItem *item);

/* Pointer-based fallback for items with no keyboard shortcut (issue
* #63): opens the menu strip with a genuine synthesized RMB-down,
* moves the pointer onto topItem (letting Intuition's own tracking
* highlight it and, if it has sub-items, auto-open the one-level
* submenu -- no separate "open submenu" event exists to synthesize),
* moves onto subItem if non-NULL, then releases (RMB-up) over the
* final target -- exactly what Intuition turns into IDCMP_MENUPICK.
* `menu` is topItem's own parent struct Menu* (from
* AmipFindMenuItemWithParents()); geometry for every box is resolved
* live, immediately before each move, never cached or precomputed --
* menu layout depends on the user's own screen/menu font. Does NOT
* check whether topItem/subItem actually has a shortcut -- callers
* decide which pick function to use. */
AmipMenuPickResult AmipMenuPickByPointer(struct Window *window, struct Menu *menu,
struct MenuItem *topItem, struct MenuItem *subItem);

#endif /* AMIPILOT_ACTION_ENGINE_H */
Loading
Loading