fix: Add Forcefield Wizard Update (#1629)

src/gui/importSpeciesDialog.cpp

@@ -22,21 +22,21 @@ ImportSpeciesDialog::ImportSpeciesDialog(QWidget *parent, Dissolve &dissolve)
             SLOT(speciesSelectionChanged(const QItemSelection &, const QItemSelection &)));
 
     // Set model, signals, and lambdas for atom types list
-    atomTypesModel_.setIconFunction([&](const std::shared_ptr<AtomType> &atomType)
-                                    { return dissolve_.coreData().findAtomType(atomType->name()) != nullptr; });
+    atomTypesModel_.setQueryFunction([&](const std::shared_ptr<AtomType> &atomType)
+                                     { return dissolve_.coreData().findAtomType(atomType->name()) != nullptr; });
     ui_.AtomTypesList->setModel(&atomTypesModel_);
     connect(ui_.AtomTypesList->selectionModel(), SIGNAL(selectionChanged(const QItemSelection &, const QItemSelection &)), this,
             SLOT(atomTypeSelectionChanged(const QItemSelection &, const QItemSelection &)));
 
     // Set model and signals for the master terms tree
-    masterTermModel_.setBondIconFunction([&](std::string_view name)
-                                         { return dissolve_.coreData().getMasterBond(name).has_value(); });
-    masterTermModel_.setAngleIconFunction([&](std::string_view name)
-                                          { return dissolve_.coreData().getMasterAngle(name).has_value(); });
-    masterTermModel_.setTorsionIconFunction([&](std::string_view name)
-                                            { return dissolve_.coreData().getMasterTorsion(name).has_value(); });
-    masterTermModel_.setImproperIconFunction([&](std::string_view name)
-                                             { return dissolve_.coreData().getMasterImproper(name).has_value(); });
+    masterTermModel_.setBondQueryFunction([&](std::string_view name)
+                                          { return dissolve_.coreData().getMasterBond(name).has_value(); });
+    masterTermModel_.setAngleQueryFunction([&](std::string_view name)
+                                           { return dissolve_.coreData().getMasterAngle(name).has_value(); });
+    masterTermModel_.setTorsionQueryFunction([&](std::string_view name)
+                                             { return dissolve_.coreData().getMasterTorsion(name).has_value(); });
+    masterTermModel_.setImproperQueryFunction([&](std::string_view name)
+                                              { return dissolve_.coreData().getMasterImproper(name).has_value(); });
     ui_.MasterTermsTree->setModel(&masterTermModel_);
     connect(&masterTermModel_, SIGNAL(dataChanged(const QModelIndex &, const QModelIndex &, const QVector<int> &)), this,
             SLOT(masterTermDataChanged(const QModelIndex &, const QModelIndex &)));

src/gui/models/addForcefieldDialogModel.cpp

@@ -53,7 +53,8 @@ void AddForcefieldDialogModel::next()
             Q_EMIT assignErrors({});
             break;
         case AddForcefieldDialogModel::Page::AtomTypesPage:
-            if (!ff_) // No valud forcefield
+            // Must have a valid forcefield
+            if (!ff_)
                 return;
             modifiedSpecies_ = temporaryDissolve_->coreData().addSpecies();
             modifiedSpecies_->copyBasic(species_);
@@ -104,6 +105,7 @@ void AddForcefieldDialogModel::next()
             index_ = AddForcefieldDialogModel::Page::IntramolecularPage;
             break;
         case AddForcefieldDialogModel::Page::IntramolecularPage:
+            assignIntramolecularTerms(ff_);
             Q_EMIT mastersChanged();
             index_ = AddForcefieldDialogModel::Page::MasterTermsPage;
             break;
@@ -122,20 +124,19 @@ void AddForcefieldDialogModel::setDissolve(Dissolve &dissolve)
     dissolve_ = &dissolve;
 
     temporaryDissolve_ = std::make_unique<Dissolve>(temporaryCoreData_);
-    auto node = dissolve_->coreData().serialiseMaster();
-    temporaryCoreData_.deserialiseMaster(node);
-    masters_ = std::make_unique<MasterTermTreeModel>(dissolve_->coreData());
+    masters_ = std::make_unique<MasterTermTreeModel>(temporaryCoreData_);
 
     // Set model and signals for the master terms tree
