fixed EditExecutablesDialog being opened without a parent

removed Executable's constructor with values, replaced with default ctor + setters, all these strings were much too error-prone
added Executable ctor overload to convert from ExecutableInfo
plugin executables now override most of the custom changes
renamed browseButton to browseBinaryButton to avoid confusion with the other browseDirButton
fixed both browse dialogs not handling cancel
EditExecutablesDialog's list used to change the text color for custom executables, replaced with italics

6 files changed, 131 insertions, 57 deletions

diff --git a/src/editexecutablesdialog.cpp b/src/editexecutablesdialog.cpp
index 83756ac8..a3b2808a 100644
--- a/src/editexecutablesdialog.cpp
+++ b/src/editexecutablesdialog.cpp
@@ -81,7 +81,14 @@ void EditExecutablesDialog::refreshExecutablesWidget()
 
   for(const auto& exe : m_ExecutablesList) {
     QListWidgetItem *newItem = new QListWidgetItem(exe.title());
-    newItem->setTextColor(exe.isCustom() ? QColor(Qt::black) : QColor(Qt::darkGray));
+
+    if (!exe.isCustom()) {
+      auto f = newItem->font();
+      f.setItalic(true);
+
+      newItem->setFont(f);
+    }
+
     ui->executablesListBox->addItem(newItem);
   }
 
@@ -142,13 +149,13 @@ void EditExecutablesDialog::saveExecutable()
   if (ui->useAppIconCheckBox->isChecked())
     flags |= Executable::UseApplicationIcon;
 
