Update: EmulationStation random video screensaver - use only media from a particular system, back and favorites controls

[Lolonois]

Thanks. Nice additions, I esp. like the add/remove Favortite during while in screensaver.

However, I identified some issues:
Note: I tested with the Slideshow Screensaver. It may or may not that these effects also appear with the video screensaver.

1. Cursor not visible after returning from Screensaver
2. Back (Left) shows blank screen instead of previous
3. UX surprises

Details:

1. Cursor not visible after returning from Screensaver

How to reproduce:

1. Launch ES
2. Start direct into Screensaver
3. Wait for a game to display that you know is not shown on the first page in the Systemview (i.e. a system with many games and a game name in the second half in the alphabet, assuming sort order is alphabetical)
4. Press A to exit to that game's systemview
5. Result: Correct game details (if using Detailsview) is shown, but the game list does not show the game selected.

Expected: Game is highlighted in list and list cursor is visible on screen.

Additional context: This effect is only observable when the system view in questions has not been visited/entered before (i.e. after an ES launch). Once a system view has been navigated into the effect is not reproducible.

2. Back (Left) shows blank screen instead of previous

How to reproduce:

1. Launch Screensaver anytime
2. Press (Left) once
3. Result: Blank screen shown. However Start launches the correct game (i.e. only the image is blank during screensaver)

Expected: After pressing Left the previous game screensaver content is shown.

There is also a follow-up (related to this issue). When pressing Left twice while in screensaver and then pressing A or Start it makes ES to show/start the game two Left presses ago (i.e. before having pressed Left at all).

3. UX surprises

These are observations that surprised me, as I was expecting a different behaviour. So YMMV.

General: In the options menu, maybe it is more consistent with the other menu entries to use "Launch Screensaver" instead of "Launch Systemscreensaver"

For these observations below I used the custom media screensaver option.

Left ends the custom screensaver (OK), but in contrast Y (Favortites) has no function and does not end the screensaver. Expected: Y also disables the screensaver.

The menu entry "Launch Systemscreensaver" is not present when being in a systemview. While there is no "per system" slideshow with custom media, a user may expect to be able to launche the screensaver from the menu nevertheless.

[pjft]

@Lolonois thank you - this is great! Thanks for the feedback and for testing these out so thoroughly.

Some feedback on these:

2 - This should be fixed in https://github.com/RetroPie/EmulationStation/pull/847 . I missed those in the last code merge. Same for the Favorites in (3). Thanks for spotting them!

1 - I cannot, for the life of me, replicate this one. A small request and a couple of questions, to help narrow it down: is it possible for you to send over a video of it, to see if anything stands out? Might be anything from theme, view type, screen size, etc, but in my current setup I'm struggling. Questions: does it also happen when it's a video screensaver (I would assume so)? Does it also happen if you choose to launch the game with start instead of just pressing A?

3 - Good inputs, and food for thought. Help me think this through, as I hear your comments but I do not have a solid opinion on it.

a) General: I chose the name "Launch System Screensaver" to make it explicit that it is a different behavior to just the regular screensaver. I could change it to "Launch Screensaver", but I fear that people would expect that it'd behave the same and they'd find it weird that it only shows games from that specific system. Happy to hear multiple opinions here. I tried to play it safe, but maybe I leaned too much to the safe side-

b) Menu Entry: correct. I disabled it when we were not going to launch a system screensaver - one can launch the screensaver from the systems list by pressing select. I don't oppose exploring having a systemview-wide option to launch screensaver, if useful - I believe other projects' version of ES have a "Launch Screensaver" option from the select menu just like you describe it. I didn't aim to add that feature, though if there's wide interest in it, it could be considered as a separate PR, not necessarily in this one.

If you want to check the PR above, let me know. I'd love to add there the fix for the cursor, but I need to replicate it beforehand. If it also happens when you press START, if you could check if it happened in the current stable ES that'd be great - so I can know if this was a "bug" and fix it separately, or a regression.

Thanks a lot, and have a great week!

[Lolonois]

@pjft said in Update: EmulationStation random video screensaver - use only media from a particular system, back and favorites controls:

2 - This should be fixed in https://github.com/RetroPie/EmulationStation/pull/847 . I

Can confirm, with that PR, both issues are gone. However, is it intentional that when using Left multiple times (without Right in between), that the images toggle on each Left between current and previous?

@pjft said in Update: EmulationStation random video screensaver - use only media from a particular system, back and favorites controls:

1 - I cannot, for the life of me, replicate this one.