-    atomTypes_.setIconFunction([this](const auto type) { return dissolve_->coreData().findAtomType(type->name()) != nullptr; });
-    masters_->setBondIconFunction([this](std::string_view name)
-                                  { return dissolve_->coreData().getMasterBond(name).has_value(); });
-    masters_->setAngleIconFunction([this](std::string_view name)
-                                   { return dissolve_->coreData().getMasterAngle(name).has_value(); });
-    masters_->setTorsionIconFunction([this](std::string_view name)
-                                     { return dissolve_->coreData().getMasterTorsion(name).has_value(); });
-    masters_->setImproperIconFunction([this](std::string_view name)
-                                      { return dissolve_->coreData().getMasterImproper(name).has_value(); });
+    atomTypes_.setQueryFunction([this](const auto type)
+                                { return dissolve_->coreData().findAtomType(type->name()) != nullptr; });
+    masters_->setBondQueryFunction([this](std::string_view name)
+                                   { return dissolve_->coreData().getMasterBond(name).has_value(); });
+    masters_->setAngleQueryFunction([this](std::string_view name)
+                                    { return dissolve_->coreData().getMasterAngle(name).has_value(); });
+    masters_->setTorsionQueryFunction([this](std::string_view name)
+                                      { return dissolve_->coreData().getMasterTorsion(name).has_value(); });
+    masters_->setImproperQueryFunction([this](std::string_view name)
+                                       { return dissolve_->coreData().getMasterImproper(name).has_value(); });
 }
 
 // Supply the species to operate on
@@ -245,9 +246,6 @@ void AddForcefieldDialogModel::finalise()
         original.setCharge(modified.charge());
     }
 
-    // Assign intramolecular terms if we need to
-    assignIntramolecularTerms(ff_);
-
     // Copy intramolecular terms
     if (intramolecularRadio_ != Radio::None)
     {
@@ -331,16 +329,16 @@ void AddForcefieldDialogModel::addMasterSuffix(int type, int index, QString suff
     switch (type)
     {
         case 0:
-            termData = masters_->bondModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
+            masters_->bondModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
             break;
         case 1:
-            termData = masters_->angleModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
+            masters_->angleModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
             break;
         case 2:
-            termData = masters_->torsionModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
+            masters_->torsionModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
             break;
         case 3:
-            termData = masters_->improperModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
+            masters_->improperModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
             break;
     };
 }
@@ -367,16 +365,16 @@ void AddForcefieldDialogModel::addMasterPrefix(int type, int index, QString pref
     switch (type)
     {
         case 0:
-            termData = masters_->bondModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
+            masters_->bondModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
             break;
         case 1:
-            termData = masters_->angleModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
+            masters_->angleModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
             break;
         case 2:
-            termData = masters_->torsionModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
+            masters_->torsionModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
             break;
         case 3:
-            termData = masters_->improperModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
+            masters_->improperModel_.setTermData(index, MasterTermModelData::DataType::Name, newName);
             break;
     };
 }
@@ -409,13 +407,12 @@ void AddForcefieldDialogModel::assignIntramolecularTerms(const Forcefield *ff)
         if (intramolecularRadio_ == Radio::Selected)
             flags += Forcefield::SelectionOnlyFlag;
 
+        // Try to assign terms across the species
         if (!ff->assignIntramolecular(modifiedSpecies_, flags))
-            // Assigning failed
             return;
 
         // Reduce to master terms?
         if (!noMasterTerms_)
             modifiedSpecies_->reduceToMasterTerms(temporaryCoreData_, intramolecularRadio_ == Radio::Selected);
     }
-    return;
 }

src/gui/models/atomTypeModel.cpp

@@ -31,10 +31,10 @@ void AtomTypeModel::setData(const std::vector<std::shared_ptr<AtomType>> &atomTy
     modelUpdater.connectModelSignals();
 }
 
-// Set function to return QIcon for item
-void AtomTypeModel::setIconFunction(std::function<bool(const std::shared_ptr<AtomType> &atomType)> func)
+// Set query function for item
+void AtomTypeModel::setQueryFunction(std::function<bool(const std::shared_ptr<AtomType> &atomType)> func)
 {
-    iconFunction_ = func;
+    queryFunction_ = func;
 }
 
 // Set vector containing checked items
@@ -80,7 +80,7 @@ QVariant AtomTypeModel::data(const QModelIndex &index, int role) const
             case Qt::UserRole + 1:
                 return QString::fromStdString(std::string(data->name()));
             case Qt::UserRole + 2:
-                return QVariant(iconFunction_(rawData(index)));
+                return QVariant(queryFunction_(rawData(index)));
         }
     }
     if (role == Qt::DisplayRole || role == Qt::EditRole)
@@ -109,8 +109,8 @@ QVariant AtomTypeModel::data(const QModelIndex &index, int role) const
                 return {};
         }
     }
