ES Screensaver UX "puzzles" and suggestions

Lolonois

According to "The details are not the details. These make the design." [Charles Eames] I found some unexpected UX behaviour in ES.

Most of them can be fixed with almost no effort. Just seeking feedback before PRing.

The select button is bound to enable/disable screensaver in the system view (the "carousel" in default theme). Good. Works as designed. However if you use the sleep feature and wake your system with the select button while you have been in the carousel before the sleep mode, ES will wake but jump right away into screensaver.
Expected: It should show the carousel view again in this case.
In Video Screensaver: When browsing the displayed videos (Joy Right or Start Button) and using the sleep functionality. When sleep time out is reached it is executed, regardless of the user input.
Expected: Sleep does get started after the last user input times out, including browsing in screensaver mode. So you reset the sleep time out with every Right / Select during screensaver.
Actions in Screensaver are bound to Select / Right for next video and to Start for directly launching the displayed game.
Expected: Launching a game should also be possible with Button A, this is the default behaviour in ES to start a game.
Currently only video screensaver allows navigation / launch. I could not come up with a reason why the image screensaver should not be able to behave the same (as long as you do not use a custom image folder). Thus,
expected: Image screensaver allows also browsing and direct launching.
Slight off topic: If there is only one game I expect the banner in the carousel to read "1 GAME" instead of "1 GAMES".

Thoughts?
If you agree with the expectations I can provide two PR (one for 1,2,3,5) and one for 4.

Regarding item 1: Why is it that launch screensaver is bound to release button event (input.value = 0), instead of press button (input.value = 1) event? (Scroll to the beginning of the outer if). I don't see the rationale behind it.

Cheers

Hex

The select button is bound to enable/disable screensaver in the system view (the "carousel" in default theme). Good. Works as designed. However if you use the sleep feature and wake your system with the select button while you have been in the carousel before the sleep mode, ES will wake but jump right away into screensaver.

This is expected and not a UX issue. ScreenSaver is disabled on button click and restarted if the button was Select

Currently only video screensaver allows navigation / launch. I could not come up with a reason why the image screensaver should not be able to behave the same

The Video screensaver is almost a hybrid along the lines of Attract mode (like the arcade). On the other hand Image screensaver is a true screensaver just like Windows/Mac/Linux desktop screen savers and exits on user input. This is the intended behavior and it is not expected to act like a navigation enabled slideshow. A key difference between Video and Image is that we know when a video terminates next one can be started. Image slideshow needs to also support switch time, blending mode, cropping, and a lot more image specific behavior that doesn't result in any proportional benefits to its writing and maintenance.

Lolonois

@Hex said in ES Screensaver UX "puzzles" and suggestions:

The select button is bound to enable/disable screensaver in the system view (the "carousel" in default theme). Good. Works as designed. However if you use the sleep feature and wake your system with the select button while you have been in the carousel before the sleep mode, ES will wake but jump right away into screensaver.

This is expected and not a UX issue. ScreenSaver is disabled on button click and restarted if the button was Select

NP with that.
But why is screensaver enabled on button release, while all other functions (at least to my knowledge) act on button press?

Currently only video screensaver allows navigation / launch. I could not come up with a reason why the image screensaver should not be able to behave the same

The Video screensaver is almost a hybrid along the lines of Attract mode (like the arcade). On the other hand Image screensaver is a true screensaver just like Windows/Mac/Linux desktop screen savers and exits on user input. This is the intended behavior and it is not expected to act like a navigation enabled slideshow. A key difference between Video and Image is that we know when a video terminates next one can be started. Image slideshow needs to also support switch time, blending mode, cropping, and a lot more image specific behavior that doesn't result in any proportional benefits to its writing and maintenance.

Hmm. Not sure if we are on the same side. If you change the condition from "random video" to "slideshow" (or add an or'ed condition for that) you can see what I relate to.

Additionally, the startScreensaver code for random video and slideshow look similar and mVideoChangeTime is even reused as variable for image swap).
It seems to me that the image slideshow should act like the random video including navigation, but for some reason the initial if is missing the condition for the slideshow.

