Configuration path should have single source of truth

[jstsmthrgk]

Back when I made #2047 I only changed one of apparently four (I realized it because of #1437) places where the configuration path is being decided. It would make sense to just have one single place where the selection logic is run.

My suggestions would be to either:

• have a static function in PreferencesManager that computes the configuration path and returns it, or
• compute it in main.cpp and somehow store it in PreferencesManager (either as a preference or with its own getter/setter)

If I get told which way is prefered, I will implement it and make a pull request. (Also if wished I could directly incorporate #1437 in it.)

[daid]

I've been moving stuff around and cleaning things up in the ECS branch around this, I think that makes it better? https://github.com/daid/EmptyEpsilon/blob/ECS/src/init/config.cpp

[jstsmthrgk]

not quite, a quick search for getenv shows that it is still there (the USER and USERNAME is something else, but every getenv("HOME") is another place it is calculated)

master branch:

$ grep -nr getenv
src/main.cpp:135:    if (getenv("EE_CONF_DIR"))
src/main.cpp:136:        configuration_path = string(getenv("EE_CONF_DIR"));
src/main.cpp:137:    else if (getenv("HOME"))
src/main.cpp:138:        configuration_path = string(getenv("HOME")) + "/.emptyepsilon";
src/main.cpp:183:        if (getenv("HOME"))
src/main.cpp:185:            new DirectoryResourceProvider(string(getenv("HOME")) + "/.emptyepsilon/resources/mods/" + mod);
src/main.cpp:186:            PackResourceProvider::addPackResourcesForDirectory(string(getenv("HOME")) + "/.emptyepsilon/resources/mods/" + mod);
src/main.cpp:195:    if (getenv("HOME"))
src/main.cpp:197:        new DirectoryResourceProvider(string(getenv("HOME")) + "/.emptyepsilon/resources/");
src/main.cpp:198:        new DirectoryResourceProvider(string(getenv("HOME")) + "/.emptyepsilon/scripts/");
src/main.cpp:199:        PackResourceProvider::addPackResourcesForDirectory(string(getenv("HOME")) + "/.emptyepsilon/packs/");
src/main.cpp:229:        if (getenv("USERNAME"))
src/main.cpp:230:            PreferencesManager::setTemporary("username", getenv("USERNAME"));
src/main.cpp:231:        else if (getenv("USER"))
src/main.cpp:232:            PreferencesManager::setTemporary("username", getenv("USER"));
src/menus/optionsMenu.cpp:204:        if (getenv("HOME"))
src/menus/optionsMenu.cpp:205:            PreferencesManager::save(string(getenv("HOME")) + "/.emptyepsilon/options.ini");
src/scriptDataStorage.cpp:11:        if (getenv("HOME"))
src/scriptDataStorage.cpp:13:            scriptstorage_path = string(getenv("HOME")) + "/.emptyepsilon/" + scriptstorage_path;

ECS branch:

$ grep -nr getenv
src/menus/optionsMenu.cpp:204:        if (getenv("HOME"))
src/menus/optionsMenu.cpp:205:            PreferencesManager::save(string(getenv("HOME")) + "/.emptyepsilon/options.ini");
src/script/dataStorage.cpp:11:    if (getenv("HOME"))
src/script/dataStorage.cpp:13:        scriptstorage_path = string(getenv("HOME")) + "/.emptyepsilon/" + scriptstorage_path;
src/init/config.cpp:18:    if (getenv("EE_CONF_DIR"))
src/init/config.cpp:19:        configuration_path = string(getenv("EE_CONF_DIR"));
src/init/config.cpp:20:    else if (getenv("HOME"))
src/init/config.cpp:21:        configuration_path = string(getenv("HOME")) + "/.emptyepsilon";
src/init/config.cpp:45:        if (getenv("USERNAME"))
src/init/config.cpp:46:            PreferencesManager::setTemporary("username", getenv("USERNAME"));
src/init/config.cpp:47:        else if (getenv("USER"))
src/init/config.cpp:48:            PreferencesManager::setTemporary("username", getenv("USER"));
src/init/resources.cpp:11:        if (getenv("HOME"))
src/init/resources.cpp:13:            new DirectoryResourceProvider(string(getenv("HOME")) + "/.emptyepsilon/resources/mods/" + mod);
src/init/resources.cpp:14:            PackResourceProvider::addPackResourcesForDirectory(string(getenv("HOME")) + "/.emptyepsilon/resources/mods/" + mod);
src/init/resources.cpp:23:    if (getenv("HOME"))
src/init/resources.cpp:25:        new DirectoryResourceProvider(string(getenv("HOME")) + "/.emptyepsilon/resources/");
src/init/resources.cpp:26:        new DirectoryResourceProvider(string(getenv("HOME")) + "/.emptyepsilon/scripts/");
src/init/resources.cpp:27:        PackResourceProvider::addPackResourcesForDirectory(string(getenv("HOME")) + "/.emptyepsilon/packs/");

The way I interpret this (in the ECS branch) is that each getenv outside of config.cpp should be eliminated somehow.

[daid]

Resources providers have nothing to do with preferences, the whole point of those is that there can be multiple sources of resources.

[jstsmthrgk]

Well, I'll just go through them

src/menus/optionsMenu.cpp:204:        if (getenv("HOME"))
src/menus/optionsMenu.cpp:205:            PreferencesManager::save(string(getenv("HOME")) + "/.emptyepsilon/> options.ini");

This one definitely must be the same one as the one in config.cpp, as else the options are saved somewhere where they are not going to be loaded from.

src/script/dataStorage.cpp:11:    if (getenv("HOME"))
src/script/dataStorage.cpp:13:        scriptstorage_path = string(getenv("HOME")) + "/.emptyepsilon/" + scriptstorage_path;

This seems to not be a resource provider but rather a way to store script state, there is only one place it is stored, it would be confusing if it would still use ~/.emptyepsilon if $EE_CONF_DIR is set. (Maybe it would make sense to make it configurable via the options and have $EE_CONF_DIR/scriptstorage.json as the default for that?)

src/init/resources.cpp:11:        if (getenv("HOME"))
src/init/resources.cpp:13:            new DirectoryResourceProvider(string(getenv("HOME")) + "/.emptyepsilon/resources/mods/" + mod);
src/init/resources.cpp:14:            PackResourceProvider::addPackResourcesForDirectory(string(getenv("HOME")) + "/.emptyepsilon/resources/mods/" + mod);
src/init/resources.cpp:23:    if (getenv("HOME"))
src/init/resources.cpp:25:        new DirectoryResourceProvider(string(getenv("HOME")) + "/.emptyepsilon/resources/");
src/init/resources.cpp:26:        new DirectoryResourceProvider(string(getenv("HOME")) + "/.emptyepsilon/scripts/");
src/init/resources.cpp:27:        PackResourceProvider::addPackResourcesForDirectory(string(getenv("HOME")) + "/.emptyepsilon/packs/");

Yes, these are resource providers and they don't do any harm when they look there and there's nothing there, but maybe same point, it is a bit confusing and configurability via options would be nice?

[jstsmthrgk]

Maybe something along these lines:

PreferencesManager::set("resource_path", configuration_path + "/resources/:" + configuration_path + "/scripts/");

std:string resource_path = PreferencesManager::get("resource_path");
size_t left = 0;
size_t right = 0;
while (left < resource_path.size()) {
  right = resource_path.find(':', left);
  std::string resource_directory = resource_path.substr(left, right-left);
  new DirectoryResourceProvider(resource_directory);
  left = right + 1;
}

(I know the loop is not nice at all, I'm not that good with string handling in C++)

So basically just a setting (with the current stuff as defaults) where one can set a : delimited list of directories (just like the $PATH environment variable syntax)

[daid]