-    else if (role == Qt::DecorationRole && iconFunction_)
-        return QIcon(iconFunction_(rawData(index)) ? ":/general/icons/warn.svg" : ":/general/icons/warn.svg");
+    else if (role == Qt::DecorationRole && queryFunction_)
+        return QIcon(queryFunction_(rawData(index)) ? ":/general/icons/warn.svg" : ":/general/icons/warn.svg");
     else if (role == Qt::CheckStateRole && checkedItems_)
         return std::find(checkedItems_->get().begin(), checkedItems_->get().end(), rawData(index)) == checkedItems_->get().end()
                    ? Qt::Unchecked

src/gui/models/atomTypeModel.h

@@ -39,15 +39,15 @@ class AtomTypeModel : public QAbstractListModel
     OptionalReferenceWrapper<const std::vector<std::shared_ptr<AtomType>>> atomTypes_;
     // Vector containing checked items (if relevant)
     OptionalReferenceWrapper<std::vector<std::shared_ptr<AtomType>>> checkedItems_;
-    // Icon return function
-    std::function<bool(const std::shared_ptr<AtomType> &atomType)> iconFunction_;
+    // Query function
+    std::function<bool(const std::shared_ptr<AtomType> &atomType)> queryFunction_;
 
     public:
     // Set source AtomType data
     void setData(const std::vector<std::shared_ptr<AtomType>> &atomTypes,
                  OptionalReferenceWrapper<const CoreData> coreData = std::nullopt);
     // Set function to return QIcon for item
-    void setIconFunction(std::function<bool(const std::shared_ptr<AtomType> &atomType)> func);
+    void setQueryFunction(std::function<bool(const std::shared_ptr<AtomType> &atomType)> func);
     // Set vector containing checked items
     void setCheckStateData(std::vector<std::shared_ptr<AtomType>> &checkedItemsVector);
     // Return object represented by specified model index

src/gui/models/masterTermModel.cpp

@@ -5,7 +5,10 @@
 
 MasterTermModel::MasterTermModel(CoreData &coreData) : QAbstractTableModel(), coreData_(coreData) {}
 
-void MasterTermModel::setIconFunction(std::function<bool(std::string_view termName)> func) { iconFunction_ = std::move(func); }
+void MasterTermModel::setQueryFunction(std::function<bool(std::string_view termName)> func)
+{
+    queryFunction_ = std::move(func);
+}
 
 int MasterTermModel::columnCount(const QModelIndex &parent) const { return parent.isValid() ? 0 : 3; }
 
@@ -47,13 +50,13 @@ QVariant MasterTermModel::data(const QModelIndex &index, int role) const
     if (index.row() < 0 || index.row() >= rowCount())
         return {};
 
-    if (role == MasterTermModelData::Roles::HasMaster && iconFunction_)
-        return iconFunction_(getTermData(index.row(), MasterTermModelData::DataType::Name).toString().toStdString());
+    if (role == MasterTermModelData::Roles::HasMaster && queryFunction_)
+        return queryFunction_(getTermData(index.row(), MasterTermModelData::DataType::Name).toString().toStdString());
 
-    if (role == MasterTermModelData::Roles::Icon && iconFunction_)
-        return QIcon(iconFunction_(getTermData(index.row(), MasterTermModelData::DataType::Name).toString().toStdString())
+    if (role == MasterTermModelData::Roles::Icon && queryFunction_)
+        return QIcon(queryFunction_(getTermData(index.row(), MasterTermModelData::DataType::Name).toString().toStdString())
                          ? ":/general/icons/warn.svg"
-                         : ":/general/icons/warn.svg");
+                         : ":/general/icons/true.svg");
 
     if (role == MasterTermModelData::Roles::Display || role == MasterTermModelData::Roles::Edit)
         return getTermData(index.row(), static_cast<MasterTermModelData::DataType>(index.column()));

src/gui/models/masterTermModel.h

@@ -44,15 +44,15 @@ class MasterTermModel : public QAbstractTableModel
 
     protected:
     // Icon return function
-    std::function<bool(std::string_view termName)> iconFunction_;
+    std::function<bool(std::string_view termName)> queryFunction_;
 
     public:
     // Return model data, by type, for specified term index
     virtual QVariant getTermData(int row, MasterTermModelData::DataType dataType) const = 0;
     // Set model data, by type, for specified term index
     virtual bool setTermData(int row, MasterTermModelData::DataType dataType, const QVariant &value) = 0;
     // Set function to return QIcon for item
