Generalize Configuration class

[DanRStevens]

The Configuration class contains quite a bit of code specific to Mixer and Renderer. It can be generalized to support loading of arbitrary configuration data.

Additionally, the Configuration class can serve as a bridge between the XML serialized format, and the in memory configuration data, which is largely key/value storage with support for various data types, now implemented as Dictionary. It is one of the few places in NAS2D that is heavily dependent on XML code, the other being Sprite, and can be used as a basis for the Sprite, along with downstream uses in OPHD.

One of the end goals of this work, is to decouple the library and downstream project code from the XML code. That will make it easier to upgrade or replace the TinyXML1 dependency. In particular, it can be made an external dependency installed through vcpkg, it can be upgraded to TinyXML2, or it can even possibly be swapped out for another serialization format such as JSON.

Reference: #211, #239

---

A lot of work has already been put into this effort, though was not previously tracked using a specific issue. By opening this issue, that work from various PRs can be collected into a single place, along with new work to complete that goal.

[DanRStevens]

Mostly thinking out loud here.

Looking at the Configuration class, there are sections of methods relating to video and audio that should be removed. Each method comes in get/set pairs.

Video methods to remove:

	// Video Options
	int graphicsWidth() const;
	int graphicsHeight() const;
	int graphicsColorDepth() const;
	bool fullscreen() const;
	bool vsync() const;

	void graphicsWidth(int width);
	void graphicsHeight(int height);
	void graphicsColorDepth(int bpp);
	void fullscreen(bool fullscreen);
	void vsync(bool vsync);

Audio methods to remove:

	// Audio Options
	int audioMixRate() const;
	int audioStereoChannels() const;
	int audioSfxVolume() const;
	int audioMusicVolume() const;
	int audioBufferSize() const;
	std::string mixer() const;

	void audioMixRate(int mixrate);
	void audioStereoChannels(int channels);
	void audioSfxVolume(int volume);
	void audioMusicVolume(int volume);
	void audioBufferSize(int size);
	void mixer(const std::string& mixer);

Similarly the ability to set defaults is currently too specific to video and audio settings, and so may be removed:

	void setDefaultValues();

Related to the above are the private methods to parse those specific options, which will need to be removed:

	void parseGraphics(const Dictionary& dictionary);
	void parseAudio(const Dictionary& dictionary);

---

Implementation details will need to change. Currently the Configuration implementation contains many constants for the video and audio settings. In particular, it contains allowed options, allowed values, default values, as well as code to detect out of range values and replace them with defaults. Particular settings needed by various components, as well as their allowed values, are best determined by the components themselves, and so this code should be pushed out to those components.

For the RendererOpenGL class, it should be responsible for allowed options (screenwidth, screenheight, bitdepth, fullscreen, vsync), allowed values (minimum size 800x600, maximum size limited by graphics hardware), default values (1080p resolution), and mapping of out of range values to defaults (size of 640x480 -> 800x600 or maybe 1080p).

Similarly the MixerSDL class should be responsible for allowed options (mixrate, channels, sfxvolume, musicvolume, bufferlength), allowed values (audio quality 11025, 22050, 44100), default values (medium quality 22050), and mapping of out of range values to defaults (audio quality 100 -> 22050).

---

One slightly unfortunately aspect of the above design, is that configuration setting names found in configuration files become somewhat dictated by the objects that need initialization settings (Renderer and Mixer subclasses). That may not be appropriate if the configuration file is already of a pre-determined format which is not compatible with the default configuration setting names or format. Perhaps a developer is using a custom binary settings file, and needs to initialize Renderer or Mixer objects using their custom format.

One way to keep coupling loose is to use Options structs that can be passed to their constructors, rather than passing Configuration related objects directly. That way if a developer has custom configuration data, they can provide their own mapping to an Options struct, and use that to initialize a Renderer or Mixer object. As a convenience, a method can be provided that takes Configuration data and using default configuration setting names maps settings to the class specific Options struct. That provides a default way for objects to be initialized with very little code, while also not locking in developers to a specific configuration data format. Such convenience conversion methods should be packaged with the component being initialized, rather than as part of the Configuration class.

Effectively these class specific Options parsing methods will replace the private methods parseGraphics, and parseAudio being remove above.

---

Tied to the parsing of class specific Options structs, is handling of out of range values. They may result in exceptions raised by constructors, or it might be handled by mapping out of range values to default values. If out of range values are mapped to default values, one way might be to convert an Options struct to a new modified Options struct. That way such checking and re-mapping can be done for any data source, not just data coming from a Configuration object.

---

One area that needs further refinement, is how setting changes can be saved. That might involve reading back into a class specific Options struct, and then either saving it a custom way, or using a convenience method to write a class specific Options struct to a Configuration data store. Getting data back into an Options struct may require a custom method for each Renderer or Mixer. As the type of Options struct being returned is class specific, this method could not possibly be made virtual. Instead, it would be a regular class method on each subclass, and knowledge of the subclass type would be needed to call it.