I should have noted, that it is after a fresh ES start and only on the very first launch of the screensaver, sorry for the confusion.
Then launch the screensaver, before navigating into any system.
In the screensaver push Right until you identify a game that is (from your memory) not in the first page of the gamelist in the respective system view.
Then press A to get to the system's gamelist. This should reproduce the effect.

However, if you Start a game from the screensaver and exit again, the cursor is placed correctly and visible.

I tested the Start from screensaver and return to game with the same procedure as above also in b8800f0, which is just before your updates. Result: The cursor is placed correctly and visible.

I tested only with images, I have no videos configured.

If it matters here's how I compiled ES:
cmake . -DFREETYPE_INCLUDE_DIRS=/usr/include/freetype2/ -DRPI=On -DGL=On -DUSE_GL21=On -DOMX=On && make clean && make VERBOSE=1 -j 4 (the same as the scriptmodule does. I am using an Rpi4). Screensize is 1280x1024 with ca. 16 gametitles visible on one systemview screen, using the Carbon theme. If you need more input on my config or a video, just ask.

@pjft said in Update: EmulationStation random video screensaver - use only media from a particular system, back and favorites controls:

3 - Good inputs, and food for thought.

This I got wrong: I was thinking of the class Systemscreensaver instead of the Screensaver with media limited to the selected system. Then your proposal makes sense to disable the menu entry when custom media is used. So no claims from my side unless you may want to rename the menu entry to "Run Screensaver on this System" or "Run Screensaver on <Systemname>", but that is really cosmetic.

Greetings and have a marvelous week, too.

[pjft]

@Lolonois Thanks for the response!

Well, I still can't, but let me know more:

• Is it that the game entry shows on screen but isn't selected, or
• The game entry is selected, but it's not on screen, or
• Another game entry remains selected

Send over your es_settings.cfg just in case and, while we're at it, do try out the latest code in the PR. To be fair, the code path is exactly the same for launching or selecting - except that, you know, we don't launch the game when you press A :) So, yes, maybe we're doing something there that fixes it when you launch the game, but more info would help, for sure. Even a small video or screenshot of the end result would help.

Thank you!

[roslof]

@pjft said in Update: EmulationStation random video screensaver - use only media from a particular system, back and favorites controls:

We've been at work in the last few weeks to adjust the user experience for EmulationStation game discovery. There are a few more ideas in the pipeline, but the current state seems good enough to share for those of you who want to try it out and test it.

Loving the contribution, @pjft . Been imagining 3 of your functions for a while now -- and now they exist. No issues found on my end testing with the es-epicnoir theme. Everything as expected.

The only peanut gallery suggestion would be to move the LAUNCH SYSTEM SCREENSAVER item directly below JUMP TO ...

JUMP TO ... has been at the top of the menu for quite some time and is probably going to be more widely used, Anecdotally, my muscle memory has already fallen victim to this, as I often open the menu and immediately start moving left or right thinking I'm changing the jump value, only to learn I'm above the JUMP TO ... selection.

I really like this overall though. Fun addition and appreciated.

-Ros

[Lolonois]

@pjft said in Update: EmulationStation random video screensaver - use only media from a particular system, back and favorites controls:

Is it that the game entry shows on screen but isn't selected, or

The game details show right.

The game entry is selected, but it's not on screen, or

It is selected but the cursor is not visible on the screen resp. the visible part of the gamelist is not aligned with the selection.

Another game entry remains selected

No.

I have put together a video: https://jumpshare.com/v/0H8mGGhc0JL1veSdZOPp

From the video: The screensaver starts with "Gunforce" in Systen Mame, but the view on the gamelist is still at the beginning. (Note: you can also use Right during screensaver to identify a game that has these conditions (longer gamelist and game shown in screensaver not on first page) at your setup),

Hope you can reconstruct the issue now.

Here is my es_settings.cfg: https://gist.github.com/Gemba/225d926e52a1a4e90e046370186c708c

I have been using f604dcf from main/HEAD: https://github.com/RetroPie/EmulationStation
I have your latest PR #847 left aside for this testcase, because I get a compile error:

[ 70%] Building CXX object es-app/CMakeFiles/emulationstation.dir/src/CollectionSystemManager.cpp.o
/home/pi/git/emu_master_clean/es-app/src/SystemScreenSaver.cpp: In member function ‘virtual void SystemScreenSaver::selectGame(bool)’:
/home/pi/git/emu_master_clean/es-app/src/SystemScreenSaver.cpp:604:4: error: ‘mCurrentView’ was not declared in this scope
    mCurrentView->onShow();
    ^~~~~~~~~~~~

[pjft]

