Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10193 - unified the game menus and some improvements

Attached to Project: Rockbox
Opened by Johannes Schwarz (Ubuntuxer) - Saturday, 09 May 2009, 16:45 GMT+2
Last edited by Johannes Schwarz (Ubuntuxer) - Tuesday, 17 November 2009, 07:58 GMT+2
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

The aim of this patch is to improve the usability of the games.

Changes:
- add a standart menu to brickmania, pegbox, spacerocks, jewels, rockblox
(including Playback Control)
- add change of difficult to brickmania (if you select easy and have less than two lifes, you'll get a extra life.
- add highscore list to the games
- add help text to the games (expandable)
- clean the code and delete unnecessary variables
- change the keymap of FUZE in pegbox
- fixed a bug in pegbox, which prevent to save a game
- you can save a game in brickmania now, but just the
This task depends upon

Closed by  Johannes Schwarz (Ubuntuxer)
Tuesday, 17 November 2009, 07:58 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  last commit r23653
Comment by Johannes Schwarz (Ubuntuxer) - Saturday, 09 May 2009, 16:51 GMT+2
The patch needs  FS#10099 , which I have attached.
Please test the patch and let me know your opinion.
   game_improvements.diff (172.8 KiB) 
 apps/plugins/spacerocks.c |  617 +++++++++++----------
 apps/plugins/brickmania.c |  895 +++++++++++--------------------
 apps/plugins/jewels.c     | 1328 +++++++++++++++-------------------------------
 apps/plugins/rockblox.c   |  232 +++++++-
 apps/plugins/pegbox.c     |  722 +++++++------------------
 5 files changed, 1531 insertions(+), 2263 deletions(-)

   lib_display_text.diff (7.7 KiB) 
 plugins/lib/SOURCES     |    1 
 apps/plugins/lib/text.c |  151 ++++++++++++++++++++++++++++++++++++++++++++++++
 apps/plugins/lib/text.h |   41 +++++++++++++
 3 files changed, 193 insertions(+)

   lib_highscore.diff (0.4 KiB) 
 apps/plugins/lib/highscore.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comment by Thomas Martitz (kugel.) - Sunday, 10 May 2009, 04:20 GMT+2
Please, don't put so much changes into 1 patch. Ideally, 1 change per path (it's ok if the same changes applies to more plugins).
Comment by Johannes Schwarz (Ubuntuxer) - Saturday, 16 May 2009, 23:48 GMT+2
I will try to divide the patch.
The attached patch change the game pegbox.
- add standart menu to pegbox
- saves your current level and the highest level
- remove save game, because it didn't work and isn't really necessary
- rename functions, so it begins with pegbox_...
- remove level up and down from Fuze, because often by accident you select the button
- clean up the code

The bitmaps pegbox_menu_items.bmp are now useless and should be removed.

PS: The patch is based on "lib_highscore.diff" and "lib_display_text.diff".

I know in the patch still features and non-functional changes are mixed, but it's really difficult to diffide it rigorous.
   pegbox.diff (31.7 KiB) 
 apps/plugins/pegbox.c |  734 +++++++++++++-------------------------------------
 1 file changed, 204 insertions(+), 530 deletions(-)

Comment by Johannes Schwarz (Ubuntuxer) - Sunday, 12 July 2009, 22:34 GMT+2
I fixed lots of bugs and synced the patch to svn. Besides I matched the manual to the changes in the patch.
   game_improvements2.diff (167.4 KiB) 
 apps/plugins/snake.c      |  264 ++++++-----
 apps/plugins/spacerocks.c |  614 ++++++++++++-------------
 apps/plugins/brickmania.c |  871 +++++++++++++-----------------------
 apps/plugins/jewels.c     | 1095 ++++++++++++++++++----------------------------
 apps/plugins/pegbox.c     |  740 ++++++++-----------------------
 5 files changed, 1412 insertions(+), 2172 deletions(-)

   game_manuals.diff (3.5 KiB) 
 manual/plugins/jewels.tex     |   30 +++++++++++-------------------
 manual/plugins/brickmania.tex |    4 +++-
 manual/plugins/pegbox.tex     |    2 +-
 3 files changed, 15 insertions(+), 21 deletions(-)

Comment by Teruaki Kawashima (teru) - Monday, 13 July 2009, 05:06 GMT+2
why in the world you want to change everything at once?
it would be better to separate some parts.
* displaying help
* score related stuff.
* adding standard menu.
* maybe the others? i'm not sure.
and there seems to be sooo many tabs.
Comment by Johannes Schwarz (Ubuntuxer) - Monday, 13 July 2009, 20:04 GMT+2
I know I had wrong seized my plans. The patch is considerable to comprehensive, but it's hard to divide the patch, because everything coheres, so it'll make much effort.
If you help me to fix the remaining bugs, I would like to commit the patch and next time I'll do it better. :-)

replaced all tabs with spaces
minor fixes
   game_improvements3.diff (164.5 KiB) 
 apps/plugins/snake.c      |  264 ++++++------
 apps/plugins/spacerocks.c |  584 +++++++++++++--------------
 apps/plugins/brickmania.c |  871 +++++++++++++++--------------------------
 apps/plugins/jewels.c     |  969 +++++++++++++++++-----------------------------
 apps/plugins/pegbox.c     |  740 ++++++++++-------------------------
 5 files changed, 1333 insertions(+), 2095 deletions(-)

