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.

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

Only define keep_screen_on project setting once #64014

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

RedMser
Copy link
Contributor

@RedMser RedMser commented Aug 6, 2022

Discussed in #63882

The .editor feature override does not currently work, since overrides are disabled in the editor it seems.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good.

@@ -1481,7 +1481,6 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
OS::get_singleton()->_allow_layered = false;
}

OS::get_singleton()->_keep_screen_on = GLOBAL_DEF("display/window/energy_saving/keep_screen_on", true);
Copy link
Member

Choose a reason for hiding this comment

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

Actually this might change behavior. I don't see the Android implementation querying the setting so it might break now.

I also wonder why it needs to be done manually in each platform instead of just done in main.cpp after the DisplaySerer is instantiated?

Copy link
Contributor Author

@RedMser RedMser Aug 6, 2022

Choose a reason for hiding this comment

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

I don't see the Android implementation querying the setting

Isn't this the android implementation? I also couldn't find any uses for the OS singleton's variable.

why it needs to be done manually in each platform

I'm not sure, maybe it was done this way since not all platforms implement the function? I could try changing how it's being called, but can't really test it for all platforms.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see the Android implementation querying the setting

Isn't this the android implementation? I also couldn't find any uses for the OS singleton's variable.

Ah indeed.

Ah I was going to suggest making OS::_keep_screen_on private but it seems like it's already the case, this was just abusing the fact that OS is a friend class to Main.

@akien-mga akien-mga merged commit 5df8eae into godotengine:master Aug 8, 2022
@akien-mga
Copy link
Member

Thanks!

reduz added a commit to reduz/godot that referenced this pull request Jan 13, 2023
* Overrides no longer happen for set/get.
* They must be checked with a new function: `ProjectSettings::get_setting_with_override()`.
* GLOBAL_DEF/GLOBAL_GET updated to use this

This change solves many problems:
* General confusion about getting the actual or overriden setting.
* Feature tags available after settings are loaded were being ignored, they are now considered.
* Hacks required for the Project Settings editor to work.

Fixes godotengine#64100. Fixes godotengine#64014. Fixes godotengine#61908.
reduz added a commit to reduz/godot that referenced this pull request Jan 13, 2023
* Overrides no longer happen for set/get.
* They must be checked with a new function: `ProjectSettings::get_setting_with_override()`.
* GLOBAL_DEF/GLOBAL_GET updated to use this

This change solves many problems:
* General confusion about getting the actual or overriden setting.
* Feature tags available after settings are loaded were being ignored, they are now considered.
* Hacks required for the Project Settings editor to work.

Fixes godotengine#64100. Fixes godotengine#64014. Fixes godotengine#61908.
reduz added a commit to reduz/godot that referenced this pull request Jan 13, 2023
* Overrides no longer happen for set/get.
* They must be checked with a new function: `ProjectSettings::get_setting_with_override()`.
* GLOBAL_DEF/GLOBAL_GET updated to use this

This change solves many problems:
* General confusion about getting the actual or overriden setting.
* Feature tags available after settings are loaded were being ignored, they are now considered.
* Hacks required for the Project Settings editor to work.

Fixes godotengine#64100. Fixes godotengine#64014. Fixes godotengine#61908.
reduz added a commit to reduz/godot that referenced this pull request Jan 13, 2023
* Overrides no longer happen for set/get.
* They must be checked with a new function: `ProjectSettings::get_setting_with_override()`.
* GLOBAL_DEF/GLOBAL_GET updated to use this

This change solves many problems:
* General confusion about getting the actual or overriden setting.
* Feature tags available after settings are loaded were being ignored, they are now considered.
* Hacks required for the Project Settings editor to work.

Fixes godotengine#64100. Fixes godotengine#64014. Fixes godotengine#61908.
reduz added a commit to reduz/godot that referenced this pull request Jan 13, 2023
* Overrides no longer happen for set/get.
* They must be checked with a new function: `ProjectSettings::get_setting_with_override()`.
* GLOBAL_DEF/GLOBAL_GET updated to use this

This change solves many problems:
* General confusion about getting the actual or overriden setting.
* Feature tags available after settings are loaded were being ignored, they are now considered.
* Hacks required for the Project Settings editor to work.

Fixes godotengine#64100. Fixes godotengine#64014. Fixes godotengine#61908.
reduz added a commit to reduz/godot that referenced this pull request Jan 13, 2023
* Overrides no longer happen for set/get.
* They must be checked with a new function: `ProjectSettings::get_setting_with_override()`.
* GLOBAL_DEF/GLOBAL_GET updated to use this

This change solves many problems:
* General confusion about getting the actual or overriden setting.
* Feature tags available after settings are loaded were being ignored, they are now considered.
* Hacks required for the Project Settings editor to work.

Fixes godotengine#64100. Fixes godotengine#64014. Fixes godotengine#61908.
Streq pushed a commit to Streq/godot that referenced this pull request Feb 9, 2023
* Overrides no longer happen for set/get.
* They must be checked with a new function: `ProjectSettings::get_setting_with_override()`.
* GLOBAL_DEF/GLOBAL_GET updated to use this

This change solves many problems:
* General confusion about getting the actual or overriden setting.
* Feature tags available after settings are loaded were being ignored, they are now considered.
* Hacks required for the Project Settings editor to work.

Fixes godotengine#64100. Fixes godotengine#64014. Fixes godotengine#61908.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants