NOISSUE set groups for instances by not setting groups for instances

So simple. Better in every way.

api/logic/BaseInstance.cpp

@@ -99,8 +99,6 @@ void BaseInstance::iconUpdated(QString key)
 void BaseInstance::invalidate()
 {
     changeStatus(Status::Gone);
-    m_group = QString();
-    emit groupChanged();
     qDebug() << "Instance" << id() << "has been invalidated.";
 }
 
@@ -209,31 +207,6 @@ void BaseInstance::setLastLaunch(qint64 val)
     emit propertiesChanged(this);
 }
 
-void BaseInstance::setGroupInitial(QString val)
-{
-    if(m_group == val)
-    {
-        return;
-    }
-    m_group = val;
-    emit propertiesChanged(this);
-}
-
-void BaseInstance::setGroupPost(QString val)
-{
-    if(m_group == val)
-    {
-        return;
-    }
-    setGroupInitial(val);
-    emit groupChanged();
-}
-
-QString BaseInstance::group() const
-{
-    return m_group;
-}
-
 void BaseInstance::setNotes(QString val)
 {
     //FIXME: if no change, do not set. setting involves saving a file.

api/logic/BaseInstance.h

@@ -68,7 +68,6 @@ public:
     /// virtual destructor to make sure the destruction is COMPLETE
     virtual ~BaseInstance() {};
 
-    virtual void init() = 0;
     virtual void saveNow() = 0;
 
     /***
@@ -112,10 +111,6 @@ public:
     QString notes() const;
     void setNotes(QString val);
 
-    QString group() const;
-    void setGroupInitial(QString val);
-    void setGroupPost(QString val);
-
     QString getPreLaunchCommand();
     QString getPostExitCommand();
     QString getWrapperCommand();
@@ -245,10 +240,6 @@ signals:
      * \brief Signal emitted when properties relevant to the instance view change
      */
     void propertiesChanged(BaseInstance *inst);
-    /*!
-     * \brief Signal emitted when groups are affected in any way
-     */
-    void groupChanged();
 
     void launchTaskChanged(std::shared_ptr<LaunchTask>);
 
@@ -261,7 +252,6 @@ protected slots:
 
 protected: /* data */
     QString m_rootDir;
-    QString m_group;
     SettingsObjectPtr m_settings;
     // InstanceFlags m_flags;
     bool m_isRunning = false;

api/logic/InstanceCreationTask.cpp

@@ -25,7 +25,6 @@ void InstanceCreationTask::executeTask()
         components->setComponentVersion("net.minecraft", m_version->descriptor(), true);
         inst.setName(m_instName);
         inst.setIconKey(m_instIcon);
-        inst.init();
         instanceSettings->resumeSave();
     }
     emitSucceeded();

api/logic/InstanceImportTask.cpp

@@ -274,7 +274,6 @@ void InstanceImportTask::processFlame()
             instance.setIconKey("flame");
         }
     }
-    instance.init();
     QString jarmodsPath = FS::PathCombine(m_stagingPath, "minecraft", "jarmods");
     QFileInfo jarmodsInfo(jarmodsPath);
     if(jarmodsInfo.isDir())

api/logic/InstanceList.cpp

@@ -51,7 +51,6 @@ InstanceList::InstanceList(SettingsObjectPtr settings, const QString & instDir,
     }
 
     connect(this, &InstanceList::instancesChanged, this, &InstanceList::providerUpdated);
-    connect(this, &InstanceList::groupsChanged, this, &InstanceList::groupsPublished);
 
     // NOTE: canonicalPath requires the path to exist. Do not move this above the creation block!
     m_instDir = QDir(instDir).canonicalPath();
@@ -111,7 +110,7 @@ QVariant InstanceList::data(const QModelIndex &index, int role) const
     // HACK: see GroupView.h in gui!
     case GroupRole:
     {
-        return pdata->group();
+        return getInstanceGroup(pdata->id());
     }
     default:
         break;