Hex

@Lolonois said in ES Screensaver UX "puzzles" and suggestions:

But why is screensaver enabled on button release, while all other functions (at least to my knowledge) act on button press?

Did you try using git blame to see why/when or by who hat change was made ?

Hmm. Not sure if we are on the same side

The question is not why or how easily the code can be change to do it. It is most certainly, should we do it. And honestly, I don't agree that there is real benefit to implement and maintain the feature. I would recommend making a poll to see how many users are interested in it and if other devs welcome the feature. Once the proposal clears that hurdle, we will have to access a priority and find a dev willing to work on it. Then the PR needs to pass the review and/or iterate til it gets approved before being merged. You can volunteer to be the dev to work on it if you want to.

cc: @mitu @Tomaz @jdrassa @pjft tagging in case i missed something/ have outdated dev pocess

pjft

I have no problems with most of these PRs, by all means. I can't recall why the mapping of the action is mapped to button release but I do recall that now that you mention it and there could probably be a good reason.

Have you tried it on button press and does everything work - right, select and start during screensaver? I seem to recall that I had some concerns with the game launching (on key press) and then ES not capturing the key release event, or specifically for the "next game" video launch action. If you try them out and they work on both omx player and VLC, as well as the other actions, you can chalk it up to my inexperience with that part of the code at the time:)

The only change I'm not thrilled about is changing A to launch a game from the screensaver. In fact, going back to your remark around expectations and consistency, the expectation is that any key ends a screensaver. Having added a shortcut to launch games from a screensaver - with the option to turn it off which, if I recall correctly, is the default - is the thing that breaks that consistency and expectation.

The keys mapped to actions during the screensaver were deliberately chosen for not being common keys that you'd press, so that you wouldn't mistakenly have an action triggered when you were just trying to exit a screensaver. The main face buttons should only cause the screensaver to exit, in my opinion - that was the reasoning behind the implementation and I still haven't had any meaningful feedback or reason to change my mind in the past 3 years that this has been around. I take the point you're making, but that would be fair if it was a more explicit navigation/game selection mechanism of sorts. The main action that most users will want to perform when in a screensaver will be to have it exit, and launching a game is a very secondary action, and the buttons should map to that. The odds of it always having a game you want to launch would be slim, statistically speaking.

As for the image screensaver, I don't think there's any key reason for it not to behave the same way - it was just developed at a different time by a different person, and with the latch game use case not being a primary one, I guess it was never really implemented. I imagine it would be a welcome addition.

I can't comment much on the Sleep interactions - @Hex would be the right person there.

On my phone, so I might be missing other parts of your post - I'll reply later if that's the case.

Thanks for the interest here!

Lolonois

@Hex said in ES Screensaver UX "puzzles" and suggestions:

But why is screensaver enabled on button release, while all other functions (at least to my knowledge) act on button press?

Did you try using git blame to see why/when or by who hat change was made ?

I like to talk to humans first. However, the commit with the if (input.value != 0) and the else dates back to 2016-06-25 and was made by Aloshi in a major refactoring/file move. So I guess this is of no use.

I would recommend making a poll to see how many users are interested in it and if other devs welcome the feature.

IMHO the devs should not decide about the usefulness of any feature, if at all it should be the endusers, but still running a poll for these small changes is stiring the pot (while an dev can also be an enduser, the userbase is much more larger than the dev headcount). From my opinion the core maintainers are at the driver seat (maybe anticipating the endusers need) to steer the project/product to success (not monetary but in terms of other benefits).

This does not release neither the PR developer nor the QA from doing his or her best.

Cheers

Lolonois

@pjft said in ES Screensaver UX "puzzles" and suggestions:

