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

[WIP] Dynamic configuration framework #385

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

375gnu
Copy link
Member

@375gnu 375gnu commented Sep 2, 2019

The problem:

  • Currently configuration file is read by frontend while actual consumers are spread over the code. In current implementation frontend must know everything about configuration options, i.e. their types, valid values. There is an example for SolarSystemMaxDistance used in render.cpp:
src/celestia/qt/qtglwidget.cpp:    appRenderer->setSolarSystemMaxDistance(appCore->getConfig()->SolarSystemMaxDistance);
src/celestia/configfile.cpp:    configParams->getNumber("SolarSystemMaxDistance", maxDist);
src/celestia/configfile.cpp:    config->SolarSystemMaxDistance = min(max(maxDist, 1.0f), 10.0f);
src/celestia/configfile.h:    float SolarSystemMaxDistance;
src/celestia/win32/winmain.cpp:    appCore->getRenderer()->setSolarSystemMaxDistance(appCore->getConfig()->SolarSystemMaxDistance);
src/celestia/glutmain.cpp:    appCore->getRenderer()->setSolarSystemMaxDistance(appCore->getConfig()->SolarSystemMaxDistance);
src/celestia/gtk/main.cpp:    app->renderer->setSolarSystemMaxDistance(app->core->getConfig()->SolarSystemMaxDistance);

So not only routines responsible for celestia.cfg reading aware of this option but GUI code as well. While the latter can be fixed with some common routines in celestiacore.cpp this will not make out config code more robust.

  • Configuration files are read on application startup only, to reconfigure we should restart it.

  • Frontends save some different options using their specific methods so Gtk or win-native frontends are unable to read configuration saved by Qt frontend and vice verse.

So the requirements:

  1. Any limitation on a configuration option should be applied on a consumer's side.
  2. Config reader should just parse the file and put it into a structure designed for it. This will allow us to keep plugins' configuration in the same file, without any need for per-plugin config files.
  3. Our code should provide means to dynamically reconfigure options in runtime.
  4. Our code should provide means to save configuration file, frontends should save only frontend-specific options like window position and size but not rendering options.

This PR is a very early attempt to provide such configuration framework, it provides only basic functions and even not integrated into Celestia codebase. src/celengine/testprops.cc provides an example of usage.

Let's discuss its pros and cons.

This reverts commit d37f2e7.

# Conflicts:
#	src/celengine/config.h
#	src/celengine/property.h
Copy link
Contributor

@pirogronian pirogronian left a comment

Choose a reason for hiding this comment

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

Something like Qt's QSettings, I like it. Namespaces are also better than "celengine" and so on. But I wonder if is something similar already implemented as independend project.

@pirogronian
Copy link
Contributor

Btw I would be also happy to see Parser class integration.

@375gnu
Copy link
Member Author

375gnu commented Sep 3, 2019

Btw I would be also happy to see Parser class integration.

What do you mean? Parser is an absolutely independent entity, it's only required to parse files.

@Adriankhl
Copy link
Contributor

I generally agree on the direction, have to take a closer look to comment on the actual implementation.

@Adriankhl
Copy link
Contributor

Adriankhl commented Sep 3, 2019

By the way, I realize that something like DATADIR (which is related to SPLASH_DIR, etc. ) are hard-coded in compile-time, is it better to load these in runtime through the configuration process?

@375gnu
Copy link
Member Author

375gnu commented Sep 3, 2019

By the way, I realize that something like DATADIR (which is related to SPLASH_DIR, etc. ) are hard-coded in compile-time, is it better to load these in runtime through the configuration proc

Good point

@pirogronian
Copy link
Contributor

pirogronian commented Sep 3, 2019

I meant Array and Value classed from parser.h

@375gnu
Copy link
Member Author

375gnu commented Sep 3, 2019

I meant Array and Value classed from parser.h

I will. Currently it's just a proof-of-concept so it doesn't contain all required code.

Instead of replying I edited your comment, lol

@375gnu 375gnu marked this pull request as draft April 20, 2020 12:51
@375gnu 375gnu removed the request for review from Adriankhl April 20, 2020 12:51
@375gnu 375gnu force-pushed the master branch 2 times, most recently from 690ab56 to 2f8f3b0 Compare May 15, 2023 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants