Launcher warns when setting conflicting args

Display a warning when the user tries to set arguments in additional
settings which the launcher will also set / conflict with. Blacklist
is still evolving, and we don’t actually prevent the user from running,
since maybe they are doing something special

src/GUI/LauncherArgumentTokenizer.cxx

@@ -1,5 +1,8 @@
 #include "LauncherArgumentTokenizer.hxx"
 
+#include <algorithm>
+#include <set>
+
 #include <QVariantMap>
 #include <QJSEngine>
 
@@ -8,14 +11,17 @@ LauncherArgumentTokenizer::LauncherArgumentTokenizer()
 
 }
 
-QList<ArgumentToken> LauncherArgumentTokenizer::tokenize(QString in) const
+void LauncherArgumentTokenizer::tokenize(QString in)
 {
+    m_tokens.clear();
+    m_valid = false;
+
     int index = 0;
     const int len = in.count();
     QChar c, nc;
     State state = Start;
     QString key, value;
-    QList<ArgumentToken> result;
+    std::vector<ArgumentToken> result;
 
     for (; index < len; ++index) {
         c = in.at(index);
@@ -31,7 +37,7 @@ QList<ArgumentToken> LauncherArgumentTokenizer::tokenize(QString in) const
                 } else {
                     // should we pemit single hyphen arguments?
                     // choosing to fail for now
-                    return {};
+                    return;
                 }
             } else if (c == QChar('#')) {
                 state = Comment;
@@ -47,7 +53,7 @@ QList<ArgumentToken> LauncherArgumentTokenizer::tokenize(QString in) const
                 value.clear();
             } else if (c.isSpace()) {
                 state = Start;
-                result.append(ArgumentToken{key});
+                result.emplace_back(key);
             } else {
                 // could check for illegal charatcers here
                 key.append(c);
@@ -59,7 +65,7 @@ QList<ArgumentToken> LauncherArgumentTokenizer::tokenize(QString in) const
                 state = Quoted;
             } else if (c.isSpace()) {
                 state = Start;
-                result.append(ArgumentToken{key, value});
+                result.emplace_back(key, value);
             } else {
                 value.append(c);
             }
@@ -93,18 +99,22 @@ QList<ArgumentToken> LauncherArgumentTokenizer::tokenize(QString in) const
 
     // ensure last argument isn't lost
     if (state == Key) {
-        result.append(ArgumentToken{key});
+        result.emplace_back(key);
     } else if (state == Value) {
-        result.append(ArgumentToken{key, value});
+        result.emplace_back(key, value);
     }
 