@@ -129,6 +128,53 @@ Qt::ItemFlags InstanceList::flags(const QModelIndex &index) const
     return f;
 }
 
+GroupId InstanceList::getInstanceGroup(const InstanceId& id) const
+{
+    auto inst = getInstanceById(id);
+    if(!inst)
+    {
+        return GroupId();
+    }
+    auto iter = m_groupMap.find(inst->id());
+    if(iter != m_groupMap.end())
+    {
+        return *iter;
+    }
+    return GroupId();
+}
+
+void InstanceList::setInstanceGroup(const InstanceId& id, const GroupId& name)
+{
+    auto inst = getInstanceById(id);
+    if(!inst)
+    {
+        qDebug() << "Attempt to set a null instance's group";
+        return;
+    }
+
+    bool changed = false;
+    auto iter = m_groupMap.find(inst->id());
+    if(iter != m_groupMap.end())
+    {
+        if(*iter != name)
+        {
+            *iter = name;
+            changed = true;
+        }
+    }
+    else
+    {
+        changed = true;
+        m_groupMap[id] = name;
+    }
+
+    if(changed)
+    {
+        m_groups.insert(name);
+        saveGroupList();
+    }
+}
+
 QStringList InstanceList::getGroups()
 {
     return m_groups.toList();
@@ -136,14 +182,28 @@ QStringList InstanceList::getGroups()
 
 void InstanceList::deleteGroup(const QString& name)
 {
+    bool removed = false;
+    qDebug() << "Delete group" << name;
     for(auto & instance: m_instances)
     {
-        auto instGroupName = instance->group();
+        const auto & instID = instance->id();
+        auto instGroupName = getInstanceGroup(instID);
         if(instGroupName == name)
         {
-            instance->setGroupPost(QString());
+            m_groupMap.remove(instID);
+            qDebug() << "Remove" << instID << "from group" << name;
+            removed = true;
+            auto idx = getInstIndex(instance.get());
+            if(idx > 0)
+            {
+                emit dataChanged(index(idx), index(idx), {GroupRole});
+            }
         }
     }
+    if(removed)
+    {
+        saveGroupList();
+    }
 }
 
 void InstanceList::deleteInstance(const InstanceId& id)
@@ -155,6 +215,11 @@ void InstanceList::deleteInstance(const InstanceId& id)
         return;
     }
 
+    if(m_groupMap.remove(id))
+    {
+        saveGroupList();
+    }
+
     qDebug() << "Will delete instance" << id;
     if(!FS::deletePath(inst->instanceRoot()))
     {
@@ -340,11 +405,6 @@ void InstanceList::providerUpdated()
     }
 }
 
-void InstanceList::groupsPublished(QSet<QString> newGroups)
-{
-    m_groups.unite(newGroups);
-}
-
 InstancePtr InstanceList::getInstanceById(QString instId) const
 {
     if(instId.isEmpty())
@@ -413,13 +473,6 @@ InstancePtr InstanceList::loadInstance(const InstanceId& id)
     {
         inst.reset(new NullInstance(m_globalSettings, instanceSettings, instanceRoot));
     }
-    inst->init();
-    auto iter = groupMap.find(id);
-    if (iter != groupMap.end())
-    {
-        inst->setGroupInitial((*iter));
-    }
-    connect(inst.get(), &BaseInstance::groupChanged, this, &InstanceList::groupChanged);
     qDebug() << "Loaded instance " << inst->name() << " from " << inst->instanceRoot();
     return inst;
 }
@@ -435,7 +488,7 @@ void InstanceList::saveGroupList()
     WatchLock foo(m_watcher, m_instDir);
     QString groupFileName = m_instDir + "/instgroups.json";
     QMap<QString, QSet<QString>> reverseGroupMap;
-    for (auto iter = groupMap.begin(); iter != groupMap.end(); iter++)
+    for (auto iter = m_groupMap.begin(); iter != m_groupMap.end(); iter++)
     {
         QString id = iter.key();
         QString group = iter.value();
@@ -543,7 +596,7 @@ void InstanceList::loadGroupList()
         return;
     }
 
