Resolve warnings and low-hanging TODO items

forms/maplisttoolbar.ui

@@ -31,8 +31,11 @@
    </property>
    <item>
     <widget class="QToolButton" name="button_AddFolder">
+     <property name="toolTip">
+      <string>Add a new folder to the list.</string>
+     </property>
      <property name="text">
-      <string>Add a folder to the list.</string>
+      <string/>
      </property>
      <property name="icon">
       <iconset resource="../resources/images.qrc">
@@ -110,7 +113,7 @@
    <item>
     <widget class="QToolButton" name="button_ToggleEdit">
      <property name="toolTip">
-      <string>Toggle editability of folders in the list.</string>
+      <string>If enabled, folders may be renamed and items in the list may be rearranged.</string>
      </property>
      <property name="text">
       <string/>

include/mainwindow.h

@@ -369,8 +369,6 @@ private:
     bool closeProject();
     void showProjectOpenFailure();
 
-    QStandardItem* createMapItem(QString mapName, int groupNum, int inGroupNum);
-
     bool setInitialMap();
     void saveGlobalConfigs();
 

include/ui/maplistmodels.h

@@ -150,7 +150,6 @@ private:
 
     QMap<QString, QStandardItem *> areaItems;
     QMap<QString, QStandardItem *> mapItems;
-    // TODO: if reordering, will the item be the same?
 
     QString openMap;
 

src/mainwindow.cpp

@@ -465,7 +465,6 @@ void MainWindow::updateWindowTitle() {
     } else {
         ui->mainTabBar->setTabIcon(MainTab::Map, QIcon(QStringLiteral(":/icons/map.ico")));
     }
