From 9a975a2e684e030650d064c70e05dcdfcd956df9 Mon Sep 17 00:00:00 2001
From: cfcohen <cfcohen@verizon.net>
Date: Mon, 23 Nov 2015 18:10:33 -0500
Subject: [PATCH 01/12] Substantial rewrite of code to save settings.cfg file,
 allowing comments to persist, ordering of settings to be retained, additional
 reporting of changed settings, preservation of the settings.cfg timestamp
 when no changes are made, and foundational changes for possible future
 features.  Due to poor interaction with the openmw-launcher settings code,
 the launcher will still discard all of these benefits.

---
 components/settings/settings.cpp | 241 +++++++++++++++++++++++++++++--
 1 file changed, 229 insertions(+), 12 deletions(-)

diff --git a/components/settings/settings.cpp b/components/settings/settings.cpp
index 90fd300ec..7abfaff2c 100644
--- a/components/settings/settings.cpp
+++ b/components/settings/settings.cpp
@@ -2,6 +2,7 @@
 
 #include <stdexcept>
 #include <sstream>
+#include <iostream>
 
 #include <components/misc/stringops.hpp>
 
@@ -58,6 +59,7 @@ CategorySettingValueMap Manager::mDefaultSettings = CategorySettingValueMap();
 CategorySettingValueMap Manager::mUserSettings = CategorySettingValueMap();
 CategorySettingVector Manager::mChangedSettings = CategorySettingVector();
 
+typedef std::map< CategorySetting, bool > CategorySettingStatusMap;
 
 class SettingsFileParser
 {
@@ -69,6 +71,7 @@ public:
         mFile = file;
         boost::filesystem::ifstream stream;
         stream.open(boost::filesystem::path(file));
+        std::cout << "Loading settings file: " << file << std::endl;
         std::string currentCategory;
         mLine = 0;
         while (!stream.eof() && !stream.fail())
@@ -117,6 +120,230 @@ public:
         }
     }
 
+    void saveSettingsFile (const std::string& file, CategorySettingValueMap& settings)
+    {
+        // No options have been written to the file yet.
+        CategorySettingStatusMap written;
+        for (CategorySettingValueMap::iterator it = settings.begin(); it != settings.end(); ++it) {
+            written[it->first] = false;
+        }
+
+        // Have we substantively changed the settings file?
+        bool changed = false;
+
+        // Was there anything in the existing file?  This is intended to permit the deletion of
+        // the settings.cfg file and it's automatic recreation -- a feature not currently
+        // supported elsewhere in the code.
+        bool existing = false;
+
+        // The category/section we're currently in.
+        std::string currentCategory;
+
+        // Open the existing settings.cfg file to copy comments.  This might not be the same file
+        // as the output file if we're copying the setting from the default settings.cfg for the
+        // first time.  A minor change in API to pass the source file might be in order here.
+        boost::filesystem::ifstream istream;
+        boost::filesystem::path ipath(file);
+        istream.open(ipath);
+        //std::cout << "Reading previous settings file: " << ipath << std::endl;
+
+        // Create a new string stream to write the current settings to.  It's likely that the
+        // input file and the output file are the same, so this stream serves as a temporary file
+        // of sorts.  The setting files aren't very large so there's no performance issue.
+        std::stringstream ostream;
+
+        // For every line in the input file...
+        while (!istream.eof() && !istream.fail()) {
+            std::string line;
+            std::getline(istream, line);
+
+            // The current character position in the line.
+            size_t i = 0;
+
+            // Don't add additional newlines at the end of the file.
+            if (istream.eof()) continue;
+
+            // Copy entirely blank lines.
+            if (!skipWhiteSpace(i, line)) {
+                ostream << line << std::endl;
+                continue;
+            }
+
+            // There were at least some comments in the input file.
+            existing = true;
+
+            // Copy comments.
+            if (line[i] == '#') {
+                ostream << line << std::endl;
+                continue;
+            }
+
+            // Category heading.
+            if (line[i] == '[') {
+                size_t end = line.find(']', i);
+                // This should never happen unless the player edited the file while playing.
+                if (end == std::string::npos) {
+                    ostream << "# unterminated category: " << line << std::endl;
+                    changed = true;
+                    continue;
+                }
+
+                // Ensure that all options in the current category have been written.
+                for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) {
+                    bool missed = false;
+                    if (mit->second == false && mit->first.first == currentCategory) {
+                        std::cout << "Added new setting: [" << currentCategory << "] "
+                                  << mit->first.second << " = " << settings[mit->first] << std::endl;
+			// With some further code changes, this comment could be more descriptive.
+                        ostream << "# Automatically added setting." << std::endl;
+                        ostream << mit->first.second << " = " << settings[mit->first] << std::endl;
+                        mit->second = true;
+                        missed = true;
+                        changed = true;
+                    }
+                    // Ensure that there's a newline before the next section if we added settings.
+                    if (missed) ostream << std::endl;
+                }
+
+                // Update the current category.
+                currentCategory = line.substr(i+1, end - (i+1));
+                boost::algorithm::trim(currentCategory);
+
+                // Write the (new) current category to the file.
+                ostream << "[" << currentCategory << "]" << std::endl;
+                //std::cout << "Wrote category: " << currentCategory << std::endl;
+
+                // A setting can apparently follow the category on an input line.  That's rather
+                // inconvenient, since it makes it more likely to have duplicative sections,
+                // which our algorithm doesn't like.  Do the best we can.
+                i = end+1;
+            }
+
+            // Truncate trailing whitespace, since we're changing the file anayway.
+            if (!skipWhiteSpace(i, line))
+                continue;
+
+	    // If we'cve found settings befor ethe first category, something's wrong.  This
+            // should never happen unless the player edited the file while playing, since
+	    // the loadSettingsFile() logic rejects it.
+            if (currentCategory.empty()) {
+                ostream << "# empty category name: " << line << std::endl;
+                changed = true;
+                continue;
+            }
+
+            // Which setting was at this location in the input file?
+            size_t settingEnd = line.find('=', i);
+            // This should never happen unless the player edited the file while playing.
+            if (settingEnd == std::string::npos) {
+                ostream << "# unterminated setting name: " << line << std::endl;
+                changed = true;
+                continue;
+            }
+            std::string setting = line.substr(i, (settingEnd-i));
+            boost::algorithm::trim(setting);
+
+            // Get the existing value so we can see if we've changed it.
+            size_t valueBegin = settingEnd+1;
+            std::string value = line.substr(valueBegin);
+            boost::algorithm::trim(value);
+
+            // Construct the setting map key to determine whether the setting has already been
+            // written to the file.
+            CategorySetting key = std::make_pair(currentCategory, setting);
+            CategorySettingStatusMap::iterator finder = written.find(key);
+
+            // Settings not in the written map are definitely invalid.  Currently, this can only
+            // happen if the player edited the file while playing, because loadSettingsFile()
+            // will accept anything and pass it along in the map, but in the future, we might
+            // want to handle invalid settings more gracefully here.
+            if (finder == written.end()) {
+                ostream << "# invalid setting: " << line << std::endl;
+                changed = true;
+                continue;
+            }
+
+            // Write the current value of the setting to the file.  The key must exist in the
+            // settings map because of how written was initialized and finder != end().
+            ostream << setting << " = " << settings[key] << std::endl;
+            // Mark that setting as written.
+            finder->second = true;
+            // Did we really change it?
+            if (value != settings[key]) {
+                std::cout << "Changed setting: [" << currentCategory << "] "
+                          << setting << " = " << settings[key] << std::endl;
+                changed = true;
+            }
+            // No need to write the current line, because we just emitted a replacement.
+
+	    // Curiously, it appears that comments at the ends of lines with settings are not
+            // allowed, and the comment becomes part of the value.  Was that intended?
+        }
+
+        // We're done with the input stream file.
+        istream.close();
+
+        // Ensure that all options in the current category have been written.  We must complete
+        // the current category at the end of the file before moving on to any new categories.
+        for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) {
+            bool missed = false;
+            if (mit->second == false && mit->first.first == currentCategory) {
+                std::cout << "Added new setting: [" << mit->first.first << "] "
+                          << mit->first.second << " = " << settings[mit->first] << std::endl;
+		// With some further code changes, this comment could be more descriptive.
+                ostream << std::endl << "# Automatically added setting." << std::endl;
+                ostream << mit->first.second << " = " << settings[mit->first] << std::endl;
+                mit->second = true;
+                missed = true;
+                changed = true;
+            }
+            // Ensure that there's a newline before the next section if we added settings.
+            if (missed) ostream << std::endl;
+        }
+
+	// This logic triggers when the input file was effectively empty.  It could be extended
+	// to doing something more intelligent instead, like make a copy of the default settings
+	// file (complete with comments) before continuing.  Other code prevents OpenMW from
+	// executing to this point with a missing config file.
+        if (!existing) {
+            ostream << "# This file was automatically generated." << std::endl << std::endl;
+        }
+
+        // We still have one more thing to do before we're completely done writing the file.
+        // It's possible that there are entirely new categories, or that the input file had
+        // disappeared completely, so we need ensure that all settings are written to the file
+        // regardless of those issues.
+        for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) {
+            // If the setting hasn't been written yet.
+            if (mit->second == false) {
+                // If the catgory has changed, write a new category header.
+                if (mit->first.first != currentCategory) {
+                    currentCategory = mit->first.first;
+                    std::cout << "Created new setting section: " << mit->first.first << std::endl;
+                    ostream << std::endl;
+                    // Add new category comments only if we're amending an existing file.
+                    if (existing) ostream << "# Automatically added category." << std::endl;
+                    ostream << "[" << currentCategory << "]" << std::endl;
+                }
+                std::cout << "Added new setting: [" << mit->first.first << "] "
+                          << mit->first.second << " = " << settings[mit->first] << std::endl;
+                // Then write the setting.  No need to mark it as written because we're done.
+                ostream << std::endl;
+                ostream << mit->first.second << " = " << settings[mit->first] << std::endl;
+                changed = true;
+            }
+        }
+
+        // Now install the newly written file in the requested place.
+        if (changed) {
+            std::cout << "Updating settings file: " << ipath << std::endl;
+            boost::filesystem::ofstream ofstream;
+            ofstream.open(ipath);
+            ofstream << ostream.rdbuf();
+            ofstream.close();
+        }
+    }
+
 private:
     /// Increment i until it longer points to a whitespace character
     /// in the string or has reached the end of the string.