-    groupMap.clear();
+    m_groupMap.clear();
 
     // Iterate through all the groups.
     QJsonObject groupMapping = rootObj.value("groups").toObject();
@@ -580,25 +633,14 @@ void InstanceList::loadGroupList()
 
         for (QJsonArray::iterator iter2 = instancesArray.begin(); iter2 != instancesArray.end(); iter2++)
         {
-            groupMap[(*iter2).toString()] = groupName;
+            m_groupMap[(*iter2).toString()] = groupName;
         }
     }
     m_groupsLoaded = true;
-    emit groupsChanged(groupSet);
+    m_groups.unite(groupSet);
     qDebug() << "Group list loaded.";
 }
 
-void InstanceList::groupChanged()
-{
-    // save the groups. save all of them.
-    auto instance = (BaseInstance *) QObject::sender();
-    auto id = instance->id();
-    groupMap[id] = instance->group();
-    emit groupsChanged({instance->group()});
-    saveGroupList();
-}
-
-
 void InstanceList::instanceDirContentsChanged(const QString& path)
 {
     Q_UNUSED(path);
@@ -731,9 +773,9 @@ bool InstanceList::commitStagedInstance(const QString& path, const QString& inst
             qWarning() << "Failed to move" << path << "to" << destination;
             return false;
         }
-        groupMap[instID] = groupName;
+        m_groupMap[instID] = groupName;
         instanceSet.insert(instID);
-        emit groupsChanged({groupName});
+        m_groups.insert(groupName);
         emit instancesChanged();
     }
     saveGroupList();

api/logic/InstanceList.h

@@ -41,6 +41,13 @@ enum class InstCreateError
     CantCreateDir
 };
 
+enum class GroupsState
+{
+    NotLoaded,
+    Steady,
+    Dirty
+};
+
 
 class MULTIMC_LOGIC_EXPORT InstanceList : public QAbstractListModel
 {
@@ -90,6 +97,8 @@ public:
     InstancePtr getInstanceById(QString id) const;
     QModelIndex getInstanceIndexById(const QString &id) const;
     QStringList getGroups();
+    GroupId getInstanceGroup(const InstanceId & id) const;
+    void setInstanceGroup(const InstanceId & id, const GroupId& name);
 
     void deleteGroup(const GroupId & name);
     void deleteInstance(const InstanceId & id);
@@ -125,10 +134,8 @@ public slots:
 
 private slots:
     void propertiesChanged(BaseInstance *inst);
-    void groupsPublished(QSet<QString>);
     void providerUpdated();
     void instanceDirContentsChanged(const QString &path);
-    void groupChanged();
 
 private:
     int getInstIndex(BaseInstance *inst) const;
@@ -149,7 +156,7 @@ private:
     SettingsObjectPtr m_globalSettings;
     QString m_instDir;
     QFileSystemWatcher * m_watcher;
-    QMap<InstanceId, GroupId> groupMap;
+    QMap<InstanceId, GroupId> m_groupMap;
     QSet<InstanceId> instanceSet;
     bool m_groupsLoaded = false;
     bool m_instancesProbed = false;

api/logic/NullInstance.h

@@ -10,9 +10,6 @@ public:
         setVersionBroken(true);
     }
     virtual ~NullInstance() {};
-    virtual void init() override
-    {
-    }
     virtual void saveNow() override
     {
     }

api/logic/minecraft/MinecraftInstance.cpp

@@ -112,10 +112,6 @@ MinecraftInstance::MinecraftInstance(SettingsObjectPtr globalSettings, SettingsO
     m_components->setOldConfigVersion("com.mumfrey.liteloader", m_settings->get("LiteloaderVersion").toString());
 }
 