-    updateMapList(); // TODO: Why is this function responsible for this
 }
 
 void MainWindow::markMapEdited() {
@@ -479,6 +478,7 @@ void MainWindow::markSpecificMapEdited(Map* map) {
 
     if (editor && editor->map == map)
         updateWindowTitle();
+    updateMapList();
 }
 
 void MainWindow::loadUserSettings() {
@@ -870,6 +870,7 @@ bool MainWindow::setMap(QString map_name) {
     refreshMapScene();
     displayMapProperties();
     updateWindowTitle();
+    updateMapList();
     resetMapListFilters();
 
     connect(editor->map, &Map::modified, this, &MainWindow::markMapEdited, Qt::UniqueConnection);
@@ -918,8 +919,9 @@ bool MainWindow::setLayout(QString layoutId) {
 
     unsetMap();
 
-    // TODO: Using the 'id' instead of the layout name here is inconsistent with how we treat maps.
+    // Prefer logging the name of the layout as displayed in the map list.
-    logInfo(QString("Setting layout to '%1'").arg(layoutId));
+    const QString layoutName = this->editor->project ? this->editor->project->layoutIdsToNames.value(layoutId, layoutId) : layoutId;
+    logInfo(QString("Setting layout to '%1'").arg(layoutName));
 
     if (!this->editor->setLayout(layoutId)) {
         return false;
@@ -929,6 +931,7 @@ bool MainWindow::setLayout(QString layoutId) {
 
     refreshMapScene();
     updateWindowTitle();
+    updateMapList();
     resetMapListFilters();
 
     connect(editor->layout, &Layout::needsRedrawing, this, &MainWindow::redrawMapScene, Qt::UniqueConnection);
@@ -1241,8 +1244,6 @@ bool MainWindow::setProjectUI() {
     this->layoutListProxyModel->setSourceModel(this->layoutTreeModel);
     ui->layoutList->setModel(layoutListProxyModel);
 
-    //on_toolButton_EnableDisable_EditGroups_clicked();//TODO
-
     return true;
 }
 
@@ -1366,7 +1367,6 @@ void MainWindow::mapListAddGroup() {
     QDialog dialog(this, Qt::WindowTitleHint | Qt::WindowCloseButtonHint);
     dialog.setWindowModality(Qt::ApplicationModal);
     QDialogButtonBox newItemButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, Qt::Horizontal, &dialog);
-    connect(&newItemButtonBox, &QDialogButtonBox::accepted, &dialog, &QDialog::accept);
     connect(&newItemButtonBox, &QDialogButtonBox::rejected, &dialog, &QDialog::reject);
 
     QLineEdit *newNameEdit = new QLineEdit(&dialog);
@@ -1375,15 +1375,24 @@ void MainWindow::mapListAddGroup() {
     static const QRegularExpression re_validChars("[A-Za-z_]+[\\w]*");
     newNameEdit->setValidator(new QRegularExpressionValidator(re_validChars, newNameEdit));
 
+    QLabel *errorMessageLabel = new QLabel(&dialog);
+    errorMessageLabel->setVisible(false);
+    errorMessageLabel->setStyleSheet("QLabel { background-color: rgba(255, 0, 0, 25%) }");
+
     connect(&newItemButtonBox, &QDialogButtonBox::accepted, [&](){
-        if (!this->editor->project->groupNames.contains(newNameEdit->text()))
+        const QString mapGroupName = newNameEdit->text();
+        if (this->editor->project->groupNames.contains(mapGroupName)) {
+            errorMessageLabel->setText(QString("A map group with the name '%1' already exists").arg(mapGroupName));
+            errorMessageLabel->setVisible(true);
+        } else {
             dialog.accept();
-        // TODO: Else display error?
+        }
     });
 
     QFormLayout form(&dialog);
 
     form.addRow("New Group Name", newNameEdit);
+    form.addRow("", errorMessageLabel);
     form.addRow(&newItemButtonBox);
 
     if (dialog.exec() == QDialog::Accepted) {
@@ -1426,7 +1435,6 @@ void MainWindow::mapListAddLayout() {
     QLabel *errorMessageLabel = new QLabel(&dialog);
     errorMessageLabel->setVisible(false);
     errorMessageLabel->setStyleSheet("QLabel { background-color: rgba(255, 0, 0, 25%) }");
-    QString errorMessage;
 
     QComboBox *primaryCombo = new QComboBox(&dialog);
     primaryCombo->addItems(this->editor->project->primaryTilesetLabels);
@@ -1463,28 +1471,25 @@ void MainWindow::mapListAddLayout() {
 
     connect(&newItemButtonBox, &QDialogButtonBox::accepted, [&](){
         // verify some things
-        bool issue = false;
+        QString errorMessage;
         QString tryLayoutName = newNameEdit->text();
         // name not empty
         if (tryLayoutName.isEmpty()) {
             errorMessage = "Name cannot be empty";
-            issue = true;
         }
         // unique layout name & id
         else if (this->editor->project->mapLayoutsTable.contains(newId->text())
               || this->editor->project->layoutIdsToNames.find(tryLayoutName) != this->editor->project->layoutIdsToNames.end()) {
             errorMessage = "Layout Name / ID is not unique";
-            issue = true;
         }
         // from id is existing value
         else if (useExistingCheck->isChecked()) {
             if (!this->editor->project->mapLayoutsTable.contains(useExistingCombo->currentText())) {
                 errorMessage = "Existing layout ID is not valid";
-                issue = true;
             }
         }
 
-        if (issue) {
+        if (!errorMessage.isEmpty()) {
             // show error
             errorMessageLabel->setText(errorMessage);
             errorMessageLabel->setVisible(true);
@@ -1532,16 +1537,24 @@ void MainWindow::mapListAddArea() {
         newNameDisplay->setText(prefix + text);
     });
 
+    QLabel *errorMessageLabel = new QLabel(&dialog);
+    errorMessageLabel->setVisible(false);
+    errorMessageLabel->setStyleSheet("QLabel { background-color: rgba(255, 0, 0, 25%) }");
+
     static const QRegularExpression re_validChars("[A-Za-z_]+[\\w]*");
     newNameEdit->setValidator(new QRegularExpressionValidator(re_validChars, newNameEdit));
 
     connect(&newItemButtonBox, &QDialogButtonBox::accepted, [&](){
-        if (!this->editor->project->mapSectionNameToValue.contains(newNameDisplay->text()))
+        const QString newAreaName = newNameDisplay->text();
+        if (this->editor->project->mapSectionNameToValue.contains(newAreaName)){
+            errorMessageLabel->setText(QString("An area with the name '%1' already exists").arg(newAreaName));
+            errorMessageLabel->setVisible(true);
+        } else {
             dialog.accept();
-        // TODO: Else display error?
+        }
     });
 
-    QLabel *newNameEditLabel = new QLabel("New Map Section Name", &dialog);
+    QLabel *newNameEditLabel = new QLabel("New Area Name", &dialog);
     QLabel *newNameDisplayLabel = new QLabel("Constant Name", &dialog);
     newNameDisplayLabel->setEnabled(false);
 
@@ -1549,6 +1562,7 @@ void MainWindow::mapListAddArea() {
 
     form.addRow(newNameEditLabel, newNameEdit);
     form.addRow(newNameDisplayLabel, newNameDisplay);
+    form.addRow("", errorMessageLabel);
     form.addRow(&newItemButtonBox);
 
     if (dialog.exec() == QDialog::Accepted) {
@@ -1855,11 +1869,13 @@ void MainWindow::updateMapList() {
 void MainWindow::on_action_Save_Project_triggered() {
     editor->saveProject();
     updateWindowTitle();
+    updateMapList();
 }
 
 void MainWindow::on_action_Save_triggered() {
     editor->save();
     updateWindowTitle();
+    updateMapList();
 }
 
 void MainWindow::duplicate() {

src/ui/maplistmodels.cpp

@@ -131,7 +131,7 @@ QMimeData *MapGroupModel::mimeData(const QModelIndexList &indexes) const {
     return mimeData;
 }
 
-bool MapGroupModel::dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parentIndex) {
+bool MapGroupModel::dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int, const QModelIndex &parentIndex) {
     if (action == Qt::IgnoreAction)
         return true;
 
@@ -154,7 +154,6 @@ bool MapGroupModel::dropMimeData(const QMimeData *data, Qt::DropAction action, i
         QByteArray encodedData = data->data("application/porymap.mapgroupmodel.group");
         QDataStream stream(&encodedData, QIODevice::ReadOnly);
         QString groupName;
-        int rowCount = 1;
 
         while (!stream.atEnd()) {
             stream >> groupName;
@@ -402,6 +401,7 @@ bool MapGroupModel::setData(const QModelIndex &index, const QVariant &value, int
     if (QStandardItemModel::setData(index, value, role)) {
         this->updateProject();
     }
+    return true;
 }
 
 
@@ -425,7 +425,7 @@ QStandardItem *MapAreaModel::createAreaItem(QString mapsecName, int areaIndex) {
     return area;
 }
 
-QStandardItem *MapAreaModel::createMapItem(QString mapName, int groupIndex, int mapIndex) {
+QStandardItem *MapAreaModel::createMapItem(QString mapName, int, int) {
     QStandardItem *map = new QStandardItem;
     map->setText(mapName);
     map->setEditable(false);
@@ -603,6 +603,7 @@ QStandardItem *LayoutTreeModel::createMapItem(QString mapName) {
 QStandardItem *LayoutTreeModel::insertLayoutItem(QString layoutId) {
     QStandardItem *layoutItem = this->createLayoutItem(layoutId);
     this->root->appendRow(layoutItem);
+    return layoutItem;
 }
 
 QStandardItem *LayoutTreeModel::insertMapItem(QString mapName, QString layoutId) {

src/ui/maplisttoolbar.cpp

@@ -117,20 +117,22 @@ void MapListToolBar::collapseList() {
 }
 
 void MapListToolBar::applyFilter(const QString &filterText) {
-    if (!m_list || m_filterLocked)
+    if (m_filterLocked)
         return;
 
     const QSignalBlocker b(ui->lineEdit_filterBox);
     ui->lineEdit_filterBox->setText(filterText);
 
-    auto model = static_cast<FilterChildrenProxyModel*>(m_list->model());
+    if (m_list) {
-    if (model) model->setFilterRegularExpression(QRegularExpression(filterText, QRegularExpression::CaseInsensitiveOption));
+        auto model = static_cast<FilterChildrenProxyModel*>(m_list->model());
+        if (model) model->setFilterRegularExpression(QRegularExpression(filterText, QRegularExpression::CaseInsensitiveOption));
 
-    if (filterText.isEmpty()) {
+        if (filterText.isEmpty()) {
-        m_list->collapseAll();
+            m_list->collapseAll();
-        emit filterCleared(m_list);
+            emit filterCleared(m_list);
-    } else {
+        } else {
-        m_list->expandToDepth(0);
+            m_list->expandToDepth(0);
+        }
     }
 }