Jump to content

Configuration system


Recommended Posts

Re wacko's changes on Github (readable diff here skipping spurious line-ending changes (clearly Git is inferior to SVN :ok:)), which updates the configuration system: I think it's useful first to look at what we actually need from the config system.

A random collection of things I think are needed:

* Key/value string pairs. (In particular we haven't wanted to extend it to support complex value types, nested sections, etc, so the simple approach is still fine.)

* Clean integration with engine code that uses configured values (simple type conversions, some mechanism to ensure consistency when a value used by various things changes, etc).

* Default values for everything, specified in a human-editable (i.e. containing comments) config file. The file will be shared by all users so it has to be a read-only file.

* Per-user non-default settings, edited through an in-game options screen or by hand (in case they can't even launch the game).

* The options screen allows changing options (usually(?) without them having immediate effect), then clicking "ok" (which applies changs and saves to disk immediately) or "cancel" or "restore all defaults" or "restore this particular setting to default" etc.

* When changing video mode, if the game fails to set the new mode then it should revert back to the old working mode (so it needs to support some kind of simple rollback operation).

* Some options can be changed outside of the options screen, e.g. using alt+enter to toggle fullscreen mode. Those changes probably(?) shouldn't get saved to disk.

* When a user installs a new version of the game, they shouldn't lose their custom settings, but they should get our new default values (e.g. if we change a default hotkey then they should be using the new default unless they explicitly overrode it before).

* We want powerful modding support, and mods may want to add hotkeys but probably shouldn't change any other options (since the other options are engine things, not gameplay things; gameplay things need to be network-synchronised instead of loaded from config files). So mods at least need a way to define their default hotkeys (in a way that supports multiple mods being active at once). Users should be able to enable and disable mods, and to override the mods' default hotkey bindings.

I'm not sure if I'm missing lots here, or if some of these aren't really needed, but I think they're probably reasonable. My main concern with wacko's changes is that they help satisfy a couple of these requirements (which is good) but appear to ignore the rest (any may need to be largely rewritten to support them).

(Also, looking at implementation details, we ought to be trying to minimise use of singletons and global variables (mainly since they make unit testing much harder - better to be explicit about object lifetimes and entry points when possible), and I'd prefer to avoid using ScriptGlue.cpp and CJSObject and ToPrimitive etc (I've been migrating code away from them in the hope of eventually deleting them entirely, to simplify our C++/JS bridging). Also there's various trivial issues, like code in lib/ is meant to be independent of the game and shouldn't depend on code in ps/, and like using strtol (unsafe since it depends on locale - probably should use/extend our existing code that handles these things more safely already).)

Anyway, I believe it's important to fix the current config system (since it certainly doesn't handle the requirements above and it has similar implementation problems), which probably involves largely rewriting it - I just don't think this patch is really the right direction to go in. I don't know exactly what is a right direction, but I'm happy to discuss it and help work out what's needed.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share

×
×
  • Create New...