Launcher: ensure extra args override launcher ones

Categorise arguments to ensure user-specified ones take precedence
over values set by the launcher.

When any positional arg is set, exclude all positional ones from being
set at all, to prevent strange interactions.

src/GUI/LaunchConfig.cxx

@@ -1,5 +1,7 @@
 #include "LaunchConfig.hxx"
 
+#include <set>
+
 #include <Main/options.hxx>
 #include <simgear/misc/sg_path.hxx>
 
@@ -20,31 +22,50 @@ void LaunchConfig::reset()
 
 void LaunchConfig::applyToOptions() const
 {
+    // build a set of all the extra args we have defined
+    std::set<std::string> extraArgNames;
+    for (auto arg : m_values) {
+        // don't override prop: arguments
+        if (arg.arg == "prop") continue;
+
+        if (arg.origin == ExtraArgs)
+            extraArgNames.insert(arg.arg.toStdString());
+    }
+
     flightgear::Options* options = flightgear::Options::sharedInstance();
-    std::for_each(m_values.begin(), m_values.end(), [options](const Arg& arg)
+    std::for_each(m_values.begin(), m_values.end(),
+                  [options, &extraArgNames](const Arg& arg)
     {
-                      options->addOption(arg.arg.toStdString(), arg.value.toStdString());
+        const auto name = arg.arg.toStdString();
+        if (arg.origin == Launcher) {
+            auto it = extraArgNames.find(name);
+            if (it != extraArgNames.end()) {
+                qInfo() << "skipping arg:" << arg.arg << "=" << arg.value << "because the user has over-ridden it";
+                return;
+            }
+        }
+        options->addOption(name, arg.value.toStdString());
     });
 }
 
-void LaunchConfig::setArg(QString name, QString value)
+void LaunchConfig::setArg(QString name, QString value, Origin origin)
 {
-    m_values.push_back(Arg(name, value));
+    m_values.push_back(Arg(name, value, origin));
 }
 
 void LaunchConfig::setArg(const std::string &name, const std::string &value)
 {
-    setArg(QString::fromStdString(name), QString::fromStdString(value));
+    setArg(QString::fromStdString(name), QString::fromStdString(value), Launcher);
 }
 
-void LaunchConfig::setProperty(QString path, QVariant value)
+void LaunchConfig::setProperty(QString path, QVariant value, Origin origin)
 {
-    m_values.push_back(Arg("prop", path + "=" + value.toString()));
+    m_values.push_back(Arg("prop", path + "=" + value.toString(), origin));
 }
 
 void LaunchConfig::setEnableDisableOption(QString name, bool value)
 {
-    m_values.push_back(Arg((value ? "enable-" : "disable-") + name));
+    m_values.push_back(Arg((value ? "enable-" : "disable-") + name, "", Launcher));
 }
 
 QString LaunchConfig::htmlForCommandLine()
@@ -59,20 +80,26 @@ QString LaunchConfig::htmlForCommandLine()
         }
         html += "</ul>\n";
     }
-#if 0
-    if (m_extraSettings) {
-        LauncherArgumentTokenizer tk;
-        Q_FOREACH(auto arg, tk.tokenize(m_extraSettings->argsText())) {
-    //        m_config->setArg(arg.arg, arg.value);
-        }
-    }
-#endif
+
     reset();
     collect();
 
     html += tr("<p>Options set in the launcher:</p>\n");
     html += "<ul>\n";