-    void setIconFunction(std::function<bool(std::string_view termName)> func);
+    void setQueryFunction(std::function<bool(std::string_view termName)> func);
 
     /*
      * QAbstractItemModel overrides

src/gui/models/masterTermTreeModel.cpp

@@ -25,24 +25,24 @@ MasterTermModel &MasterTermTreeModel::modelForTopLevelRow(int row)
     }
 }
 
-void MasterTermTreeModel::setBondIconFunction(std::function<bool(std::string_view termName)> func)
+void MasterTermTreeModel::setBondQueryFunction(std::function<bool(std::string_view termName)> func)
 {
-    bondModel_.setIconFunction(func);
+    bondModel_.setQueryFunction(std::move(func));
 }
 
-void MasterTermTreeModel::setAngleIconFunction(std::function<bool(std::string_view termName)> func)
+void MasterTermTreeModel::setAngleQueryFunction(std::function<bool(std::string_view termName)> func)
 {
-    angleModel_.setIconFunction(func);
+    angleModel_.setQueryFunction(std::move(func));
 }
 
-void MasterTermTreeModel::setTorsionIconFunction(std::function<bool(std::string_view termName)> func)
+void MasterTermTreeModel::setTorsionQueryFunction(std::function<bool(std::string_view termName)> func)
 {
-    torsionModel_.setIconFunction(func);
+    torsionModel_.setQueryFunction(std::move(func));
 }
 
-void MasterTermTreeModel::setImproperIconFunction(std::function<bool(std::string_view termName)> func)
+void MasterTermTreeModel::setImproperQueryFunction(std::function<bool(std::string_view termName)> func)
 {
-    improperModel_.setIconFunction(func);
+    improperModel_.setQueryFunction(std::move(func));
 }
 
 void MasterTermTreeModel::prefixNames(QList<QModelIndex> indices, QString prefix)

src/gui/models/masterTermTreeModel.h

@@ -33,10 +33,10 @@ class MasterTermTreeModel : public QAbstractItemModel
     MasterTermModel &modelForTopLevelRow(int row);
 
     public:
-    void setBondIconFunction(std::function<bool(std::string_view termName)> func);
-    void setAngleIconFunction(std::function<bool(std::string_view termName)> func);
-    void setTorsionIconFunction(std::function<bool(std::string_view termName)> func);
-    void setImproperIconFunction(std::function<bool(std::string_view termName)> func);
+    void setBondQueryFunction(std::function<bool(std::string_view termName)> func);
+    void setAngleQueryFunction(std::function<bool(std::string_view termName)> func);
+    void setTorsionQueryFunction(std::function<bool(std::string_view termName)> func);
+    void setImproperQueryFunction(std::function<bool(std::string_view termName)> func);
     void prefixNames(QList<QModelIndex> indices, QString prefix);
     void suffixNames(QList<QModelIndex> indices, QString suffix);
 

src/gui/qml/AddForcefieldTermsDialog.qml

@@ -11,6 +11,7 @@ Page {
     font.pointSize: 10
     palette.buttonText: palette.active.text
     palette.disabled.buttonText: palette.disabled.text
+    spacing: 4
     visible: true
 
     AddForcefieldDialogModel {

src/gui/qml/IconListView.qml

@@ -21,6 +21,7 @@ ListView {
             anchors.bottom: label.bottom
             anchors.left: parent.left
             anchors.top: label.top
+            height: 10
             source: icon ? "qrc:/general/icons/warn.svg" : "qrc:/general/icons/true.svg"
             width: height
         }

src/gui/qml/ImageRadio.qml

@@ -21,9 +21,10 @@ RadioButton {
 
             anchors.left: padding.right
             fillMode: Image.PreserveAspectFit
-            height: instructions.height
+            height: width
             opacity: control.enabled ? 1.0 : 0.3
             source: control.source
+            width: 16
         }
         D.Text {
             id: instructions

src/gui/qml/widgets/Text.qml

@@ -2,6 +2,6 @@ import QtQuick
 
 Text {
     anchors.margins: 4
-    font.pointSize: 11
+    font.pointSize: 10
     wrapMode: Text.Wrap
 }

src/io/import/cif.cpp

@@ -976,12 +976,13 @@ void CIFHandler::finalise(CoreData &coreData, const Flags<OutputFlags> &flags) c
         if (flags.isSet(OutputFlags::OutputSupermolecule))
         {
             sp->removePeriodicBonds();
-            sp->updateIntramolecularTerms();
             sp->removeBox();
         }
         else
             sp->createBox(supercellSpecies_.box()->axisLengths(), supercellSpecies_.box()->axisAngles());
 
+        sp->updateIntramolecularTerms();
+
         if (flags.isSet(OutputFlags::OutputConfiguration))
         {
             configuration = coreData.addConfiguration();