Comment by Teruaki Kawashima (teru) - Tuesday, 14 July 2009, 00:54 GMT+2
i'd suggest to commit each part separately istead of creating diff of all changes and divide patch.
i.e. commit only displaying help part, then commit adding standard menu part, and so on.
how about complete  FS#10099  first?
Comment by Thomas Martitz (kugel.) - Tuesday, 14 July 2009, 00:58 GMT+2
/me whistles git :)

But yea, try to divide the patch into smaller pieces.
Comment by Teruaki Kawashima (teru) - Tuesday, 14 July 2009, 12:02 GMT+2
quick look.

surrounding only content of plugin_start by #ifdef HAVE_LCD_BITMAP doesn't make sense for me.

you use MENUITEM_STRINGLIST and rb->do_menu to set option like difficulty and level.
i think rb->set_option and rb->set_int are usually used.

is it allowed to use float?

chages of jewel seems overkill for me.
current system works find and i dislike auto saveing...
i don't see the reason of the cahnges.

according to docs/CONTRIBUTING, enum names should be all lower case.
i mean,
1693: +enum Difficulty{
should be
1693: +enum difficulty{
or something like in your patch.

i think it would be better to avoid to use same variable for arguments and to set return value like follows.
choice = rb->do_menu(&main_menu, &choice, NULL, false);
because pressing left reset selection.

adding preffix such as pegbox_ to function names seems to only make patch complicated...
it's not necessary after all, right?

i'm thinking it's nice feature so-called attract mode is shown when start spacerocks game.
and it would be bit strange that selecting start in menu starts attract mode.
so, can it be change to show attract mode first and then, pressing start will start game and pressing menu (or somthing simmiler) will open the menu?

btw, i wonder why no one seems to consider adding menu to balckjack.
Comment by Johannes Schwarz (Ubuntuxer) - Tuesday, 14 July 2009, 18:47 GMT+2
"is it allowed to use float?"
why not ?

"chages of jewel seems overkill for me.
current system works find and i dislike auto saveing...
i don't see the reason of the cahnges."
It takes a long time to finish a game in jewels, so often you must pause it and exit the player. I think, it's very annoying if you quit by accident a game without saving.
Why do you dislike auto saving?
The main disadvantage of your commit is that if you start jewels and without playing you have to save the game still.
Comment by Thomas Martitz (kugel.) - Tuesday, 14 July 2009, 18:50 GMT+2
It's not exactly disallowed, but *****highly***** discouraged.

Our targets don't have hardware units to do floating point calculation, hence GCC has to emulate this routines. And this is *horribly* slow.
Comment by Dominik Riebeling (bluebrother) - Tuesday, 14 July 2009, 20:53 GMT+2
There is also another reason to not use float calculations: if no float calculations are used the compiler doesn't need to link the floating-point math library, hence it saves space. A single use of floating point calculations kills this advantage. Not sure if this affects the core for plugins and if the core itself uses floating point operations (though I assume the core is fixed-point only).
Comment by Teruaki Kawashima (teru) - Wednesday, 15 July 2009, 10:23 GMT+2
> Why do you dislike auto saving?
Because it causes disk access which takes ~2 secconds to finish and i rarely want to save game.
Comment by Dominik Riebeling (bluebrother) - Wednesday, 15 July 2009, 18:00 GMT+2
You also shouldn't forget that every disk access is expensive on non-flash targets, of which we support quite a noticable amount.
Comment by Johannes Schwarz (Ubuntuxer) - Thursday, 16 July 2009, 20:13 GMT+2
Ok, you are right auto saving doesn't make sense. From this it follows that also highscore tables shouldn't be saved, if they weren't changed.
I suggest to prevent to save a unchanged highscore direct in the lib. Besides I would move the function ..._show_highscores to the lib, too.
   highscore_save.diff (0.6 KiB) 
 apps/plugins/lib/highscore.c |    6 ++++++
 1 file changed, 6 insertions(+)

Comment by Thomas Martitz (kugel.) - Friday, 17 July 2009, 00:07 GMT+2
Why stopping saving highscores? If you made one, it's worth a spin-up IMO. It's not comparable to autosave.
Comment by Johannes Schwarz (Ubuntuxer) - Friday, 17 July 2009, 10:18 GMT+2
I don't want to stop saving highscore, but if the highscore weren't changed, there is no need to save it.
Comment by Johannes Linke (Jaykay) - Sunday, 19 July 2009, 11:43 GMT+2
just a small note... you wrote colission in game_improvements3.diff :)
Comment by Johannes Schwarz (Ubuntuxer) - Tuesday, 17 November 2009, 07:48 GMT+2
I would like to close this task, so here is the last patch, which adds a menu to rockblox.
   rockblox_menu.diff (7.6 KiB) 
 apps/plugins/rockblox.c |  173 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 143 insertions(+), 30 deletions(-)

Loading...