-    for (auto arg : values()) {
+    for (auto arg : valuesFromLauncher()) {
+        if (arg.value.isEmpty()) {
+            html += QString("<li>--") + arg.arg + "</li>\n";
+        } else if (arg.arg == "prop") {
+            html += QString("<li>--") + arg.arg + ":" + arg.value + "</li>\n";
+        } else {
+            html += QString("<li>--") + arg.arg + "=" + arg.value + "</li>\n";
+        }
+    }
+    html += "</ul>\n";
+
+    html += tr("<p>Options set as additional arguments:</p>\n");
+    html += "<ul>\n";
+    for (auto arg : valuesFromExtraArgs()) {
         if (arg.value.isEmpty()) {
             html += QString("<li>--") + arg.arg + "</li>\n";
         } else if (arg.arg == "prop") {
@@ -127,3 +154,19 @@ auto LaunchConfig::values() const -> std::vector<Arg>
 {
     return m_values;
 }
+
+auto LaunchConfig::valuesFromLauncher() const -> std::vector<Arg>
+{
+    std::vector<Arg> result;
+    std::copy_if(m_values.begin(), m_values.end(), std::back_inserter(result), [](const Arg& a)
+    { return a.origin == Launcher; });
+    return result;
+}
+
+auto LaunchConfig::valuesFromExtraArgs() const -> std::vector<Arg>
+{
+    std::vector<Arg> result;
+    std::copy_if(m_values.begin(), m_values.end(), std::back_inserter(result), [](const Arg& a)
+    { return a.origin == ExtraArgs; });
+    return result;
+}

src/GUI/LaunchConfig.hxx

@@ -13,13 +13,22 @@ class LaunchConfig : public QObject
     Q_PROPERTY(QString defaultDownloadDir READ defaultDownloadDir CONSTANT)
     Q_PROPERTY(bool enableDownloadDirUI READ enableDownloadDirUI CONSTANT)
 public:
+    enum Origin
+    {
+        Launcher = 0,
+        ExtraArgs
+    };
+
+    Q_ENUMS(Origin);
+
     class Arg
     {
     public:
-        explicit Arg(QString k, QString v = QString()) : arg(k), value(v) {}
+        explicit Arg(QString k, QString v, Origin o) : arg(k), value(v), origin(o) {}
 
         QString arg;
         QString value;
+        Origin origin;
     };
 
 
@@ -30,11 +39,13 @@ public:
 
     std::vector<Arg> values() const;
 
-    Q_INVOKABLE void setArg(QString name, QString value = QString());
+
+
+    Q_INVOKABLE void setArg(QString name, QString value = QString(), Origin origin = Launcher);
 
     Q_INVOKABLE void setArg(const std::string& name, const std::string& value = std::string());
 
-    Q_INVOKABLE void setProperty(QString path, QVariant value);
+    Q_INVOKABLE void setProperty(QString path, QVariant value, Origin origin = Launcher);
 
     Q_INVOKABLE void setEnableDisableOption(QString name, bool value);
 
@@ -54,6 +65,9 @@ public:
 
     static void setEnableDownloadDirUI(bool enableDownloadDirUI);
 
+    std::vector<Arg> valuesFromLauncher() const;
+    std::vector<Arg> valuesFromExtraArgs() const;
+
 signals:
     void collect();
     

src/GUI/LauncherArgumentTokenizer.cxx

@@ -128,6 +128,14 @@ bool LauncherArgumentTokenizer::isValid() const
     return m_valid;
 }
 
+const std::set<std::string> unsupportedArgs({
+    "fg-aircraft", "fg-root", "fg-home", "aircraft-dir", "fg-scenery"});
+
+bool LauncherArgumentTokenizer::haveUnsupportedArgs() const
+{
+    return haveArgsIn(unsupportedArgs);
+}
+
 void LauncherArgumentTokenizer::setArgString(QString argString)
 {
     if (m_argString == argString)
@@ -138,20 +146,29 @@ void LauncherArgumentTokenizer::setArgString(QString argString)
     emit argStringChanged(m_argString);
 }
 
-const std::set<std::string> argBlacklist({
-    "lat", "lon", "aircraft", "airport", "parkpos", "season",
-    "runway", "vor", "time-offset", "timeofday"});
+const std::set<std::string> positionalArgs({
+    "lat", "lon", "vc", "vor", "ndb", "fix"
+    "airport", "parkpos", "runway",
+    "heading", "altitude", "offset-azimuth",
+    "offset-distance", "glideslope",
+    "on-ground"});
 
-bool LauncherArgumentTokenizer::haveProtectedArgs() const
+bool LauncherArgumentTokenizer::haveArgsIn(const std::set<std::string>& args) const
 {
     if (!m_valid)
         return false;
 
     auto n = std::count_if(m_tokens.begin(), m_tokens.end(),
-                           [](const ArgumentToken& tk)
+                           [&args](const ArgumentToken& tk)
     {
-        return (argBlacklist.find(tk.arg.toStdString()) != argBlacklist.end());
+        return (args.find(tk.arg.toStdString()) != args.end());
     });
 
     return (n > 0);
 }
+
+
+bool LauncherArgumentTokenizer::havePositionalArgs() const
+{
+    return haveArgsIn(positionalArgs);
+}

src/GUI/LauncherArgumentTokenizer.hxx

@@ -1,6 +1,8 @@
 #ifndef LAUNCHERARGUMENTTOKENIZER_HXX
 #define LAUNCHERARGUMENTTOKENIZER_HXX
 
+#include <set>
+
 #include <QString>
 #include <QList>
 #include <QObject>
@@ -23,7 +25,8 @@ class LauncherArgumentTokenizer : public QObject
     Q_PROPERTY(QVariantList tokens READ tokens NOTIFY argStringChanged)
 
     Q_PROPERTY(bool valid READ isValid NOTIFY argStringChanged)
-    Q_PROPERTY(bool warnProtectedArgs READ haveProtectedArgs NOTIFY argStringChanged)
+    Q_PROPERTY(bool havePositionalArgs READ havePositionalArgs NOTIFY argStringChanged)
+    Q_PROPERTY(bool haveUnsupportedArgs READ haveUnsupportedArgs NOTIFY argStringChanged)
 
 public:
     LauncherArgumentTokenizer();
@@ -38,7 +41,8 @@ public:
 
     bool isValid() const;
 
-    bool haveProtectedArgs() const;
+    bool haveUnsupportedArgs() const;
+    bool havePositionalArgs() const;
 public slots:
     void setArgString(QString argString);
 
@@ -46,6 +50,8 @@ signals:
     void argStringChanged(QString argString);
 
 private:
+    bool haveArgsIn(const std::set<std::string> &args) const;
+
     void tokenize(QString in);
 
     enum State {

src/GUI/LocationController.cxx

@@ -943,6 +943,11 @@ void LocationController::applyPositionOffset()
 
 void LocationController::onCollectConfig()
 {
+    if (m_skipFromArgs) {
+        qInfo() << Q_FUNC_INFO << "skipping argument collection";
+        return;
+    }
+
     if (m_locationIsLatLon) {
         m_config->setArg("lat", QString::number(m_geodLocation.getLatitudeDeg()));
         m_config->setArg("lon", QString::number(m_geodLocation.getLongitudeDeg()));

src/GUI/LocationController.hxx

@@ -70,6 +70,10 @@ class LocationController : public QObject
     Q_PROPERTY(QmlPositioned* base READ baseLocation CONSTANT)
     Q_PROPERTY(QmlPositioned* detail READ detail CONSTANT)
     Q_PROPERTY(bool isBaseLatLon READ isBaseLatLon NOTIFY baseLocationChanged)
+
+    // allow collecting the location properties to be disabled, if the
+    // user is setting conflicting ones
+    Q_PROPERTY(bool skipFromArgs MEMBER m_skipFromArgs NOTIFY skipFromArgsChanged)
 public:
     explicit LocationController(QObject *parent = nullptr);
     ~LocationController();
@@ -184,6 +188,7 @@ Q_SIGNALS:
     void offsetChanged();
     void baseLocationChanged();
     void configChanged();
+    void skipFromArgsChanged();
 
 private Q_SLOTS:
     void onCollectConfig();
@@ -229,6 +234,7 @@ private:
     bool m_speedEnabled = false;
     AltitudeType m_altitudeType = Off;
     int m_flightLevel = 0;
+    bool m_skipFromArgs = false;
 };
 
 #endif // LOCATION_CONTROLLER_HXX

src/GUI/qml/SettingExtraArguments.qml

@@ -29,12 +29,22 @@ SettingControl {
         SettingDescription {
             id: warningText
             enabled: root.enabled
-            visible: tokenizer.warnProtectedArgs
+            visible: tokenizer.haveUnsupportedArgs
             width: parent.width
-            text: qsTr("<b>Warning:</b> certain arguments such as the aircraft, location and time are set directly " +
-                       "by this launcher. Attempting to set them here will <b>not</b> work as expected, " +
-                       "since the same or conflicting arguments may be set. (For exmmple, this may cause " +
-                       "you to start at the wrong position)");
+            text: qsTr("<b>Warning:</b> specifying <tt>fg-root</tt>, <tt>fg-aircraft</tt>, <tt>fg-scenery</tt> or <tt>fg-home</tt> " +
+                       "using this section is not recommended, and may cause problem or prevent the simulator from running. " +
+                       "Please use the add-ons page to setup scenery and aircrft directories, and the 'Select data files location' " +
+                       "menu item to change the root data directory.");
+        }
+
+        SettingDescription {
+            id: positionalArgsText
+            enabled: root.enabled
+            visible: tokenizer.havePositionalArgs
+            width: parent.width
+            text: qsTr("<b>Note:</b> you have entered arguments relating to the startup location below. " +
+                       "To prevent problems caused by conflicting settings, the values entered on the location " +
+                       "page (for example, airport or altitude) will be ignored.");
         }
 
         Rectangle {
@@ -73,13 +83,19 @@ SettingControl {
         for (var i = 0; i < tokens.length; i++) {
             var tk = tokens[i];
             if (tk.arg.substring(0, 5) === "prop:") {
-                _config.setProperty(tk.arg.substring(5), tk.value);
+                _config.setProperty(tk.arg.substring(5), tk.value, LaunchConfig.ExtraArgs);
             } else {
-                _config.setArg(tk.arg, tk.value);
+                _config.setArg(tk.arg, tk.value, LaunchConfig.ExtraArgs);
             }
         }
     }
 
+    Binding {
+        target: _location
+        property: "skipFromArgs"
+        value: tokenizer.havePositionalArgs
+    }
+
     ArgumentTokenizer {
         id: tokenizer
         argString: edit.text