-    return result;
+    m_valid = true;
+    m_tokens = result;
 }
 
 QVariantList LauncherArgumentTokenizer::tokens() const
 {
+    if (!m_valid)
+        return {};
+
     QVariantList result;
-    Q_FOREACH(auto tk, tokenize(m_argString)) {
+    for (auto tk : m_tokens) {
         QVariantMap m;
         m["arg"] = tk.arg;
         m["value"] = tk.value;
@@ -113,11 +123,35 @@ QVariantList LauncherArgumentTokenizer::tokens() const
     return result;
 }
 
+bool LauncherArgumentTokenizer::isValid() const
+{
+    return m_valid;
+}
+
 void LauncherArgumentTokenizer::setArgString(QString argString)
 {
     if (m_argString == argString)
         return;
 
     m_argString = argString;
+    tokenize(m_argString);
     emit argStringChanged(m_argString);
 }
+
+const std::set<std::string> argBlacklist({
+    "lat", "lon", "aircraft", "airport", "parkpos", "season",
+    "runway", "vor", "time-offset", "timeofday"});
+
+bool LauncherArgumentTokenizer::haveProtectedArgs() const
+{
+    if (!m_valid)
+        return false;
+
+    auto n = std::count_if(m_tokens.begin(), m_tokens.end(),
+                           [](const ArgumentToken& tk)
+    {
+        return (argBlacklist.find(tk.arg.toStdString()) != argBlacklist.end());
+    });
+
+    return (n > 0);
+}

src/GUI/LauncherArgumentTokenizer.hxx

@@ -21,12 +21,14 @@ class LauncherArgumentTokenizer : public QObject
 
     Q_PROPERTY(QString argString READ argString WRITE setArgString NOTIFY argStringChanged)
     Q_PROPERTY(QVariantList tokens READ tokens NOTIFY argStringChanged)
+
+    Q_PROPERTY(bool valid READ isValid NOTIFY argStringChanged)
+    Q_PROPERTY(bool warnProtectedArgs READ haveProtectedArgs NOTIFY argStringChanged)
+
 public:
     LauncherArgumentTokenizer();
 
 
-    Q_INVOKABLE QList<ArgumentToken> tokenize(QString in) const;
-
     QString argString() const
     {
         return m_argString;
@@ -34,6 +36,9 @@ public:
 
     QVariantList tokens() const;
 
+    bool isValid() const;
+
+    bool haveProtectedArgs() const;
 public slots:
     void setArgString(QString argString);
 
@@ -41,6 +46,8 @@ signals:
     void argStringChanged(QString argString);
 
 private:
+    void tokenize(QString in);
+
     enum State {
         Start = 0,
         Key,
@@ -49,7 +56,9 @@ private:
         Comment
     };
 
+    std::vector<ArgumentToken> m_tokens;
     QString m_argString;
+    bool m_valid = false;
 };
 
 #endif // LAUNCHERARGUMENTTOKENIZER_HXX

src/GUI/qml/SettingExtraArguments.qml

@@ -4,56 +4,67 @@ import "."
 
 SettingControl {
     id: root
-    implicitHeight: childrenRect.height
+    implicitHeight: col.height + Style.margin * 2
     readonly property string placeholder: "--option=value --prop:/sim/name=value"
     property alias value: edit.text
     option: "xxx" // non-empty value so apply() is called
     property string defaultValue: "" // needed to type save/restore logic to string
 
-    SettingDescription {
+    Column
-        id: description
+    {
-        enabled: root.enabled
+        id: col
-        anchors.top: parent.top
+        y: Style.margin
-        anchors.topMargin: Style.margin
         width: parent.width
-        text: qsTr("Enter additional command-line arguments if any are required. " +
+        spacing: Style.margin
-                   "See <a href=\"http://flightgear.sourceforge.net/getstart-en/getstart-enpa2.html#x5-450004.5\">here</a> " +
-                   "for documentation on possible arguments.");
-    }
 
-    Rectangle {
+        SettingDescription {
-        id: editFrame
+            id: description
-
-        anchors.left: parent.left
-        anchors.margins: Style.margin
-        anchors.right: parent.right
-        anchors.top: description.bottom
-
-        height: edit.height + Style.margin
-
-        border.color: edit.activeFocus ? Style.frameColor : Style.minorFrameColor
-        border.width: 1
-
-        TextEdit {
-            id: edit
             enabled: root.enabled
-            anchors.verticalCenter: parent.verticalCenter
+            text: qsTr("Enter additional command-line arguments if any are required. " +
-            anchors.left: parent.left
+                       "See <a href=\"http://flightgear.sourceforge.net/getstart-en/getstart-enpa2.html#x5-450004.5\">here</a> " +
-            anchors.right: parent.right
+                       "for documentation on possible arguments.");
-            anchors.margins: Style.margin
+            width: parent.width
-            height: Math.max(Style.strutSize * 4, implicitHeight)
+        }
-            textFormat: TextEdit.PlainText
-            font.family: "Courier"
-            selectByMouse: true
-            wrapMode: TextEdit.WordWrap
 
-            Text {
+        SettingDescription {
-                id: placeholder
+            id: warningText
-                visible: (edit.text.length == 0) && !edit.activeFocus
+            enabled: root.enabled
-                text: root.placeholder
+            visible: tokenizer.warnProtectedArgs
-                color: Style.baseTextColor
+            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)");
+        }
+
+        Rectangle {
+            id: editFrame
+            height: edit.height + Style.margin
+            border.color: edit.activeFocus ? Style.frameColor : Style.minorFrameColor
+            border.width: 1
+            width: parent.width
+
+            TextEdit {
+                id: edit
+                enabled: root.enabled
+                anchors.verticalCenter: parent.verticalCenter
+                anchors.left: parent.left
+                anchors.right: parent.right
+                anchors.margins: Style.margin
+                height: Math.max(Style.strutSize * 4, implicitHeight)
+                textFormat: TextEdit.PlainText
+                font.family: "Courier"
+                selectByMouse: true
+                wrapMode: TextEdit.WordWrap
+
+                Text {
+                    id: placeholder
+                    visible: (edit.text.length == 0) && !edit.activeFocus
+                    text: root.placeholder
+                    color: Style.baseTextColor
+                }
+           }
+        }
     }
 
     function apply()
@@ -61,7 +72,7 @@ SettingControl {
         var tokens = tokenizer.tokens;
         for (var i = 0; i < tokens.length; i++) {
             var tk = tokens[i];
-            if (tk.arg.substring(0, 5) == "prop:") {
+            if (tk.arg.substring(0, 5) === "prop:") {
                 _config.setProperty(tk.arg.substring(5), tk.value);
             } else {
                 _config.setArg(tk.arg, tk.value);
@@ -73,5 +84,4 @@ SettingControl {
         id: tokenizer
         argString: edit.text
     }
-
 }