-void MinecraftInstance::init()
-{
-}
-
 void MinecraftInstance::saveNow()
 {
     m_components->saveNow();

api/logic/minecraft/MinecraftInstance.h

@@ -18,7 +18,6 @@ class MULTIMC_LOGIC_EXPORT MinecraftInstance: public BaseInstance
 public:
     MinecraftInstance(SettingsObjectPtr globalSettings, SettingsObjectPtr settings, const QString &rootDir);
     virtual ~MinecraftInstance() {};
-    virtual void init() override;
     virtual void saveNow() override;
 
     // FIXME: remove

api/logic/minecraft/legacy/LegacyInstance.h

@@ -34,7 +34,6 @@ public:
 
     explicit LegacyInstance(SettingsObjectPtr globalSettings, SettingsObjectPtr settings, const QString &rootDir);
 
-    virtual void init() override {}
     virtual void saveNow() override {}
 
     /// Path to the instance's minecraft.jar

api/logic/minecraft/legacy/LegacyUpgradeTask.cpp

@@ -67,7 +67,6 @@ void LegacyUpgradeTask::copyFinished()
     {
         MinecraftInstance inst(m_globalSettings, instanceSettings, m_stagingPath);
         inst.setName(m_instName);
-        inst.init();
 
         QString preferredVersionNumber = decideVersion(legacyInst->currentVersionId(), legacyInst->intendedVersionId());
         if(preferredVersionNumber.isNull())

api/logic/modplatform/ftb/FtbPackInstallTask.cpp

@@ -175,14 +175,12 @@ void FtbPackInstallTask::install()
 
     progress(4, 4);
 
-    instance.init();
     instance.setName(m_instName);
     if(m_instIcon == "default")
     {
         m_instIcon = "ftb_logo";
     }
     instance.setIconKey(m_instIcon);
-    instance.setGroupInitial(m_instGroup);
     instanceSettings->resumeSave();
 
     emitSucceeded();

application/MainWindow.cpp

@@ -1446,7 +1446,8 @@ void MainWindow::on_actionChangeInstGroup_triggered()
         return;
 
     bool ok = false;
-    QString name(m_selectedInstance->group());
+    InstanceId instId = m_selectedInstance->id();
+    QString name(MMC->instances()->getInstanceGroup(instId));
     auto groups = MMC->instances()->getGroups();
     groups.insert(0, "");
     groups.sort(Qt::CaseInsensitive);
@@ -1455,7 +1456,9 @@ void MainWindow::on_actionChangeInstGroup_triggered()
     name = QInputDialog::getItem(this, tr("Group name"), tr("Enter a new group name."), groups, foo, true, &ok);
     name = name.simplified();
     if (ok)
-        m_selectedInstance->setGroupPost(name);
+    {
+        MMC->instances()->setInstanceGroup(instId, name);
+    }
 }
 
 void MainWindow::deleteGroup()

application/dialogs/CopyInstanceDialog.cpp

@@ -45,7 +45,7 @@ CopyInstanceDialog::CopyInstanceDialog(InstancePtr original, QWidget *parent)
     groupList.removeOne("");
     groupList.push_front("");
     ui->groupBox->addItems(groupList);
-    int index = groupList.indexOf(m_original->group());
+    int index = groupList.indexOf(MMC->instances()->getInstanceGroup(m_original->id()));
     if(index == -1)
     {
         index = 0;

application/pages/instance/LegacyUpgradePage.cpp

@@ -38,7 +38,7 @@ void LegacyUpgradePage::on_upgradeButton_clicked()
     QString newName = tr("%1 (Migrated)").arg(m_inst->name());
     auto upgradeTask = new LegacyUpgradeTask(m_inst);
     upgradeTask->setName(newName);
-    upgradeTask->setGroup(m_inst->group());
+    upgradeTask->setGroup(MMC->instances()->getInstanceGroup(m_inst->id()));
     upgradeTask->setIcon(m_inst->iconKey());
     std::unique_ptr<Task> task(MMC->instances()->wrapInstanceTask(upgradeTask));
     runModalTask(task.get());