-  m_ExecutablesList.setExecutable({
-    ui->titleEdit->text(),
-    QDir::fromNativeSeparators(ui->binaryEdit->text()),
-    ui->argumentsEdit->text(),
-    QDir::fromNativeSeparators(ui->workingDirEdit->text()),
-    ui->overwriteAppIDBox->isChecked() ? ui->appIDOverwriteEdit->text() : "",
-    flags});
+  m_ExecutablesList.setExecutable(Executable()
+    .title(ui->titleEdit->text())
+    .binaryInfo(QDir::fromNativeSeparators(ui->binaryEdit->text()))
+    .arguments(ui->argumentsEdit->text())
+    .steamAppID(ui->overwriteAppIDBox->isChecked() ? ui->appIDOverwriteEdit->text() : "")
+    .workingDirectory(QDir::fromNativeSeparators(ui->workingDirEdit->text()))
+    .flags(flags));
 
   if (ui->newFilesModCheckBox->isChecked()) {
     m_Profile->storeSetting("custom_overwrites", ui->titleEdit->text(),
@@ -198,12 +205,17 @@ void EditExecutablesDialog::on_addButton_clicked()
   refreshExecutablesWidget();
 }
 
-void EditExecutablesDialog::on_browseButton_clicked()
+void EditExecutablesDialog::on_browseBinaryButton_clicked()
 {
   QString binaryName = FileDialogMemory::getOpenFileName(
       "editExecutableBinary", this, tr("Select a binary"), QString(),
       tr("Executable (%1)").arg("*.exe *.bat *.jar"));
 
+  if (binaryName.isNull()) {
+    // canceled
+    return;
+  }
+
   if (binaryName.endsWith(".jar", Qt::CaseInsensitive)) {
     QString binaryPath;
     { // try to find java automatically
@@ -250,6 +262,11 @@ void EditExecutablesDialog::on_browseDirButton_clicked()
   QString dirName = FileDialogMemory::getExistingDirectory("editExecutableDirectory", this,
                                                            tr("Select a directory"));
 
+  if (dirName.isNull()) {
+    // canceled
+    return;
+  }
+
   ui->workingDirEdit->setText(dirName);
 }
 
diff --git a/src/editexecutablesdialog.h b/src/editexecutablesdialog.h
index bee3cba6..3a856afa 100644
--- a/src/editexecutablesdialog.h
+++ b/src/editexecutablesdialog.h
@@ -78,7 +78,7 @@ private slots:
 

   void on_addButton_clicked();

 

-  void on_browseButton_clicked();

+  void on_browseBinaryButton_clicked();

 

   void on_removeButton_clicked();

 

diff --git a/src/editexecutablesdialog.ui b/src/editexecutablesdialog.ui
index 4f9223d5..adda050c 100644
--- a/src/editexecutablesdialog.ui
+++ b/src/editexecutablesdialog.ui
@@ -80,7 +80,7 @@
       </widget>

      </item>

      <item>

-      <widget class="QPushButton" name="browseButton">

+      <widget class="QPushButton" name="browseBinaryButton">

        <property name="toolTip">

         <string>Browse filesystem</string>

        </property>

@@ -310,7 +310,7 @@ Right now the only case I know of where this needs to be overwritten is for the
   <tabstop>executablesListBox</tabstop>

   <tabstop>titleEdit</tabstop>

   <tabstop>binaryEdit</tabstop>

-  <tabstop>browseButton</tabstop>

+  <tabstop>browseBinaryButton</tabstop>

   <tabstop>workingDirEdit</tabstop>

   <tabstop>browseDirButton</tabstop>

   <tabstop>argumentsEdit</tabstop>

diff --git a/src/executableslist.cpp b/src/executableslist.cpp
index 9c291708..d884bdf2 100644
--- a/src/executableslist.cpp
+++ b/src/executableslist.cpp
@@ -81,13 +81,13 @@ void ExecutablesList::load(const MOBase::IPluginGame* game, QSettings& settings)
     if (settings.value("ownicon", false).toBool())

       flags |= Executable::UseApplicationIcon;

 

-    setExecutable({

-      settings.value("title").toString(),

-      settings.value("binary").toString(),

-      settings.value("arguments").toString(),

-      settings.value("workingDirectory", "").toString(),

-      settings.value("steamAppID", "").toString(),

-      flags});

+    setExecutable(Executable()

+      .title(settings.value("title").toString())

+      .binaryInfo(settings.value("binary").toString())

+      .arguments(settings.value("arguments").toString())

+      .steamAppID(settings.value("steamAppID", "").toString())

+      .workingDirectory(settings.value("workingDirectory", "").toString())

+      .flags(flags));

   }

 

   settings.endArray();

@@ -125,27 +125,22 @@ void ExecutablesList::addFromPlugin(IPluginGame const *game)
 

   for (const ExecutableInfo &info : game->executables()) {

     if (info.isValid()) {

-      setExecutable({

-        info.title(),

-        info.binary().absoluteFilePath(),

-        info.arguments().join(" "),

-        info.workingDirectory().absolutePath(),

-        info.steamAppID(),

-        Executable::UseApplicationIcon});

+      setExecutable({info, Executable::UseApplicationIcon});

     }

   }

 

-  ExecutableInfo explorerpp = ExecutableInfo("Explore Virtual Folder", QFileInfo(QCoreApplication::applicationDirPath() + "/explorer++/Explorer++.exe" ))

-      .withArgument(QString("\"%1\"").arg(QDir::toNativeSeparators(game->dataDirectory().absolutePath())));

+  const QFileInfo eppBin(QCoreApplication::applicationDirPath() + "/explorer++/Explorer++.exe");

 

-  if (explorerpp.isValid()) {

-    setExecutable({

-      explorerpp.title(),

-      explorerpp.binary().absoluteFilePath(),

-      explorerpp.arguments().join(" "),

-      explorerpp.workingDirectory().absolutePath(),

-      explorerpp.steamAppID(),

-      Executable::UseApplicationIcon});

+  if (eppBin.exists()) {

+    const auto args = QString("\"%1\"")

+      .arg(QDir::toNativeSeparators(game->dataDirectory().absolutePath()));

+

+    setExecutable(Executable()

+      .title("Explore Virtual Folder")

+      .binaryInfo(eppBin)

+      .arguments(args)

+      .workingDirectory(eppBin.absolutePath())

+      .flags(Executable::UseApplicationIcon));

   }

 }

 

@@ -213,15 +208,13 @@ void ExecutablesList::remove(const QString &title)
 }

 

 

-Executable::Executable(

-  QString title, QFileInfo binaryInfo, QString arguments,

-  QString steamAppID, QString workingDirectory, Flags flags) :

-    m_title(std::move(title)),

-    m_binaryInfo(std::move(binaryInfo)),

-    m_arguments(std::move(arguments)),

-    m_steamAppID(std::move(steamAppID)),

-    m_workingDirectory(std::move(workingDirectory)),

-    m_flags(flags)

+Executable::Executable(const MOBase::ExecutableInfo& info, Flags flags) :

+  m_title(info.title()),

+  m_binaryInfo(info.binary()),

+  m_arguments(info.arguments().join(" ")),

+  m_steamAppID(info.steamAppID()),

+  m_workingDirectory(info.workingDirectory().absolutePath()),

+  m_flags(flags)

 {

 }

 

@@ -255,6 +248,42 @@ Executable::Flags Executable::flags() const
   return m_flags;

 }

 

