From 10aa6f6c3f6872a904e65368979ca356bad0d411 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Thu, 17 Oct 2024 14:01:27 -0400 Subject: [PATCH] Fix new name regexes, some assumptions about MAPSEC_NONE, memory leak --- include/project.h | 2 +- src/mainwindow.cpp | 58 +++++++++++++++++++++--------------- src/project.cpp | 46 +++++++++++++++++----------- src/scriptapi/apiutility.cpp | 3 -- src/ui/maplistmodels.cpp | 13 +++++--- 5 files changed, 73 insertions(+), 49 deletions(-) diff --git a/include/project.h b/include/project.h index d93f5601..6782769b 100644 --- a/include/project.h +++ b/include/project.h @@ -132,7 +132,6 @@ class Project : public QObject QString getProjectTitle(); QString readMapLayoutId(QString map_name); - QString readMapLayoutName(QString mapName); QString readMapLocation(QString map_name); bool readWildMonData(); @@ -243,6 +242,7 @@ class Project : public QObject static bool mapDimensionsValid(int width, int height); bool calculateDefaultMapSize(); static int getMaxObjectEvents(); + static QString getEmptyMapsecName(); private: void updateLayout(Layout *); diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index dea3d210..2946dbeb 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1370,9 +1370,13 @@ void MainWindow::mapListAddGroup() { QLineEdit *newNameEdit = new QLineEdit(&dialog); newNameEdit->setClearButtonEnabled(true); - static const QRegularExpression re_validChars("[_A-Za-z0-9]*$"); - QRegularExpressionValidator *validator = new QRegularExpressionValidator(re_validChars); - newNameEdit->setValidator(validator); + static const QRegularExpression re_validChars("[A-Za-z_]+[\\w]*"); + newNameEdit->setValidator(new QRegularExpressionValidator(re_validChars, newNameEdit)); + + connect(&newItemButtonBox, &QDialogButtonBox::accepted, [&](){ + if (!this->editor->project->groupNames.contains(newNameEdit->text())) + dialog.accept(); + }); QFormLayout form(&dialog); @@ -1397,10 +1401,10 @@ void MainWindow::mapListAddLayout() { QLineEdit *newNameEdit = new QLineEdit(&dialog); newNameEdit->setClearButtonEnabled(true); - static const QRegularExpression re_validChars("[_A-Za-z0-9]*$"); - QRegularExpressionValidator *validator = new QRegularExpressionValidator(re_validChars); - newNameEdit->setValidator(validator); + static const QRegularExpression re_validChars("[A-Za-z_]+[\\w]*"); + newNameEdit->setValidator(new QRegularExpressionValidator(re_validChars, newNameEdit)); + // TODO: Support arbitrary LAYOUT_ ID names (Note from GriffinR: This is already handled in an unopened PR) QLabel *newId = new QLabel("LAYOUT_", &dialog); connect(newNameEdit, &QLineEdit::textChanged, [&](QString text){ newId->setText(Layout::layoutConstantFromName(text.remove("_Layout"))); @@ -1499,7 +1503,7 @@ void MainWindow::mapListAddLayout() { layoutSettings.tileset_secondary_label = secondaryCombo->currentText(); } Layout *newLayout = this->editor->project->createNewLayout(layoutSettings); - QStandardItem *item = this->layoutTreeModel->insertLayoutItem(newLayout->id); + this->layoutTreeModel->insertLayoutItem(newLayout->id); setLayout(newLayout->id); } } @@ -1511,13 +1515,18 @@ void MainWindow::mapListAddArea() { QDialogButtonBox newItemButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, Qt::Horizontal, &dialog); connect(&newItemButtonBox, &QDialogButtonBox::rejected, &dialog, &QDialog::reject); + const QString prefix = projectConfig.getIdentifier(ProjectIdentifier::define_map_section_prefix); QLineEdit *newNameEdit = new QLineEdit(&dialog); - newNameEdit->setText(projectConfig.getIdentifier(ProjectIdentifier::define_map_section_prefix)); - newNameEdit->setClearButtonEnabled(false); + QLineEdit *newNameDisplay = new QLineEdit(&dialog); + newNameDisplay->setText(prefix); + newNameDisplay->setEnabled(false); + connect(newNameEdit, &QLineEdit::textEdited, [newNameDisplay, prefix] (const QString &text) { + // As the user types a name, update the label to show the name with the prefix. + newNameDisplay->setText(prefix + text); + }); - QRegularExpression re_validChars(QString("%1[_A-Za-z0-9]+$").arg(projectConfig.getIdentifier(ProjectIdentifier::define_map_section_prefix))); - QRegularExpressionValidator *validator = new QRegularExpressionValidator(re_validChars); - newNameEdit->setValidator(validator); + 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(newNameEdit->text())) @@ -1527,12 +1536,12 @@ void MainWindow::mapListAddArea() { QFormLayout form(&dialog); form.addRow("New Map Section Name", newNameEdit); + form.addRow("Constant Name", newNameDisplay); form.addRow(&newItemButtonBox); if (dialog.exec() == QDialog::Accepted) { - QString newFieldName = newNameEdit->text(); - if (newFieldName.isEmpty()) return; - this->mapAreaModel->insertAreaItem(newFieldName); + if (newNameEdit->text().isEmpty()) return; + this->mapAreaModel->insertAreaItem(newNameDisplay->text()); } } @@ -1540,13 +1549,13 @@ void MainWindow::mapListAddItem() { if (!this->editor || !this->editor->project) return; switch (this->ui->mapListContainer->currentIndex()) { - case 0: + case MapListTab::Groups: this->mapListAddGroup(); break; - case 1: + case MapListTab::Areas: this->mapListAddArea(); break; - case 2: + case MapListTab::Layouts: this->mapListAddLayout(); break; } @@ -1596,14 +1605,14 @@ void MainWindow::mapListRemoveItem() { if (!this->editor || !this->editor->project) return; switch (this->ui->mapListContainer->currentIndex()) { - case 0: + case MapListTab::Groups: this->mapListRemoveGroup(); break; - case 1: + case MapListTab::Areas: // Disabled // this->mapListRemoveArea(); break; - case 2: + case MapListTab::Layouts: // Disabled // this->mapListRemoveLayout(); break; @@ -2913,10 +2922,8 @@ void MainWindow::onTilesetsSaved(QString primaryTilesetLabel, QString secondaryT } else { this->editor->project->getTileset(secondaryTilesetLabel, true); } - if (updated) { - this->editor->layout->clearBorderCache(); + if (updated) redrawMapScene(); - } } void MainWindow::onMapRulerStatusChanged(const QString &status) { @@ -3232,6 +3239,7 @@ void MainWindow::do_CollapseAll() { } } +// TODO: Save this state in porymapConfig void MainWindow::do_HideShow() { switch (ui->mapListContainer->currentIndex()) { case MapListTab::Groups: @@ -3265,6 +3273,7 @@ void MainWindow::on_toolButton_CollapseAll_Groups_clicked() { } } +// TODO: Save this state in porymapConfig void MainWindow::on_toolButton_EnableDisable_EditGroups_clicked() { this->ui->mapList->clearSelection(); if (this->ui->toolButton_EnableDisable_EditGroups->isChecked()) { @@ -3607,6 +3616,7 @@ bool MainWindow::closeProject() { return true; // Check loaded maps for unsaved changes + // TODO: This needs to check for unsaved changes in layouts too. bool unsavedChanges = false; for (auto map : editor->project->mapCache.values()) { if (map && map->hasUnsavedChanges()) { diff --git a/src/project.cpp b/src/project.cpp index 32c61fed..e6cd7a8e 100644 --- a/src/project.cpp +++ b/src/project.cpp @@ -412,10 +412,6 @@ QString Project::readMapLayoutId(QString map_name) { return ParseUtil::jsonToQString(mapObj["layout"]); } -QString Project::readMapLayoutName(QString mapName) { - return this->layoutIdsToNames[readMapLayoutId(mapName)]; -} - QString Project::readMapLocation(QString map_name) { if (mapCache.contains(map_name)) { return mapCache.value(map_name)->location; @@ -537,6 +533,8 @@ bool Project::loadMapLayout(Map* map) { void Project::clearMapLayouts() { qDeleteAll(mapLayouts); mapLayouts.clear(); + qDeleteAll(mapLayoutsMaster); + mapLayoutsMaster.clear(); mapLayoutsTable.clear(); layoutIdsToNames.clear(); } @@ -763,7 +761,7 @@ void Project::saveMapSections() { longestLength += 1; - // mapSectionValueToName + // TODO: Maybe print as an enum now that we can? for (int value : this->mapSectionValueToName.keys()) { QString line = QString("#define %1 0x%2\n") .arg(this->mapSectionValueToName[value], -1 * longestLength) @@ -771,6 +769,8 @@ void Project::saveMapSections() { text += line; } + // TODO: We should maybe consider another way to update MAPSEC values in this file, in case we break anything by relocating it to the bottom of the file. + // (or alternatively keep separate strings for text before/after the MAPSEC values) text += "\n" + this->extraFileText[projectConfig.getFilePath(ProjectFilePath::constants_region_map_sections)] + "\n"; text += QString("#endif // GUARD_REGIONMAPSEC_H\n"); @@ -2307,10 +2307,12 @@ bool Project::readRegionMapSections() { continue; } // include guards + // TODO: Assuming guard name is the same across projects (it isn't) else if (currentLine.contains("GUARD_REGIONMAPSEC_H")) { continue; } - // defines captured (not considering comments) + // defines captured + // TODO: Regex to consider comments/extra space else if (currentLine.contains("#define " + projectConfig.getIdentifier(ProjectIdentifier::define_map_section_prefix))) { continue; } @@ -2324,18 +2326,28 @@ bool Project::readRegionMapSections() { return true; } +QString Project::getEmptyMapsecName() { + return projectConfig.getIdentifier(ProjectIdentifier::define_map_section_prefix) + projectConfig.getIdentifier(ProjectIdentifier::define_map_section_empty); +} + +// This function assumes a valid and unique name. +// Will return the new index. +// TODO: We're not currently tracking map/layout agonstic changes like this as unsaved, so there's no warning if you close the project after doing this. int Project::appendMapsec(QString name) { - // This function assumes a valid and unique name. - // Will return the new index. - const QString emptyMapsecName = projectConfig.getIdentifier(ProjectIdentifier::define_map_section_prefix) - + projectConfig.getIdentifier(ProjectIdentifier::define_map_section_empty); - - int noneBefore = this->mapSectionNameToValue[emptyMapsecName]; - this->mapSectionNameToValue[name] = noneBefore; - this->mapSectionValueToName[noneBefore] = name; - this->mapSectionNameToValue[emptyMapsecName] = noneBefore + 1; - this->mapSectionValueToName[noneBefore + 1] = emptyMapsecName; - return noneBefore; + const QString emptyMapsecName = getEmptyMapsecName(); + int newMapsecValue = mapSectionValueToName.isEmpty() ? 0 : mapSectionValueToName.lastKey(); + + // If the user has the 'empty' MAPSEC value defined last in the list we'll shift it so that it stays last in the list. + if (this->mapSectionNameToValue.contains(emptyMapsecName) && this->mapSectionNameToValue.value(emptyMapsecName) == newMapsecValue) { + this->mapSectionNameToValue.insert(emptyMapsecName, newMapsecValue + 1); + this->mapSectionValueToName.insert(newMapsecValue + 1, emptyMapsecName); + } + + // TODO: Update 'define_map_section_count'? + + this->mapSectionNameToValue[name] = newMapsecValue; + this->mapSectionValueToName[newMapsecValue] = name; + return newMapsecValue; } // Read the constants to preserve any "unused" heal locations when writing the file later diff --git a/src/scriptapi/apiutility.cpp b/src/scriptapi/apiutility.cpp index c12db39a..27bd5677 100644 --- a/src/scriptapi/apiutility.cpp +++ b/src/scriptapi/apiutility.cpp @@ -154,9 +154,6 @@ void ScriptUtility::setMainTab(int index) { // Can't select tab if it's disabled if (!window->ui->mainTabBar->isTabEnabled(index)) return; - // don't change tab when not editing a map - if (!window->editor || !window->editor->map) - return; window->on_mainTabBar_tabBarClicked(index); } diff --git a/src/ui/maplistmodels.cpp b/src/ui/maplistmodels.cpp index f5835357..7e0d7e62 100644 --- a/src/ui/maplistmodels.cpp +++ b/src/ui/maplistmodels.cpp @@ -19,10 +19,9 @@ void MapTree::removeSelected() { QWidget *GroupNameDelegate::createEditor(QWidget *parent, const QStyleOptionViewItem &, const QModelIndex &) const { QLineEdit *editor = new QLineEdit(parent); - static const QRegularExpression expression("[A-Za-z0-9_]+"); + static const QRegularExpression expression("[A-Za-z_]+[\\w]*"); editor->setPlaceholderText("gMapGroup_"); - QRegularExpressionValidator *validator = new QRegularExpressionValidator(expression, parent); - editor->setValidator(validator); + editor->setValidator(new QRegularExpressionValidator(expression, parent)); editor->setFrame(false); return editor; } @@ -401,7 +400,12 @@ QStandardItem *MapAreaModel::insertAreaItem(QString areaName) { int newAreaIndex = this->project->appendMapsec(areaName); QStandardItem *item = createAreaItem(areaName, newAreaIndex); this->root->insertRow(newAreaIndex, item); - this->areaItems["MAPSEC_NONE"]->setData(newAreaIndex + 1, MapListUserRoles::GroupRole); + + // MAPSEC_NONE may have shifted to accomodate the new item, update it in the list. + const QString emptyMapsecName = Project::getEmptyMapsecName(); + if (this->areaItems.contains(emptyMapsecName)) + this->areaItems[emptyMapsecName]->setData(this->project->mapSectionNameToValue.value(emptyMapsecName), MapListUserRoles::GroupRole); + return item; } @@ -427,6 +431,7 @@ void MapAreaModel::initialize() { this->mapItems.clear(); this->setSortRole(MapListUserRoles::GroupRole); + // TODO: Ignore 'define_map_section_count' and/or 'define_map_section_empty'? for (int i : this->project->mapSectionNameToValue) { QString mapsecName = project->mapSectionValueToName.value(i); QStandardItem *areaItem = createAreaItem(mapsecName, i);