RetroPie / EmulationStation

A Fork of Emulation Station for RetroPie. Emulation Station is a flexible emulator front-end supporting keyboardless navigation and custom system themes.
Other
858 stars 344 forks source link

Fix cursor overrun in last played collection when LR/LB paging is used #874

Closed Gemba closed 5 months ago

Gemba commented 5 months ago

cf. https://retropie.org.uk/forum/topic/35365/

pjft commented 5 months ago

Thanks for sending this over.

Questions:

1) in the post someone suggested this only happened when using the full page RB/LB controls. Does this change the behavior when that setting isn't on? 2) I'd remove the link from the code comments, that's not common practice here in ES. We have the tracking here on the PR :)

Cheers.

Gemba commented 5 months ago
  1. in the post someone suggested this only happened when using the full page RB/LB controls. Does this change the behavior when that setting isn't on?

I tested it both ways (LB/RB on and off): If LB/RB is off the forced recalculation does not matter as it simply puts/keeps the cursor in the middle as before (before means before the last played collection has been updated).

pjft commented 5 months ago

Cool, thank you. Happy to merge - could you squash the commits?

Thanks for sending this over!

Gemba commented 5 months ago

As there has been reported another cosmetic issue in the forum when changing the theme: It can be mitigated with a one-line similar to this (i am leaning towards applyTheme() in TextListComponent.h).

Should the second reported issue go in a separate PR or should I update this one?

pjft commented 5 months ago

It's ok to update this one, thank you.

Gemba commented 5 months ago

Ok. Both aspects fixed and squashed.

I tested as reported in the forum with two themes which have different display length in the gamelist (carbon and snes-mini), with LB/RB off and on. Looks good.

FYI: I removed the GuiMenu.cpp -> ViewController -> goToStart() call when the changed theme is applied. I had no regression by that during my tests (the forum user claimed that he had to re-navigate into the system after changing a theme, because a theme change returned him to the system carousel).

pjft commented 5 months ago

Thank you.