Menu
▾
▴
#151 Modifying Config File Loading Behavior
Attached is a patch to change how GemRB loads config files. This patch should apply cleanly to svn revision 7491.
This patch is similar to:
https://sourceforge.net/tracker/?func=detail&aid=2766054&group_id=10122&atid=310122
This patch makes two major changes to the current behavior:
* Unless ``-c`` is specified, three config files are searched for settings in this order:
1. ./gemrb.cfg
2. ~/.gemrb/gemrb.cfg
3. (SYSCONFDIR)/gemrb/gemrb.cfg
* Only new keys that haven't been found in previous config files are loaded. If a user sets all their config file keys in the current directory's file, the fact that it reads other config files is irrelevant and the same as the current behavior.
This allows us to use the current behavior (load every key from only one config file) if we use ``-c`` to specify the name. This behavior is unchanged.
This also allows more intelligent behavior, making it much easier to set up per-installation configs. This fixes a limitation that the launcher has had to work around, which it does somewhat clunkily at best. With the current functionality, it's impossible to have two different installations of a single game with different configuration options (the launcher can currently only rename config files per game-name in the ``~/.gemrb`` directory). With the proposed patch, setting up multiple configurations of the same game is trivial. The launcher can use this functionality if we keep a registry of installations (something easy to implement in the installer).
If you'd be interested, I'd be happy to extend the current patch by:
* Changing the ``-c`` option to also check HOME and SYSCONF directories unless a new ``--only-c`` option was specified.
Making the last change I mentioned would make the system even more modular and still easier for the launcher and users (otherwise we have to change working directories, etc). But, that's a lot of current behavior changes for the first post of a patch.
Isn't it easier to load the files in reverse order? That way you wouldn't need to check if a key was already loaded.
Yes and no. There are two main reasons to prefer the current patch:
1. It's more efficient. If we load all the keys in each of three files, we could be loading 3x as many keys as we need. This probably isn't much of an issue in the end, really, as we don't have thousands of keys.
2. We're only loading each key once. If we load all the keys in each file we come across, we might end up setting the same value several times. This is fine when each key simply replaces the previous value when it's loaded. However, if any of the loading code doesn't replace already existing values, we're in trouble. If, for example, we append values to a list during the load (without re-newing the list), that list will have several extra values the user never intended to use. I haven't reviewed the key loading code to make sure this never happens, but it's something to keep in mind.
So, it's really a trade-off. Getting rid of the list of loaded keys makes the config-reading code simpler, but requires us to always replace values when loading data (making loading data a little more complicated).
As encouragement for this patch, I recently purchased a flash drive big enough to install all of the IE games at once. With this patch applied to the branch svn checkout I'm using, the gemrb.cfg files for each installation are 7 lines long and involve no data duplication: changing the resolution in all of my installations at once is as simple as changing a single line.
I've uploaded a new version of the patch to fix some default-value setting behavior. In LoadConfig( filename ), the function set default values for any unspecified keys. This meant that everything broke when you didn't specify the "GemRBPath=" key in the working directory's gemrb.cfg file.
The new patch I've attached moves the default value setting behavior into a new FixConfig() function, called after all the config files are loaded (as opposed to once per config file).
And one final patch version. This one is just like the previous patches except that this one finally includes the long promised ``--single-config`` option.
The behaviors are as follow:
1. When no config file is specified, all normal configs ("./gemrb.cfg", "~/.gemrb/gemrb.cfg", "SYSCONFDIR/gemrb/gemrb.cfg", in that order) will be read.
2. When a config file is specified with "-c", the specified config file will be read first, followed by all normal configs.
3. When a config file is specified with "-c" and "--single-config" is also specified, only that single config file will be read.
4. If "--single-config" is specified without an associated "-c" option, it's ignored.
In this case, "read" means loading all the values in each config file located. Each key that is found is read only once in the first config file that specifies it.
The upshot of all this is that if the user doesn't create any config files other than the "gemrb.cfg" that exists in each game's directory, we use the file in that directory. If the user creates other config files (which are normally used), those files can be ignored by adding the "--single-config" option.
This makes multiple games much easier to manage (forcing users to specify only game-directory and CD-specific options), as well as making GemRB act much more like a normal Unix application (like Apache, X, etc). This also makes life much easier on the launcher, as the only magic now required to make GemRB launch is to specify command line options.
This is also hosted as a git branch at:
http://github.com/NickDaly/GemRB-Multiple-Configs-Branch
I added some comments to the relevant commit on github.
I agree that it would be simpler to just load them in the reverse order. The two reasons you mentioned do not apply. It is also needed for overriding the resolution settings, which are not safe across the games, even more so if you use the widescreen mod.
A shorthand form of --single-config would be nice. I think the default should be reversed too, since this implicit multiloading isn't obvious. Or we should have include directives in the configs, like it was suggested in the other bug.
Didn't read the patch, but we now also check SYSCONFDIR/gemrb.cfg.
Fuzzie, where did you post your comments? I'm not finding them anywhere. I'll post an update regarding lynx's comments later today, hopefully.
Never mind, I found your comments, Fuzzie (that's a neat feature!). I think I've uploaded a version that addresses everyone's concerns (http://github.com/NickDaly/GemRB-Multiple-Configs-Branch):
- The single file loading method is now the default.
- ``--multiple-configs`` has a shorthand form, ``-m``.
- "GemRB.cfg" is renamed "gemrb.cfg". A single case is easier on users and more predictable overall. Different config file cases per directory location is just nuts.
- ``FixConfig()`` is now run correctly on the Windows path.
- If Windows has a sense of ``SYSCONFIG`` and ``UserConfig`` in each version of Windows that we're supporting, then they can be included, but I'm coding conservatively and assuming that it doesn't until I know otherwise. So, for now, Windows platforms only have access to "./gemrb.cfg".
If I screwed anything up about the Git commit, let me know and I'll update it, but I think I've straightened everything out. If there are any other changes you'd like to see in the patch, just let me know.
Interface.h looks badly merged, with extraneus includes and there are some indentation errors.
Interface.h should be corrected. Actually, the only file that should be changed at all by this branch is ``Interface.cpp``. Nothing else actually needed changes. so if they're there, ignore/reject them.
Updated the patches against the latest head:
git://github.com/NickDaly/GemRB-MultipleConfigs.git