Rockbox

Hi, I doesn't like the idea to use the same menu for both in-game and not in game, because resume game shouldn't be displayed, if you can't select it. In my opinion the different menus increase the usability.
I think it would be make sense, that you can't enter the menu while the game is paused.
I wrote a patch, which fixes the typos and illogical entrys in the menu.

   xobox.diff (2.8 KiB)

it's not good things to have duplicated settings and so on.
and i dont like something like
>if(ingame)
> MENUITEM_STRINGLIST(…)
>else
> MENUITEM_STRINGLIST(…)
another approach is using MAKE_MENU, MENUITEM_RETURNVALUE, and menu_callback function.

   xobox.2.patch (5.8 KiB)

I think the usage of MENUITEM_RETURNVALUE and a menu_callback function is unnecessary complicated, because it's just code style. But your patch works fine for me.

hmm… then, how about this one? use MENUITEM_STRINGLIST with a menu_callback.

   xobox.3.patch (5.8 KiB)

It's simply great; a very neat solution. Are you sure the change in menu.c doesn't affect any other plugin?

I made a similar change for menus of some other plugins.

At least, the change in apps/menu.c won't affect existing code.
It only affects MT_RETURN_ID with menu_callback function. because if menu_callback is not set, current_subitems[n] is n.
And as far as i checked, MT_RETURN_ID is only used by MENUITEM_STRINGLIST and none of MENUITEM_STRINGLIST is used with callback.
I think it's almost ok for MENUITEM_STRINGLIST with menu_callback but I'm not sure.

   simplify_menus_by_using_strin... (13.6 KiB)

I wrote a patch, which fix the warnings and adapt the behaviour of chooper to xobox.

   simplify_menus_by_using_strin... (14.5 KiB)

Please dont change the behavior of calendar. "Edit" shouldn't be displayed if the day dose't have memos.
Could you write the warnings you got if you remember? i haven't got any warning while compiling.

warnings:
/home/johannes/Rockbox/rockbox/apps/plugins/chopper.c: In function ‘chopMenuCb’:
/home/johannes/Rockbox/rockbox/apps/plugins/chopper.c:707: warning: comparison between pointer and integer"
/home/johannes/Rockbox/rockbox/apps/plugins/calendar.c: In function ‘edit_menu_cb’:
/home/johannes/Rockbox/rockbox/apps/plugins/calendar.c:630: warning: initialization makes integer from pointer without a cast

small fix for mazezam.

   simplify_menus_by_using_strin... (14.6 KiB)

Update patch.
*apply same chage to solitaire.
*divide fix for xobox and simplifying menu.

   fix_xobox.patch (1.8 KiB)
   simplify_menus_by_using_strin... (9.8 KiB)
   simplify_menus_by_using_strin... (3.8 KiB)

Oops, diff of solitaire was not included..
apply the attached patch with the 2nd patch of previous comment.

   simplify_menus_for_solitaire.... (3 KiB)

I notice something weird:

For example in simplify_menus_by_using_stringlist_with_callback_b.5.patch :

- if (action == ACTION_REQUEST_MENUITEM && memos_in_shown_memory ⇐ 0)
+ int i = (intptr_t)this_item;
+ if (action == ACTION_REQUEST_MENUITEM
+ && memos_in_shown_memory ⇐ 0 && (i==0 || i==1))

You cast this_item (which is a pointer) to an integer (intptr_t) and compare it to 0 or 1.

What are you trying to do exactly ?

The commit r21523 contains other uses of intptr_t and I think they all are illegal here.

If you want to check if a pointer is not null, you should do if(pointer != NULL) or even if(!pointer) but not cast it to intptr_t.

I tried to compare this_item as an interger.
In this context, i, 0, and 1 are indexes of the list of text.
I don't know the reason but index is passed as a pointer casted like "(void*)(intptr_t)i" in apps/menu.c at line around 192 if the menu is from MENUITEM_STRINGLIST.
I thought that maybe I can use this value to simplify menu and I need to cast this_item to an integer to compare.

I think this is a special case.

The names you pass to MENUITEM_STRINGLIST don't have an address anyway as they're literal constants (so it can't actually be a pointer). Hence their ID is used. In this case, casting to int_ptr seems weird but correct.