NOISSUE do not lose selection on mod enable/disable toggle

MultiMC · Aug 4, 2019 · c291946 · c291946
1 parent dfb30d9
commit c291946
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 60 deletions.
diff --git a/api/logic/FileSystem.cpp b/api/logic/FileSystem.cpp
@@ -174,6 +174,11 @@ bool copy::operator()(const QString &offset)
 bool deletePath(QString path)
 {
     bool OK = true;
+    QFileInfo finfo(path);
+    if(finfo.isFile()) {
+        return QFile::remove(path);
+    }
+
     QDir dir(path);
 
     if (!dir.exists())

diff --git a/api/logic/minecraft/mod/Mod.cpp b/api/logic/minecraft/mod/Mod.cpp
@@ -18,6 +18,7 @@
 
 #include "Mod.h"
 #include <QDebug>
+#include <FileSystem.h>
 
 namespace {
 
@@ -100,34 +101,15 @@ bool Mod::enable(bool value)
         if (!foo.rename(path))
             return false;
     }
-    m_file = QFileInfo(path);
+    repath(QFileInfo(path));
     m_enabled = value;
     return true;
 }
 
 bool Mod::destroy()
 {
-    if (m_type == MOD_FOLDER)
-    {
-        QDir d(m_file.filePath());
-        if (d.removeRecursively())
-        {
-            m_type = MOD_UNKNOWN;
-            return true;
-        }
-        return false;
-    }
-    else if (m_type == MOD_SINGLEFILE || m_type == MOD_ZIPFILE || m_type == MOD_LITEMOD)
-    {
-        QFile f(m_file.filePath());
-        if (f.remove())
-        {
-            m_type = MOD_UNKNOWN;
-            return true;
-        }
-        return false;
-    }
-    return true;
+    m_type = MOD_UNKNOWN;
+    return FS::deletePath(m_file.filePath());
 }
 
 

diff --git a/api/logic/minecraft/mod/ModFolderModel.cpp b/api/logic/minecraft/mod/ModFolderModel.cpp
@@ -303,7 +303,7 @@ bool ModFolderModel::installMod(const QString &filename)
     return false;
 }
 
-bool ModFolderModel::enableMods(const QModelIndexList& indexes, bool enable)
+bool ModFolderModel::setModStatus(const QModelIndexList& indexes, ModStatusAction enable)
 {
     if(interaction_disabled) {
         return false;
@@ -314,27 +314,14 @@ bool ModFolderModel::enableMods(const QModelIndexList& indexes, bool enable)
 
     for (auto index: indexes)
     {
-        Mod &m = mods[index.row()];
-        m.enable(enable);
-        emit dataChanged(index, index);
+        if(index.column() != 0) {
+            continue;
+        }
+        setModStatus(index.row(), enable);
     }
     return true;
 }
 
-void ModFolderModel::toggleEnabled(const QModelIndex& index)
-{
-    if(interaction_disabled) {
-        return;
-    }
-    if(!index.isValid()) {
-        return;
-    }
-
-    Mod &m = mods[index.row()];
-    m.enable(!m.enabled());
-    emit dataChanged(index, index);
-}
-
 bool ModFolderModel::deleteMods(const QModelIndexList& indexes)
 {
     if(interaction_disabled) {
@@ -418,16 +405,52 @@ bool ModFolderModel::setData(const QModelIndex &index, const QVariant &value, in
 
     if (role == Qt::CheckStateRole)
     {
-        auto &mod = mods[index.row()];
-        if (mod.enable(!mod.enabled()))
-        {
-            emit dataChanged(index, index);
-            return true;
-        }
+        return setModStatus(index.row(), Toggle);
     }
     return false;
 }
 
+bool ModFolderModel::setModStatus(int row, ModFolderModel::ModStatusAction action)
+{
+    if(row < 0 || row >= mods.size()) {
+        return false;
+    }
+
+    auto &mod = mods[row];
+    bool desiredStatus;
+    switch(action) {
+        case Enable:
+            desiredStatus = true;
+            break;
+        case Disable:
+            desiredStatus = false;
+            break;
+        case Toggle:
+        default:
+            desiredStatus = !mod.enabled();
+            break;
+    }
+
+    if(desiredStatus == mod.enabled()) {
+        return true;
+    }
+
+    // preserve the row, but change its ID
+    auto oldId = mod.mmc_id();
+    if(!mod.enable(!mod.enabled())) {
+        return false;
+    }
+    auto newId = mod.mmc_id();
+    if(modsIndex.contains(newId)) {
+        // NOTE: this could handle a corner case, where we are overwriting a file, because the same 'mod' exists both enabled and disabled
+        // But is it necessary?
+    }
+    modsIndex.remove(oldId);
+    modsIndex[newId] = row;
+    emit dataChanged(index(row, 0), index(row, columnCount(QModelIndex()) - 1));
+    return true;
+}
+
 QVariant ModFolderModel::headerData(int section, Qt::Orientation orientation, int role) const
 {
     switch (role)

diff --git a/api/logic/minecraft/mod/ModFolderModel.h b/api/logic/minecraft/mod/ModFolderModel.h
@@ -48,6 +48,11 @@ class MULTIMC_LOGIC_EXPORT ModFolderModel : public QAbstractListModel
         DateColumn,
         NUM_COLUMNS
     };
+    enum ModStatusAction {
+        Disable,
+        Enable,
+        Toggle
+    };
     ModFolderModel(const QString &dir);
 
     virtual QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override;
@@ -93,8 +98,7 @@ class MULTIMC_LOGIC_EXPORT ModFolderModel : public QAbstractListModel
     bool deleteMods(const QModelIndexList &indexes);
 
     /// Enable or disable listed mods
-    bool enableMods(const QModelIndexList &indexes, bool enable = true);
-    void toggleEnabled(const QModelIndex &index);
+    bool setModStatus(const QModelIndexList &indexes, ModStatusAction action);
 
     void startWatching();
     void stopWatching();
@@ -125,6 +129,7 @@ private
 
 private:
     void resolveMod(Mod& m);
+    bool setModStatus(int index, ModStatusAction action);
 
 protected:
     QFileSystemWatcher *m_watcher;

diff --git a/application/pages/instance/ModFolderPage.cpp b/application/pages/instance/ModFolderPage.cpp
@@ -147,12 +147,13 @@ ModFolderPage::ModFolderPage(
     connect(m_inst, &BaseInstance::runningStatusChanged, this, &ModFolderPage::on_RunningState_changed);
 }
 
-void ModFolderPage::modItemActivated(const QModelIndex& index)
+void ModFolderPage::modItemActivated(const QModelIndex&)
 {
-    auto modsModelIndex = m_filterModel->mapToSource(index);
-    if(modsModelIndex.isValid()) {
-        m_mods->toggleEnabled(modsModelIndex);
+    if(!m_controlsEnabled) {
+        return;
     }
+    auto selection = m_filterModel->mapSelectionToSource(ui->modTreeView->selectionModel()->selection());
+    m_mods->setModStatus(selection.indexes(), ModFolderModel::Toggle);
 }
 
 QMenu * ModFolderPage::createPopupMenu()
@@ -297,7 +298,7 @@ void ModFolderPage::on_actionEnable_triggered()
         return;
     }
     auto selection = m_filterModel->mapSelectionToSource(ui->modTreeView->selectionModel()->selection());
-    m_mods->enableMods(selection.indexes(), true);
+    m_mods->setModStatus(selection.indexes(), ModFolderModel::Enable);
 }
 
 void ModFolderPage::on_actionDisable_triggered()
@@ -306,7 +307,7 @@ void ModFolderPage::on_actionDisable_triggered()
         return;
     }
     auto selection = m_filterModel->mapSelectionToSource(ui->modTreeView->selectionModel()->selection());
-    m_mods->enableMods(selection.indexes(), false);
+    m_mods->setModStatus(selection.indexes(), ModFolderModel::Disable);
 }
 
 void ModFolderPage::on_actionRemove_triggered()

