Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Plugins
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by syfou - 2007-03-19
Last edited by petur - 2007-08-30

FS#6854 - Add scrolling to the credits

With more than 280 contributors to Rockbox so far, the credits plugin takes about two minutes to list everyone – it seems to me it could benefit from some added interactivity; this patch adds simple scrolling capabilities to the credits plugin, so that the user can browse the list of contributors back and forth. Once applied:

- The original text sliding effect is kept intact.
- As with most interactive plugins, there is only one button to quit the plugin (this is model-dependant) – pressing any button is not good enough anymore.
- The model-dependent up and down buttons can be used to scroll the list, one half of the screen at a time, so that every contributor gets listed at least once.
- At the end of the list, the credits plugin exits if no manual scrolling occurred, and loop otherwise.

There are many other improvements that could be incorporated to make it feel better (automated scrolling in the direction of last manual scroll for instance), but this is a simple, working solution that adds the scrolling feature with little changes to current tree.

Closed by  petur
2007-08-30 19:57
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

maybe the navigation isn't 100% fluid but it will do for now…

petur commented on 2007-04-13 20:46

Nice tinkering, but here’s why this patch doesn’t make it into the code tree: Please read the CONTRIBUTING document regarding coding rules, specifically regarding the use of tabs. Some of the lines of the patch are only replacing spaces with tabs (which we do not want). Yes I could clean it up for you, but this is the way I learned it too ;)

syfou commented on 2007-04-18 16:45

This should correct the spaces/tabs issue. CONTRIBUTING also states “Keep lines below 80 columns length”… The current file in subversion (rev. 13201) does not respect this in a few places I modified, hence I didn’t change the original formatting when it occurred.

petur commented on 2007-05-19 22:12

looks ok.
could you either generate your patch from the root of the rockbox tree or state where to execute it (option one is preferred)

one more thing, the scrolling only starts working if the screen is filled… how’s that for a task ;)

syfou commented on 2007-05-19 22:35

Here you go (patch generated from root).

As for the part about scrolling only working once the screen is filled, it is just because I did want to have minimal impact on current code… Maybe I am just overly cautious, but I didn’t fully understand the justification of a few timed operations in the plugin, and I do not have access to other real devices besides the Sansa e200 target.

petur commented on 2007-05-20 00:55

more work for you… it crashed the sim here (reproducable)
just move up/sown a few times and then proceed down to the bottom. Once at the end you’ll get a segmentation fault (probably a div by 0). I guess the scrollin code conflicts with the moving down, causing some index to go wrong?

too bad, I was going to commit after a final check.

petur commented on 2007-08-01 13:46

Will you fix this or face the patch being rejected?

syfou commented on 2007-08-01 15:22

Thanks for the reminder – I will fix this: just allow me a few days (let’s say until after this week-end). If it bugs you to see this task open after so long, feel free to close it and I will reopen or resubmit as appropriate.

petur commented on 2007-08-01 15:25

no problem…. I’m just poking around on sleeping patches
If you intend to work on it it’s fine

syfou commented on 2007-08-06 21:23

Here is a fourth patch addressing this issue; It is not an incremental change on the third one, but rather a different approach: it implements a new plugin (called credits2 for now – I guess that credits_browser or something similar would be better in the end but it is just a proof of concept) that just displays the credits on a synclist.

I did this because I found the credit browsing I implemented before hard to use and behaving in a counter-intuitive fashion: I think this is far better for the end-user. I tested in in the uisimulator for a while, and it seems stable this time.

Right now, it duplicates the content of ‘credits.raw’ in the final binary – I guess this could be changed later if this proved to be a good idea – there is nothing preventing to merge this into the original credits plugin either.

petur commented on 2007-08-10 23:06

hmmmm I don’t know about a second credits plugin, one seems enough for me

petur commented on 2007-08-21 18:06

Note to Bagder:
re-opened because OP will supply new patch. The “I just don’t think this code should go in anymore…” was referring to the old patch.

syfou commented on 2007-08-22 01:48

This merges the latest patch into the original credits plugin (no more second plugin): once a key is pressed, it provides the credits on a synclist instead of exiting right away, so that the user gets a chance to interactively browse through the names. I think this is both simple (code-wise) and natural (user-wise). I tested it for a while on both a charcell and non-charcell ui simulator, without getting any segfaults.

I am not sure I got the USB states handling right though: on the simulator, I got a few hangs (rockbox not waking up once plugged, then unplugged when credits is running) I couldn’t reproduce on my sansa e280. The bug, if any, eludes me… I seems to get it both with this patch applied or not.

petur commented on 2007-08-22 19:03

hmmm I’m certainly not in favor of this version, it is not intuitive, not a nice way of working at all… Also doesn’t handle USB well, it gets stuck in the listview.

What was wrong with the first way of working anyway, why couldn’t you just fix the bug and be done with it?

syfou commented on 2007-08-22 19:40
hmmm I’m certainly not in favor of this version, it is not intuitive, not a nice way of working at all…
[…] What was wrong with the first way of working anyway?

Not intuitive? It’s just a regular list. I showed initial first version to two people, how were clearly struggling with it (they both got lost with the controls a couple of times): that’s the reason I reused rockbox pre-written, uniform interface instead of the custom effect.

Is there anything I can do to make it better (or to convince you it’s better ;-D )?

Also doesn’t handle USB well, it gets stuck in the listview.

I know: I didn’t understand how signaling is supposed to work… I read rockbox code a little more yesterday, and I think I got it now. I could fix this.

Why couldn’t you just fix the bug and be done with it?

I will post this one-liner next Monday (I travel for a few days) but I sure hope you’ll reconsider. Thanks for your time once again.

syfou commented on 2007-08-27 19:57

Here is an updated version of rockbox-scrolling-credits-3.patch. It fixes:

- the indexing problem that caused a segmentation fault in previous version (appears stable on simulator).
- another slight display glitch due to incorrect refreshing on the title line on loop.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing