daid
Configuration path should have single source of truth
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.)
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
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.
Resources providers have nothing to do with preferences, the whole point of those is that there can be multiple sources of resources.
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?
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)