M2M$FORCE_MENU panics when asked to set an item in a group that is already set

[Rhialto]

In the MegaPET I added some code in OSM_SEL_PRE to make it impossible to select item combinations that are inherently incompatible.

One example: There is a single on/off option "8296 memory exp + HRE / User Port control" which uses the user port to control a memory configuration.
There is also group of 3 options for joystick emulation:

• " Keyboard\n"
• " 1 (Space Invaders+)\n"
• " 2 (Stupid PET Tricks)\n"
  The last 2 provide inputs from a joystick to the user port, and that would of course interfere weirdly with using it as outputs for memory configurations.

Therefore, if you select "User Port control" I want to de-select options " 1 (Space Invaders+)" and " 2 (Stupid PET Tricks)", by selecting " Keyboard".

However there is a problem in case "Keyboard" was already selected.

The code in OPTM_SET which is called from M2M$FORCE_MENU seems to select the asked item, and then looks for the previously selected item to de-select. However, if the item to be selected was already selected, it finds nothing to deselect, and panics:

                MOVE    OPTM_CLBK_FATAL, R7
                MOVE    OPTM_F_MENUGRP, R8      ; if we land here then somethg
                MOVE    R3, R9                  ; went wrong: go fatal
                RBRA    _OPTM_CALL, 1

with this text

OPTM_F_MENUGRP  .ASCII_P "menu.asm: OPTM_SET\n"
                .ASCII_P "Corrupt memory layout or structural error\n"
                .ASCII_P "in current menu group (config.vhd):\n"
                .ASCII_P "Did not find any menu group item that\n"
                .ASCII_W "can be deselected. Only one item in group?\n"

For this use case, this is unwanted.

I hacked around the issue by essentially ignoring the condition:

diff --git a/M2M/rom/menu.asm b/M2M/rom/menu.asm
index 916b5da..2b5cdde 100644
--- a/M2M/rom/menu.asm
+++ b/M2M/rom/menu.asm
@@ -59,11 +59,11 @@ OPTM_F_MENUIDX  .ASCII_P "menu.asm: OPTM_RUN:\n"
                 .ASCII_P "Corrupt memory layout or logic error:\n"
                 .ASCII_P "Menu index does not exist in currently\n"
                 .ASCII_W "active menu level.\n"
-OPTM_F_MENUGRP  .ASCII_P "menu.asm: OPTM_SET\n"
-                .ASCII_P "Corrupt memory layout or structural error\n"
-                .ASCII_P "in current menu group (config.vhd):\n"
-                .ASCII_P "Did not find any menu group item that\n"
-                .ASCII_W "can be deselected. Only one item in group?\n"
+;OPTM_F_MENUGRP  .ASCII_P "menu.asm: OPTM_SET\n"
+;                .ASCII_P "Corrupt memory layout or structural error\n"
+;                .ASCII_P "in current menu group (config.vhd):\n"
+;                .ASCII_P "Did not find any menu group item that\n"
+;                .ASCII_W "can be deselected. Only one item in group?\n"
 OPTM_F_MENUGRP2 .ASCII_P "menu.asm: OPTM_SET\n"
                 .ASCII_P "Unsetting (R9=0) is illegal for menu\n"
                 .ASCII_W "groups. One item always needs to be 1.\n"
@@ -1149,10 +1149,18 @@ _OPTM_SET_1D    ADD     1, R5                   ; next menu group item
                 CMP     R6, R4                  ; done?
                 RBRA    _OPTM_SET_1C, !Z        ; no: iterate
 
-                MOVE    OPTM_CLBK_FATAL, R7
-                MOVE    OPTM_F_MENUGRP, R8      ; if we land here then somethg
-                MOVE    R3, R9                  ; went wrong: go fatal
-                RBRA    _OPTM_CALL, 1
+                ;MOVE    OPTM_CLBK_FATAL, R7
+                ;MOVE    OPTM_F_MENUGRP, R8      ; if we land here then somethg
+                ;MOVE    R3, R9                  ; went wrong: go fatal
+                ;RBRA    _OPTM_CALL, 1
+                ;;;; Rhialto: if we call OPTM_SET (or M2M$FORCE_MENU which calls it) to
+                ; set an item in a group and it is already selected, then this code
+                ; doesn't find any other item to de-select, and panics.
+                ; So we patch it out as a hack. It would be better to check beforehand
+                ; somewhere if the item is already selected ot not.
+                MOVE    0xFFFF, R12
+                RBRA    _OPTM_SET_R, 1
+                ;;;; End of hack by Rhialto.
 
                 ; Transform the menu index from flat coordinates to
                 ; screen coordinates

As noted, it would be better to detect that the requested item to select is already selected, and then do nothing.

I might guess that this could happen around this bit of code but I haven't studied it deeply enough yet to be sure if and how.

_OPTM_SET_1C    CMP     R6, R0                  ; skip currently selected item
                RBRA    _OPTM_SET_1D, Z

[Rhialto]

Maybe this is a reasonable place to check in advance:

                ; Update menu data structure which is in flat coordinates:
                ; Step #1: Set new value
                ; Step #2: In case of menu group: unset old value
                ;
                ; Step #1
_OPTM_SET_1A    MOVE    OPTM_DATA, R8
                MOVE    @R8, R8
                ADD     OPTM_IR_STDSEL, R8
                MOVE    @R8, R8
                ADD     R0, R8                  ; R0 contains menu index
new             CMP     R1, @R8                 ; is it already in the requested state?
new             RBRA    _OPTM_SET_S, Z          ; if so, skip everything
                MOVE    R1, @R8                 ; set new value

(or even branch to _OPTM_SET_R to skip even more)

As a disadvantage it seems to have that if you request to un-set an item that is part of a group, the check for that is skipped. Currently this panics for all cases (with error text OPTM_F_MENUGRP2), with this change it would only panic when you actually unset the active item.

Edit: I tried this and it seems to work.