- Status Closed
- Percent Complete
- Task Type Patches
- Category Plugins
- Assigned To No-one
- Operating System All players
- Severity Low
- Priority Very Low
- Reported Version Daily build (which?)
- Due in Version Undecided
-
Due Date
Undecided
- Votes
- Private
FS#10283 - Simplify Xobox menu and fix a issue
*Use same menu for both in-game and not in game.
-”Restart Level” and “Start Game” are combined as “Start New Game”.
“Restart Level” sounds like to restart the level I stoped for me. but it starts game from level 1.
-Show message “No game to resume” if “Resume Game” is selecetd while not in game. this is similar to chopper.
*fix typo “Difficult” to “Difficulty”.
*Fix the following issue.
pause game and enter menu. then resume game. the menu will stay shown untill game is unpaused. it is confusable.
With this patch, redraw the board and show “Paused”.
another way to fix it would be to force unpausing. i.e. somthing like “pause = false;”.
or do not allow to enter menu while the game is pausing.
I’m not sure which is the best.
Loading...
Available keyboard shortcuts
- Alt + ⇧ Shift + l Login Dialog / Logout
- Alt + ⇧ Shift + a Add new task
- Alt + ⇧ Shift + m My searches
- Alt + ⇧ Shift + t focus taskid search
Tasklist
- o open selected task
- j move cursor down
- k move cursor up
Task Details
- n Next task
- p Previous task
- Alt + ⇧ Shift + e ↵ Enter Edit this task
- Alt + ⇧ Shift + w watch task
- Alt + ⇧ Shift + y Close Task
Task Editing
- Alt + ⇧ Shift + s save task
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.
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.
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.
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.
I wrote a patch, which fix the warnings and adapt the behaviour of chooper to xobox.
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.
Update patch.
*apply same chage to solitaire.
*divide fix for xobox and simplifying menu.
Oops, diff of solitaire was not included..
apply the attached patch with the 2nd patch of previous comment.
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.