Add INI file versioning

This introduces a new INI config option called "IniVersion". Whenever
default values change or options are renmaed, the latest version number is
incremented and an update path is run.
This change also rewrites part of the initialization routine by adding
more safety checks before creating or reading the configuration file.

src/wx/cmdevents.cpp

@@ -208,10 +208,10 @@ EVT_HANDLER(RecentReset, "Reset recent ROM list")
         while (gopts.recent->GetCount())
             gopts.recent->RemoveFileFromHistory(0);
 
-        wxFileConfig* cfg = wxGetApp().cfg;
+        wxConfigBase* cfg = wxConfigBase::Get();
-        cfg->SetPath(wxT("/Recent"));
+        cfg->SetPath("/Recent");
         gopts.recent->Save(*cfg);
-        cfg->SetPath(wxT("/"));
+        cfg->SetPath("/");
         cfg->Flush();
     }
 }
@@ -2857,14 +2857,12 @@ EVT_HANDLER(UpdateEmu, "Check for updates...")
 
 EVT_HANDLER(FactoryReset, "Factory Reset...")
 {
-    wxMessageDialog dlg(NULL, wxString(wxT(
+    wxMessageDialog dlg(
-"YOUR CONFIGURATION WILL BE DELETED!\n\n")) + wxString(wxT(
+        nullptr, _("YOUR CONFIGURATION WILL BE DELETED!\n\nAre you sure?"),
-"Are you sure?")),
+        _("FACTORY RESET"), wxYES_NO | wxNO_DEFAULT | wxCENTRE);
-                        wxT("FACTORY RESET"), wxYES_NO | wxNO_DEFAULT | wxCENTRE);
 
     if (dlg.ShowModal() == wxID_YES) {
-        wxGetApp().cfg->DeleteAll();
+        wxConfigBase::Get()->DeleteAll();
-
         wxExecute(wxStandardPaths::Get().GetExecutablePath(), wxEXEC_ASYNC);
         Close(true);
     }

src/wx/config/internal/option-internal.cpp

@@ -161,6 +161,9 @@ std::array<Option, kNbOptions>& Option::All() {
         double video_scale = 3;
         bool retain_aspect = true;
 
+        /// General
+        uint32_t ini_version = kIniLatestVersion;
+
         /// Geometry
         bool window_maximized = false;
         uint32_t window_height = 0;
@@ -226,6 +229,7 @@ std::array<Option, kNbOptions>& Option::All() {
         Option(OptionID::kGenScreenshotDir, &gopts.scrshot_dir),
         Option(OptionID::kGenStateDir, &gopts.state_dir),
         Option(OptionID::kGenStatusBar, &gopts.statusbar),
+        Option(OptionID::kGenIniVersion, &g_owned_opts.ini_version, 0, std::numeric_limits<uint32_t>::max()),
 
         /// Joypad
         Option(OptionID::kJoy),
@@ -402,6 +406,7 @@ const std::array<OptionData, kNbOptions + 1> kAllOptionsData = {
                _("Directory to store saved state files (relative paths are "
                  "relative to BatteryDir)")},
     OptionData{"General/StatusBar", "StatusBar", _("Enable status bar")},
+    OptionData{"General/IniVersion", "", _("INI file version (DO NOT MODIFY)")},
 
     /// Joypad
     OptionData{"Joypad/*/*", "",

src/wx/config/option-id.h

@@ -53,6 +53,7 @@ enum class OptionID {
     kGenScreenshotDir,
     kGenStateDir,
     kGenStatusBar,
+    kGenIniVersion,
 
     /// Joypad
     kJoy,

src/wx/config/option-proxy.h

@@ -59,6 +59,7 @@ static constexpr std::array<Option::Type, kNbOptions> kOptionsTypes = {
     /*kGenScreenshotDir*/ Option::Type::kString,
     /*kGenStateDir*/ Option::Type::kString,
     /*kGenStatusBar*/ Option::Type::kBool,
+    /*kGenIniVersion*/ Option::Type::kUnsigned,
 
     /// Joypad
     /*kJoy*/ Option::Type::kNone,

src/wx/config/option.h

@@ -79,6 +79,10 @@ enum class RenderMethod {
 static constexpr size_t kNbRenderMethods =
     static_cast<size_t>(RenderMethod::kLast);
 
+// This is incremented whenever we want to change a default value between
+// release versions. The option update code is in load_opts.
+static constexpr uint32_t kIniLatestVersion = 1;
+
 // Represents a single option saved in the INI file. Option does not own the
 // individual option, but keeps a pointer to where the data is actually saved.
 //

src/wx/opts.cpp

@@ -1,11 +1,12 @@
 #include "opts.h"
 
 #include <algorithm>
+#include <limits>
 #include <memory>
 #include <unordered_set>
 
+#include <wx/defs.h>
 #include <wx/log.h>
-#include <wx/display.h>
 
 #include "config/option-observer.h"
 #include "config/option-proxy.h"
@@ -25,7 +26,7 @@
 namespace {
 
 void SaveOption(config::Option* option) {
-    wxFileConfig* cfg = wxGetApp().cfg;
+    wxConfigBase* cfg = wxConfigBase::Get();
 
     switch (option->type()) {
         case config::Option::Type::kNone:
@@ -72,6 +73,33 @@ void InitializeOptionObservers() {
     }
 }
 
+// Helper function to work around wxWidgets limitations when converting string
+// to unsigned int.
+uint32_t LoadUnsignedOption(wxConfigBase* cfg,
+                            const wxString& option_name,
+                            uint32_t default_value) {
+    wxString temp;
+    if (!cfg->Read(option_name, &temp)) {
+        return default_value;
+    }
+    if (!temp.IsNumber()) {
+        return default_value;
+    }
+
+    // Go through ulonglong to get enough space to work with. Also, older
+    // versions do not have a conversion function for unsigned int.
+    wxULongLong_t out;
+    if (!temp.ToULongLong(&out)) {
+        return default_value;
+    }
+
+    if (out > std::numeric_limits<uint32_t>::max()) {
+        return default_value;
+    }
+
+    return out;
+}
+
 }  // namespace
 
 #define WJKB config::UserInput
@@ -317,7 +345,7 @@ opts_t::opts_t()
 }
 
 // FIXME: simulate MakeInstanceFilename(vbam.ini) using subkeys (Slave%d/*)
-void load_opts() {
+void load_opts(bool first_time_launch) {
     // just for sanity...
     static bool did_init = false;
     assert(!did_init);
@@ -326,8 +354,9 @@ void load_opts() {
     // enumvals should not be translated, since they would cause config file
     // change after lang change
     // instead, translate when presented to user
-    wxFileConfig* cfg = wxGetApp().cfg;
+    wxConfigBase* cfg = wxConfigBase::Get();
-    cfg->SetPath(wxT("/"));
+    cfg->SetPath("/");
+
     // enure there are no unknown options present
     // note that items cannot be deleted until after loop or loop will fail
     wxArrayString item_del, grp_del;
@@ -341,9 +370,22 @@ void load_opts() {
         item_del.push_back(s);
     }
 
-    // Date of last online update check;
+    // Read the IniVersion now since the Option initialization code will reset
-    gopts.last_update = cfg->Read(wxT("General/LastUpdated"), (long)0);
+    // it to kIniLatestVersion if it is unset.
-    cfg->Read(wxT("General/LastUpdatedFileName"), &gopts.last_updated_filename);
+    uint32_t ini_version = 0;
+    if (first_time_launch) {
+        // Just go with the default values for the first time launch.
+        ini_version = config::kIniLatestVersion;
+    } else {
+        // We want to default to 0 if the option is not set.
+        ini_version = LoadUnsignedOption(cfg, "General/IniVersion", 0);
+        if (ini_version > config::kIniLatestVersion) {
+            wxLogWarning(
+                _("The INI file was written for a more recent version of "
+                  "VBA-M. Some INI option values may have been reset."));
+            ini_version = config::kIniLatestVersion;
+        }
+    }
 
     for (cont = cfg->GetFirstGroup(s, grp_idx); cont;
          cont = cfg->GetNextGroup(s, grp_idx)) {
@@ -464,8 +506,8 @@ void load_opts() {
             break;
         }
         case config::Option::Type::kUnsigned: {
-            int temp;
+            uint32_t temp =
-            cfg->Read(opt.config_name(), &temp, opt.GetUnsigned());
+                LoadUnsignedOption(cfg, opt.config_name(), opt.GetUnsigned());
             opt.SetUnsigned(temp);
             break;
         }
@@ -537,11 +579,6 @@ void load_opts() {
         }
     }
 
-    // Make sure link_timeout is not set to 1, which was the previous default.
-    if (gopts.link_timeout <= 1) {
-        gopts.link_timeout = 500;
-    }
-
     // recent is special
     // Recent does not get written with defaults
     cfg->SetPath(wxT("/Recent"));
@@ -549,6 +586,8 @@ void load_opts() {
     cfg->SetPath(wxT("/"));
     cfg->Flush();
 
+    InitializeOptionObservers();
+
     // We default the MaxThreads option to 0, so set it to the CPU count here.
     config::OptionProxy<config::OptionID::kDispMaxThreads> max_threads;
     if (max_threads == 0) {
@@ -563,13 +602,32 @@ void load_opts() {
         }
     }
 
-    InitializeOptionObservers();
+    // Apply Option updates.
+    while (ini_version < config::kIniLatestVersion) {
+        // Update the ini version as we go in case we fail halfway through.
+        OPTION(kGenIniVersion) = ini_version;
+        switch (ini_version) {
+            case 0: { // up to 2.1.5 included.
+#ifndef NO_LINK
+                // Previous default was 1.
+                if (OPTION(kGBALinkTimeout) == 1) {
+                    OPTION(kGBALinkTimeout) = 500;
+                }
+#endif
+                // Previous default was true.
+                OPTION(kGBALCDFilter) = false;
+            }
+        }
+        ini_version++;
+    }
+
+    // Finally, overwrite the value to the current version.
+    OPTION(kGenIniVersion) = config::kIniLatestVersion;
 }
 
 // Note: run load_opts() first to guarantee all config opts exist
-void update_opts()
+void update_opts() {
-{
+    wxConfigBase* cfg = wxConfigBase::Get();
-    wxFileConfig* cfg = wxGetApp().cfg;
 
     for (config::Option& opt : config::Option::All()) {
         SaveOption(&opt);

src/wx/opts.h

@@ -53,8 +53,6 @@ extern struct opts_t {
     bool autoload_state = false;
     bool autoload_cheats = false;
     wxString battery_dir;
-    long last_update;
-    wxString last_updated_filename;
     bool recent_freeze = false;
     wxString recording_dir;
     int rewind_interval = 0;
@@ -116,7 +114,7 @@ extern const int num_def_accels;
 // call to load config (once)
 // will write defaults for options not present and delete bad opts
 // will also initialize opts[] array translations
-void load_opts();
+void load_opts(bool first_time_launch);
 // call whenever opt vars change
 // will detect changes and write config if necessary
 void update_opts();

src/wx/panel.cpp

@@ -170,14 +170,14 @@ void GameArea::LoadGame(const wxString& name)
     }
 
     {
-        wxFileConfig* cfg = wxGetApp().cfg;
+        wxConfigBase* cfg = wxConfigBase::Get();
 
         if (!gopts.recent_freeze) {
             gopts.recent->AddFileToHistory(name);
             wxGetApp().frame->SetRecentAccels();
-            cfg->SetPath(wxT("/Recent"));
+            cfg->SetPath("/Recent");
             gopts.recent->Save(*cfg);
-            cfg->SetPath(wxT("/"));
+            cfg->SetPath("/");
             cfg->Flush();
         }
     }

src/wx/wxvbam.cpp

@@ -38,6 +38,11 @@
 #include "strutils.h"
 #include "wayland.h"
 
+namespace {
+static const wxString kOldConfigFileName("vbam.conf");
+static const wxString knewConfigFileName("vbam.ini");
+}  // namespace
+
 #ifdef __WXMSW__
 
 int __stdcall WinMain(HINSTANCE hInstance,
@@ -197,16 +202,14 @@ const wxString wxvbamApp::GetPluginsDir()
     return wxStandardPaths::Get().GetPluginsDir();
 }
 
-wxString wxvbamApp::GetConfigurationPath()
+wxString wxvbamApp::GetConfigurationPath() {
-{
-    wxString config(wxT("vbam.ini"));
     // first check if config files exists in reverse order
     // (from system paths to more local paths.)
     if (data_path.empty()) {
         get_config_path(config_path);
 
         for (int i = config_path.size() - 1; i >= 0; i--) {
-            wxFileName fn(config_path[i], config);
+            wxFileName fn(config_path[i], knewConfigFileName);
 
             if (fn.FileExists() && fn.IsFileWritable()) {
                 data_path = config_path[i];
@@ -312,53 +315,52 @@ bool wxvbamApp::OnInit() {
 
     wxSetWorkingDirectory(cwd);
 
-    if (!cfg) {
+    if (!config_file_.IsOk()) {
-        // set up config file
+        // Set up the default configuration file.
-        // this needs to be in a subdir to support other config as well
+        // This needs to be in a subdir to support other config as well.
-        // but subdir flag behaves differently 2.8 vs. 2.9.  Oh well.
+        // NOTE: this does not support XDG (freedesktop.org) paths.
-        // NOTE: this does not support XDG (freedesktop.org) paths
+        // We rely on wx to build the paths in a cross-platform manner. However,
-        wxString confname(wxT("vbam.ini"));
+        // the wxFileName APIs are weird and don't quite work as intended so we
-        wxFileName vbamconf(GetConfigurationPath(), confname);
+        // use the wxString APIs for files instead.
-        // /MIGRATION
+        const wxString old_conf_file(
-        // migrate from 'vbam.{cfg,conf}' to 'vbam.ini' to manage a single config
+            wxFileName(GetConfigurationPath(), kOldConfigFileName)
-        // file for all platforms.
+                .GetFullPath());
-        wxString oldConf(GetConfigurationPath() + wxT(FILE_SEP) + wxT("vbam.conf"));
+        const wxString new_conf_file(
-        wxString newConf(GetConfigurationPath() + wxT(FILE_SEP) + wxT("vbam.ini"));
+            wxFileName(GetConfigurationPath(), knewConfigFileName)
+                .GetFullPath());
 
-        if (!wxFileExists(newConf) && wxFileExists(oldConf))
+        if (wxDirExists(new_conf_file)) {
-            wxRenameFile(oldConf, newConf, false);
+            wxLogError(_("Invalid configuration file provided: %s"),
+                       new_conf_file);
+            return false;
+        }
+
+        // /MIGRATION
+        // Migrate from 'vbam.conf' to 'vbam.ini' to manage a single config
+        // file for all platforms.
+        if (!wxFileExists(new_conf_file) && wxFileExists(old_conf_file)) {
+            wxRenameFile(old_conf_file, new_conf_file, false);
+        }
         // /END_MIGRATION
 
-        cfg = new wxFileConfig(wxT("vbam"), wxEmptyString,
+        config_file_ = new_conf_file;
-            vbamconf.GetFullPath(),
-            wxEmptyString, wxCONFIG_USE_LOCAL_FILE);
-        // set global config for e.g. Windows font mapping
-        wxFileConfig::Set(cfg);
-        // yet another bug/deficiency in wxConfig: dirs are not created if needed
-        // since a default config is always written, dirs are always needed
-        // Can't figure out statically if using wxFileConfig w/o duplicating wx's
-        // logic, so do it at run-time
-        // wxFileConfig *f = wxDynamicCast(cfg, wxFileConfig);
-        // wxConfigBase does not derive from wxObject!!! so no wxDynamicCast
-        wxFileConfig* fc = dynamic_cast<wxFileConfig*>(cfg);
-
-        if (fc) {
-            wxFileName s(wxFileConfig::GetLocalFileName(GetAppName()));
-            // at least up to 2.8.12, GetLocalFileName returns the dir if
-            // SUBDIR is specified instead of actual file name
-            // and SUBDIR only affects UNIX
-#if defined(__UNIX__) && !wxCHECK_VERSION(2, 9, 0)
-            s.AppendDir(s.GetFullName());
-#endif
-            // only the path part gets created
-            // note that 0777 is default (assumes umask will do og-w)
-            s.Mkdir(0777, wxPATH_MKDIR_FULL);
-            s = wxFileName::DirName(GetConfigurationPath());
-            s.Mkdir(0777, wxPATH_MKDIR_FULL);
-        }
     }
 
-    load_opts();
+    if (!config_file_.IsOk() || wxDirExists(config_file_.GetFullPath())) {
+        wxLogError(_("Invalid configuration file provided: %s"),
+                   config_file_.GetFullPath());
+        return false;
+    }
+
+    // wx takes ownership of the wxFileConfig here. It will be deleted on app
+    // destruction.
+    wxConfigBase::DontCreateOnDemand();
+    wxConfigBase::Set(new wxFileConfig("vbam", wxEmptyString,
+                                       config_file_.GetFullPath(),
+                                       wxEmptyString, wxCONFIG_USE_LOCAL_FILE));
+
+    // Load the default options.
+    load_opts(!config_file_.Exists());
 
     // wxGLCanvas segfaults under wayland before wx 3.2
 #if defined(HAVE_WAYLAND_SUPPORT) && !defined(HAVE_WAYLAND_EGL)
@@ -703,9 +705,7 @@ bool wxvbamApp::OnCmdLineParsed(wxCmdLineParser& cl)
             wxLogError(_("Configuration file not found."));
             return false;
         }
-        cfg = new wxFileConfig(wxT("vbam"), wxEmptyString,
+        config_file_ = s;
-            vbamconf.GetFullPath(),
-            wxEmptyString, wxCONFIG_USE_LOCAL_FILE);
     }
 
 #if !defined(NO_LINK) && !defined(__WXMSW__)

src/wx/wxvbam.h

@@ -104,8 +104,6 @@ public:
         return accels;
     }
 
-    // the main configuration
-    wxFileConfig* cfg = nullptr;
     // vba-over.ini
     wxFileConfig* overrides = nullptr;
 
@@ -144,6 +142,9 @@ protected:
 private:
     wxPathList config_path;
     char* home = nullptr;
+
+    // Main configuration file.
+    wxFileName config_file_;
 };
 
 DECLARE_APP(wxvbamApp);