Rockbox

I reviewed the patch and I think the patch is sensible. Just one request, add a short usage description in file highscore.h, please.

I like the patch. While we're at it, I'd put the check for "fd<0" in the function 'highscore_load' more close to "open" call, without interleaving the both with "memset" (move memset before open or after if). Some comments about what the parameters and the return values mean would be very welcome!

Also, in the highscore_update function, I'd introduce another parameter for the score buffer capacity. The existing one tells us how many entries there are now, and the new parameter would tell how many entries we can store so that we can avoid overflow if a score is inserted. Or do I miss how this API is used?

Update patch.
*add some comments to source code.
*move "memset" before "open" in function highscore_load.
*change function highscore_update a bit. hopefully this change makes it clearer.

In my understanding, num_scores, parameter of function highscore_update, is how many entries we can store.
And the code I wrote wouldn't cause overflow. but it didn't seems undarstandable a bit.

   lib_highscore.2.patch (6.2 KiB)

Ok, now I understand how it should be used. No need for an additional parameter. But now I have another question. Why isn't there a parameter for the name in the update function? The name member is never used! In rockblox we could use date/time (on RTC targets) as the name.

I updated some comments.

   lib_highscore.3.patch (7 KiB)

hmm. I assumed the name is intended to be input by user.
In this case, it makes sense that name is not passed to the update function.
e.g.
int pos;
if1) > 0)

  rb->kbd_input(scores[pos-1].name, sizeof(scores[pos-1].name));

Maybe. If so, we should write it as a comment about the intended (or possible, since rockblox doesn't do it) use. But I think it's not a good API. If you need an input from the user (we can provide a function to test whether the entry would be inserted) then you should first get the input and then call score_update, passing all the values for the entry. In rockblox, no user input is required. We could pass the date/time on RTC and an empty string on non-RTC targets. Or the name of the currently playing song (on both)!

BTW: wouldn't it be better (and more C-like) to return a 0-based value in the update function (and a negative one if the entry was not inserted)? You could then use it as the index in your example above, no need to subtract 1 (in a message for the user you'd have to add 1 though). But I think it's more natural to work with 0-based values in C.

This is how I would do it. (Populating of the name with the current time in rockblox is not implemented.)

   lib_highscore.4.patch (8.2 KiB)

Here is the patch that adds the date/time of the score (for RTC targets only, for non-RTC targets, "???" is written). The date/time is not displayed in the plugin but can be seen e.g. in an editor.

   lib_highscore.5.patch (8.5 KiB)

In rockblox, there is no need to check for RTC since get_time work on non-RTC platforms as well.

   lib_highscore.6.patch (8.4 KiB)

I wrote a extension for the highscore lib, which prevents to save an unchanged highscore. Besides I move the function …_show_highscores to the lib.

   lib_highscore.7.patch (3 KiB)
   games_changes.patch (14.3 KiB)

i wonder why you reopened this instead of creating new one…

*use "highscore_update" & "highscore_show" for names so that it would looks more consistent.
*remove weird "num_scores > 1" from lib/highscore.c
*add some "highsocre_updated = false;"
*disable "highscore_show" for charcells lcd.

   lib_highscroe.8.patch (18.3 KiB)

sorry, forgat to change "show_highscores" to "highscore_show" in some files.

   lib_highscore.9.patch (18.3 KiB)

fix bugs in bubbles.

   lib_highscore.10.patch (18.3 KiB)

i wonder why you reopened this instead of creating new one…

Same here I also wonder why you didn't implement the "show" function using the standard list widget. Because it doesn't allow to display data in columns?

I also wonder why you didn't implement the "show" function using the standard list widget. Because it doesn't allow to display data in columns?
As I started to write the "show_highscore" function I don't know anything about the standard list widget. I wouldn't change the function, even though it looks strange on big screens if sysfont is used.

The patch from terru works fine for me.