+Executable& Executable::title(const QString& s)

+{

+  m_title = s;

+  return *this;

+}

+

+Executable& Executable::binaryInfo(const QFileInfo& fi)

+{

+  m_binaryInfo = fi;

+  return *this;

+}

+

+Executable& Executable::arguments(const QString& s)

+{

+  m_arguments = s;

+  return *this;

+}

+

+Executable& Executable::steamAppID(const QString& s)

+{

+  m_steamAppID = s;

+  return *this;

+}

+

+Executable& Executable::workingDirectory(const QString& s)

+{

+  m_workingDirectory = s;

+  return *this;

+}

+

+Executable& Executable::flags(Flags f)

+{

+  m_flags = f;

+  return *this;

+}

+

 bool Executable::isCustom() const

 {

   return m_flags.testFlag(CustomExecutable);

@@ -281,12 +310,25 @@ bool Executable::usesOwnIcon() const
 

 void Executable::mergeFrom(const Executable& other)

 {

-  if (!isCustom()) {

-    // this happens when the user is trying to modify a plugin executable

+  // flags on plugin executables that the user is allowed to chnage

+  const auto allow = ShowInToolbar;

+

+

+  if (!isCustom() && !other.isCustom()) {

+    // this happens when loading plugin executables in addFromPlugin(), replace

+    // everything in case the plugin has changed

 

-    // only change some of the flags

-    const auto allow = ShowInToolbar;

+    // remember the flags though

+    const auto flags = m_flags;

 

+    // overwrite everything

+    *this = other;

+

+    // set the user flags

+    m_flags |= (flags & allow);

+  }

+  else if (!isCustom()) {

+    // this happens when the user is trying to modify a plugin executable

     m_flags |= (other.flags() & allow);

   } else {

     // this happens after executables are loaded from settings and plugin

diff --git a/src/executableslist.h b/src/executableslist.h
index e35cb550..54a105d0 100644
--- a/src/executableslist.h
+++ b/src/executableslist.h
@@ -27,7 +27,7 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 #include <QFileInfo>

 #include <QMetaType>

 

-namespace MOBase { class IPluginGame; }

+namespace MOBase { class IPluginGame; class ExecutableInfo; }

 

 /*!

  * @brief Information about an executable

@@ -44,9 +44,12 @@ public:
 

   Q_DECLARE_FLAGS(Flags, Flag);

 

-  Executable(

-    QString title, QFileInfo binaryInfo, QString arguments,

-    QString steamAppID, QString workingDirectory, Flags flags);

+  Executable() = default;

+

+  /**

+  * @brief Executable from plugin

+  */

+  Executable(const MOBase::ExecutableInfo& info, Flags flags);

 

   const QString& title() const;

   const QFileInfo& binaryInfo() const;

@@ -55,6 +58,13 @@ public:
   const QString& workingDirectory() const;

   Flags flags() const;

 

+  Executable& title(const QString& s);

+  Executable& binaryInfo(const QFileInfo& fi);

+  Executable& arguments(const QString& s);

+  Executable& steamAppID(const QString& s);

+  Executable& workingDirectory(const QString& s);

+  Executable& flags(Flags f);

+

   bool isCustom() const;

   bool isShownOnToolbar() const;

   void setShownOnToolbar(bool state);

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 926ba43c..267a2971 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -2483,7 +2483,9 @@ bool MainWindow::modifyExecutablesDialog()
     EditExecutablesDialog dialog(*m_OrganizerCore.executablesList(),
                                  *m_OrganizerCore.modList(),
                                  m_OrganizerCore.currentProfile(),
-                                 m_OrganizerCore.managedGame());
+                                 m_OrganizerCore.managedGame(),
+                                 this);
+
     QSettings &settings = m_OrganizerCore.settings().directInterface();
     QString key = QString("geometry/%1").arg(dialog.objectName());
     if (settings.contains(key)) {
@@ -5507,9 +5509,12 @@ void MainWindow::addAsExecutable()
 
         if (!name.isEmpty()) {
           //Note: If this already exists, you'll lose custom settings
-          m_OrganizerCore.executablesList()->setExecutable({
-            name, binaryInfo, arguments, targetInfo.absolutePath(), QString(),
-            Executable::CustomExecutable});
+          m_OrganizerCore.executablesList()->setExecutable(Executable()
+            .title(name)
+            .binaryInfo(binaryInfo)
+            .arguments(arguments)
+            .workingDirectory(targetInfo.absolutePath())
+            .flags(Executable::CustomExecutable));
 
           refreshExecutablesList();
         }