Abstract away InstanceCopyPrefs' internals through new getSelectedFiltersAsRegex() function

+ fix typo in comment
+ remove unused import
+ add [[nodiscard]] to methods

Signed-off-by: Marcelo Hernandez <marcelohdez.inq@gmail.com>

5 files changed, 45 insertions, 60 deletions

diff --git a/launcher/InstanceCopyPrefs.cpp b/launcher/InstanceCopyPrefs.cpp
index fad55d1e..6432a535 100644
--- a/launcher/InstanceCopyPrefs.cpp
+++ b/launcher/InstanceCopyPrefs.cpp
@@ -14,3 +14,36 @@ bool InstanceCopyPrefs::allTrue() const
         copyServers &&
         copyMods;
 }
+
+// Returns a single RegEx string of the selected folders/files to filter out (ex: ".minecraft/saves|.minecraft/server.dat")
+QString InstanceCopyPrefs::getSelectedFiltersAsRegex() const
+{
+    QStringList filters;
+
+    if(!copySaves)
+        filters << "saves";
+
+    if(!copyGameOptions)
+        filters << "options.txt";
+
+    if(!copyResourcePacks)
+        filters << "resourcepacks" << "texturepacks";
+
+    if(!copyShaderPacks)
+        filters << "shaderpacks";
+
+    if(!copyServers)
+        filters << "servers.dat" << "servers.dat_old" << "server-resource-packs";
+
+    if(!copyMods)
+        filters << "coremods" << "mods" << "config";
+
+    // If we have any filters to add, join them as a single regex string to return:
+    if (!filters.isEmpty()) {
+        const QString MC_ROOT = "[.]?minecraft/";
+        // Ensure first filter starts with root, then join other filters with OR regex before root (ex: ".minecraft/saves|.minecraft/mods"):
+        return MC_ROOT + filters.join("|" + MC_ROOT);
+    }
+
+    return {};
+}
diff --git a/launcher/InstanceCopyPrefs.h b/launcher/InstanceCopyPrefs.h
index c5c1a7ae..432d67c4 100644
--- a/launcher/InstanceCopyPrefs.h
+++ b/launcher/InstanceCopyPrefs.h
@@ -5,6 +5,8 @@
 #ifndef LAUNCHER_INSTANCECOPYPREFS_H
 #define LAUNCHER_INSTANCECOPYPREFS_H
 
+#include <QStringList>
+
 struct InstanceCopyPrefs {
     bool copySaves = true;
     bool keepPlaytime = true;
@@ -14,7 +16,8 @@ struct InstanceCopyPrefs {
     bool copyServers = true;
     bool copyMods = true;
 
-    bool allTrue() const;
+    [[nodiscard]] bool allTrue() const;
+    [[nodiscard]] QString getSelectedFiltersAsRegex() const;
 };
 
 #endif  // LAUNCHER_INSTANCECOPYPREFS_H
diff --git a/launcher/InstanceCopyTask.cpp b/launcher/InstanceCopyTask.cpp
index e0f68224..7fbf8636 100644
--- a/launcher/InstanceCopyTask.cpp
+++ b/launcher/InstanceCopyTask.cpp
@@ -9,62 +9,16 @@ InstanceCopyTask::InstanceCopyTask(InstancePtr origInstance, const InstanceCopyP
 {
     m_origInstance = origInstance;
     m_keepPlaytime = prefs.keepPlaytime;
-    QString filter;
 
-    if(!prefs.copySaves)
+    QString filters = prefs.getSelectedFiltersAsRegex();
+    if (!filters.isEmpty())
     {
-        appendToFilter(filter, "saves");
+        // Set regex filter:
+        // FIXME: get this from the original instance type...
+        auto matcherReal = new RegexpMatcher(filters);
+        matcherReal->caseSensitive(false);
+        m_matcher.reset(matcherReal);
     }
-
-    if(!prefs.copyGameOptions) {
-        appendToFilter(filter, "options.txt");
-    }
-
-    if(!prefs.copyResourcePacks)
-    {
-        appendToFilter(filter, "resourcepacks");
-        appendToFilter(filter, "texturepacks");
-    }
-
-    if(!prefs.copyShaderPacks)
-    {
-        appendToFilter(filter, "shaderpacks");
-    }
-
-    if(!prefs.copyServers)
-    {
-        appendToFilter(filter, "servers.dat");
-        appendToFilter(filter, "servers.dat_old");
-        appendToFilter(filter, "server-resource-packs");
-    }
-
-    if(!prefs.copyMods)
-    {
-        appendToFilter(filter, "coremods");
-        appendToFilter(filter, "mods");
-        appendToFilter(filter, "config");
-    }
-
-    if (!filter.isEmpty())
-    {
-        resetFromMatcher(filter);
-    }
-}
-
-void InstanceCopyTask::appendToFilter(QString& filter, const QString& append)
-{
-    if (!filter.isEmpty())
-        filter.append('|'); // OR regex
-
-    filter.append("[.]?minecraft/" + append);
-}
-
-void InstanceCopyTask::resetFromMatcher(const QString& regexp)
-{
-    // FIXME: get this from the original instance type...
-    auto matcherReal = new RegexpMatcher(regexp);
-    matcherReal->caseSensitive(false);
-    m_matcher.reset(matcherReal);
 }
 
 void InstanceCopyTask::executeTask()
diff --git a/launcher/InstanceCopyTask.h b/launcher/InstanceCopyTask.h
index 4abbf6e6..1f29b854 100644
--- a/launcher/InstanceCopyTask.h
+++ b/launcher/InstanceCopyTask.h
@@ -24,10 +24,6 @@ protected:
     void copyAborted();
 
 private:
-    // Helper functions to avoid repeating code
-    static void appendToFilter(QString &filter, const QString &append);
-    void resetFromMatcher(const QString &regexp);
-
     /* data */
     InstancePtr m_origInstance;
     QFuture<bool> m_copyFuture;
diff --git a/launcher/ui/dialogs/CopyInstanceDialog.cpp b/launcher/ui/dialogs/CopyInstanceDialog.cpp
index 8445f0a9..e658f26d 100644
--- a/launcher/ui/dialogs/CopyInstanceDialog.cpp
+++ b/launcher/ui/dialogs/CopyInstanceDialog.cpp
@@ -44,7 +44,6 @@
 
 #include "BaseVersion.h"
 #include "icons/IconList.h"
-#include "tasks/Task.h"
 #include "BaseInstance.h"
 #include "InstanceList.h"
 
@@ -138,7 +137,7 @@ void CopyInstanceDialog::checkAllCheckboxes(const bool& b)
     ui->copyModsCheckbox->setChecked(b);
 }
 
-// Check the "Select all" checkbox checked if all options are already checked:
+// Check the "Select all" checkbox if all options are already selected:
 void CopyInstanceDialog::updateSelectAllCheckbox()
 {
     ui->selectAllCheckbox->blockSignals(true);