From 236ad9b73c220008623089df506c2c51b962e568 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Tue, 11 Jun 2024 12:26:34 -0400 Subject: [PATCH 1/6] Fix Add Region Map button --- src/ui/regionmapeditor.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ui/regionmapeditor.cpp b/src/ui/regionmapeditor.cpp index 220f9cca..92da19c2 100644 --- a/src/ui/regionmapeditor.cpp +++ b/src/ui/regionmapeditor.cpp @@ -298,7 +298,7 @@ bool RegionMapEditor::buildConfigDialog() { form.addRow(addMapButton); // allow user to add region maps - connect(addMapButton, &QPushButton::clicked, [this, regionMapList] { + connect(addMapButton, &QPushButton::clicked, [this, regionMapList, &updateJsonFromList] { poryjson::Json resultJson = configRegionMapDialog(); poryjson::Json::object resultObj = resultJson.object_items(); @@ -310,6 +310,7 @@ bool RegionMapEditor::buildConfigDialog() { newItem->setText(resultObj["alias"].string_value()); newItem->setData(Qt::UserRole, resultStr); regionMapList->addItem(newItem); + updateJsonFromList(); }); QPushButton *delMapButton = new QPushButton("Delete Selected Region Map"); From 0954fe26ff603fbf26a0c01526bcf03343353a14 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Mon, 17 Jun 2024 11:26:45 -0400 Subject: [PATCH 2/6] Fix confusing error logging during region map setup --- include/ui/regionmapeditor.h | 5 +++++ src/mainwindow.cpp | 7 +++---- src/ui/regionmapeditor.cpp | 38 +++++++++++++++--------------------- 3 files changed, 24 insertions(+), 26 deletions(-) diff --git a/include/ui/regionmapeditor.h b/include/ui/regionmapeditor.h index 3cbc2515..ef72b5ee 100644 --- a/include/ui/regionmapeditor.h +++ b/include/ui/regionmapeditor.h @@ -27,6 +27,7 @@ public: ~RegionMapEditor(); bool load(bool silent = false); + bool setupErrored() const { return setupError; } void onRegionMapTileSelectorSelectedTileChanged(unsigned id); void onRegionMapTileSelectorHoveredTileChanged(unsigned id); @@ -53,9 +54,13 @@ private: RegionMap *region_map = nullptr; tsl::ordered_map region_maps; + QString configFilepath; + QString mapSectionFilepath; + poryjson::Json rmConfigJson; bool configSaved = false; + bool setupError = false; QUndoGroup history; diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 6f3b87d7..ae498fb4 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -2960,15 +2960,14 @@ bool MainWindow::initRegionMapEditor(bool silent) { this->regionMapEditor = new RegionMapEditor(this, this->editor->project); bool success = this->regionMapEditor->load(silent); if (!success) { - delete this->regionMapEditor; - this->regionMapEditor = nullptr; - if (!silent) { + if (!silent && this->regionMapEditor->setupErrored()) { QMessageBox msgBox(this); - QString errorMsg = QString("There was an error opening the region map data. Please see %1 for full error details.\n\n%3") + QString errorMsg = QString("There was an error opening the region map data. Please see %1 for full error details.\n\n%2") .arg(getLogPath()) .arg(getMostRecentError()); msgBox.critical(nullptr, "Error Opening Region Map Editor", errorMsg); } + delete this->regionMapEditor; return false; } diff --git a/src/ui/regionmapeditor.cpp b/src/ui/regionmapeditor.cpp index 92da19c2..16ea4a5a 100644 --- a/src/ui/regionmapeditor.cpp +++ b/src/ui/regionmapeditor.cpp @@ -28,6 +28,8 @@ RegionMapEditor::RegionMapEditor(QWidget *parent, Project *project) : { this->ui->setupUi(this); this->project = project; + this->configFilepath = QString("%1/%2").arg(this->project->root).arg(projectConfig.getFilePath(ProjectFilePath::json_region_porymap_cfg)); + this->mapSectionFilepath = QString("%1/%2").arg(this->project->root).arg(projectConfig.getFilePath(ProjectFilePath::json_region_map_entries)); this->initShortcuts(); this->restoreWindowState(); } @@ -110,12 +112,10 @@ void RegionMapEditor::applyUserShortcuts() { bool RegionMapEditor::loadRegionMapEntries() { this->region_map_entries.clear(); - QString regionMapSectionFilepath = QString("%1/%2").arg(this->project->root).arg(projectConfig.getFilePath(ProjectFilePath::json_region_map_entries)); - ParseUtil parser; QJsonDocument sectionsDoc; - if (!parser.tryParseJsonFile(§ionsDoc, regionMapSectionFilepath)) { - logError(QString("Failed to read map data from %1").arg(regionMapSectionFilepath)); + if (!parser.tryParseJsonFile(§ionsDoc, this->mapSectionFilepath)) { + logError(QString("Failed to read map data from %1").arg(this->mapSectionFilepath)); return false; } @@ -140,11 +140,9 @@ bool RegionMapEditor::loadRegionMapEntries() { } bool RegionMapEditor::saveRegionMapEntries() { - QString regionMapSectionFilepath = QString("%1/%2").arg(this->project->root).arg(projectConfig.getFilePath(ProjectFilePath::json_region_map_entries)); - - QFile sectionsFile(regionMapSectionFilepath); + QFile sectionsFile(this->mapSectionFilepath); if (!sectionsFile.open(QIODevice::WriteOnly)) { - logError(QString("Error: Could not open %1 for writing").arg(regionMapSectionFilepath)); + logError(QString("Could not open %1 for writing").arg(this->mapSectionFilepath)); return false; } @@ -477,6 +475,7 @@ bool RegionMapEditor::setup() { if (!newMap->loadMapData(o)) { delete newMap; // TODO: consider continue, just reporting error loading single map? + this->setupError = true; return false; } @@ -499,26 +498,21 @@ bool RegionMapEditor::setup() { if (!region_maps.empty()) { setRegionMap(region_maps.begin()->second); } + this->setupError = false; return true; } bool RegionMapEditor::load(bool silent) { // check for config json file - QString jsonConfigFilepath = this->project->root + "/" + projectConfig.getFilePath(ProjectFilePath::json_region_porymap_cfg); - bool badConfig = true; - - if (QFile::exists(jsonConfigFilepath)) { - logInfo("Region map configuration file found."); + if (QFile::exists(this->configFilepath)) { ParseUtil parser; OrderedJson::object obj; - if (parser.tryParseOrderedJsonFile(&obj, jsonConfigFilepath)) { + if (parser.tryParseOrderedJsonFile(&obj, this->configFilepath)) { this->rmConfigJson = OrderedJson(obj); this->configSaved = true; } badConfig = !verifyConfig(this->rmConfigJson); - } else { - logWarn("Region Map config file not found."); } if (badConfig) { @@ -534,14 +528,15 @@ bool RegionMapEditor::load(bool silent) { if (warning.exec() == QMessageBox::Ok) { // there is a separate window that allows to load multiple region maps, if (!buildConfigDialog()) { - logError("Region map loading interrupted [user]"); + // User canceled config set up return false; } } else { - // do not open editor - logError("Region map loading interrupted [user]"); + // User declined config set up return false; } + } else { + logInfo("Successfully loaded region map configuration file."); } return setup(); @@ -583,10 +578,9 @@ void RegionMapEditor::saveConfig() { mapsObject["region_maps"] = mapArray; OrderedJson newConfigJson(mapsObject); - QString filepath = QString("%1/%2").arg(this->project->root).arg(projectConfig.getFilePath(ProjectFilePath::json_region_porymap_cfg)); - QFile file(filepath); + QFile file(this->configFilepath); if (!file.open(QIODevice::WriteOnly)) { - logError(QString("Error: Could not open %1 for writing").arg(filepath)); + logError(QString("Could not open %1 for writing").arg(this->configFilepath)); return; } OrderedJsonDoc jsonDoc(&newConfigJson); From 4a79114b98ec7d07c1a4f286f48baec73581afc7 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Mon, 17 Jun 2024 11:38:01 -0400 Subject: [PATCH 3/6] Fix crash if region map tileset is missing --- src/core/regionmap.cpp | 10 ++++++++-- src/ui/regionmapeditor.cpp | 10 ---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/core/regionmap.cpp b/src/core/regionmap.cpp index 68da1fa7..9c66860b 100644 --- a/src/core/regionmap.cpp +++ b/src/core/regionmap.cpp @@ -79,14 +79,20 @@ bool RegionMap::loadTilemap(poryjson::Json tilemapJson) { this->palette_path = tilemapObject["palette"].string_value(); } + QFileInfo tilesetFileInfo(fullPath(this->tileset_path)); + if (!tilesetFileInfo.exists() || !tilesetFileInfo.isFile()) { + logError(QString("Failed to open region map tileset file '%1'.").arg(tileset_path)); + return false; + } + QFile tilemapFile(fullPath(this->tilemap_path)); if (!tilemapFile.open(QIODevice::ReadOnly)) { - logError(QString("Failed to open region map tilemap file %1.").arg(tilemap_path)); + logError(QString("Failed to open region map tilemap file '%1'.").arg(tilemap_path)); return false; } if (tilemapFile.size() < tilemapBytes()) { - logError(QString("The region map tilemap at %1 is too small.").arg(tilemap_path)); + logError(QString("The region map tilemap at '%1' is too small.").arg(tilemap_path)); return false; } diff --git a/src/ui/regionmapeditor.cpp b/src/ui/regionmapeditor.cpp index 16ea4a5a..ee491a3e 100644 --- a/src/ui/regionmapeditor.cpp +++ b/src/ui/regionmapeditor.cpp @@ -402,16 +402,6 @@ bool RegionMapEditor::verifyConfig(poryjson::Json cfg) { logError("Region map config json has no map list."); return false; } - - OrderedJson::array arr = obj["region_maps"].array_items(); - - for (auto ref : arr) { - RegionMap tempMap(this->project); - if (!tempMap.loadMapData(ref)) { - return false; - } - } - return true; } From 1c2be70ff0215112718196031ba70787a0831302 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Mon, 17 Jun 2024 12:11:55 -0400 Subject: [PATCH 4/6] Allow users to fix faulty region map settings --- include/mainwindow.h | 1 + include/ui/regionmapeditor.h | 2 ++ src/core/regionmap.cpp | 2 +- src/mainwindow.cpp | 35 ++++++++++++++++++++++++++++------- src/ui/regionmapeditor.cpp | 10 ++++++++-- 5 files changed, 40 insertions(+), 10 deletions(-) diff --git a/include/mainwindow.h b/include/mainwindow.h index f3aa9375..7fda1b09 100644 --- a/include/mainwindow.h +++ b/include/mainwindow.h @@ -388,6 +388,7 @@ private: void initTilesetEditor(); bool initRegionMapEditor(bool silent = false); + bool askToFixRegionMapEditor(); void initShortcutsEditor(); void initCustomScriptsEditor(); void connectSubEditorsToShortcutsEditor(); diff --git a/include/ui/regionmapeditor.h b/include/ui/regionmapeditor.h index ef72b5ee..3d889f88 100644 --- a/include/ui/regionmapeditor.h +++ b/include/ui/regionmapeditor.h @@ -42,6 +42,8 @@ public: void resizeTilemap(int width, int height); + bool reconfigure(); + QObjectList shortcutableObjects() const; public slots: diff --git a/src/core/regionmap.cpp b/src/core/regionmap.cpp index 9c66860b..45fc1141 100644 --- a/src/core/regionmap.cpp +++ b/src/core/regionmap.cpp @@ -303,7 +303,7 @@ bool RegionMap::loadLayout(poryjson::Json layoutJson) { } setLayout("main", layout); } else { - logError("Region map layout is not readable."); + logError(QString("Failed to read region map layout from '%1'.").arg(this->layout_path)); return false; } } diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index ae498fb4..0b92c1c9 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -2958,14 +2958,12 @@ void MainWindow::on_pushButton_CreatePrefab_clicked() { bool MainWindow::initRegionMapEditor(bool silent) { this->regionMapEditor = new RegionMapEditor(this, this->editor->project); - bool success = this->regionMapEditor->load(silent); - if (!success) { + if (!this->regionMapEditor->load(silent)) { + // The region map editor either failed to load, + // or the user declined configuring their settings. if (!silent && this->regionMapEditor->setupErrored()) { - QMessageBox msgBox(this); - QString errorMsg = QString("There was an error opening the region map data. Please see %1 for full error details.\n\n%2") - .arg(getLogPath()) - .arg(getMostRecentError()); - msgBox.critical(nullptr, "Error Opening Region Map Editor", errorMsg); + if (this->askToFixRegionMapEditor()) + return true; } delete this->regionMapEditor; return false; @@ -2974,6 +2972,29 @@ bool MainWindow::initRegionMapEditor(bool silent) { return true; } +bool MainWindow::askToFixRegionMapEditor() { + QMessageBox msgBox; + msgBox.setIcon(QMessageBox::Critical); + msgBox.setText(QString("There was an error opening the region map data. Please see %1 for full error details.").arg(getLogPath())); + msgBox.setDetailedText(getMostRecentError()); + msgBox.setStandardButtons(QMessageBox::Ok); + msgBox.setDefaultButton(QMessageBox::Ok); + auto reconfigButton = msgBox.addButton("Reconfigure", QMessageBox::ActionRole); + msgBox.exec(); + if (msgBox.clickedButton() == reconfigButton) { + if (this->regionMapEditor->reconfigure()) { + // User fixed error + return true; + } + if (this->regionMapEditor->setupErrored()) { + // User's new settings still fail, show error and ask again + return this->askToFixRegionMapEditor(); + } + } + // User accepted error + return false; +} + void MainWindow::closeSupplementaryWindows() { delete this->tilesetEditor; delete this->regionMapEditor; diff --git a/src/ui/regionmapeditor.cpp b/src/ui/regionmapeditor.cpp index ee491a3e..0f75a730 100644 --- a/src/ui/regionmapeditor.cpp +++ b/src/ui/regionmapeditor.cpp @@ -599,6 +599,11 @@ void RegionMapEditor::on_actionSave_All_triggered() { } void RegionMapEditor::on_action_Configure_triggered() { + reconfigure(); +} + +bool RegionMapEditor::reconfigure() { + this->setupError = false; if (this->modified()) { QMessageBox warning; warning.setIcon(QMessageBox::Warning); @@ -609,15 +614,16 @@ void RegionMapEditor::on_action_Configure_triggered() { if (warning.exec() == QMessageBox::Ok) { if (buildConfigDialog()) { - reload(); + return reload(); } } } else { if (buildConfigDialog()) { - reload(); + return reload(); } } + return false; } void RegionMapEditor::displayRegionMap() { From 79955715dd36051a31cdaf414465cc516b2ef446 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Mon, 17 Jun 2024 14:32:02 -0400 Subject: [PATCH 5/6] Fix crash if region map tileset file is too small --- src/core/regionmap.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/core/regionmap.cpp b/src/core/regionmap.cpp index 45fc1141..7dbd4c5f 100644 --- a/src/core/regionmap.cpp +++ b/src/core/regionmap.cpp @@ -79,12 +79,17 @@ bool RegionMap::loadTilemap(poryjson::Json tilemapJson) { this->palette_path = tilemapObject["palette"].string_value(); } - QFileInfo tilesetFileInfo(fullPath(this->tileset_path)); - if (!tilesetFileInfo.exists() || !tilesetFileInfo.isFile()) { + QImage tilesetFile(fullPath(this->tileset_path)); + if (tilesetFile.isNull()) { logError(QString("Failed to open region map tileset file '%1'.").arg(tileset_path)); return false; } + if (tilesetFile.width() < 8 || tilesetFile.height() < 8) { + logError(QString("Region map tileset file '%1' must be at least 8x8.").arg(tileset_path)); + return false; + } + QFile tilemapFile(fullPath(this->tilemap_path)); if (!tilemapFile.open(QIODevice::ReadOnly)) { logError(QString("Failed to open region map tilemap file '%1'.").arg(tilemap_path)); From 06b6651e46cd5e52cd327d7b1e1bb8cf910d5edf Mon Sep 17 00:00:00 2001 From: GriffinR Date: Mon, 17 Jun 2024 14:32:19 -0400 Subject: [PATCH 6/6] Update changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6d13ce3..9c9977a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,10 @@ and this project somewhat adheres to [Semantic Versioning](https://semver.org/sp The **"Breaking Changes"** listed below are changes that have been made in the decompilation projects (e.g. pokeemerald), which porymap requires in order to work properly. It also includes changes to the scripting API that may change the behavior of existing porymap scripts. If porymap is used with a project or API script that is not up-to-date with the breaking changes, then porymap will likely break or behave improperly. ## [Unreleased] -Nothing, yet. +### Fixed +- Fix `Add Region Map...` not updating the region map settings file. +- Fix some crashes on invalid region map tilesets. +- Improve error reporting for invalid region map editor settings. ## [5.4.1] - 2024-03-21 ### Fixed