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

Move all ProjectSetting and EditorSetting definitions to a centralized place #4771

Open
KoBeWi opened this issue Jun 28, 2022 · 3 comments
Open

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Jun 28, 2022

Describe the project you are working on

Godot etc

Describe the problem or limitation you are having in your project

We have GLOBAL_DEF and EDITOR_DEF macros that define a project setting and editor settings. They can be used anywhere. They are used everywhere. There is no policy at all on how they should be used, which led to e.g. defining the same setting multiple times (I took some time to fix that in godotengine/godot#58847 and godotengine/godot#61549).

There are some efforts to document Project Settings and Editor Settings. The problem is that, since they are defined all over the codebase, not all settings can be fetched reliably. When doctool is run (the tool to generate docs), it will use all currently defined properties, but it can happen that some properties are defined after doctool code or may not be defined at all in a doctool run (see #1339).

Describe the feature / enhancement and how it helps to overcome the problem or limitation

We could just move all the settings to one place. I get that it's convenient to just define something in the place where you use it, but it can easily create inconsistency and unpredictable behavior.

For Project Settings, the place is ProjectSettings constructor.
For EditorSettings, there is _load_defaults() method.

This way there is one entry point for all settings (no duplicate definitions by mistake etc.), you don't need to search for their definitions in hundreds of files and they are available when needed.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

https://github.com/godotengine/godot/blob/9d29166bc6f8f9f4359f186b43dfb869c476f70b/core/config/project_settings.cpp#L1165
https://github.com/godotengine/godot/blob/9d29166bc6f8f9f4359f186b43dfb869c476f70b/editor/editor_settings.cpp#L330

If this enhancement will not be used often, can it be worked around with a few lines of script?

No.

Is there a reason why this should be core and not an add-on in the asset library?

Can't be addon.

@YuriSizov
Copy link
Contributor

I guess the main good reason why they are so spread out is that if their owning module or engine part is disabled, then the setting won't appear. So we should probably keep that in mind and evaluate every case individually. Ideally, even those cases have to be standardized.

@KoBeWi
Copy link
Member Author

KoBeWi commented Jun 28, 2022

Maybe we could add a method to modules to define their own settings, similar to register types.

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 23, 2023

I gave it another another though and indeed moving everything to a single method does not make much sense.

As for a "method for stuff to define their own settings", _bind_methods() can be used for that. It's called for every class and it's where properties are defined for the documentation, so it's a good place for settings. If a setting is global to a module, it can be defined in register_types(). The _load_defaults() can be used for settings that don't belong anywhere, but I doubt there are such 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants