godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.97k stars 21.16k forks source link

Use of ProjectSettings is inconsistent across the engine #70354

Open albinaask opened 1 year ago

albinaask commented 1 year ago

Godot version

3.5

System information

Windows 10

Issue description

The use of the project settings are inconsistent across the engine, some settings can be changed on the fly, while others have no effect changing.

For example the "rendering/quality/voxel_cone_tracing/high_quality" seem to have a immediate effect on the engine behaviour according to the tps demo line 13. While the "display/window/size/resizable" seem to have no effect on the window being resizable. I can set the parameter to false without error and even read it back from project settings and get false, while still being able to resize the window just fine. Why can I set it if it has no bearing on the execution?

It could of course be set through the OS class, but That's really beside the point. When I can change a setting, from a user's perspective I would assume that the project behaves according to that new setting value if the documentation doesn't say otherwise. Or at least get a flag back that tells me whether the setting has no effect until I save the settings and then rerun the project or if the engine will alter it's behavior. Now it feels like the method "set_setting" really has a misleading title and description since it in some cases alter the engine behaviour and sometimes seem to be completely irrelevant to anything other than to save a copy of the settings. (Which you can't then read back in to set settings since that has no effect...)

Steps to reproduce

After having a look at the link provided above, you may want to paste the following code into a script to understand how weird it feels:

    ProjectSettings["display/window/size/resizable"] = false
    #ProjectSettings.set_setting("display/window/size/resizable", false)
    print(ProjectSettings.get_setting("display/window/size/resizable"))

Minimal reproduction project

N/a

Calinou commented 1 year ago

Why can I set it if it has no bearing on the execution?

The ProjectSettings singleton doesn't know if settings will be used at run-time or not, so it can't prevent you from changing those. There is a PROPERTY_USAGE_RESTART_IF_CHANGED property hint applied to those project settings, but it's purely declarative (it must be manually set on each project setting). I don't know if it's available in exported projects too. A warning message will likely have to be constrained to debug builds for performance reasons anyway.

We could also make more project settings adjustable at run-time, which is possible now that we have a project_settings_changed signal (so we don't have to read values every frame, which is expensive). cc @lawnjelly

Still, not all project settings can be made adjustable at run-time, so a warning will still be useful for at least a dozen project settings or so.

lawnjelly commented 1 year ago

Yeah there's certainly quite a few I'm personally responsible for (in batching for instance) I've been meaning to make hot swappable on the fly.

There are full details of how to make hot swappable in comments on the project_settings.h file, but it's one of those housekeeping tasks that isn't the most exciting.

I did actually do a PR to make all this even easier #60926 (as developers are lazy), but reduz was against this approach and thought we should do it the longhand way (with predictable results :smile: , which proves my point about developers being lazy ). Actually, eat my words, it didn't work as is for Variant because of order of destruction, but we could still do this for e.g. bool / int / float.

Some should be reset requiring project settings, but this is a little annoying, because you only require reset if you want to see the effect in the editor, it will still take effect if you run the project from within the editor.

Maybe we could mark some as non hot-swappable, but that might be as involved as actually making them hot swappable in some cases...