diff --git a/application/pages/instance/ModFolderPage.ui b/application/pages/instance/ModFolderPage.ui
@@ -87,6 +87,7 @@
     <bool>false</bool>
    </attribute>
    <addaction name="actionAdd"/>
+   <addaction name="separator"/>
    <addaction name="actionRemove"/>
    <addaction name="actionEnable"/>
    <addaction name="actionDisable"/>
@@ -95,35 +96,47 @@
   </widget>
   <action name="actionAdd">
    <property name="text">
-    <string>Add</string>
+    <string>&amp;Add</string>
+   </property>
+   <property name="toolTip">
+    <string>Add mods</string>
    </property>
   </action>
   <action name="actionRemove">
    <property name="text">
-    <string>Remove</string>
+    <string>&amp;Remove</string>
+   </property>
+   <property name="toolTip">
+    <string>Remove selected mods</string>
    </property>
   </action>
   <action name="actionEnable">
    <property name="text">
-    <string>Enable</string>
+    <string>&amp;Enable</string>
+   </property>
+   <property name="toolTip">
+    <string>Enable selected mods</string>
    </property>
   </action>
   <action name="actionDisable">
    <property name="text">
-    <string>Disable</string>
+    <string>&amp;Disable</string>
+   </property>
+   <property name="toolTip">
+    <string>Disable selected mods</string>
    </property>
   </action>
   <action name="actionView_configs">
    <property name="text">
-    <string>View configs</string>
+    <string>View &amp;Configs</string>
    </property>
    <property name="toolTip">
     <string>Open the 'config' folder in the system file manager.</string>
    </property>
   </action>
   <action name="actionView_Folder">
    <property name="text">
-    <string>View Folder</string>
+    <string>View &amp;Folder</string>
    </property>
   </action>
  </widget>
@@ -145,6 +158,10 @@
    <header>widgets/WideBar.h</header>
   </customwidget>
  </customwidgets>
+ <tabstops>
+  <tabstop>modTreeView</tabstop>
+  <tabstop>filterEdit</tabstop>
+ </tabstops>
  <resources/>
  <connections/>
 </ui>
diff --git a/changelog.md b/changelog.md
@@ -16,6 +16,12 @@ There are some accessibility fixes thrown in too.
 
   It now also shows disabled mods, and has prefix and suffix that shows if the mod is enabled, and if it is a folder.
 
+- You can now enable and disable mods with the keyboard.
+
+  Toggle with enter.
+
+- Enabling and disabling mods no longer makes the list forget what was selected.
+
 - GH-358: Switched all the dialog pages from using buttons in layouts to toolbars.
 
   Toolbar buttons are smaller, and the toolbars can overflow buttons into an overflow space. This allows requiring a lot less space for the windows.