@Lolonois Thank you so much - I'll review these later in the day! If I can't narrow it down I will very likely just force an up and a subsequent-immediate-down input to force the List component to refresh <awkward smile>. :) But we'll get to the bottom of this.

Also, apologies - I didn't mean to ignore some of your previous questions or comments, so if I miss any please call me out - I reply on the phone at times, or in the middle of other things and it happens.

You had asked me whether it was intentional that when using Left multiple times (without Right in between), that the images toggle on each Left between current and previous.

That was a very deliberate battle against my own instincts, but happy to be persuaded otherwise.

The main driver for the back use case was those occasions when I suddenly glance at the screensaver in the background - that every now and then I enjoy having on during the day - and notice a game that I'm curious about but, when I get there, it has moved on to the next. That has happened more often than not, so I thought of implementing "back".

The main question here was, then, that if one would move back once, why not move back more times? Well then, how many times? Infinite times, potentially? A predefined number of times (5? 10?). Sure enough, I could do them all - and I did hesitate. And what if you interrupt the screensaver in the middle of multiple back motions? Would you resume the screensaver where you left off? Could you potentially start the screensaver again and continue going back - storing the entire queue of videos that had been played so far?

Since the use case that I found more often was "darn, what was that game that I just too a glimpse of but it moved on to the next", and honestly I don't think there's often a "hey, let me go back to that specific game I saw 3 videos ago" use case, I kept it at a single back scenario.

Now the question was: well, what happens after you go back? What if now you want to return to the video that was playing since you did see it (for whatever reason - maybe you just want to go back to add it to a collection, and then resume the normal screensaver). We could either have "back" not to anything and stay in the same video, or - since it wouldn't hurt and it's still technically correct as it was "the video that was showing right before the one that's playing right now", I thought that this would be a workable compromise of sorts, and that I would not be overengineering things with the full-length queue just because I could :) The alternative would be for me to implement that when someone presses "right" after pressing "back", which could also be done, I suppose.

Well, this was the long story of that design decision. None of this is set in stone - hence the post here, so that folks try it out and get a feel for what works, what times some time to get used to, and what's just poorly designed (I am very confident that one of my future PRs has a high risk of falling in that category, and that's why it's still being slowly iterated on).

Let me know your thoughts in this regard, and how you see it.