I have no problems with most of these PRs, by all means. I can't recall why the mapping of the action is mapped to button release but I do recall that now that you mention it and there could probably be a good reason.
Have you tried it on button press and does everything work - right, select and start during screensaver? I seem to recall that I had some concerns with the game launching (on key press) and then ES not capturing the key release event

This is currently (master branch) acting on button press, did not change this. Only launch screensaver is performed on button release and the listinput is refreshed after releasing a joystick axis.

@pjft thanks for the extended explaination and setting the scene.

So to keep close to my initial suggestions.

Suggestion 1: Won't do - Agree with @Hex.
Suggestion 2: not decided yet, @Hex do you have a opinion here?
Suggestion 3: Won't do - Agree with you: Having a button to launch a game directly is the initial "Huh?" in UX. So I will not use an additional button (A) for in screensaver actions:

The only change I'm not thrilled about is changing A to launch a game from the screensaver. In fact, going back to your remark around expectations and consistency, the expectation is that any key ends a screensaver.
Suggestion 4: Should do. Thanks for the thumbs up. ("should" is the same as could as this is a voluntary effort, just to highlight this is higher ranked than the next one).
Suggestion 5: Could do (no opinion here atm, most likely the i18n branch team is remotely affected)

Thanks for the interest here!

My pleasure! (-:

pjft

@Lolonois Thanks for the thorough reply and for being keen on making this even better. I hope that my explanations from memory help.

I don't have a better explanation for the "on release" and "on press" discrepancy - I offered what my memory suggested could be valid reasons for it, but I'm also kind of grasping at straws.

Thanks for taking a stab at number 4.

For number 5, I agree with the change - it is, indeed, a bit awkward to have it as "1 GAMES". I am not aware of any current i18n efforts undergoing for ES, so feel free to go at it if you thing it'd be something you'd be keen on taking on!

Thanks, and have a great weekend.

Hex

@Lolonois said in ES Screensaver UX "puzzles" and suggestions:

I like to talk to humans first. However, the commit with the if (input.value != 0) and the else dates back to 2016-06-25 and was made by Aloshi in a major refactoring/file move. So I guess this is of no use.

I expected this to happen. I asked you to check git blame cos I dont remember who changed that code last and it they are still active on the forums.

and if other devs welcome the feature.

As in if any other dev is willing to work on the Feature. We do need someone to work on it for the initial implementation. No gate keeping there. If you are interested to work on it, that would be great too.

Suggestion 2: not decided yet, @Hex do you have a opinion here?

Actually I dont have any opinion on this. My priority is to make PowerSaver feature work as expected. UX change is not an issue as I am more of an Infra dev o this project. I think that decision should come from core devs responsible for UI/UX.

I suggested you create a poll so we can get feedback from actual users and not just the subset of users frequenting the Ideas and Development section.

Lolonois

Hi @pjft just filed the PRs for No 2 (PR 672), 4 (PR 673) and 5 (PR 671) of the suggestions here.

Feel free to review them.

pjft

@Lolonois said in ES Screensaver UX "puzzles" and suggestions:

In Video Screensaver: When browsing the displayed videos (Joy Right or Start Button) and using the sleep functionality. When sleep time out is reached it is executed, regardless of the user input.

Ah - I now understood what #672 was about. Thanks for detailing it here.

Hex

Looks good

Lolonois

@Hex said in ES Screensaver UX "puzzles" and suggestions:

Looks good

Thanks. :-)

Lolonois

For those want to testdrive the navigation (Joy-right/Select) and launch game (Start) also for the image slideshow screen saver -see suggestion 4 above- are invited to do so:

git clone https://github.com/RetroPie/EmulationStation.git
cd EmulationStation
git fetch origin pull/673/head:slideshow-controls
git checkout slideshow-controls
cmake -DFREETYPE_INCLUDE_DIRS=/usr/include/freetype2/ -DGL=On -DRPI=On . && make -j 5  all
./emulationstation --debug