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

Use variables as setting names instead of strings #1395

Open
RiscadoA opened this issue Dec 1, 2024 · 0 comments
Open

Use variables as setting names instead of strings #1395

RiscadoA opened this issue Dec 1, 2024 · 0 comments
Labels
A-Engine B-Settings C-Code-Quality A section of code that is hard to understand or change P-Normal This issue isn't a big priority, but it would still be nice to have it closed soon

Comments

@RiscadoA
Copy link
Member

RiscadoA commented Dec 1, 2024

Problem

Currently our settings system is a bit flaky. Here are some of its problems:

  • Settings are identified by strings, without any enforced format. The IDE can't help us find the names of settings, and it's very easy to mispell them and then get a runtime error, or worse, simply sligthly different behavior.
  • Settings have string values which aren't type checked. For example, if you do settings.setString("foo", "true");, settings.getBool("foo", false) will return true, although that isn't the right type.
  • There isn't a single location where you define or initialize a given setting. Instead, whenever you do .getString(name, defaultValue), the setting is initialized with the given default value if there isn't one already. This means that you may end up having multiple places with different default values for the same setting, or even different types. This is very bug prone and difficult to maintain.

Solution

We could make settings something similar to the tags we currently have.
E.g., on a plugin.hpp, we could declare all of it's settings:

/// @brief We can even document settings and have them appear in the docs!
CUBOS_ENGINE_API extern Setting<glm::ivec2> windowSizeSetting;

// ...
CUBOS_ENGINE_API extern Tag windowInitTag;
// ...
CUBOS_ENGINE_API void windowPlugin(Cubos& cubos);

On the plugin.cpp:

CUBOS_DEFINE_SETTING(cubos::engine::windowSizeSetting, {800, 600} /* default value */);

// ...
CUBOS_DEFINE_TAG(cubos::engine::windowInitTag);
// ...

void cubos::engine::windowPlugin(Cubos& cubos)
{
  // ...
  cubos.startupSystem("open Window")
            .call([](/*other args*/, Settings& settings) {
                glm::ivec2 windowSize = settings.get(windowSizeSetting);
            });
  // ...
}

These settings could be (de)serialized to/from JSON using our JSON (de)serializers. E.g.:

{
  "cubos::engine::windowSizeSetting": {
    "x": 800,
    "y": 600
  }
}
@RiscadoA RiscadoA added A-Engine B-Settings C-Code-Quality A section of code that is hard to understand or change labels Dec 1, 2024
@RiscadoA RiscadoA added the P-Normal This issue isn't a big priority, but it would still be nice to have it closed soon label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Engine B-Settings C-Code-Quality A section of code that is hard to understand or change P-Normal This issue isn't a big priority, but it would still be nice to have it closed soon
Projects
None yet
Development

No branches or pull requests

1 participant