@roslof Thank you for the feedback - happy that this is useful! Very valid remark, and of course I'll change that - I never want to break muscle memory for anything unless there's a very good reason (which, frankly, this isn't). I personally do not use the Jump To menu often, and since - unless I'm misremembering - some of the other projects have a "Launch Screensaver" entry on top of the Select menu (that's what I call it, but I should probably know better!), I thought I'd just adopt that position.

It should be changed now in the PR I mentioned above - https://github.com/RetroPie/EmulationStation/pull/847 - if you want to test it and see how it feels. I'm used to having SORT and JUMP be next to one another, so maybe I'll push it down one more row, but let me know your thoughts. I don't know if SORT gets used a lot, to be honest.

Thanks both - I'll try to send some updates to this in the coming days, and same for setting the default collection that one can add the games to from the screensaver.

Have a great week!

[pjft]

@Lolonois got it - this was perfect. From reading your settings file and the relevant options you had set, I changed the option on my end and I can confirm that it relates to

<bool name="UseFullscreenPaging" value="true" />

I believe you're more familiar with that part of the code, any thoughts at first glance of what might be the issue there? If not, I'll happily stumble through it later, but maybe it's something obvious to you? :)

[Lolonois]

@pjft said in Update: EmulationStation random video screensaver - use only media from a particular system, back and favorites controls:

maybe it's something obvious to you

Quick update: Obvious not, but I dealt with it in the past, right. I have an idea, let me vet it. Leave this with me. And thanks for pinpointing the complete context of this glitch.

[pjft]

@Lolonois Not at all, you did all the hard work to help pinpoint this!

To be fair, the two other scenarios where I expected the same to happen - using JUMP TO and FILTERing the gamelist - don't cause this behavior. And, furthermore, when the game returns from being launched it, indeed, resumes the normal behavior, so it must be some weird case there.

The only thing I'm doing on my end is calling setFocus(game) which, I suppose, I expected to set the focus on that game. It seems it does, but the mList component for the view does not follow it.

I suppose that, for the sake of abstraction, I kind of expected it to work the same regardless of that option being enabled (that'd be the preferred scenario). If not, sure enough, I can accommodate that on the selection flow when exiting the screensaver, though it will feel more like a workaround rather than a proper fix for the underlying cause :)

I may have more time for this in the weekend or next week, so if I have some time I'll also check it out.

Thank you!

[sleve_mcdichael]

emulationstation-dev f604dc built from source:

When select the first (top) game in any system or auto-collection (I don't use custom-collection), then when exit game and return to ES, game titles are blank and cursor is misplaced.

Scroll left-right to another system and back, issue remains. Scroll up or down to another title, issue corrects.

Does not present on emulationstation-dev 01de76 install from binary.

Does not present when any game except the first is selected.

Does present across multiple themes/systems/collections.

Screenshots:

es_settings.cfg:

pi@retropie:~ $ cat .emulationstation/es_settings.cfg
<?xml version="1.0"?>
<bool name="BackgroundIndexing" value="false" />
<bool name="BackgroundJoystickInput" value="false" />
<bool name="CaptionsCompatibility" value="true" />
<bool name="CollectionShowSystemInfo" value="true" />
<bool name="DisableKidStartMenu" value="true" />
<bool name="DoublePressRemovesFromFavs" value="false" />
<bool name="DrawFramerate" value="false" />
<bool name="EnableSounds" value="false" />
<bool name="ForceDisableFilters" value="false" />
<bool name="IgnoreLeadingArticles" value="true" />
<bool name="LocalArt" value="false" />
<bool name="MoveCarousel" value="true" />
<bool name="ParseGamelistOnly" value="false" />
<bool name="QuickSystemSelect" value="true" />
<bool name="ScrapeRatings" value="false" />
<bool name="ScreenSaverControls" value="true" />
<bool name="ScreenSaverOmxPlayer" value="false" />
<bool name="ScreenSaverVideoMute" value="true" />
<bool name="ShowHelpPrompts" value="true" />
<bool name="ShowHiddenFiles" value="false" />
<bool name="SlideshowScreenSaverCustomMediaSource" value="true" />
<bool name="SlideshowScreenSaverRecurse" value="false" />
<bool name="SlideshowScreenSaverStretch" value="false" />
<bool name="SortAllSystems" value="false" />
<bool name="StretchVideoOnScreenSaver" value="false" />
<bool name="SystemSleepTimeHintDisplayed" value="false" />
<bool name="ThreadedLoading" value="true" />
<bool name="UseCustomCollectionsSystem" value="true" />
<bool name="UseFullscreenPaging" value="true" />
<bool name="VideoAudio" value="false" />
<bool name="VideoOmxPlayer" value="false" />
<int name="MaxVRAM" value="80" />
<int name="ScraperResizeHeight" value="0" />
<int name="ScraperResizeWidth" value="400" />
<int name="ScreenSaverSwapMediaTimeout" value="10000" />
<int name="ScreenSaverSwapVideoTimeout" value="30000" />
<int name="ScreenSaverTime" value="300000" />
<int name="SubtitleSize" value="55" />
<int name="SystemSleepTime" value="0" />
<string name="AudioCard" value="default" />
<string name="AudioDevice" value="HDMI" />
<string name="CollectionSystemsAuto" value="all,recent" />
<string name="CollectionSystemsCustom" value="" />
<string name="GamelistViewStyle" value="automatic" />
<string name="LeadingArticles" value="a,an,the" />
<string name="OMXAudioDev" value="both" />
<string name="PowerSaverMode" value="disabled" />
<string name="SaveGamelistsMode" value="on exit" />
<string name="Scraper" value="ScreenScraper" />
<string name="ScreenSaverBehavior" value="random video" />
<string name="ScreenSaverGameInfo" value="start &amp; end" />
<string name="SlideshowScreenSaverBackgroundAudioFile" value="/home/pi/.emulationstation/slideshow/audio/slideshow_bg.wav" />
<string name="SlideshowScreenSaverImageFilter" value=".png,.jpg" />
<string name="SlideshowScreenSaverMediaDir" value="/home/pi/.emulationstation/slideshow/image" />
<string name="SlideshowScreenSaverVideoFilter" value=".mp4,.avi" />
<string name="StartupSystem" value="" />
<string name="SubtitleAlignment" value="center" />
<string name="SubtitleFont" value="/usr/share/fonts/truetype/freefont/FreeSans.ttf" />
<string name="SubtitleItalicFont" value="/usr/share/fonts/truetype/freefont/FreeSansOblique.ttf" />
<string name="ThemeSet" value="carbon" />
<string name="TransitionStyle" value="instant" />
<string name="UIMode" value="Full" />
<string name="UIMode_passkey" value="uuddlrlrba" />
<string name="VlcScreenSaverResolution" value="original" />

[pjft]

@sleve_mcdichael said in Update: EmulationStation random video screensaver - use only media from a particular system, back and favorites controls:

<bool name="UseFullscreenPaging" value="true" />

Thanks! It's actually a different/related symptom to what @Lolonois is looking into. Can you disable "Use LB/RB for full screen paging" and see if it changes things?

[pjft]

@sleve_mcdichael However, just to add: if you do confirm that this is effectively something that you can absolutely not reproduce in 01de7618d0d248fa2ff1eacde09a20d9d2af5f10 that's both an useful insight for bisecting things, but also unexpected as most commits since then are mine and related to this, and I don't think I changed anything that would have changed that behavior...

...though that's what anyone would say :)

This is helpful, thank you so much.

Assuming I can replicate it, this will go a long way towards narrowing it down. I'll try to bisect it here - thanks!

[pjft]

EDIT: TL;DR: yes, it's not reproduceable in 01de76. I was tired and running the wrong binary.

@sleve_mcdichael Ok, just to close this and I'll start editing this last post for updates, but so nobody spends more time on this without this update:

• I can (EDIT: NOT) reproduce this on 01de76 for some weird reason related to my failure to run the binary I was building. I will try to go a bit further back and see when I can't stop reproducing the issue and bisect it. I will curl in a corner and cry, and then bisect it properly!

This shouldn't happen if you disable the full page scroll with the LB/RB buttons. Did you per chance enable them today (I think not)?

This happened to me when I got into that messed up state @Lolonois reported, so since I suppose fixing this may help with fixing the original symptom reported as well, I am invested in fixing this :) I'll report back and update this post with the bisect results.

Still, if you can 100% confirm that it does not happen on your 01de76, I'd love to dig a bit more into this, but I can 100% replicate this now by:
- Starting ES
- Moving right (just, because, why not?)
- Opening that system
- Starting the first game
- Returning to ES (in my case, I just go to runcommand and tell it to exit without launching. It should not affect this at all!).

[sleve_mcdichael]

@pjft I am still seeing it with full-screen paging disabled:

(disabled via cfg before launching ES)

[sleve_mcdichael]

@pjft also, when returning from a game at or near the bottom of the list, the selected title appears vertically centered, even if that means blank lines after the final entry. Full screen paging does not affect this either.

(When browsing normally, the list stops scrolling at the bottom and the cursor itself moves down the last few entries instead of remaining centered.)

[pjft]

@sleve_mcdichael Awesome. Yes, this is on me, I just bisected it to an attempt at improving ES performance. Sigh. I'll revert it for now.

Can you test the latest emulationstation-dev code?

Also, for the last examples, can you check whether it is fixed - and, if not, if they were working on the previous commit you mentioned? Thank you.

@Lolonois alas, this was unrelated to your symptoms, the behavior you shared is still present. I can help dig into it if you give me guidance or directions.

[Lolonois]

@pjft said in Update: EmulationStation random video screensaver - use only media from a particular system, back and favorites controls:

The only thing I'm doing on my end is calling setFocus(game) which, I suppose, I expected to set the focus on that game. It seems it does, but the mList component for the view does not follow it.

You are right and were pretty close with setting setCursor(). However, the list renderer got confused as there was no user navigation to the game in question.

Two things happened and cumulated.

One: Without the setViewportTop(-1) in the selectGame() the renderer of TextListComponent.h [1] is not pre-initialize with "place cursor mid on screen with selected game", in turn this de-railed the cursor placement in the subsequent viewportTop() [2].

Two: For some reasons the mCursorPrev was not -1 (initial value) even with setViewportTop(-1), which then made an assumption fail in viewportTop(). (The assumption was that a user can only navigate to game after entering the system manually). It came to me when forcing the cursor to be mid-aligned, the previous cursor position is irrelevant (=it is safe to set to -1).

This is what the patch [3] does.

PS: I had to inspect the values in render() and viewportTop() to get to this conclusion.

PPS: Once you find this working ok, I have a followup PR to make the code more understandable (i.e. replacing -1 with an constant with a meaningful name).

[1] https://github.com/RetroPie/EmulationStation/blob/19fd7d9ae91f22caca948acc46fb9dac587b2023/es-app/src/components/TextListComponent.h#L142
[2] https://github.com/RetroPie/EmulationStation/blob/19fd7d9ae91f22caca948acc46fb9dac587b2023/es-app/src/components/TextListComponent.h#L257
[3] https://github.com/RetroPie/EmulationStation/pull/850

[pjft]

@Lolonois Thank you! I'll test it out later, but I trust your judgement there. I left a comment on the PR.

[pjft]

Just updating everyone here, I believe most of the reported issues have been fixed by now. Thanks @Lolonois for the help with the bugfixing as well.

If you all can update to the latest code, do let us know if there's anything missing on this end.

Thanks all.