Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

display: Add support for controling screensaver state. part 1/2 #1734

Merged
merged 4 commits into from May 23, 2020
Merged

display: Add support for controling screensaver state. part 1/2 #1734

merged 4 commits into from May 23, 2020

Conversation

nthykier
Copy link
Contributor

@nthykier nthykier commented May 14, 2020 •
edited by illume

... on platforms that support it with SDL2.

SDL2 does not provide any visible feedback on whether the controls
have an effect and may "silently" say it changed the state without
actually doing anything (except update the boolean variable, which
SDL_IsScreenSaverEnabled returns). Accordingly, we document that
these functions may be unreliable on certain platforms without any
visible clue.

For SDLv1, we just raise a TypeError as these functions are not
supported there.

Closes: #1707
Signed-off-by: Niels Thykier niels@thykier.net

https://wiki.libsdl.org/SDL_HINT_VIDEO_ALLOW_SCREENSAVER

@nthykier nthykier force-pushed the sdl2-screensaver-support branch 2 times, most recently from a084293 to a49132e Compare May 14, 2020 18:23
@nthykier
display: Add support for controling screensaver state
6baadc7 
... on platforms that support it with SDL2.

SDL2 does not provide any visible feedback on whether the controls
have an effect and may "silently" say it changed the state without
actually doing anything (except update the boolean variable, which
`SDL_IsScreenSaverEnabled` returns).  Accordingly, we document that
these functions may be unreliable on certain platforms without any
visible clue.

For SDLv1, we just raise a `TypeError` as these functions are not
supported there.

Closes: pygame#1707
Signed-off-by: Niels Thykier <niels@thykier.net>
@nthykier nthykier requested a review from MightyJosip May 14, 2020 20:50
MightyJosip
MightyJosip approved these changes May 15, 2020
View reviewed changes
Copy link
Contributor

@MightyJosip MightyJosip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks now everything should be ok

MyreMylar
MyreMylar suggested changes May 15, 2020
View reviewed changes
docs/reST/ref/display.rst Outdated Show resolved Hide resolved
@MyreMylar MyreMylar added enhancement display pygame.display labels May 15, 2020
@illume
Copy link
Member

illume commented May 17, 2020

I made a few doc changes, and am making a few changes to display.c now.

nthykier
nthykier commented May 17, 2020
View reviewed changes
docs/reST/ref/display.rst Outdated Show resolved Hide resolved
@illume
Copy link
Member

illume commented May 17, 2020 •
edited

This may just be me, but I think the word 'enable' is misleading here?

SDL_EnableScreenSaver and set_screensaver_enable when read by themselves suggest that the screensaver will be turned on right then. Not that the screen saver will kick in after 10 minutes of inactivity (or whatever the setting is).

How about we follow the environment variable name a bit? 'Allow' from SDL_HINT_VIDEO_ALLOW_SCREENSAVER seems a better word choice than enable as used in the SDL function names.

  • display.set_allow_screensaver
  • display.get_allow_screensaver

https://wiki.libsdl.org/SDL_HINT_VIDEO_ALLOW_SCREENSAVER
https://wiki.libsdl.org/SDL_EnableScreenSaver
https://wiki.libsdl.org/SDL_DisableScreenSaver
https://wiki.libsdl.org/SDL_IsScreenSaverEnabled

@nthykier if you're ok with it, I can change them to get_allow_screensaver/set_allow_screensaver ?

@nthykier
Copy link
Contributor Author

nthykier commented May 23, 2020 •
edited

@illume Renaming them is fine with me. :)

@nthykier nthykier requested a review from MyreMylar May 23, 2020 08:58
@nthykier
Copy link
Contributor Author

@MyreMylar I pinged you for a review because you had a changes requested that have not been resolved.

@illume
Copy link
Member

illume commented May 23, 2020

Ok, thanks :) I'll work on renaming now.

@illume illume changed the base branch from master to sdl2-screensaver-support May 23, 2020 16:59
@illume illume changed the title display: Add support for controling screensaver state display: Add support for controling screensaver state. part 1/2 May 23, 2020
@illume
Copy link
Member

illume commented May 23, 2020

Merging into sdl2-screensaver-support branch so I can continue.

@illume illume merged commit edaa706 into pygame:sdl2-screensaver-support May 23, 2020
@illume illume mentioned this pull request May 23, 2020
Merged
@nthykier nthykier deleted the sdl2-screensaver-support branch September 17, 2020 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@illume illume illume left review comments

@MyreMylar MyreMylar MyreMylar approved these changes

@MightyJosip MightyJosip MightyJosip approved these changes

Assignees
No one assigned
Labels
display pygame.display enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow screen blanking while pygame is running
4 participants
@nthykier @illume @MyreMylar @MightyJosip