Another option is to not worry about reading back data. Instead, make it a user interface issue. The user interface will somehow be responsible for setting (most) options, and so when they are set, they can be recorded there, by the user interface. The user interface is then responsible for updating the Configuration data store. That eliminates the need to read back values from the Renderer or Mixer subclass.

One limitation of saving settings only through the user interface is being able to detect when out of range options are re-mapped. If there is a function that can error check and re-map class specific Options structs before object initialization, then the actual Options struct data can be stored, before the object is initialized. That should be fine, provided the object doesn't modify settings further during or after construction. At least, not without being told to update settings by a call from the user interface.

Similarly, if an object might update it's own state, maybe the OS forces a minimized/maximized type change due to alt-tab or similar, the object might be responsible for providing an event that the user interface can listen to and make appropriate changes. Effectively, push the burden off to the user interface. This seems like the most reasonable approach.

[DanRStevens]

The external interface for the Configuration class has been simplified by removing all methods relating to:
"graphics", "audio", "options"

There's still a bit of work needed to cleanup the internal implementation of Configuration. Some of the above named sections still have special internal processing.

[DanRStevens]

To remove final internal dependence on section names "graphics", "audio", and "options", some aspects of the Configuration class design will need to be updated. Currently the Configuration class reports errors in the loadData method when those required configuration section names are not found. This error reporting will need to be lifted out of the Configuration class. It would instead be reported in Game, or in main in OPHD.

Error reporting is done using the ReportProblemNames method. This method will need to move out of the anonymous namespace. Perhaps the value calculating version of it can be moved to ContainerUtils.h. Converting those values to exception methods might then be done inline. With error checking lifted to the calling code in place, we can then remove the ReportProblemNames method.

In the reverse direction, the saveData method currently calls DictionaryToXmlElementOptions, so that the "options" section can be written in its special sub-tag attribute format, while the "graphics", and "audio" sections are collected to be written in the standard attribute format. Instead, all sections should simply be output in the standard attribute format. (Previously other section names were ignored and not written to the file). At this point the DictionaryToXmlElementOptions method can be removed.

The ParseOptionsToDictionary method can then be deprecated. We may want to delay removal for some time, to allow saved game files to be loaded and resaved to convert their format to standard attributes for the "options" section.

There is a merge method for combining Dictionary objects from two collections. Potentially this could be generalized and moved into ContainerUtils.h. This task would be considered optional.

[DanRStevens]

It seems the existing code for ReportProblemNames contains a bit of a bug:

const auto missing = names - required; // Bug

This should be the opposite:

const auto missing = required - names;

---

The exception message formatting code calls join, so I want to keep that part of of ContainerUtils.h, and creating extra internal library dependencies between header files.

Actually, seeing as how the calculation involves using a custom operator- defined for std::vector, it would be fairly easy to inline the whole method where it is used.

[DanRStevens]

Had an additional thought on this. Since OPHD specifies default configuration settings for each of the "graphics", "audio", and "options" sections, those sections can never be missing once defaults are merged with the loaded settings. Hence, if that check were lifted from the Configuration class to the caller, there would actually be no need to do the check from OPHD. Other downstream projects may be in a similar situation, where they can provide a default, rather than check for the absence of a Configuration setting after load.

With that in mind, we could reasonably safely remove the internal check without worrying about having a corresponding check already being done at the point of call. Effectively this allows us to defer on how we refactor ReportProblemNames.

[DanRStevens]

Currently there are some outstanding tasks related to this issue:

1. Generalizing and making public the ability to check keys against a required and optional set, and report back any missing or unexpected keys.
2. Loading/saving data to a section with a name other than "configuration". This may make the code more reusable for things like the Sprite class.
3. Related to the Sprite class, is the ability to check the file for a required or minimum version number.

[DanRStevens]

I think I'd like to limit the scope of this issue to generalizing Configuration to handle an arbitrary set of sections, where each section is an arbitrary set of key/value pairs. In particular, there should be no special handling of any given section or key. With all the work so far completed, that seems to be the current state of affairs. As such, I'd like to close this issue as now completed.

Remaining work to refactor the Sprite loading code should be handled under a new separate issue. In particular, the Sprite loading code needs to be able to handle arrays of data (the frame list), which is not a feature needed or supported by the Configuration class. The ability to check for and ensure a minimum required version number can also be tracked under such a new issue for similar reasons, along with being able to load elements other than a "configuration" section.

The support code for required and optional sets of keys might also be considered out of scope. It's not clear we'll need such code going forward, so it might get removed. More likely though, it may get generalized and made public, perhaps as a split set of functions. It seems likely text input file processing code may need such features, whether it be for Configuration or Sprite, or XML or JSON formats. I think such work should be deferred to the new issue, as future work may better inform where to place such updated code, so it is accessible to all text processing methods that might need it.