@@ -155,18 +382,8 @@ void Manager::loadUser(const std::string &file)
 
 void Manager::saveUser(const std::string &file)
 {
-    boost::filesystem::ofstream stream;
-    stream.open(boost::filesystem::path(file));
-    std::string currentCategory;
-    for (CategorySettingValueMap::iterator it = mUserSettings.begin(); it != mUserSettings.end(); ++it)
-    {
-        if (it->first.first != currentCategory)
-        {
-            currentCategory = it->first.first;
-            stream << "\n[" << currentCategory << "]\n";
-        }
-        stream << it->first.second << " = " << it->second << "\n";
-    }
+    SettingsFileParser parser;
+    parser.saveSettingsFile(file, mUserSettings);
 }
 
 std::string Manager::getString(const std::string &setting, const std::string &category)

From 6882e6451ab027b565c03de5e8da233d72fd937a Mon Sep 17 00:00:00 2001
From: cfcohen <cfcohen@verizon.net>
Date: Mon, 23 Nov 2015 18:55:48 -0500
Subject: [PATCH 02/12] Remove tabs. :-[

---
 components/settings/settings.cpp | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/components/settings/settings.cpp b/components/settings/settings.cpp
index 7abfaff2c..c72e1ace4 100644
--- a/components/settings/settings.cpp
+++ b/components/settings/settings.cpp
@@ -194,7 +194,7 @@ public:
                     if (mit->second == false && mit->first.first == currentCategory) {
                         std::cout << "Added new setting: [" << currentCategory << "] "
                                   << mit->first.second << " = " << settings[mit->first] << std::endl;
-			// With some further code changes, this comment could be more descriptive.
+                        // With some further code changes, this comment could be more descriptive.
                         ostream << "# Automatically added setting." << std::endl;
                         ostream << mit->first.second << " = " << settings[mit->first] << std::endl;
                         mit->second = true;
@@ -223,9 +223,9 @@ public:
             if (!skipWhiteSpace(i, line))
                 continue;
 
-	    // If we'cve found settings befor ethe first category, something's wrong.  This
+            // If we'cve found settings befor ethe first category, something's wrong.  This
             // should never happen unless the player edited the file while playing, since
-	    // the loadSettingsFile() logic rejects it.
+            // the loadSettingsFile() logic rejects it.
             if (currentCategory.empty()) {
                 ostream << "# empty category name: " << line << std::endl;
                 changed = true;
@@ -276,7 +276,7 @@ public:
             }
             // No need to write the current line, because we just emitted a replacement.
 
-	    // Curiously, it appears that comments at the ends of lines with settings are not
+            // Curiously, it appears that comments at the ends of lines with settings are not
             // allowed, and the comment becomes part of the value.  Was that intended?
         }
 
@@ -290,7 +290,7 @@ public:
             if (mit->second == false && mit->first.first == currentCategory) {
                 std::cout << "Added new setting: [" << mit->first.first << "] "
                           << mit->first.second << " = " << settings[mit->first] << std::endl;
-		// With some further code changes, this comment could be more descriptive.
+                // With some further code changes, this comment could be more descriptive.
                 ostream << std::endl << "# Automatically added setting." << std::endl;
                 ostream << mit->first.second << " = " << settings[mit->first] << std::endl;
                 mit->second = true;
@@ -301,10 +301,10 @@ public:
             if (missed) ostream << std::endl;
         }
 
-	// This logic triggers when the input file was effectively empty.  It could be extended
-	// to doing something more intelligent instead, like make a copy of the default settings
-	// file (complete with comments) before continuing.  Other code prevents OpenMW from
-	// executing to this point with a missing config file.
+        // This logic triggers when the input file was effectively empty.  It could be extended
+        // to doing something more intelligent instead, like make a copy of the default settings
+        // file (complete with comments) before continuing.  Other code prevents OpenMW from
+        // executing to this point with a missing config file.
         if (!existing) {
             ostream << "# This file was automatically generated." << std::endl << std::endl;
         }

From 18da95e4f8dd82fb7058b0cb5e17c06259e0961f Mon Sep 17 00:00:00 2001
From: cfcohen <cfcohen@verizon.net>
Date: Wed, 25 Nov 2015 13:08:53 -0500
Subject: [PATCH 03/12] Make openmw-launcher pass comments through
 settings.cfg, and reuse the Settings::Manager code to do most of the work. 
 Stop loading both the global settings-default.cfg and the one in the current
 directory, while continuing to prefer the latter one.  Cleanup paths slightly
 and remove what appears to have been debugging in the launcher settings.

---
 apps/launcher/settings/graphicssettings.cpp | 44 ---------------------
 apps/launcher/settings/graphicssettings.hpp | 18 ---------
 2 files changed, 62 deletions(-)
 delete mode 100644 apps/launcher/settings/graphicssettings.cpp
 delete mode 100644 apps/launcher/settings/graphicssettings.hpp

diff --git a/apps/launcher/settings/graphicssettings.cpp b/apps/launcher/settings/graphicssettings.cpp
deleted file mode 100644
index 9dad3dee6..000000000
--- a/apps/launcher/settings/graphicssettings.cpp
+++ /dev/null
@@ -1,44 +0,0 @@
-#include "graphicssettings.hpp"
-
-#include <QTextStream>
-#include <QString>
-#include <QRegExp>
-#include <QMap>
-
-Launcher::GraphicsSettings::GraphicsSettings()
-{
-}
-
-Launcher::GraphicsSettings::~GraphicsSettings()
-{
-}
-
-bool Launcher::GraphicsSettings::writeFile(QTextStream &stream)
-{
-    QString sectionPrefix;
-    QRegExp sectionRe("([^/]+)/(.+)$");
-    QMap<QString, QString> settings = SettingsBase::getSettings();
-
-    QMapIterator<QString, QString> i(settings);
-    while (i.hasNext()) {
-        i.next();
-
-        QString prefix;
-        QString key;
-
-        if (sectionRe.exactMatch(i.key())) {
-             prefix = sectionRe.cap(1);
-             key = sectionRe.cap(2);
-        }
-
-        if (sectionPrefix != prefix) {
-            sectionPrefix = prefix;
-            stream << "\n[" << prefix << "]\n";
-        }
-
-        stream << key << " = " << i.value() << "\n";
-    }
-
-    return true;
-
-}
diff --git a/apps/launcher/settings/graphicssettings.hpp b/apps/launcher/settings/graphicssettings.hpp
deleted file mode 100644
index a52e0aa84..000000000
--- a/apps/launcher/settings/graphicssettings.hpp
+++ /dev/null
@@ -1,18 +0,0 @@
-#ifndef GRAPHICSSETTINGS_HPP
-#define GRAPHICSSETTINGS_HPP
-
-#include <components/config/settingsbase.hpp>
-
-namespace Launcher
-{
-    class GraphicsSettings : public Config::SettingsBase<QMap<QString, QString> >
-    {
-    public:
-        GraphicsSettings();
-        ~GraphicsSettings();
-
-        bool writeFile(QTextStream &stream);
-
-    };
-}
-#endif // GRAPHICSSETTINGS_HPP

From 67c4b17581850ced0439b2f107dd7b0e0971203a Mon Sep 17 00:00:00 2001
From: cfcohen <cfcohen@verizon.net>
Date: Wed, 25 Nov 2015 13:17:03 -0500
Subject: [PATCH 04/12] Commit files that I thought wre in the previous commit.
  :-[ I'm accustomed to the hg behavior of commiting all modified files by
 default.

---
 apps/launcher/CMakeLists.txt           |   4 -
 apps/launcher/datafilespage.cpp        |   2 +-
 apps/launcher/graphicspage.cpp         |  62 ++++++------
 apps/launcher/graphicspage.hpp         |   6 +-
 apps/launcher/maindialog.cpp           | 125 ++++++++++++-------------
 apps/launcher/maindialog.hpp           |   4 +-
 apps/openmw/engine.cpp                 |   6 +-
 components/config/launchersettings.cpp |   2 -
 8 files changed, 102 insertions(+), 109 deletions(-)

diff --git a/apps/launcher/CMakeLists.txt b/apps/launcher/CMakeLists.txt
index 274239fb0..75f76a532 100644
--- a/apps/launcher/CMakeLists.txt
+++ b/apps/launcher/CMakeLists.txt
@@ -7,8 +7,6 @@ set(LAUNCHER
     textslotmsgbox.cpp
     settingspage.cpp
 
-    settings/graphicssettings.cpp
-
     utils/profilescombobox.cpp
     utils/textinputdialog.cpp
     utils/lineedit.cpp
@@ -24,8 +22,6 @@ set(LAUNCHER_HEADER
     textslotmsgbox.hpp
     settingspage.hpp
 
-    settings/graphicssettings.hpp
-
     utils/profilescombobox.hpp
     utils/textinputdialog.hpp
     utils/lineedit.hpp
diff --git a/apps/launcher/datafilespage.cpp b/apps/launcher/datafilespage.cpp
index ec2e1246e..0b0f8c75e 100644
--- a/apps/launcher/datafilespage.cpp
+++ b/apps/launcher/datafilespage.cpp
@@ -75,7 +75,7 @@ bool Launcher::DataFilesPage::loadSettings()
     QStringList profiles = mLauncherSettings.getContentLists();
     QString currentProfile = mLauncherSettings.getCurrentContentListName();
 
-    qDebug() << "current profile is: " << currentProfile;
+    qDebug() << "The current profile is: " << currentProfile;
 
     foreach (const QString &item, profiles)
         addProfile (item, false);
diff --git a/apps/launcher/graphicspage.cpp b/apps/launcher/graphicspage.cpp
index 2128c08f7..a3bfbe5db 100644
--- a/apps/launcher/graphicspage.cpp
+++ b/apps/launcher/graphicspage.cpp
@@ -18,7 +18,7 @@
 
 #include <components/contentselector/model/naturalsort.hpp>
 
-#include "settings/graphicssettings.hpp"
+#include <components/settings/settings.hpp>
 
 QString getAspect(int x, int y)
 {
@@ -32,10 +32,10 @@ QString getAspect(int x, int y)
     return QString(QString::number(xaspect) + ":" + QString::number(yaspect));
 }
 
-Launcher::GraphicsPage::GraphicsPage(Files::ConfigurationManager &cfg, GraphicsSettings &graphicsSetting, QWidget *parent)
+Launcher::GraphicsPage::GraphicsPage(Files::ConfigurationManager &cfg, Settings::Manager &engineSettings, QWidget *parent)
     : QWidget(parent)
     , mCfgMgr(cfg)
-    , mGraphicsSettings(graphicsSetting)
+    , mEngineSettings(engineSettings)
 {
     setObjectName ("GraphicsPage");
     setupUi(this);
@@ -80,25 +80,26 @@ bool Launcher::GraphicsPage::loadSettings()
     if (!setupSDL())
         return false;
 
-    if (mGraphicsSettings.value(QString("Video/vsync")) == QLatin1String("true"))
+    if (mEngineSettings.getBool("vsync", "Video"))
         vSyncCheckBox->setCheckState(Qt::Checked);
 
-    if (mGraphicsSettings.value(QString("Video/fullscreen")) == QLatin1String("true"))
+    if (mEngineSettings.getBool("fullscreen", "Video"))
         fullScreenCheckBox->setCheckState(Qt::Checked);
 
-    if (mGraphicsSettings.value(QString("Video/window border")) == QLatin1String("true"))
+    if (mEngineSettings.getBool("window border", "Video"))
         windowBorderCheckBox->setCheckState(Qt::Checked);
 
-    int aaIndex = antiAliasingComboBox->findText(mGraphicsSettings.value(QString("Video/antialiasing")));
+    // aaValue is the actual value (0, 1, 2, 4, 8, 16)
+    int aaValue = mEngineSettings.getInt("antialiasing", "Video");
+    // aaIndex is the index into the allowed values in the pull down.
+    int aaIndex = antiAliasingComboBox->findText(QString::number(aaValue));
     if (aaIndex != -1)
         antiAliasingComboBox->setCurrentIndex(aaIndex);
 
-    QString width = mGraphicsSettings.value(QString("Video/resolution x"));
-    QString height = mGraphicsSettings.value(QString("Video/resolution y"));
-    QString resolution = width + QString(" x ") + height;
-    QString screen = mGraphicsSettings.value(QString("Video/screen"));
-
-    screenComboBox->setCurrentIndex(screen.toInt());
+    int width = mEngineSettings.getInt("resolution x", "Video");
+    int height = mEngineSettings.getInt("resolution y", "Video");
+    QString resolution = QString::number(width) + QString(" x ") + QString::number(height);
+    screenComboBox->setCurrentIndex(mEngineSettings.getInt("screen", "Video"));
 
     int resIndex = resolutionComboBox->findText(resolution, Qt::MatchStartsWith);
 
@@ -107,9 +108,8 @@ bool Launcher::GraphicsPage::loadSettings()
         resolutionComboBox->setCurrentIndex(resIndex);
     } else {
         customRadioButton->toggle();
-        customWidthSpinBox->setValue(width.toInt());
-        customHeightSpinBox->setValue(height.toInt());
-
+        customWidthSpinBox->setValue(width);
+        customHeightSpinBox->setValue(height);
     }
 
     return true;
@@ -117,31 +117,29 @@ bool Launcher::GraphicsPage::loadSettings()
 
 void Launcher::GraphicsPage::saveSettings()
 {
-    vSyncCheckBox->checkState() ? mGraphicsSettings.setValue(QString("Video/vsync"), QString("true"))
-                                 : mGraphicsSettings.setValue(QString("Video/vsync"), QString("false"));
-
-    fullScreenCheckBox->checkState() ? mGraphicsSettings.setValue(QString("Video/fullscreen"), QString("true"))
-                                      : mGraphicsSettings.setValue(QString("Video/fullscreen"), QString("false"));
-
-    windowBorderCheckBox->checkState() ? mGraphicsSettings.setValue(QString("Video/window border"), QString("true"))
-                                      : mGraphicsSettings.setValue(QString("Video/window border"), QString("false"));
-
-    mGraphicsSettings.setValue(QString("Video/antialiasing"), antiAliasingComboBox->currentText());
+    mEngineSettings.setBool("vsync", "Video", vSyncCheckBox->checkState());
+    mEngineSettings.setBool("fullscreen", "Video", fullScreenCheckBox->checkState());
+    mEngineSettings.setBool("window border", "Video", windowBorderCheckBox->checkState());
 
+    // The atoi() call is safe because the pull down constrains the string values.
+    int aaValue = atoi(antiAliasingComboBox->currentText().toLatin1().data());
+    mEngineSettings.setInt("antialiasing", "Video", aaValue);
 
     if (standardRadioButton->isChecked()) {
         QRegExp resolutionRe(QString("(\\d+) x (\\d+).*"));
-
         if (resolutionRe.exactMatch(resolutionComboBox->currentText().simplified())) {
-            mGraphicsSettings.setValue(QString("Video/resolution x"), resolutionRe.cap(1));
-            mGraphicsSettings.setValue(QString("Video/resolution y"), resolutionRe.cap(2));
+            // The atoi() call is safe because the pull down constrains the string values.
+            int width = atoi(resolutionRe.cap(1).toLatin1().data());
+            int height = atoi(resolutionRe.cap(2).toLatin1().data());
+            mEngineSettings.setInt("resolution x", "Video", width);
+            mEngineSettings.setInt("resolution y", "Video", height);
         }
     } else {
-        mGraphicsSettings.setValue(QString("Video/resolution x"), QString::number(customWidthSpinBox->value()));
-        mGraphicsSettings.setValue(QString("Video/resolution y"), QString::number(customHeightSpinBox->value()));
+        mEngineSettings.setInt("resolution x", "Video", customWidthSpinBox->value());
+        mEngineSettings.setInt("resolution y", "Video", customHeightSpinBox->value());
     }
 
-    mGraphicsSettings.setValue(QString("Video/screen"), QString::number(screenComboBox->currentIndex()));
+    mEngineSettings.setInt("screen", "Video", screenComboBox->currentIndex());
 }
 
 QStringList Launcher::GraphicsPage::getAvailableResolutions(int screen)
diff --git a/apps/launcher/graphicspage.hpp b/apps/launcher/graphicspage.hpp
index fb96c39d7..e6eb53a3b 100644
--- a/apps/launcher/graphicspage.hpp
+++ b/apps/launcher/graphicspage.hpp
@@ -5,6 +5,8 @@
 
 #include "ui_graphicspage.h"
 
+#include <components/settings/settings.hpp>
+
 namespace Files { struct ConfigurationManager; }
 
 namespace Launcher
@@ -16,7 +18,7 @@ namespace Launcher
         Q_OBJECT
 
     public:
-        GraphicsPage(Files::ConfigurationManager &cfg, GraphicsSettings &graphicsSettings, QWidget *parent = 0);
+        GraphicsPage(Files::ConfigurationManager &cfg, Settings::Manager &engineSettings, QWidget *parent = 0);
 
         void saveSettings();
         bool loadSettings();
@@ -30,7 +32,7 @@ namespace Launcher
 
     private:
         Files::ConfigurationManager &mCfgMgr;
-        GraphicsSettings &mGraphicsSettings;
+        Settings::Manager &mEngineSettings;
 
         QStringList getAvailableResolutions(int screen);
         QRect getMaximumResolution();
diff --git a/apps/launcher/maindialog.cpp b/apps/launcher/maindialog.cpp
index 9a6f91a0f..1c56a9efd 100644
--- a/apps/launcher/maindialog.cpp
+++ b/apps/launcher/maindialog.cpp
@@ -105,7 +105,7 @@ void Launcher::MainDialog::createPages()
 {
     mPlayPage = new PlayPage(this);
     mDataFilesPage = new DataFilesPage(mCfgMgr, mGameSettings, mLauncherSettings, this);
-    mGraphicsPage = new GraphicsPage(mCfgMgr, mGraphicsSettings, this);
+    mGraphicsPage = new GraphicsPage(mCfgMgr, mEngineSettings, this);
     mSettingsPage = new SettingsPage(mCfgMgr, mGameSettings, mLauncherSettings, this);
 
     // Set the combobox of the play page to imitate the combobox on the datafilespage
@@ -381,55 +381,64 @@ bool Launcher::MainDialog::setupGameSettings()
     return true;
 }
 
+void cfgError(const QString& title, const QString& msg) {
+    QMessageBox msgBox;
+    msgBox.setWindowTitle(title);
+    msgBox.setIcon(QMessageBox::Critical);
+    msgBox.setStandardButtons(QMessageBox::Ok);
+    msgBox.setText(msg);
+    msgBox.exec();
+}
+
 bool Launcher::MainDialog::setupGraphicsSettings()
 {
-    mGraphicsSettings.setMultiValueEnabled(false);
-
-    QString userPath = QString::fromUtf8(mCfgMgr.getUserConfigPath().string().c_str());
-    QString globalPath = QString::fromUtf8(mCfgMgr.getGlobalPath().string().c_str());
-
-    QFile localDefault(QString("settings-default.cfg"));
-    QFile globalDefault(globalPath + QString("settings-default.cfg"));
-
-    if (!localDefault.exists() && !globalDefault.exists()) {
-        QMessageBox msgBox;
-        msgBox.setWindowTitle(tr("Error reading OpenMW configuration file"));
-        msgBox.setIcon(QMessageBox::Critical);
-        msgBox.setStandardButtons(QMessageBox::Ok);
-        msgBox.setText(tr("<br><b>Could not find settings-default.cfg</b><br><br> \
-                                   The problem may be due to an incomplete installation of OpenMW.<br> \
-                                   Reinstalling OpenMW may resolve the problem."));
-                                   msgBox.exec();
+    // This method is almost a copy of OMW::Engine::loadSettings().  They should definitely
+    // remain consistent, and possibly be merged into a shared component.  At the very least
+    // the filenames should be in the CfgMgr component.
+
+    // Create the settings manager and load default settings file
+    const std::string localDefault = mCfgMgr.getLocalPath().string() + "settings-default.cfg";
+    const std::string globalDefault = mCfgMgr.getGlobalPath().string() + "settings-default.cfg";
+    std::string defaultPath;
+
+    // Prefer the settings-default.cfg in the current directory.
+    if (boost::filesystem::exists(localDefault))
+        defaultPath = localDefault;
+    else if (boost::filesystem::exists(globalDefault))
+        defaultPath = globalDefault;
+    // Something's very wrong if we can't find the file at all.
+    else {
+        cfgError(tr("Error reading OpenMW configuration file"),
+                 tr("<br><b>Could not find settings-default.cfg</b><br><br> \
+                     The problem may be due to an incomplete installation of OpenMW.<br> \
+                     Reinstalling OpenMW may resolve the problem."));
         return false;
     }
 
+    // Load the default settings, report any parsing errors.
+    try {
+        mEngineSettings.loadDefault(defaultPath);
+    }
+    catch (std::exception& e) {
+        std::string msg = "<br><b>Error reading settings-default.cfg</b><br><br>" +
+            defaultPath + "<br><br>" + e.what();
+        cfgError(tr("Error reading OpenMW configuration file"), tr(msg.c_str()));
+        return false;
+    }
 
-    QStringList paths;
-    paths.append(globalPath + QString("settings-default.cfg"));
-    paths.append(QString("settings-default.cfg"));
-    paths.append(userPath + QString("settings.cfg"));
+    // Load user settings if they exist
+    const std::string userPath = mCfgMgr.getUserConfigPath().string() + "settings.cfg";
+    // User settings are not required to exist, so if they don't we're done.
+    if (!boost::filesystem::exists(userPath)) return true;
 
-    foreach (const QString &path, paths) {
-        qDebug() << "Loading config file:" << qPrintable(path);
-        QFile file(path);
-        if (file.exists()) {
-            if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
-                QMessageBox msgBox;
-                msgBox.setWindowTitle(tr("Error opening OpenMW configuration file"));
-                msgBox.setIcon(QMessageBox::Critical);
-                msgBox.setStandardButtons(QMessageBox::Ok);
-                msgBox.setText(tr("<br><b>Could not open %0 for reading</b><br><br> \
-                                           Please make sure you have the right permissions \
-                                           and try again.<br>").arg(file.fileName()));
-                                           msgBox.exec();
-                return false;
-            }
-            QTextStream stream(&file);
-            stream.setCodec(QTextCodec::codecForName("UTF-8"));
-
-            mGraphicsSettings.readFile(stream);
-        }
-        file.close();
+    try {
+        mEngineSettings.loadUser(userPath);
+    }
+    catch (std::exception& e) {
+        std::string msg = "<br><b>Error reading settings-default.cfg</b><br><br>" +
+            defaultPath + "<br><br>" + e.what();
+        cfgError(tr("Error reading OpenMW configuration file"), tr(msg.c_str()));
+        return false;
     }
 
     return true;
@@ -511,27 +520,16 @@ bool Launcher::MainDialog::writeSettings()
     file.close();
 
     // Graphics settings
-    file.setFileName(userPath + QString("settings.cfg"));
-
-    if (!file.open(QIODevice::ReadWrite | QIODevice::Text | QIODevice::Truncate)) {
-        // File cannot be opened or created
-        QMessageBox msgBox;
-        msgBox.setWindowTitle(tr("Error writing OpenMW configuration file"));
-        msgBox.setIcon(QMessageBox::Critical);
-        msgBox.setStandardButtons(QMessageBox::Ok);
-        msgBox.setText(tr("<br><b>Could not open or create %0 for writing</b><br><br> \
-                          Please make sure you have the right permissions \
-                          and try again.<br>").arg(file.fileName()));
-                          msgBox.exec();
-                       return false;
+    const std::string settingsPath = mCfgMgr.getUserConfigPath().string() + "settings.cfg";
+    try {
+        mEngineSettings.saveUser(settingsPath);
+    }
+    catch (std::exception& e) {
+        std::string msg = "<br><b>Error writing settings.cfg</b><br><br>" +
+            settingsPath + "<br><br>" + e.what();
+        cfgError(tr("Error reading OpenMW configuration file"), tr(msg.c_str()));
+        return false;
     }
-
-    QTextStream stream(&file);
-    stream.setDevice(&file);
-    stream.setCodec(QTextCodec::codecForName("UTF-8"));
-
-    mGraphicsSettings.writeFile(stream);
-    file.close();
 
     // Launcher settings
     file.setFileName(userPath + QString(Config::LauncherSettings::sLauncherConfigFileName));
@@ -549,6 +547,7 @@ bool Launcher::MainDialog::writeSettings()
                        return false;
     }
 
+    QTextStream stream(&file);
     stream.setDevice(&file);
     stream.setCodec(QTextCodec::codecForName("UTF-8"));
 
diff --git a/apps/launcher/maindialog.hpp b/apps/launcher/maindialog.hpp
index 298682d20..0dfea4865 100644
--- a/apps/launcher/maindialog.hpp
+++ b/apps/launcher/maindialog.hpp
@@ -13,7 +13,7 @@
 #include <components/config/gamesettings.hpp>
 #include <components/config/launchersettings.hpp>
 
-#include "settings/graphicssettings.hpp"
+#include <components/settings/settings.hpp>
 
 #include "ui_mainwindow.h"
 
@@ -93,7 +93,7 @@ namespace Launcher
         Files::ConfigurationManager mCfgMgr;
 
         Config::GameSettings mGameSettings;
-        GraphicsSettings mGraphicsSettings;
+        Settings::Manager mEngineSettings;
         Config::LauncherSettings mLauncherSettings;
 
     };
diff --git a/apps/openmw/engine.cpp b/apps/openmw/engine.cpp
index 031eac800..a4f5f9440 100644
--- a/apps/openmw/engine.cpp
+++ b/apps/openmw/engine.cpp
@@ -298,8 +298,8 @@ void OMW::Engine::setSkipMenu (bool skipMenu, bool newGame)
 std::string OMW::Engine::loadSettings (Settings::Manager & settings)
 {
     // Create the settings manager and load default settings file
-    const std::string localdefault = mCfgMgr.getLocalPath().string() + "/settings-default.cfg";
-    const std::string globaldefault = mCfgMgr.getGlobalPath().string() + "/settings-default.cfg";
+    const std::string localdefault = mCfgMgr.getLocalPath().string() + "settings-default.cfg";
+    const std::string globaldefault = mCfgMgr.getGlobalPath().string() + "settings-default.cfg";
 
     // prefer local
     if (boost::filesystem::exists(localdefault))
@@ -310,7 +310,7 @@ std::string OMW::Engine::loadSettings (Settings::Manager & settings)
         throw std::runtime_error ("No default settings file found! Make sure the file \"settings-default.cfg\" was properly installed.");
 
     // load user settings if they exist
-    const std::string settingspath = mCfgMgr.getUserConfigPath().string() + "/settings.cfg";
+    const std::string settingspath = mCfgMgr.getUserConfigPath().string() + "settings.cfg";
     if (boost::filesystem::exists(settingspath))
         settings.loadUser(settingspath);
 
diff --git a/components/config/launchersettings.cpp b/components/config/launchersettings.cpp
index 1d4b428c9..8f3498826 100644
--- a/components/config/launchersettings.cpp
+++ b/components/config/launchersettings.cpp
@@ -25,8 +25,6 @@ QStringList Config::LauncherSettings::subKeys(const QString &key)
     QMap<QString, QString> settings = SettingsBase::getSettings();
     QStringList keys = settings.uniqueKeys();
 
-    qDebug() << keys;
-
     QRegExp keyRe("(.+)/");
 
     QStringList result;

From ad5eaaa705dcb1fe6ffe29e7be5fcf71c16c5889 Mon Sep 17 00:00:00 2001
From: cfcohen <cfcohen@verizon.net>
Date: Wed, 25 Nov 2015 21:30:04 -0500
Subject: [PATCH 05/12] Update the OpenMW Launcher so that it only writes
 changed values to the user settings.cfg file.  Add a helpful header to the
 top of new settings.cfg files.  Remove old code involve whitespace management
 that didn't work correctly anayway, and doesn't matter since we're not adding
 comments to the file.  Remove "automatically generated" comments.

---
 apps/launcher/graphicspage.cpp   | 52 ++++++++++++++++++++++++--------
 apps/launcher/graphicspage.hpp   |  1 +
 components/settings/settings.cpp | 35 ++++++---------------
 3 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/apps/launcher/graphicspage.cpp b/apps/launcher/graphicspage.cpp
index a3bfbe5db..d7f045f80 100644
--- a/apps/launcher/graphicspage.cpp
+++ b/apps/launcher/graphicspage.cpp
@@ -80,6 +80,8 @@ bool Launcher::GraphicsPage::loadSettings()
     if (!setupSDL())
         return false;
 
+    mInitialSettings = mEngineSettings;
+
     if (mEngineSettings.getBool("vsync", "Video"))
         vSyncCheckBox->setCheckState(Qt::Checked);
 
@@ -117,29 +119,53 @@ bool Launcher::GraphicsPage::loadSettings()
 
 void Launcher::GraphicsPage::saveSettings()
 {
-    mEngineSettings.setBool("vsync", "Video", vSyncCheckBox->checkState());
-    mEngineSettings.setBool("fullscreen", "Video", fullScreenCheckBox->checkState());
-    mEngineSettings.setBool("window border", "Video", windowBorderCheckBox->checkState());
-
+    bool iVSync = mInitialSettings.getBool("vsync", "Video");
+    bool cVSync = vSyncCheckBox->checkState();
+    if (iVSync != cVSync)
+        mEngineSettings.setBool("vsync", "Video", cVSync);
+
+    bool iFullScreen = mInitialSettings.getBool("fullscreen", "Video");
+    bool cFullScreen = fullScreenCheckBox->checkState();
+    if (iFullScreen != cFullScreen)
+        mEngineSettings.setBool("fullscreen", "Video", cFullScreen);
+
+    bool iWindowBorder = mInitialSettings.getBool("window border", "Video");
+    bool cWindowBorder = windowBorderCheckBox->checkState();
+    if (iWindowBorder != cWindowBorder)
+        mEngineSettings.setBool("window border", "Video", cWindowBorder);
+
+    int iAAValue = mInitialSettings.getInt("antialiasing", "Video");
     // The atoi() call is safe because the pull down constrains the string values.
-    int aaValue = atoi(antiAliasingComboBox->currentText().toLatin1().data());
-    mEngineSettings.setInt("antialiasing", "Video", aaValue);
+    int cAAValue = atoi(antiAliasingComboBox->currentText().toLatin1().data());
+    if (iAAValue != cAAValue)
+        mEngineSettings.setInt("antialiasing", "Video", cAAValue);
 
+    int cWidth = 0;
+    int cHeight = 0;
     if (standardRadioButton->isChecked()) {
         QRegExp resolutionRe(QString("(\\d+) x (\\d+).*"));
         if (resolutionRe.exactMatch(resolutionComboBox->currentText().simplified())) {
             // The atoi() call is safe because the pull down constrains the string values.
-            int width = atoi(resolutionRe.cap(1).toLatin1().data());
-            int height = atoi(resolutionRe.cap(2).toLatin1().data());
-            mEngineSettings.setInt("resolution x", "Video", width);
-            mEngineSettings.setInt("resolution y", "Video", height);
+            cWidth = atoi(resolutionRe.cap(1).toLatin1().data());
+            cHeight = atoi(resolutionRe.cap(2).toLatin1().data());
         }
     } else {
-        mEngineSettings.setInt("resolution x", "Video", customWidthSpinBox->value());
-        mEngineSettings.setInt("resolution y", "Video", customHeightSpinBox->value());
+        cWidth = customWidthSpinBox->value();
+        cHeight = customHeightSpinBox->value();
     }
 
-    mEngineSettings.setInt("screen", "Video", screenComboBox->currentIndex());
+    int iWidth = mInitialSettings.getInt("resolution x", "Video");
+    if (iWidth != cWidth)
+        mEngineSettings.setInt("resolution x", "Video", cWidth);
+
+    int iHeight = mInitialSettings.getInt("resolution y", "Video");
+    if (iHeight != cHeight)
+        mEngineSettings.setInt("resolution y", "Video", cHeight);
+
+    int iScreen = mInitialSettings.getInt("screen", "Video");
+    int cScreen = screenComboBox->currentIndex();
+    if (iScreen != cScreen)
+        mEngineSettings.setInt("screen", "Video", cScreen);
 }
 
 QStringList Launcher::GraphicsPage::getAvailableResolutions(int screen)
diff --git a/apps/launcher/graphicspage.hpp b/apps/launcher/graphicspage.hpp
index e6eb53a3b..0958f07da 100644
--- a/apps/launcher/graphicspage.hpp
+++ b/apps/launcher/graphicspage.hpp
@@ -33,6 +33,7 @@ namespace Launcher
     private:
         Files::ConfigurationManager &mCfgMgr;
         Settings::Manager &mEngineSettings;
+        Settings::Manager mInitialSettings;
 
         QStringList getAvailableResolutions(int screen);
         QRect getMaximumResolution();
diff --git a/components/settings/settings.cpp b/components/settings/settings.cpp
index c72e1ace4..acad1a98e 100644
--- a/components/settings/settings.cpp
+++ b/components/settings/settings.cpp
@@ -131,9 +131,7 @@ public:
         // Have we substantively changed the settings file?
         bool changed = false;
 
-        // Was there anything in the existing file?  This is intended to permit the deletion of
-        // the settings.cfg file and it's automatic recreation -- a feature not currently
-        // supported elsewhere in the code.
+        // Were there any lines at all in the file?
         bool existing = false;
 
         // The category/section we're currently in.
@@ -145,7 +143,6 @@ public:
         boost::filesystem::ifstream istream;
         boost::filesystem::path ipath(file);
         istream.open(ipath);
-        //std::cout << "Reading previous settings file: " << ipath << std::endl;
 
         // Create a new string stream to write the current settings to.  It's likely that the
         // input file and the output file are the same, so this stream serves as a temporary file
@@ -190,19 +187,13 @@ public:
 
                 // Ensure that all options in the current category have been written.
                 for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) {
-                    bool missed = false;
                     if (mit->second == false && mit->first.first == currentCategory) {
                         std::cout << "Added new setting: [" << currentCategory << "] "
                                   << mit->first.second << " = " << settings[mit->first] << std::endl;
-                        // With some further code changes, this comment could be more descriptive.
-                        ostream << "# Automatically added setting." << std::endl;
                         ostream << mit->first.second << " = " << settings[mit->first] << std::endl;
                         mit->second = true;
-                        missed = true;
                         changed = true;
                     }
-                    // Ensure that there's a newline before the next section if we added settings.
-                    if (missed) ostream << std::endl;
                 }
 
                 // Update the current category.
@@ -223,7 +214,7 @@ public:
             if (!skipWhiteSpace(i, line))
                 continue;
 
-            // If we'cve found settings befor ethe first category, something's wrong.  This
+            // If we've found settings before the first category, something's wrong.  This
             // should never happen unless the player edited the file while playing, since
             // the loadSettingsFile() logic rejects it.
             if (currentCategory.empty()) {
@@ -286,27 +277,24 @@ public:
         // Ensure that all options in the current category have been written.  We must complete
         // the current category at the end of the file before moving on to any new categories.
         for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) {
-            bool missed = false;
             if (mit->second == false && mit->first.first == currentCategory) {
                 std::cout << "Added new setting: [" << mit->first.first << "] "
                           << mit->first.second << " = " << settings[mit->first] << std::endl;
-                // With some further code changes, this comment could be more descriptive.
-                ostream << std::endl << "# Automatically added setting." << std::endl;
                 ostream << mit->first.second << " = " << settings[mit->first] << std::endl;
                 mit->second = true;
-                missed = true;
                 changed = true;
             }
-            // Ensure that there's a newline before the next section if we added settings.
-            if (missed) ostream << std::endl;
         }
 
-        // This logic triggers when the input file was effectively empty.  It could be extended
-        // to doing something more intelligent instead, like make a copy of the default settings
-        // file (complete with comments) before continuing.  Other code prevents OpenMW from
-        // executing to this point with a missing config file.
+        // If there was absolutely nothing in the file (or more likely the file didn't
+        // exist), start the newly created file with a helpful comment.
         if (!existing) {
-            ostream << "# This file was automatically generated." << std::endl << std::endl;
+            ostream << "# This is the OpenMW user 'settings.cfg' file.  This file only contains" << std::endl;
+            ostream << "# explicitly changed settings.  If you would like to revert a setting" << std::endl;
+            ostream << "# to its default, simply remove it from this file.  For available" << std::endl;
+            ostream << "# settings, see the file 'settings-default.cfg' or the documentation at:" << std::endl;
+            ostream << "#" << std::endl;
+            ostream << "#   https://wiki.openmw.org/index.php?title=Settings" << std::endl;
         }
 
         // We still have one more thing to do before we're completely done writing the file.
@@ -321,14 +309,11 @@ public:
                     currentCategory = mit->first.first;
                     std::cout << "Created new setting section: " << mit->first.first << std::endl;
                     ostream << std::endl;
-                    // Add new category comments only if we're amending an existing file.
-                    if (existing) ostream << "# Automatically added category." << std::endl;
                     ostream << "[" << currentCategory << "]" << std::endl;
                 }
                 std::cout << "Added new setting: [" << mit->first.first << "] "
                           << mit->first.second << " = " << settings[mit->first] << std::endl;
                 // Then write the setting.  No need to mark it as written because we're done.
-                ostream << std::endl;
                 ostream << mit->first.second << " = " << settings[mit->first] << std::endl;
                 changed = true;
             }

From 046538984c580c2dcf93920b67c9761b96ee987a Mon Sep 17 00:00:00 2001
From: cfcohen <cfcohen@verizon.net>
Date: Thu, 26 Nov 2015 01:49:14 -0500
Subject: [PATCH 06/12] Fix duplicate filename in what() message.  Use newly
 create cfgError utility function consistently throughout code.

---
 apps/launcher/maindialog.cpp | 102 +++++++++++++----------------------
 1 file changed, 38 insertions(+), 64 deletions(-)

diff --git a/apps/launcher/maindialog.cpp b/apps/launcher/maindialog.cpp
index 1c56a9efd..6453beacc 100644
--- a/apps/launcher/maindialog.cpp
+++ b/apps/launcher/maindialog.cpp
@@ -24,6 +24,15 @@
 
 using namespace Process;
 
+void cfgError(const QString& title, const QString& msg) {
+    QMessageBox msgBox;
+    msgBox.setWindowTitle(title);
+    msgBox.setIcon(QMessageBox::Critical);
+    msgBox.setStandardButtons(QMessageBox::Ok);
+    msgBox.setText(msg);
+    msgBox.exec();
+}
+
 Launcher::MainDialog::MainDialog(QWidget *parent)
     : QMainWindow(parent), mGameSettings (mCfgMgr)
 {
@@ -261,14 +270,10 @@ bool Launcher::MainDialog::setupLauncherSettings()
         QFile file(path);
         if (file.exists()) {
             if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
-                QMessageBox msgBox;
-                msgBox.setWindowTitle(tr("Error opening OpenMW configuration file"));
-                msgBox.setIcon(QMessageBox::Critical);
-                msgBox.setStandardButtons(QMessageBox::Ok);
-                msgBox.setText(tr("<br><b>Could not open %0 for reading</b><br><br> \
-                                           Please make sure you have the right permissions \
-                                           and try again.<br>").arg(file.fileName()));
-                                           msgBox.exec();
+                cfgError(tr("Error opening OpenMW configuration file"),
+                         tr("<br><b>Could not open %0 for reading</b><br><br> \
+                             Please make sure you have the right permissions \
+                             and try again.<br>").arg(file.fileName()));
                 return false;
             }
             QTextStream stream(&file);
@@ -296,14 +301,10 @@ bool Launcher::MainDialog::setupGameSettings()
 
     if (file.exists()) {
         if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
-            QMessageBox msgBox;
-            msgBox.setWindowTitle(tr("Error opening OpenMW configuration file"));
-            msgBox.setIcon(QMessageBox::Critical);
-            msgBox.setStandardButtons(QMessageBox::Ok);
-            msgBox.setText(tr("<br><b>Could not open %0 for reading</b><br><br> \
-                                       Please make sure you have the right permissions \
-                                       and try again.<br>").arg(file.fileName()));
-                                       msgBox.exec();
+            cfgError(tr("Error opening OpenMW configuration file"),
+                     tr("<br><b>Could not open %0 for reading</b><br><br> \
+                         Please make sure you have the right permissions \
+                         and try again.<br>").arg(file.fileName()));
             return false;
         }
         QTextStream stream(&file);
@@ -324,14 +325,10 @@ bool Launcher::MainDialog::setupGameSettings()
         QFile file(path);
         if (file.exists()) {
             if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
-                QMessageBox msgBox;
-                msgBox.setWindowTitle(tr("Error opening OpenMW configuration file"));
-                msgBox.setIcon(QMessageBox::Critical);
-                msgBox.setStandardButtons(QMessageBox::Ok);
-                msgBox.setText(tr("<br><b>Could not open %0 for reading</b><br><br> \
-                                           Please make sure you have the right permissions \
-                                           and try again.<br>").arg(file.fileName()));
-                                           msgBox.exec();
+                cfgError(tr("Error opening OpenMW configuration file"),
+                         tr("<br><b>Could not open %0 for reading</b><br><br> \
+                             Please make sure you have the right permissions \
+                             and try again.<br>").arg(file.fileName()));
                 return false;
             }
             QTextStream stream(&file);
@@ -381,15 +378,6 @@ bool Launcher::MainDialog::setupGameSettings()
     return true;
 }
 
-void cfgError(const QString& title, const QString& msg) {
-    QMessageBox msgBox;
-    msgBox.setWindowTitle(title);
-    msgBox.setIcon(QMessageBox::Critical);
-    msgBox.setStandardButtons(QMessageBox::Ok);
-    msgBox.setText(msg);
-    msgBox.exec();
-}
-
 bool Launcher::MainDialog::setupGraphicsSettings()
 {
     // This method is almost a copy of OMW::Engine::loadSettings().  They should definitely
@@ -420,8 +408,7 @@ bool Launcher::MainDialog::setupGraphicsSettings()
         mEngineSettings.loadDefault(defaultPath);
     }
     catch (std::exception& e) {
-        std::string msg = "<br><b>Error reading settings-default.cfg</b><br><br>" +
-            defaultPath + "<br><br>" + e.what();
+        std::string msg = std::string("<br><b>Error reading settings-default.cfg</b><br><br>") + e.what();
         cfgError(tr("Error reading OpenMW configuration file"), tr(msg.c_str()));
         return false;
     }
@@ -435,8 +422,7 @@ bool Launcher::MainDialog::setupGraphicsSettings()
         mEngineSettings.loadUser(userPath);
     }
     catch (std::exception& e) {
-        std::string msg = "<br><b>Error reading settings-default.cfg</b><br><br>" +
-            defaultPath + "<br><br>" + e.what();
+        std::string msg = std::string("<br><b>Error reading settings.cfg</b><br><br>") + e.what();
         cfgError(tr("Error reading OpenMW configuration file"), tr(msg.c_str()));
         return false;
     }
@@ -487,15 +473,11 @@ bool Launcher::MainDialog::writeSettings()
 
     if (!dir.exists()) {
         if (!dir.mkpath(userPath)) {
-            QMessageBox msgBox;
-            msgBox.setWindowTitle(tr("Error creating OpenMW configuration directory"));
-            msgBox.setIcon(QMessageBox::Critical);
-            msgBox.setStandardButtons(QMessageBox::Ok);
-            msgBox.setText(tr("<br><b>Could not create %0</b><br><br> \
-                              Please make sure you have the right permissions \
-                              and try again.<br>").arg(userPath));
-                              msgBox.exec();
-                           return false;
+            cfgError(tr("Error creating OpenMW configuration directory"),
+                     tr("<br><b>Could not create %0</b><br><br> \
+                         Please make sure you have the right permissions \
+                         and try again.<br>").arg(userPath));
+            return false;
         }
     }
 
@@ -504,15 +486,11 @@ bool Launcher::MainDialog::writeSettings()
 
     if (!file.open(QIODevice::ReadWrite | QIODevice::Text)) {
         // File cannot be opened or created
-        QMessageBox msgBox;
-        msgBox.setWindowTitle(tr("Error writing OpenMW configuration file"));
-        msgBox.setIcon(QMessageBox::Critical);
-        msgBox.setStandardButtons(QMessageBox::Ok);
-        msgBox.setText(tr("<br><b>Could not open or create %0 for writing</b><br><br> \
-                          Please make sure you have the right permissions \
-                          and try again.<br>").arg(file.fileName()));
-                          msgBox.exec();
-                       return false;
+        cfgError(tr("Error writing OpenMW configuration file"),
+                 tr("<br><b>Could not open or create %0 for writing</b><br><br> \
+                     Please make sure you have the right permissions \
+                     and try again.<br>").arg(file.fileName()));
+        return false;
     }
 
 
@@ -536,15 +514,11 @@ bool Launcher::MainDialog::writeSettings()
 
     if (!file.open(QIODevice::ReadWrite | QIODevice::Text | QIODevice::Truncate)) {
         // File cannot be opened or created
-        QMessageBox msgBox;
-        msgBox.setWindowTitle(tr("Error writing Launcher configuration file"));
-        msgBox.setIcon(QMessageBox::Critical);
-        msgBox.setStandardButtons(QMessageBox::Ok);
-        msgBox.setText(tr("<br><b>Could not open or create %0 for writing</b><br><br> \
-                          Please make sure you have the right permissions \
-                          and try again.<br>").arg(file.fileName()));
-                          msgBox.exec();
-                       return false;
+        cfgError(tr("Error writing Launcher configuration file"),
+                 tr("<br><b>Could not open or create %0 for writing</b><br><br> \
+                     Please make sure you have the right permissions \
+                     and try again.<br>").arg(file.fileName()));
+        return false;
     }
 
     QTextStream stream(&file);

From a9c9cc5508f49dc095fd5e518dab93239646620a Mon Sep 17 00:00:00 2001
From: cfcohen <cfcohen@verizon.net>
Date: Thu, 26 Nov 2015 10:42:43 -0500
Subject: [PATCH 07/12] Remove unnecessary copy of mEngineSettings in
 mInitialSettings.

---
 apps/launcher/graphicspage.cpp | 16 +++++++---------
 apps/launcher/graphicspage.hpp |  1 -
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/apps/launcher/graphicspage.cpp b/apps/launcher/graphicspage.cpp
index d7f045f80..f56c97888 100644
--- a/apps/launcher/graphicspage.cpp
+++ b/apps/launcher/graphicspage.cpp
@@ -80,8 +80,6 @@ bool Launcher::GraphicsPage::loadSettings()
     if (!setupSDL())
         return false;
 
-    mInitialSettings = mEngineSettings;
-
     if (mEngineSettings.getBool("vsync", "Video"))
         vSyncCheckBox->setCheckState(Qt::Checked);
 
@@ -119,22 +117,22 @@ bool Launcher::GraphicsPage::loadSettings()
 
 void Launcher::GraphicsPage::saveSettings()
 {
-    bool iVSync = mInitialSettings.getBool("vsync", "Video");
+    bool iVSync = mEngineSettings.getBool("vsync", "Video");
     bool cVSync = vSyncCheckBox->checkState();
     if (iVSync != cVSync)
         mEngineSettings.setBool("vsync", "Video", cVSync);
 
-    bool iFullScreen = mInitialSettings.getBool("fullscreen", "Video");
+    bool iFullScreen = mEngineSettings.getBool("fullscreen", "Video");
     bool cFullScreen = fullScreenCheckBox->checkState();
     if (iFullScreen != cFullScreen)
         mEngineSettings.setBool("fullscreen", "Video", cFullScreen);
 
-    bool iWindowBorder = mInitialSettings.getBool("window border", "Video");
+    bool iWindowBorder = mEngineSettings.getBool("window border", "Video");
     bool cWindowBorder = windowBorderCheckBox->checkState();
     if (iWindowBorder != cWindowBorder)
         mEngineSettings.setBool("window border", "Video", cWindowBorder);
 
-    int iAAValue = mInitialSettings.getInt("antialiasing", "Video");
+    int iAAValue = mEngineSettings.getInt("antialiasing", "Video");
     // The atoi() call is safe because the pull down constrains the string values.
     int cAAValue = atoi(antiAliasingComboBox->currentText().toLatin1().data());
     if (iAAValue != cAAValue)
@@ -154,15 +152,15 @@ void Launcher::GraphicsPage::saveSettings()
         cHeight = customHeightSpinBox->value();
     }
 
-    int iWidth = mInitialSettings.getInt("resolution x", "Video");
+    int iWidth = mEngineSettings.getInt("resolution x", "Video");
     if (iWidth != cWidth)
         mEngineSettings.setInt("resolution x", "Video", cWidth);
 
-    int iHeight = mInitialSettings.getInt("resolution y", "Video");
+    int iHeight = mEngineSettings.getInt("resolution y", "Video");
     if (iHeight != cHeight)
         mEngineSettings.setInt("resolution y", "Video", cHeight);
 
-    int iScreen = mInitialSettings.getInt("screen", "Video");
+    int iScreen = mEngineSettings.getInt("screen", "Video");
     int cScreen = screenComboBox->currentIndex();
     if (iScreen != cScreen)
         mEngineSettings.setInt("screen", "Video", cScreen);
diff --git a/apps/launcher/graphicspage.hpp b/apps/launcher/graphicspage.hpp
index 0958f07da..e6eb53a3b 100644
--- a/apps/launcher/graphicspage.hpp
+++ b/apps/launcher/graphicspage.hpp
@@ -33,7 +33,6 @@ namespace Launcher
     private:
         Files::ConfigurationManager &mCfgMgr;
         Settings::Manager &mEngineSettings;
-        Settings::Manager mInitialSettings;
 
         QStringList getAvailableResolutions(int screen);
         QRect getMaximumResolution();

From c26463fd91b90bba390085228712fa7396c997f5 Mon Sep 17 00:00:00 2001
From: cfcohen <cfcohen@verizon.net>
Date: Thu, 26 Nov 2015 10:52:20 -0500
Subject: [PATCH 08/12] Should have coded it the way scrawl said, since it's
 cleaner.

---
 apps/launcher/graphicspage.cpp | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/apps/launcher/graphicspage.cpp b/apps/launcher/graphicspage.cpp
index f56c97888..bc797574b 100644
--- a/apps/launcher/graphicspage.cpp
+++ b/apps/launcher/graphicspage.cpp
@@ -117,25 +117,21 @@ bool Launcher::GraphicsPage::loadSettings()
 
 void Launcher::GraphicsPage::saveSettings()
 {
-    bool iVSync = mEngineSettings.getBool("vsync", "Video");
     bool cVSync = vSyncCheckBox->checkState();
-    if (iVSync != cVSync)
+    if (cVSync != mEngineSettings.getBool("vsync", "Video"))
         mEngineSettings.setBool("vsync", "Video", cVSync);
 
-    bool iFullScreen = mEngineSettings.getBool("fullscreen", "Video");
     bool cFullScreen = fullScreenCheckBox->checkState();
-    if (iFullScreen != cFullScreen)
+    if (cFullScreen != mEngineSettings.getBool("fullscreen", "Video"))
         mEngineSettings.setBool("fullscreen", "Video", cFullScreen);
 
-    bool iWindowBorder = mEngineSettings.getBool("window border", "Video");
     bool cWindowBorder = windowBorderCheckBox->checkState();
-    if (iWindowBorder != cWindowBorder)
+    if (cWindowBorder != mEngineSettings.getBool("window border", "Video"))
         mEngineSettings.setBool("window border", "Video", cWindowBorder);
 
-    int iAAValue = mEngineSettings.getInt("antialiasing", "Video");
     // The atoi() call is safe because the pull down constrains the string values.
     int cAAValue = atoi(antiAliasingComboBox->currentText().toLatin1().data());
-    if (iAAValue != cAAValue)
+    if (cAAValue != mEngineSettings.getInt("antialiasing", "Video"))
         mEngineSettings.setInt("antialiasing", "Video", cAAValue);
 
     int cWidth = 0;
@@ -152,17 +148,14 @@ void Launcher::GraphicsPage::saveSettings()
         cHeight = customHeightSpinBox->value();
     }
 
-    int iWidth = mEngineSettings.getInt("resolution x", "Video");
-    if (iWidth != cWidth)
+    if (cWidth != mEngineSettings.getInt("resolution x", "Video"))
         mEngineSettings.setInt("resolution x", "Video", cWidth);
 
-    int iHeight = mEngineSettings.getInt("resolution y", "Video");
-    if (iHeight != cHeight)
+    if (cHeight != mEngineSettings.getInt("resolution y", "Video"))
         mEngineSettings.setInt("resolution y", "Video", cHeight);
 
-    int iScreen = mEngineSettings.getInt("screen", "Video");
     int cScreen = screenComboBox->currentIndex();
-    if (iScreen != cScreen)
+    if (cScreen != mEngineSettings.getInt("screen", "Video"))
         mEngineSettings.setInt("screen", "Video", cScreen);
 }
 

From 3747843c921a20159bc7ab048064c72ceab61480 Mon Sep 17 00:00:00 2001
From: scrawl <scrawl@baseoftrash.de>
Date: Thu, 26 Nov 2015 17:07:20 +0100
Subject: [PATCH 09/12] Use QString::toInt instead of atoi

---
 apps/launcher/graphicspage.cpp | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/apps/launcher/graphicspage.cpp b/apps/launcher/graphicspage.cpp
index bc797574b..523d0bb04 100644
--- a/apps/launcher/graphicspage.cpp
+++ b/apps/launcher/graphicspage.cpp
@@ -129,8 +129,7 @@ void Launcher::GraphicsPage::saveSettings()
     if (cWindowBorder != mEngineSettings.getBool("window border", "Video"))
         mEngineSettings.setBool("window border", "Video", cWindowBorder);
 
-    // The atoi() call is safe because the pull down constrains the string values.
-    int cAAValue = atoi(antiAliasingComboBox->currentText().toLatin1().data());
+    int cAAValue = antiAliasingComboBox->currentText().toInt();
     if (cAAValue != mEngineSettings.getInt("antialiasing", "Video"))
         mEngineSettings.setInt("antialiasing", "Video", cAAValue);
 
@@ -139,9 +138,8 @@ void Launcher::GraphicsPage::saveSettings()
     if (standardRadioButton->isChecked()) {
         QRegExp resolutionRe(QString("(\\d+) x (\\d+).*"));
         if (resolutionRe.exactMatch(resolutionComboBox->currentText().simplified())) {
-            // The atoi() call is safe because the pull down constrains the string values.
-            cWidth = atoi(resolutionRe.cap(1).toLatin1().data());
-            cHeight = atoi(resolutionRe.cap(2).toLatin1().data());
+            cWidth = resolutionRe.cap(1).toInt();
+            cHeight = resolutionRe.cap(2).toInt();
         }
     } else {
         cWidth = customWidthSpinBox->value();

From 325d208b4a74ce9f1bd976ba76c07f69b9c61525 Mon Sep 17 00:00:00 2001
From: scrawl <scrawl@baseoftrash.de>
Date: Thu, 26 Nov 2015 17:13:13 +0100
Subject: [PATCH 10/12] Fix incorrect error message

---
 apps/launcher/maindialog.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/apps/launcher/maindialog.cpp b/apps/launcher/maindialog.cpp
index 6453beacc..9bf135e7e 100644
--- a/apps/launcher/maindialog.cpp
+++ b/apps/launcher/maindialog.cpp
@@ -505,7 +505,7 @@ bool Launcher::MainDialog::writeSettings()
     catch (std::exception& e) {
         std::string msg = "<br><b>Error writing settings.cfg</b><br><br>" +
             settingsPath + "<br><br>" + e.what();
-        cfgError(tr("Error reading OpenMW configuration file"), tr(msg.c_str()));
+        cfgError(tr("Error writing user settings file"), tr(msg.c_str()));
         return false;
     }
 

From 84aceedfa26352fd85d2364555e51d744258d287 Mon Sep 17 00:00:00 2001
From: scrawl <scrawl@baseoftrash.de>
Date: Thu, 26 Nov 2015 17:15:12 +0100
Subject: [PATCH 11/12] Add comment

---
 apps/launcher/graphicspage.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/apps/launcher/graphicspage.cpp b/apps/launcher/graphicspage.cpp
index 523d0bb04..622db4da4 100644
--- a/apps/launcher/graphicspage.cpp
+++ b/apps/launcher/graphicspage.cpp
@@ -117,6 +117,8 @@ bool Launcher::GraphicsPage::loadSettings()
 
 void Launcher::GraphicsPage::saveSettings()
 {
+    // Ensure we only set the new settings if they changed. This is to avoid cluttering the
+    // user settings file (which by definition should only contain settings the user has touched)
     bool cVSync = vSyncCheckBox->checkState();
     if (cVSync != mEngineSettings.getBool("vsync", "Video"))
         mEngineSettings.setBool("vsync", "Video", cVSync);

From d894d54e41e7577dc7b54e06613712780a07bd65 Mon Sep 17 00:00:00 2001
From: scrawl <scrawl@baseoftrash.de>
Date: Thu, 26 Nov 2015 17:15:28 +0100
Subject: [PATCH 12/12] Improve path conversions

---
 apps/launcher/maindialog.cpp | 8 ++++----
 apps/openmw/engine.cpp       | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/apps/launcher/maindialog.cpp b/apps/launcher/maindialog.cpp
index 9bf135e7e..8738dae2c 100644
--- a/apps/launcher/maindialog.cpp
+++ b/apps/launcher/maindialog.cpp
@@ -385,8 +385,8 @@ bool Launcher::MainDialog::setupGraphicsSettings()
     // the filenames should be in the CfgMgr component.
 
     // Create the settings manager and load default settings file
-    const std::string localDefault = mCfgMgr.getLocalPath().string() + "settings-default.cfg";
-    const std::string globalDefault = mCfgMgr.getGlobalPath().string() + "settings-default.cfg";
+    const std::string localDefault = (mCfgMgr.getLocalPath() / "settings-default.cfg").string();
+    const std::string globalDefault = (mCfgMgr.getGlobalPath() / "settings-default.cfg").string();
     std::string defaultPath;
 
     // Prefer the settings-default.cfg in the current directory.
@@ -414,7 +414,7 @@ bool Launcher::MainDialog::setupGraphicsSettings()
     }
 
     // Load user settings if they exist
-    const std::string userPath = mCfgMgr.getUserConfigPath().string() + "settings.cfg";
+    const std::string userPath = (mCfgMgr.getUserConfigPath() / "settings.cfg").string();
     // User settings are not required to exist, so if they don't we're done.
     if (!boost::filesystem::exists(userPath)) return true;
 
@@ -498,7 +498,7 @@ bool Launcher::MainDialog::writeSettings()
     file.close();
 
     // Graphics settings
-    const std::string settingsPath = mCfgMgr.getUserConfigPath().string() + "settings.cfg";
+    const std::string settingsPath = (mCfgMgr.getUserConfigPath() / "settings.cfg").string();
     try {
         mEngineSettings.saveUser(settingsPath);
     }
diff --git a/apps/openmw/engine.cpp b/apps/openmw/engine.cpp
index a4f5f9440..66231dd98 100644
--- a/apps/openmw/engine.cpp
+++ b/apps/openmw/engine.cpp
@@ -298,8 +298,8 @@ void OMW::Engine::setSkipMenu (bool skipMenu, bool newGame)
 std::string OMW::Engine::loadSettings (Settings::Manager & settings)
 {
     // Create the settings manager and load default settings file
-    const std::string localdefault = mCfgMgr.getLocalPath().string() + "settings-default.cfg";
-    const std::string globaldefault = mCfgMgr.getGlobalPath().string() + "settings-default.cfg";
+    const std::string localdefault = (mCfgMgr.getLocalPath() / "settings-default.cfg").string();
+    const std::string globaldefault = (mCfgMgr.getGlobalPath() / "settings-default.cfg").string();
 
     // prefer local
     if (boost::filesystem::exists(localdefault))
@@ -310,7 +310,7 @@ std::string OMW::Engine::loadSettings (Settings::Manager & settings)
         throw std::runtime_error ("No default settings file found! Make sure the file \"settings-default.cfg\" was properly installed.");
 
     // load user settings if they exist
-    const std::string settingspath = mCfgMgr.getUserConfigPath().string() + "settings.cfg";
+    const std::string settingspath = (mCfgMgr.getUserConfigPath() / "settings.cfg").string();
     if (boost::filesystem::exists(settingspath))
         settings.loadUser(settingspath);