From 9a3b45117b40348ede3e302148940ef8e2606ad0 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Wed, 7 May 2025 13:45:16 -0400 Subject: [PATCH 1/4] Make map load failure non-destructive --- include/core/map.h | 4 + include/mainwindow.h | 9 +- include/project.h | 52 +++--- include/ui/maplistmodels.h | 1 + resources/icons/map_errored.ico | Bin 0 -> 606 bytes resources/images.qrc | 1 + src/core/map.cpp | 16 +- src/editor.cpp | 8 +- src/mainwindow.cpp | 80 ++++----- src/project.cpp | 284 +++++++++++++++++--------------- src/scriptapi/apiutility.cpp | 11 +- src/ui/eventframes.cpp | 4 +- src/ui/mapimageexporter.cpp | 8 +- src/ui/maplistmodels.cpp | 44 ++--- src/ui/newmapdialog.cpp | 8 +- src/ui/tileseteditor.cpp | 8 +- 16 files changed, 283 insertions(+), 255 deletions(-) create mode 100644 resources/icons/map_errored.ico diff --git a/include/core/map.h b/include/core/map.h index 2ba172ce..199968fd 100644 --- a/include/core/map.h +++ b/include/core/map.h @@ -48,6 +48,9 @@ public: void setLayout(Layout *layout); Layout* layout() const { return m_layout; } + void setLayoutId(const QString &layoutId) { m_layoutId = layoutId; } + QString layoutId() const { return layout() ? layout()->id : m_layoutId; } + int getWidth() const; int getHeight() const; int getBorderWidth() const; @@ -107,6 +110,7 @@ public: private: QString m_name; QString m_constantName; + QString m_layoutId; // Only needed if layout fails to load. QString m_sharedEventsMap = ""; QString m_sharedScriptsMap = ""; diff --git a/include/mainwindow.h b/include/mainwindow.h index 29397b4c..4714e54d 100644 --- a/include/mainwindow.h +++ b/include/mainwindow.h @@ -342,11 +342,11 @@ private: bool tilesetNeedsRedraw = false; - bool setLayout(QString layoutId); - bool setMap(QString); + bool setLayout(const QString &layoutId); + bool setMap(const QString &mapName); void unsetMap(); - bool userSetLayout(QString layoutId); - bool userSetMap(QString); + bool userSetLayout(const QString &layoutId); + bool userSetMap(const QString &mapName); void redrawMapScene(); void refreshMapScene(); void refreshMetatileViews(); @@ -380,7 +380,6 @@ private: bool closeProject(); void showRecentError(const QString &baseMessage); void showProjectOpenFailure(); - void showMapsExcludedAlert(const QStringList &excludedMapNames); bool setInitialMap(); void saveGlobalConfigs(); diff --git a/include/project.h b/include/project.h index 8b739858..e418c712 100644 --- a/include/project.h +++ b/include/project.h @@ -31,17 +31,11 @@ public: public: QString root; - QStringList mapNames; QStringList groupNames; QMap groupNameToMapNames; QStringList healLocationSaveOrder; QMap> healLocations; QMap mapConstantsToMapNames; - QString layoutsLabel; - QStringList layoutIds; - QStringList layoutIdsMaster; - QMap mapLayouts; - QMap mapLayoutsMaster; QMap gfxDefines; QString defaultSong; QStringList songNames; @@ -78,6 +72,27 @@ public: void setRoot(const QString&); + const QStringList& mapNames() const { return this->alphabeticalMapNames; } + bool isKnownMap(const QString &mapName) const { return this->maps.contains(mapName); } + bool isErroredMap(const QString &mapName) const { return this->erroredMaps.contains(mapName); } + bool isLoadedMap(const QString &mapName) const { return this->loadedMapNames.contains(mapName); } + bool isUnsavedMap(const QString &mapName) const; + + // Note: This does not guarantee the map is loaded. + Map* getMap(const QString &mapName) { return this->maps.value(mapName); } + Map* loadMap(const QString &mapName); + + const QStringList& layoutIds() const { return this->alphabeticalLayoutIds; } + bool isKnownLayout(const QString &layoutId) const { return this->mapLayouts.contains(layoutId); } + bool isLoadedLayout(const QString &layoutId) const { return this->loadedLayoutIds.contains(layoutId); } + bool isUnsavedLayout(const QString &layoutId) const; + QString getLayoutName(const QString &layoutId) const; + QStringList getLayoutNames() const; + + // Note: This does not guarantee the layout is loaded. + Layout* getLayout(const QString &layoutId) const { return this->mapLayouts.value(layoutId); } + Layout* loadLayout(const QString &layoutId); + void clearMaps(); void clearTilesetCache(); void clearMapLayouts(); @@ -88,16 +103,6 @@ public: int getSupportedMajorVersion(QString *errorOut = nullptr); bool load(); - Map* loadMap(const QString &mapName); - - // Note: This does not guarantee the map is loaded. - Map* getMap(const QString &mapName) { return this->maps.value(mapName); } - - bool isMapLoaded(const Map *map) const { return map && isMapLoaded(map->name()); } - bool isMapLoaded(const QString &mapName) const { return this->loadedMapNames.contains(mapName); } - bool isLayoutLoaded(const Layout *layout) const { return layout && isLayoutLoaded(layout->id); } - bool isLayoutLoaded(const QString &layoutId) const { return this->loadedLayoutIds.contains(layoutId); } - QMap tilesetCache; Tileset* loadTileset(QString, Tileset *tileset = nullptr); Tileset* getTileset(QString, bool forceLoad = false); @@ -158,13 +163,9 @@ public: bool hasUnsavedChanges(); bool hasUnsavedDataChanges = false; - bool readMapJson(const QString &mapName, QJsonDocument * out); bool loadMapEvent(Map *map, QJsonObject json, Event::Type defaultType = Event::Type::None); bool loadMapData(Map*); bool readMapLayouts(); - Layout *loadLayout(QString layoutId); - bool loadLayout(Layout *); - bool loadMapLayout(Map*); bool loadLayoutTilesets(Layout *); bool loadTilesetAssets(Tileset*); void loadTilesetMetatileLabels(Tileset*); @@ -265,6 +266,14 @@ private: QMap facingDirections; QHash speciesToIconPath; QHash maps; + QHash erroredMaps; + QStringList alphabeticalMapNames; + QString layoutsLabel; + QStringList alphabeticalLayoutIds; + QStringList orderedLayoutIds; + QStringList orderedLayoutIdsMaster; + QHash mapLayouts; + QHash mapLayoutsMaster; // Fields for preserving top-level JSON data that Porymap isn't expecting. QJsonObject customLayoutsData; @@ -307,6 +316,8 @@ private: }; QHash locationData; + QJsonDocument readMapJson(const QString &mapName, QString *error = nullptr); + void setNewLayoutBlockdata(Layout *layout); void setNewLayoutBorder(Layout *layout); @@ -349,7 +360,6 @@ signals: void mapSectionAdded(const QString &idName); void mapSectionDisplayNameChanged(const QString &idName, const QString &displayName); void mapSectionIdNamesChanged(const QStringList &idNames); - void mapsExcluded(const QStringList &excludedMapNames); void eventScriptLabelsRead(); }; diff --git a/include/ui/maplistmodels.h b/include/ui/maplistmodels.h index 681f0955..ed2c1a09 100644 --- a/include/ui/maplistmodels.h +++ b/include/ui/maplistmodels.h @@ -84,6 +84,7 @@ protected: QIcon mapGrayIcon; QIcon mapIcon; QIcon mapEditedIcon; + QIcon mapErroredIcon; QIcon mapOpenedIcon; QIcon mapFolderIcon; QIcon emptyMapFolderIcon; diff --git a/resources/icons/map_errored.ico b/resources/icons/map_errored.ico new file mode 100644 index 0000000000000000000000000000000000000000..c34396705b79bcded5919bbe391cf3bc11a4089f GIT binary patch literal 606 zcmV-k0-^nhP)Px%7)eAyR5*>5le=qFK@`S+cOLh(Tn&Msgn*R+3qdPMrpe-rlBMF4J}_+L;{3Aq+!;wP`NJ z=U{yS{0P<;DBhgJ`+T1u2oOSaITQfK7?LExsoiDl)&xi7SMYrN;&Jc;Sbt7&Y63pp zpwi%U9srDo7uM z1Ikhe3!pV*4j2pHS96_G2LakO@$(}{FE=FsX$Zm)qy{Cx0GM2NqN6MOtbYi64O-&oF-EB3-0I)b)CE4l8MM{J8cyM(sccnCVnq;Sk#o20ifc9{_+d9um zwFZ5I;CUcjF5ZhkXmDNV8)T(g<9+?Y-vI!kDB|VjSr%vBlGFt#&DIAu&)z#gW#$bp zHqPMte)nj`acr%%w%KgjG))h+thF|dV+)|vUQ1L*YYv0`d(CF^+3@i2icons/link.ico icons/magnifier.ico icons/map_edited.ico + icons/map_errored.ico icons/map_opened.ico icons/map.ico icons/map_grayed.ico diff --git a/src/core/map.cpp b/src/core/map.cpp index c1905f0a..a006e47c 100644 --- a/src/core/map.cpp +++ b/src/core/map.cpp @@ -56,6 +56,9 @@ void Map::setLayout(Layout *layout) { if (layout == m_layout) return; m_layout = layout; + if (layout) { + m_layoutId = layout->id; + } emit layoutChanged(); } @@ -65,19 +68,19 @@ QString Map::mapConstantFromName(const QString &name) { } int Map::getWidth() const { - return m_layout->getWidth(); + return m_layout ? m_layout->getWidth() : 0; } int Map::getHeight() const { - return m_layout->getHeight(); + return m_layout ? m_layout->getHeight() : 0; } int Map::getBorderWidth() const { - return m_layout->getBorderWidth(); + return m_layout ? m_layout->getBorderWidth() : 0; } int Map::getBorderHeight() const { - return m_layout->getBorderHeight(); + return m_layout ? m_layout->getBorderHeight() : 0; } // Get the portion of the map that can be rendered when rendered as a map connection. @@ -111,6 +114,9 @@ QRect Map::getConnectionRect(const QString &direction, Layout * fromLayout) cons } QPixmap Map::renderConnection(const QString &direction, Layout * fromLayout) { + if (!m_layout) + return QPixmap(); + QRect bounds = getConnectionRect(direction, fromLayout); if (!bounds.isValid()) return QPixmap(); @@ -365,7 +371,7 @@ void Map::setClean() { } bool Map::hasUnsavedChanges() const { - return !m_editHistory->isClean() || m_layout->hasUnsavedChanges() || m_hasUnsavedDataChanges || !m_isPersistedToFile; + return !m_editHistory->isClean() || (m_layout && m_layout->hasUnsavedChanges()) || m_hasUnsavedDataChanges || !m_isPersistedToFile; } void Map::pruneEditHistory() { diff --git a/src/editor.cpp b/src/editor.cpp index f5759e35..6b8b9a09 100644 --- a/src/editor.cpp +++ b/src/editor.cpp @@ -779,7 +779,7 @@ void Editor::displayConnection(MapConnection *connection) { connect(pixmapItem, &ConnectionPixmapItem::positionChanged, this, &Editor::maskNonVisibleConnectionTiles); // Create item for the list panel - auto listItem = new ConnectionsListItem(ui->scrollAreaContents_ConnectionsList, pixmapItem->connection, project->mapNames); + auto listItem = new ConnectionsListItem(ui->scrollAreaContents_ConnectionsList, pixmapItem->connection, project->mapNames()); ui->layout_ConnectionsList->insertWidget(ui->layout_ConnectionsList->count() - 1, listItem); // Insert above the vertical spacer // Double clicking the pixmap or clicking the list item's map button opens the connected map @@ -939,7 +939,7 @@ void Editor::removeDivingMapPixmap(MapConnection *connection) { } bool Editor::setDivingMapName(const QString &mapName, const QString &direction) { - if (!mapName.isEmpty() && !this->project->mapNames.contains(mapName)) + if (!mapName.isEmpty() && !this->project->isKnownMap(mapName)) return false; if (!MapConnection::isDiving(direction)) return false; @@ -977,7 +977,7 @@ void Editor::onDivingMapEditingFinished(NoScrollComboBox *combo, const QString & } void Editor::updateDivingMapButton(QToolButton* button, const QString &mapName) { - if (this->project) button->setDisabled(!this->project->mapNames.contains(mapName)); + if (this->project) button->setDisabled(!this->project->isKnownMap(mapName)); } void Editor::updateDivingMapsVisibility() { @@ -1227,7 +1227,7 @@ bool Editor::setMap(QString map_name) { unsetMap(); this->map = loadedMap; - setLayout(map->layout()->id); + setLayout(map->layoutId()); editGroup.addStack(map->editHistory()); editGroup.setActiveStack(map->editHistory()); diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index ee792d5d..f633606a 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -678,7 +678,6 @@ bool MainWindow::openProject(QString dir, bool initial) { connect(project, &Project::mapGroupAdded, this, &MainWindow::onNewMapGroupCreated); connect(project, &Project::mapSectionAdded, this, &MainWindow::onNewMapSectionCreated); connect(project, &Project::mapSectionDisplayNameChanged, this, &MainWindow::onMapSectionDisplayNameChanged); - connect(project, &Project::mapsExcluded, this, &MainWindow::showMapsExcludedAlert); this->editor->setProject(project); // Make sure project looks reasonable before attempting to load it @@ -798,19 +797,6 @@ void MainWindow::showProjectOpenFailure() { RecentErrorMessage::show(QStringLiteral("There was an error opening the project."), this); } -// Alert the user that one or more maps have been excluded while loading the project. -void MainWindow::showMapsExcludedAlert(const QStringList &excludedMapNames) { - auto msgBox = new RecentErrorMessage("", this); - msgBox->setAttribute(Qt::WA_DeleteOnClose); - if (excludedMapNames.length() == 1) { - msgBox->setText(QString("Failed to load map '%1'. Saving will exclude this map from your project.").arg(excludedMapNames.first())); - } else { - msgBox->setText(QStringLiteral("Failed to load the maps listed below. Saving will exclude these maps from your project.")); - msgBox->setDetailedText(excludedMapNames.join("\n")); // Overwrites error details text, user will need to check the log. - } - msgBox->open(); -} - bool MainWindow::isProjectOpen() { return editor && editor->project; } @@ -819,22 +805,22 @@ bool MainWindow::setInitialMap() { porysplash->showMessage("Opening initial map"); const QString recent = userConfig.recentMapOrLayout; - if (editor->project->mapNames.contains(recent)) { + if (editor->project->isKnownMap(recent)) { // User recently had a map open that still exists. if (setMap(recent)) return true; - } else if (editor->project->layoutIds.contains(recent)) { + } else if (editor->project->isKnownLayout(recent)) { // User recently had a layout open that still exists. if (setLayout(recent)) return true; } // Failed to open recent map/layout, or no recent map/layout. Try opening maps then layouts sequentially. - for (const auto &name : editor->project->mapNames) { + for (const auto &name : editor->project->mapNames()) { if (name != recent && setMap(name)) return true; } - for (const auto &id : editor->project->layoutIds) { + for (const auto &id : editor->project->layoutIds()) { if (id != recent && setLayout(id)) return true; } @@ -966,39 +952,39 @@ void MainWindow::unsetMap() { // setMap, but with a visible error message in case of failure. // Use when the user is specifically requesting a map to open. -bool MainWindow::userSetMap(QString map_name) { - if (editor->map && editor->map->name() == map_name) +bool MainWindow::userSetMap(const QString &mapName) { + if (editor->map && editor->map->name() == mapName) return true; // Already set - if (map_name.isEmpty()) { + if (mapName.isEmpty()) { WarningMessage::show(QStringLiteral("Cannot open map with empty name."), this); return false; } - if (map_name == editor->project->getDynamicMapName()) { - auto msgBox = new WarningMessage(QString("Cannot open map '%1'.").arg(map_name), this); + if (mapName == editor->project->getDynamicMapName()) { + auto msgBox = new WarningMessage(QString("Cannot open map '%1'.").arg(mapName), this); msgBox->setAttribute(Qt::WA_DeleteOnClose); msgBox->setInformativeText(QStringLiteral("This map name is a placeholder to indicate that the warp's map will be set programmatically.")); msgBox->open(); return false; } - if (!setMap(map_name)) { - RecentErrorMessage::show(QString("There was an error opening map '%1'.").arg(map_name), this); + if (!setMap(mapName)) { + RecentErrorMessage::show(QString("There was an error opening map '%1'.").arg(mapName), this); return false; } return true; } -bool MainWindow::setMap(QString map_name) { - if (!editor || !editor->project || map_name.isEmpty() || map_name == editor->project->getDynamicMapName()) { - logWarn(QString("Ignored setting map to '%1'").arg(map_name)); +bool MainWindow::setMap(const QString &mapName) { + if (!editor || !editor->project || mapName.isEmpty() || mapName == editor->project->getDynamicMapName()) { + logWarn(QString("Ignored setting map to '%1'").arg(mapName)); return false; } - logInfo(QString("Setting map to '%1'").arg(map_name)); - if (!editor->setMap(map_name)) { - logWarn(QString("Failed to set map to '%1'").arg(map_name)); + logInfo(QString("Setting map to '%1'").arg(mapName)); + if (!editor->setMap(mapName)) { + logWarn(QString("Failed to set map to '%1'").arg(mapName)); return false; } @@ -1018,9 +1004,9 @@ bool MainWindow::setMap(QString map_name) { connect(editor->layout, &Layout::needsRedrawing, this, &MainWindow::redrawMapScene, Qt::UniqueConnection); - userConfig.recentMapOrLayout = map_name; + userConfig.recentMapOrLayout = mapName; - Scripting::cb_MapOpened(map_name); + Scripting::cb_MapOpened(mapName); prefab.updatePrefabUi(editor->layout); updateTilesetEditor(); @@ -1043,7 +1029,7 @@ void MainWindow::setLayoutOnlyMode(bool layoutOnly) { // setLayout, but with a visible error message in case of failure. // Use when the user is specifically requesting a layout to open. -bool MainWindow::userSetLayout(QString layoutId) { +bool MainWindow::userSetLayout(const QString &layoutId) { if (!setLayout(layoutId)) { RecentErrorMessage::show(QString("There was an error opening layout '%1'.").arg(layoutId), this); return false; @@ -1055,10 +1041,10 @@ bool MainWindow::userSetLayout(QString layoutId) { return true; } -bool MainWindow::setLayout(QString layoutId) { +bool MainWindow::setLayout(const QString &layoutId) { // Prefer logging the name of the layout as displayed in the map list. - const Layout* layout = this->editor->project ? this->editor->project->mapLayouts.value(layoutId) : nullptr; - logInfo(QString("Setting layout to '%1'").arg(layout ? layout->name : layoutId)); + QString layoutName = this->editor->project ? this->editor->project->getLayoutName(layoutId) : QString(); + logInfo(QString("Setting layout to '%1'").arg(layoutName.isEmpty() ? layoutId : layoutName)); if (!this->editor->setLayout(layoutId)) { return false; @@ -1197,7 +1183,7 @@ void MainWindow::on_comboBox_LayoutSelector_currentTextChanged(const QString &te if (!this->editor || !this->editor->project || !this->editor->map) return; - if (!this->editor->project->mapLayouts.contains(text)) { + if (!this->editor->project->isKnownLayout(text)) { // User may be in the middle of typing the name of a layout, don't bother trying to load it. return; } @@ -1208,7 +1194,7 @@ void MainWindow::on_comboBox_LayoutSelector_currentTextChanged(const QString &te // New layout failed to load, restore previous layout const QSignalBlocker b(ui->comboBox_LayoutSelector); - ui->comboBox_LayoutSelector->setTextItem(this->editor->map->layout()->id); + ui->comboBox_LayoutSelector->setTextItem(this->editor->map->layoutId()); return; } this->editor->map->setLayout(layout); @@ -1222,7 +1208,7 @@ void MainWindow::onLayoutSelectorEditingFinished() { // If the user left the layout selector in an invalid state, restore it so that it displays the current layout. const QString text = ui->comboBox_LayoutSelector->currentText(); - if (!this->editor->project->mapLayouts.contains(text)) { + if (!this->editor->project->isKnownLayout(text)) { const QSignalBlocker b(ui->comboBox_LayoutSelector); ui->comboBox_LayoutSelector->setTextItem(this->editor->layout->id); } @@ -1247,17 +1233,17 @@ bool MainWindow::setProjectUI() { const QSignalBlocker b_LayoutSelector(ui->comboBox_LayoutSelector); ui->comboBox_LayoutSelector->clear(); - ui->comboBox_LayoutSelector->addItems(project->layoutIds); + ui->comboBox_LayoutSelector->addItems(project->layoutIds()); const QSignalBlocker b_DiveMap(ui->comboBox_DiveMap); ui->comboBox_DiveMap->clear(); - ui->comboBox_DiveMap->addItems(project->mapNames); + ui->comboBox_DiveMap->addItems(project->mapNames()); ui->comboBox_DiveMap->setClearButtonEnabled(true); ui->comboBox_DiveMap->setFocusedScrollingEnabled(false); const QSignalBlocker b_EmergeMap(ui->comboBox_EmergeMap); ui->comboBox_EmergeMap->clear(); - ui->comboBox_EmergeMap->addItems(project->mapNames); + ui->comboBox_EmergeMap->addItems(project->mapNames()); ui->comboBox_EmergeMap->setClearButtonEnabled(true); ui->comboBox_EmergeMap->setFocusedScrollingEnabled(false); @@ -1484,11 +1470,11 @@ void MainWindow::onNewMapCreated(Map *newMap, const QString &groupName) { // Add new map to the map lists this->mapGroupModel->insertMapItem(newMap->name(), groupName); this->mapLocationModel->insertMapItem(newMap->name(), newMap->header()->location()); - this->layoutTreeModel->insertMapItem(newMap->name(), newMap->layout()->id); + this->layoutTreeModel->insertMapItem(newMap->name(), newMap->layoutId()); // Refresh any combo box that displays map names and persists between maps // (other combo boxes like for warp destinations are repopulated when the map changes). - int mapIndex = this->editor->project->mapNames.indexOf(newMap->name()); + int mapIndex = this->editor->project->mapNames().indexOf(newMap->name()); if (mapIndex >= 0) { ui->comboBox_DiveMap->insertItem(mapIndex, newMap->name()); ui->comboBox_EmergeMap->insertItem(mapIndex, newMap->name()); @@ -1502,7 +1488,7 @@ void MainWindow::onNewLayoutCreated(Layout *layout) { logInfo(QString("Created a new layout named %1.").arg(layout->name)); // Refresh layout combo box - int layoutIndex = this->editor->project->layoutIds.indexOf(layout->id); + int layoutIndex = this->editor->project->layoutIds().indexOf(layout->id); if (layoutIndex >= 0) { const QSignalBlocker b(ui->comboBox_LayoutSelector); ui->comboBox_LayoutSelector->insertItem(layoutIndex, layout->id); @@ -2707,7 +2693,7 @@ void MainWindow::on_pushButton_AddConnection_clicked() { if (!this->editor || !this->editor->map || !this->editor->project) return; - auto dialog = new NewMapConnectionDialog(this, this->editor->map, this->editor->project->mapNames); + auto dialog = new NewMapConnectionDialog(this, this->editor->map, this->editor->project->mapNames()); connect(dialog, &NewMapConnectionDialog::newConnectionedAdded, this->editor, &Editor::addNewConnection); connect(dialog, &NewMapConnectionDialog::connectionReplaced, this->editor, &Editor::replaceConnection); dialog->open(); diff --git a/src/project.cpp b/src/project.cpp index ec2a8a6f..42dd7e20 100644 --- a/src/project.cpp +++ b/src/project.cpp @@ -280,24 +280,61 @@ Map* Project::loadMap(const QString &mapName) { return nullptr; } + // Some maps are ignored while opening the project because they have invalid or incomplete data. + // We already logged a warning about this, but now that we're trying to load the map it's an error. + auto it = this->erroredMaps.constFind(mapName); + if (it != this->erroredMaps.constEnd()) { + logError(it.value()); + return nullptr; + } + Map* map = this->maps.value(mapName); if (!map) { logError(QString("Unknown map name '%1'.").arg(mapName)); return nullptr; } - if (isMapLoaded(map)) + if (isLoadedMap(mapName)) return map; - if (!(loadMapData(map) && loadMapLayout(map))) + if (!loadMapData(map)) return nullptr; + // Load map layout + if (map->isPersistedToFile() && !map->hasUnsavedChanges()) { + if (!loadLayout(map->layoutId())) + return nullptr; + } + this->loadedMapNames.insert(mapName); emit mapLoaded(map); return map; } +Layout *Project::loadLayout(const QString &layoutId) { + Layout *layout = this->mapLayouts.value(layoutId); + if (!layout) { + logError(QString("Unknown layout ID '%1'.").arg(layoutId)); + return nullptr; + } + + if (isLoadedLayout(layoutId)) + return layout; + + // Force these to run even if one fails + bool loadedTilesets = loadLayoutTilesets(layout); + bool loadedBlockdata = loadBlockdata(layout); + bool loadedBorder = loadLayoutBorder(layout); + if (!loadedTilesets || !loadedBlockdata || !loadedBorder) { + // Error should already be logged. + return nullptr; + } + + this->loadedLayoutIds.insert(layoutId); + return layout; +} + QSet Project::getTopLevelMapFields() const { QSet fields = { "id", @@ -329,15 +366,17 @@ QSet Project::getTopLevelMapFields() const { return fields; } -bool Project::readMapJson(const QString &mapName, QJsonDocument * out) { +QJsonDocument Project::readMapJson(const QString &mapName, QString *error) { const QString mapFilepath = QString("%1%2/map.json").arg(projectConfig.getFilePath(ProjectFilePath::data_map_folders)).arg(mapName); watchFile(mapFilepath); - QString error; - if (!parser.tryParseJsonFile(out, mapFilepath, &error)) { - logError(QString("Failed to read map data from '%1': %2").arg(mapFilepath).arg(error)); - return false; + + QJsonDocument doc; + if (!parser.tryParseJsonFile(&doc, mapFilepath, error)) { + if (error) { + error->prepend(QString("Failed to read map data from '%1': ").arg(mapFilepath)); + } } - return true; + return doc; } bool Project::loadMapEvent(Map *map, QJsonObject json, Event::Type defaultType) { @@ -360,9 +399,12 @@ bool Project::loadMapData(Map* map) { return true; } - QJsonDocument mapDoc; - if (!readMapJson(map->name(), &mapDoc)) + QString error; + QJsonDocument mapDoc = readMapJson(map->name(), &error); + if (!error.isEmpty()) { + logError(error); return false; + } QJsonObject mapObj = mapDoc.object(); @@ -452,24 +494,15 @@ Map *Project::createNewMap(const Project::NewMapSettings &settings, const Map* t // Generate a unique MAP constant. map->setConstantName(toUniqueIdentifier(map->expectedConstantName())); - // Make sure we keep the order of the map names the same as in the map group order. - int mapNamePos; - if (this->groupNames.contains(settings.group)) { - mapNamePos = 0; - for (const auto &name : this->groupNames) { - mapNamePos += this->groupNameToMapNames[name].length(); - if (name == settings.group) - break; - } - } else if (isValidNewIdentifier(settings.group)) { + if (!this->groupNames.contains(settings.group)) { // Adding map to a map group that doesn't exist yet. - // Create the group, and we already know the map will be last in the list. - addNewMapGroup(settings.group); - mapNamePos = this->mapNames.length(); - } else { - logError(QString("Cannot create new map with invalid map group name '%1'.").arg(settings.group)); - delete map; - return nullptr; + if (isValidNewIdentifier(settings.group)) { + addNewMapGroup(settings.group); + } else { + logError(QString("Cannot create new map with invalid map group name '%1'.").arg(settings.group)); + delete map; + return nullptr; + } } Layout *layout = this->mapLayouts.value(settings.layout.id); @@ -483,16 +516,20 @@ Map *Project::createNewMap(const Project::NewMapSettings &settings, const Map* t } } else { // This layout already exists. Make sure it's loaded. - loadLayout(layout); + if (!loadLayout(settings.layout.id)) { + // Layout failed to load. For now we can just record the ID. + map->setLayoutId(settings.layout.id); + } } map->setLayout(layout); // Try to record the MAPSEC name in case this is a new name. addNewMapsec(map->header()->location()); - this->mapNames.insert(mapNamePos, map->name()); this->groupNameToMapNames[settings.group].append(map->name()); this->mapConstantsToMapNames.insert(map->constantName(), map->name()); + this->alphabeticalMapNames.append(map->name()); + Util::numericalModeSort(this->alphabeticalMapNames); map->setIsPersistedToFile(false); this->maps.insert(map->name(), map); @@ -503,7 +540,7 @@ Map *Project::createNewMap(const Project::NewMapSettings &settings, const Map* t } Layout *Project::createNewLayout(const Layout::Settings &settings, const Layout *toDuplicate) { - if (this->layoutIds.contains(settings.id)) + if (this->mapLayouts.contains(settings.id)) return nullptr; Layout *layout = toDuplicate ? new Layout(*toDuplicate) : new Layout(); @@ -540,60 +577,24 @@ Layout *Project::createNewLayout(const Layout::Settings &settings, const Layout } this->mapLayouts.insert(layout->id, layout); - this->layoutIds.append(layout->id); + this->orderedLayoutIds.append(layout->id); this->loadedLayoutIds.insert(layout->id); + this->alphabeticalLayoutIds.append(layout->id); + Util::numericalModeSort(this->alphabeticalLayoutIds); emit layoutCreated(layout); return layout; } -bool Project::loadLayout(Layout *layout) { - if (!isLayoutLoaded(layout)) { - // Force these to run even if one fails - bool loadedTilesets = loadLayoutTilesets(layout); - bool loadedBlockdata = loadBlockdata(layout); - bool loadedBorder = loadLayoutBorder(layout); - - if (loadedTilesets && loadedBlockdata && loadedBorder) { - this->loadedLayoutIds.insert(layout->id); - return true; - } else { - return false; - } - } - return true; -} - -Layout *Project::loadLayout(QString layoutId) { - Layout *layout = this->mapLayouts.value(layoutId); - if (!layout) { - logError(QString("Unknown layout ID '%1'.").arg(layoutId)); - return nullptr; - } - - if (!loadLayout(layout)) { - // Error should already be logged. - return nullptr; - } - return layout; -} - -bool Project::loadMapLayout(Map* map) { - if (!map->isPersistedToFile() || map->hasUnsavedChanges()) { - return true; - } else { - return loadLayout(map->layout()); - } -} - void Project::clearMapLayouts() { qDeleteAll(this->mapLayouts); this->mapLayouts.clear(); qDeleteAll(this->mapLayoutsMaster); this->mapLayoutsMaster.clear(); - this->layoutIds.clear(); - this->layoutIdsMaster.clear(); + this->alphabeticalLayoutIds.clear(); + this->orderedLayoutIds.clear(); + this->orderedLayoutIdsMaster.clear(); this->loadedLayoutIds.clear(); this->customLayoutsData = QJsonObject(); } @@ -700,8 +701,8 @@ bool Project::readMapLayouts() { this->mapLayouts.insert(layout->id, layout); this->mapLayoutsMaster.insert(layout->id, layout->copy()); - this->layoutIds.append(layout->id); - this->layoutIdsMaster.append(layout->id); + this->orderedLayoutIds.append(layout->id); + this->orderedLayoutIdsMaster.append(layout->id); } if (this->mapLayouts.isEmpty()) { @@ -709,6 +710,9 @@ bool Project::readMapLayouts() { return false; } + this->alphabeticalLayoutIds = this->orderedLayoutIds; + Util::numericalModeSort(this->alphabeticalLayoutIds); + this->customLayoutsData = layoutsObj; return true; @@ -726,7 +730,7 @@ bool Project::saveMapLayouts() { layoutsObj["layouts_table_label"] = this->layoutsLabel; OrderedJson::array layoutsArr; - for (const QString &layoutId : this->layoutIdsMaster) { + for (const QString &layoutId : this->orderedLayoutIdsMaster) { Layout *layout = this->mapLayoutsMaster.value(layoutId); OrderedJson::object layoutObj; layoutObj["id"] = layout->id; @@ -1255,7 +1259,7 @@ bool Project::saveAll() { } bool Project::saveMap(Map *map, bool skipLayout) { - if (!map || !isMapLoaded(map)) return true; + if (!map || !isLoadedMap(map->name())) return true; // Create/Modify a few collateral files for brand new maps. const QString folderPath = projectConfig.getFilePath(ProjectFilePath::data_map_folders) + map->name(); @@ -1295,7 +1299,7 @@ bool Project::saveMap(Map *map, bool skipLayout) { // Header values. mapObj["id"] = map->constantName(); mapObj["name"] = map->name(); - mapObj["layout"] = map->layout()->id; + mapObj["layout"] = map->layoutId(); mapObj["music"] = map->header()->song(); mapObj["region_map_section"] = map->header()->location(); mapObj["requires_flash"] = map->header()->requiresFlash(); @@ -1396,15 +1400,15 @@ bool Project::saveMap(Map *map, bool skipLayout) { } bool Project::saveLayout(Layout *layout) { - if (!layout || !isLayoutLoaded(layout)) + if (!layout || !isLoadedLayout(layout->id)) return true; if (!layout->save(this->root)) return false; // Update global data structures with current map data. - if (!this->layoutIdsMaster.contains(layout->id)) { - this->layoutIdsMaster.append(layout->id); + if (!this->orderedLayoutIdsMaster.contains(layout->id)) { + this->orderedLayoutIdsMaster.append(layout->id); } if (this->mapLayoutsMaster.contains(layout->id)) { @@ -1902,7 +1906,7 @@ bool Project::readWildMonData() { bool Project::readMapGroups() { clearMaps(); this->mapConstantsToMapNames.clear(); - this->mapNames.clear(); + this->alphabeticalMapNames.clear(); this->groupNames.clear(); this->groupNameToMapNames.clear(); this->customMapGroupsData = QJsonObject(); @@ -1923,7 +1927,6 @@ bool Project::readMapGroups() { const QString dynamicMapConstant = getDynamicMapDefineName(); // Process the map group lists - QStringList failedMapNames; for (int groupIndex = 0; groupIndex < mapGroupOrder.size(); groupIndex++) { const QString groupName = ParseUtil::jsonToQString(mapGroupOrder.at(groupIndex)); if (this->groupNames.contains(groupName)) { @@ -1937,87 +1940,82 @@ bool Project::readMapGroups() { // Process the names in this map group for (int j = 0; j < mapNamesJson.size(); j++) { const QString mapName = ParseUtil::jsonToQString(mapNamesJson.at(j)); + + // This list should accept all maps we find, valid or not. + // It will only be used to populate the map list panel. + // Always keeping the name prevents us from deleting it from the list + // if we're unable to load the rest of the necessary map data. + this->groupNameToMapNames[groupName].append(mapName); + if (mapName == dynamicMapName) { - logWarn(QString("Ignoring map with reserved name '%1'.").arg(mapName)); - failedMapNames.append(mapName); + logWarn(QString("Ignoring map %1 in map group '%2': Cannot use reserved map name '%3'.").arg(j).arg(groupName).arg(mapName)); continue; } - if (this->mapNames.contains(mapName)) { - logWarn(QString("Ignoring repeated map name '%1'.").arg(mapName)); - failedMapNames.append(mapName); + if (this->maps.contains(mapName)) { + logWarn(QString("Ignoring map %1 in map group '%2': Repeated map name '%3'.").arg(j).arg(groupName).arg(mapName)); continue; } // Load the map's json file so we can get its ID constant (and two other constants we use for the map list). - QJsonDocument mapDoc; - if (!readMapJson(mapName, &mapDoc)) { - failedMapNames.append(mapName); - continue; // Error message has already been logged + QString mapJsonError; + QJsonDocument mapDoc = readMapJson(mapName, &mapJsonError); + if (!mapJsonError.isEmpty()) { + this->erroredMaps.insert(mapName, mapJsonError); + logWarn(mapJsonError); + continue; } // Read and validate the map's ID from its JSON data. const QJsonObject mapObj = mapDoc.object(); const QString mapConstant = ParseUtil::jsonToQString(mapObj["id"]); if (mapConstant.isEmpty()) { - logWarn(QString("Map '%1' is missing an \"id\" value and will be ignored.").arg(mapName)); - failedMapNames.append(mapName); + QString message = QString("Map '%1' is invalid: Missing \"id\" value.").arg(mapName); + this->erroredMaps.insert(mapName, message); + logWarn(message); continue; } if (mapConstant == dynamicMapConstant) { - logWarn(QString("Ignoring map with reserved \"id\" value '%1'.").arg(mapName)); - failedMapNames.append(mapName); + QString message = QString("Map '%1' is invalid: Cannot use reserved name '%2' for \"id\" value.").arg(mapName).arg(mapConstant); + this->erroredMaps.insert(mapName, message); + logWarn(message); continue; } auto it = this->mapConstantsToMapNames.constFind(mapConstant); if (it != this->mapConstantsToMapNames.constEnd()) { - logWarn(QString("Map '%1' has the same \"id\" value '%2' as map '%3' and will be ignored.").arg(mapName).arg(it.key()).arg(it.value())); - failedMapNames.append(mapName); + QString message = QString("Map '%1' is invalid: Cannot use the same \"id\" value '%2' as map '%3'.") + .arg(mapName) + .arg(it.key()) + .arg(it.value()); + this->erroredMaps.insert(mapName, message); + logWarn(message); continue; } - // Read layout ID for map list - const QString layoutId = ParseUtil::jsonToQString(mapObj["layout"]); - if (!this->layoutIds.contains(layoutId)) { - // If a map has an unknown layout ID it won't be able to load it at all anyway, so skip it. - // Skipping these will let us assume all the map layout IDs are valid, which simplies some handling elsewhere. - logWarn(QString("Map '%1' has unknown \"layout\" value '%2' and will be ignored.").arg(mapName).arg(layoutId)); - failedMapNames.append(mapName); - continue; - } - - // Read MAPSEC name for map list - const QString mapSectionName = ParseUtil::jsonToQString(mapObj["region_map_section"]); - if (!this->mapSectionIdNames.contains(mapSectionName)) { - // An unknown location is OK. Aside from that name not appearing in the dropdowns this shouldn't cause problems. - // We'll log a warning, but allow this map to be displayed. - logWarn(QString("Map '%1' has unknown \"region_map_section\" value '%2'.").arg(mapName).arg(mapSectionName)); - } - // Success, create the Map object auto map = new Map; map->setName(mapName); map->setConstantName(mapConstant); - map->setLayout(this->mapLayouts.value(layoutId)); - map->header()->setLocation(mapSectionName); - this->maps.insert(mapName, map); - this->mapNames.append(mapName); - this->groupNameToMapNames[groupName].append(mapName); + this->maps.insert(mapName, map); + this->alphabeticalMapNames.append(mapName); this->mapConstantsToMapNames.insert(mapConstant, mapName); + + // Read layout ID for map list + const QString layoutId = ParseUtil::jsonToQString(mapObj["layout"]); + map->setLayoutId(layoutId); + map->setLayout(this->mapLayouts.value(layoutId)); // This may set layout to nullptr. Don't report anything until user tries to load this map. + + // Read MAPSEC name for map list + map->header()->setLocation(ParseUtil::jsonToQString(mapObj["region_map_section"])); } } // Note: Not successfully reading any maps or map groups is ok. We only require at least 1 map layout. - if (!failedMapNames.isEmpty()) { - // At least 1 map was excluded due to an error. - // User should be alerted of this, rather than just silently logging the details. - emit mapsExcluded(failedMapNames); - } - // Save special "Dynamic" constant this->mapConstantsToMapNames.insert(dynamicMapConstant, dynamicMapName); - this->mapNames.append(dynamicMapName); + this->alphabeticalMapNames.append(dynamicMapName); + Util::numericalModeSort(this->alphabeticalMapNames); // Chuck the "connections_include_order" field, this is only for matching. if (!projectConfig.preserveMatchingOnlyData) { @@ -2043,8 +2041,7 @@ void Project::addNewMapGroup(const QString &groupName) { QString Project::mapNameToMapGroup(const QString &mapName) const { for (auto it = this->groupNameToMapNames.constBegin(); it != this->groupNameToMapNames.constEnd(); it++) { - const QStringList mapNames = it.value(); - if (mapNames.contains(mapName)) { + if (it.value().contains(mapName)) { return it.key(); } } @@ -2060,7 +2057,7 @@ QString Project::getMapConstant(const QString &mapName, const QString &defaultVa QString Project::getMapLayoutId(const QString &mapName, const QString &defaultValue) const { Map* map = this->maps.value(mapName); - return (map && map->layout()) ? map->layout()->id : defaultValue; + return map ? map->layoutId() : defaultValue; } QString Project::getMapLocation(const QString &mapName, const QString &defaultValue) const { @@ -2068,6 +2065,29 @@ QString Project::getMapLocation(const QString &mapName, const QString &defaultVa return map ? map->header()->location() : defaultValue; } +QString Project::getLayoutName(const QString &layoutId) const { + Layout* layout = this->mapLayouts.value(layoutId); + return layout ? layout->name : QString(); +} + +QStringList Project::getLayoutNames() const { + QStringList names; + for (const auto &layoutId : this->alphabeticalLayoutIds) { + names.append(getLayoutName(layoutId)); + } + return names; +} + +bool Project::isUnsavedMap(const QString &mapName) const { + Map* map = this->maps.value(mapName); + return map ? map->hasUnsavedChanges() : false; +} + +bool Project::isUnsavedLayout(const QString &layoutId) const { + Layout* layout = this->mapLayouts.value(layoutId); + return layout ? layout->hasUnsavedChanges() : false; +} + // Determining which map a secret base ID refers to relies on assumptions about its name. // The default format is for a secret base ID of 'SECRET_BASE_FOO_#' to refer to a map with the constant // 'MAP_SECRET_BASE_FOO', so we strip the `_#` suffix and add the default map prefix 'MAP_'. If this fails, @@ -2105,7 +2125,7 @@ QString Project::secretBaseIdToMapName(const QString &secretBaseId) const { // In general this only matters to Porymap if the identifier will be added to the group it collides with, // but name collisions are likely undesirable in the project. bool Project::isIdentifierUnique(const QString &identifier) const { - if (this->mapNames.contains(identifier)) + if (this->maps.contains(identifier) || this->erroredMaps.contains(identifier)) return false; if (this->mapConstantsToMapNames.contains(identifier)) return false; @@ -2115,7 +2135,7 @@ bool Project::isIdentifierUnique(const QString &identifier) const { return false; if (this->tilesetLabelsOrdered.contains(identifier)) return false; - if (this->layoutIds.contains(identifier)) + if (this->mapLayouts.contains(identifier)) return false; for (const auto &layout : this->mapLayouts) { if (layout->name == identifier) { diff --git a/src/scriptapi/apiutility.cpp b/src/scriptapi/apiutility.cpp index 5d2072a9..dfc65617 100644 --- a/src/scriptapi/apiutility.cpp +++ b/src/scriptapi/apiutility.cpp @@ -246,7 +246,7 @@ void ScriptUtility::setMetatileLayerOpacity(QList order) { QList ScriptUtility::getMapNames() { if (!window || !window->editor || !window->editor->project) return QList(); - return window->editor->project->mapNames; + return window->editor->project->mapNames(); } QList ScriptUtility::getMapConstants() { @@ -256,18 +256,15 @@ QList ScriptUtility::getMapConstants() { } QList ScriptUtility::getLayoutNames() { - QList names; if (!window || !window->editor || !window->editor->project) - return names; - for (const auto &layout : window->editor->project->mapLayouts) - names.append(layout->name); - return names; + return {}; + return window->editor->project->getLayoutNames(); } QList ScriptUtility::getLayoutConstants() { if (!window || !window->editor || !window->editor->project) return QList(); - return window->editor->project->layoutIds; + return window->editor->project->layoutIds(); } QList ScriptUtility::getTilesetNames() { diff --git a/src/ui/eventframes.cpp b/src/ui/eventframes.cpp index 35d58831..6457e564 100644 --- a/src/ui/eventframes.cpp +++ b/src/ui/eventframes.cpp @@ -227,14 +227,14 @@ void EventFrame::populateMapNameDropdown(NoScrollComboBox * combo, Project * pro if (!project) return; - populateDropdown(combo, project->mapNames); + populateDropdown(combo, project->mapNames()); // This frame type displays map names, so when a new map is created we need to repopulate it. connect(project, &Project::mapCreated, this, &EventFrame::invalidateValues, Qt::UniqueConnection); } void EventFrame::populateIdNameDropdown(NoScrollComboBox * combo, Project * project, const QString &mapName, Event::Group group) { - if (!project || !project->mapNames.contains(mapName)) + if (!project || !project->isKnownMap(mapName)) return; Map *map = project->loadMap(mapName); diff --git a/src/ui/mapimageexporter.cpp b/src/ui/mapimageexporter.cpp index 05664aef..59efbe4b 100644 --- a/src/ui/mapimageexporter.cpp +++ b/src/ui/mapimageexporter.cpp @@ -100,11 +100,11 @@ void MapImageExporter::setModeSpecificUi() { const QSignalBlocker b(ui->comboBox_MapSelection); ui->comboBox_MapSelection->clear(); if (m_map) { - ui->comboBox_MapSelection->addItems(m_project->mapNames); + ui->comboBox_MapSelection->addItems(m_project->mapNames()); ui->comboBox_MapSelection->setTextItem(m_map->name()); ui->label_MapSelection->setText(m_mode == ImageExporterMode::Stitch ? QStringLiteral("Starting Map") : QStringLiteral("Map")); } else if (m_layout) { - ui->comboBox_MapSelection->addItems(m_project->layoutIds); + ui->comboBox_MapSelection->addItems(m_project->layoutIds()); ui->comboBox_MapSelection->setTextItem(m_layout->id); ui->label_MapSelection->setText(QStringLiteral("Layout")); } @@ -155,13 +155,13 @@ void MapImageExporter::updateMapSelection() { auto oldLayout = m_layout; const QString text = ui->comboBox_MapSelection->currentText(); - if (m_project->mapNames.contains(text)) { + if (m_project->isKnownMap(text)) { auto newMap = m_project->loadMap(text); if (newMap) { m_map = newMap; m_layout = newMap->layout(); } - } else if (m_project->layoutIds.contains(text)) { + } else if (m_project->isKnownLayout(text)) { auto newLayout = m_project->loadLayout(text); if (newLayout) { m_map = nullptr; diff --git a/src/ui/maplistmodels.cpp b/src/ui/maplistmodels.cpp index 118c7c3d..bd20ce6f 100644 --- a/src/ui/maplistmodels.cpp +++ b/src/ui/maplistmodels.cpp @@ -47,6 +47,7 @@ MapListModel::MapListModel(Project *project, QObject *parent) : QStandardItemMod this->mapGrayIcon = QIcon(QStringLiteral(":/icons/map_grayed.ico")); this->mapIcon = QIcon(QStringLiteral(":/icons/map.ico")); this->mapEditedIcon = QIcon(QStringLiteral(":/icons/map_edited.ico")); + this->mapErroredIcon = QIcon(QStringLiteral(":/icons/map_errored.ico")); this->mapOpenedIcon = QIcon(QStringLiteral(":/icons/map_opened.ico")); this->mapFolderIcon.addFile(QStringLiteral(":/icons/folder_closed_map.ico"), QSize(), QIcon::Normal, QIcon::Off); @@ -121,7 +122,7 @@ QStandardItem *MapListModel::createMapFolderItem(const QString &folderName, QSta } QStandardItem *MapListModel::insertMapItem(const QString &mapName, const QString &folderName) { - if (mapName.isEmpty() || mapName == this->project->getDynamicMapName()) // Disallow adding MAP_DYNAMIC to the map list. + if (mapName.isEmpty() || folderName.isEmpty() || mapName == this->project->getDynamicMapName()) // Disallow adding MAP_DYNAMIC to the map list. return nullptr; QStandardItem *map = createMapItem(mapName); @@ -163,11 +164,13 @@ QVariant MapListModel::data(const QModelIndex &index, int role) const { // Decorating map in the map list if (name == this->activeItemName) return this->mapOpenedIcon; - - const Map* map = this->project->getMap(name); - if (!this->project->isMapLoaded(map)) - return this->mapGrayIcon; - return map->hasUnsavedChanges() ? this->mapEditedIcon : this->mapIcon; + if (this->project->isErroredMap(name)) + return this->mapErroredIcon; + if (this->project->isUnsavedMap(name)) + return this->mapEditedIcon; + if (this->project->isLoadedMap(name)) + return this->mapIcon; + return this->mapGrayIcon; } else if (type == this->folderTypeName) { // Decorating map folder in the map list return item->hasChildren() ? this->mapFolderIcon : this->emptyMapFolderIcon; @@ -372,7 +375,6 @@ void MapGroupModel::updateProject() { if (!this->project) return; // Temporary objects in case of failure, so we won't modify the project unless it succeeds. - QStringList mapNames; QStringList groupNames; QMap groupNameToMapNames; @@ -388,11 +390,9 @@ void MapGroupModel::updateProject() { } QString mapName = mapItem->data(MapListUserRoles::NameRole).toString(); groupNameToMapNames[groupName].append(mapName); - mapNames.append(mapName); } } - this->project->mapNames = mapNames; this->project->groupNames = groupNames; this->project->groupNameToMapNames = groupNameToMapNames; this->project->hasUnsavedDataChanges = true; @@ -445,7 +445,7 @@ MapLocationModel::MapLocationModel(Project *project, QObject *parent) : MapListM for (const auto &idName : this->project->mapSectionIdNames) { insertMapFolderItem(idName); } - for (const auto &mapName : this->project->mapNames) { + for (const auto &mapName : this->project->mapNames()) { insertMapItem(mapName, this->project->getMapLocation(mapName)); } } @@ -466,10 +466,10 @@ QStandardItem *MapLocationModel::createMapFolderItem(const QString &folderName, LayoutTreeModel::LayoutTreeModel(Project *project, QObject *parent) : MapListModel(project, parent) { this->folderTypeName = "map_layout"; - for (const auto &layoutId : this->project->layoutIds) { + for (const auto &layoutId : this->project->layoutIds()) { insertMapFolderItem(layoutId); } - for (const auto &mapName : this->project->mapNames) { + for (const auto &mapName : this->project->mapNames()) { insertMapItem(mapName, this->project->getMapLayoutId(mapName)); } } @@ -483,8 +483,8 @@ QStandardItem *LayoutTreeModel::createMapFolderItem(const QString &folderName, Q // Despite using layout IDs internally, the Layouts map list shows layouts using their file path name. // We could handle this with Qt::DisplayRole in LayoutTreeModel::data, but then it would be sorted using the ID instead of the name. - const Layout* layout = this->project->mapLayouts.value(folderName); - if (layout) folder->setText(layout->name); + QString layoutName = this->project->getLayoutName(folderName); + if (!layoutName.isEmpty()) folder->setText(layoutName); // The layout ID will instead be shown as a tool tip. folder->setToolTip(folderName); @@ -501,18 +501,20 @@ QVariant LayoutTreeModel::data(const QModelIndex &index, int role) const { const QStandardItem *item = this->itemAt(index)->child(row, col); const QString type = item->data(MapListUserRoles::TypeRole).toString(); - const QString name = item->data(MapListUserRoles::NameRole).toString(); + const QString layoutId = item->data(MapListUserRoles::NameRole).toString(); if (type == this->folderTypeName) { if (role == Qt::DecorationRole) { // Map layouts are used as folders, but we display them with the same icons as maps. - if (name == this->activeItemName) + if (layoutId == this->activeItemName) return this->mapOpenedIcon; - - const Layout* layout = this->project->mapLayouts.value(name); - if (!this->project->isLayoutLoaded(layout)) - return this->mapGrayIcon; - return layout->hasUnsavedChanges() ? this->mapEditedIcon : this->mapIcon; + /*if (this->project->isErroredLayout(layoutId)) + return this->mapErroredIcon;*/ + if (this->project->isUnsavedLayout(layoutId)) + return this->mapEditedIcon; + if (this->project->isLoadedLayout(layoutId)) + return this->mapIcon; + return this->mapGrayIcon; } } return MapListModel::data(index, role); diff --git a/src/ui/newmapdialog.cpp b/src/ui/newmapdialog.cpp index 36819404..f90e7960 100644 --- a/src/ui/newmapdialog.cpp +++ b/src/ui/newmapdialog.cpp @@ -43,7 +43,7 @@ NewMapDialog::NewMapDialog(Project *project, const Map *mapToCopy, QWidget *pare ui->newLayoutForm->initUi(project); ui->comboBox_Group->addItems(project->groupNames); - ui->comboBox_LayoutID->addItems(project->layoutIds); + ui->comboBox_LayoutID->addItems(project->layoutIds()); auto validator = new IdentifierValidator(this); ui->lineEdit_Name->setValidator(validator); @@ -128,7 +128,7 @@ void NewMapDialog::saveSettings() { // (an older iteration of this dialog gave users an option to name new layouts, but it's extra clutter for // something the majority of users creating a map won't need. If they want to give a specific name to a layout // they can create the layout first, then create a new map that uses that layout.) - const Layout *layout = this->project->mapLayouts.value(settings->layout.id); + const Layout *layout = this->project->getLayout(settings->layout.id); if (!layout) { const QString newLayoutName = settings->name + QStringLiteral("_Layout"); settings->layout.name = this->project->toUniqueIdentifier(newLayoutName); @@ -198,7 +198,7 @@ bool NewMapDialog::validateLayoutID(bool allowEmpty) { QString errorText; if (layoutId.isEmpty()) { if (!allowEmpty) errorText = QString("%1 cannot be empty.").arg(ui->label_LayoutID->text()); - } else if (!this->project->layoutIds.contains(layoutId) && !this->project->isIdentifierUnique(layoutId)) { + } else if (!this->project->isKnownLayout(layoutId) && !this->project->isIdentifierUnique(layoutId)) { errorText = QString("%1 must either be the ID for an existing layout, or a unique identifier for a new layout.").arg(ui->label_LayoutID->text()); } @@ -213,7 +213,7 @@ void NewMapDialog::on_comboBox_LayoutID_currentTextChanged(const QString &text) validateLayoutID(true); // Changing the layout ID to an existing layout updates the layout settings to match. - const Layout *layout = this->project->mapLayouts.value(text); + const Layout *layout = this->project->getLayout(text); if (!layout && this->mapToCopy) { // When duplicating a map, if a new layout ID is specified the settings will be updated // to match the layout of the map we're duplicating. diff --git a/src/ui/tileseteditor.cpp b/src/ui/tileseteditor.cpp index eedc4215..3bf9f00e 100644 --- a/src/ui/tileseteditor.cpp +++ b/src/ui/tileseteditor.cpp @@ -1145,12 +1145,13 @@ void TilesetEditor::countMetatileUsage() { // do not double count this->metatileSelector->usedMetatiles.fill(0); - for (auto layout : this->project->mapLayouts) { + for (const auto &layoutId : this->project->layoutIds()) { + Layout *layout = this->project->getLayout(layoutId); bool usesPrimary = (layout->tileset_primary_label == this->primaryTileset->name); bool usesSecondary = (layout->tileset_secondary_label == this->secondaryTileset->name); if (usesPrimary || usesSecondary) { - if (!this->project->loadLayout(layout)) + if (!this->project->loadLayout(layoutId)) continue; // for each block in the layout, mark in the vector that it is used @@ -1183,7 +1184,8 @@ void TilesetEditor::countTileUsage() { QSet primaryTilesets; QSet secondaryTilesets; - for (auto &layout : this->project->mapLayouts) { + for (const auto &layoutId : this->project->layoutIds()) { + Layout *layout = this->project->getLayout(layoutId); if (layout->tileset_primary_label == this->primaryTileset->name || layout->tileset_secondary_label == this->secondaryTileset->name) { // need to check metatiles From d0587f400c1bd9a5bd8deab01cd3c277cf4ad77d Mon Sep 17 00:00:00 2001 From: GriffinR Date: Thu, 8 May 2025 13:33:51 -0400 Subject: [PATCH 2/4] Add navigation arrows --- include/mainwindow.h | 13 ++++ src/mainwindow.cpp | 138 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 131 insertions(+), 20 deletions(-) diff --git a/include/mainwindow.h b/include/mainwindow.h index 4714e54d..1efcc725 100644 --- a/include/mainwindow.h +++ b/include/mainwindow.h @@ -331,6 +331,14 @@ private: QAction *redoAction = nullptr; QPointer undoView = nullptr; + struct MapNavigation { + QStack stack; + QPointer button; + }; + MapNavigation backNavigation; + MapNavigation forwardNavigation; + bool ignoreNavigationRecords = false; + QAction *copyAction = nullptr; QAction *pasteAction = nullptr; @@ -391,6 +399,11 @@ private: void updateMapList(); void openMapListItem(const QModelIndex &index); void onMapListTabChanged(int index); + QString getActiveItemName(); + void recordNavigation(const QString &itemName); + void openMapFromHistory(bool previous); + void openPreviousMap(); + void openNextMap(); void displayMapProperties(); void checkToolButtons(); diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index f633606a..ff67e995 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -423,25 +423,52 @@ void MainWindow::initMapList() { QFrame *buttonFrame = new QFrame(this->ui->mapListContainer); buttonFrame->setFrameShape(QFrame::NoFrame); - QHBoxLayout *layout = new QHBoxLayout(buttonFrame); - layout->setSpacing(0); - layout->setContentsMargins(0, 0, 0, 0); + QHBoxLayout *buttonLayout = new QHBoxLayout(buttonFrame); + buttonLayout->setSpacing(0); + buttonLayout->setContentsMargins(0, 0, 0, 0); // Create add map/layout button QPushButton *buttonAdd = new QPushButton(QIcon(":/icons/add.ico"), ""); - buttonAdd->setToolTip("Create New Map"); + buttonAdd->setToolTip("Create new map"); connect(buttonAdd, &QPushButton::clicked, this, &MainWindow::openNewMapDialog); - layout->addWidget(buttonAdd); + buttonLayout->addWidget(buttonAdd); /* TODO: Remove button disabled, no current support for deleting maps/layouts // Create remove map/layout button QPushButton *buttonRemove = new QPushButton(QIcon(":/icons/delete.ico"), ""); connect(buttonRemove, &QPushButton::clicked, this, &MainWindow::deleteCurrentMapOrLayout); - layout->addWidget(buttonRemove); + buttonLayout->addWidget(buttonRemove); */ ui->mapListContainer->setCornerWidget(buttonFrame, Qt::TopRightCorner); + // Navigation arrows + auto navigationFrame = new QFrame(ui->mapListContainer); + navigationFrame->setFrameShape(QFrame::NoFrame); + + auto navigationLayout = new QHBoxLayout(navigationFrame); + navigationLayout->setSpacing(0); + navigationLayout->setContentsMargins(0, 0, 0, 0); + + auto backArrow = new QToolButton(navigationFrame); + backArrow->setArrowType(Qt::LeftArrow); + backArrow->setSizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed); + backArrow->setToolTip("Open previous map"); + backArrow->setEnabled(false); + connect(backArrow, &QToolButton::clicked, this, &MainWindow::openPreviousMap); + navigationLayout->addWidget(backArrow); + this->backNavigation.button = backArrow; + + auto forwardArrow = new QToolButton(navigationFrame); + forwardArrow->setArrowType(Qt::RightArrow); + forwardArrow->setToolTip("Open next map"); + forwardArrow->setEnabled(false); + connect(forwardArrow, &QToolButton::clicked, this, &MainWindow::openNextMap); + navigationLayout->addWidget(forwardArrow); + this->forwardNavigation.button = forwardArrow; + + ui->mapListContainer->setCornerWidget(navigationFrame, Qt::TopLeftCorner); + // Connect tool bars to lists ui->mapListToolBar_Groups->setList(ui->mapList); ui->mapListToolBar_Locations->setList(ui->locationList); @@ -950,12 +977,65 @@ void MainWindow::unsetMap() { setLayoutOnlyMode(true); } +void MainWindow::openPreviousMap() { + openMapFromHistory(true); +} + +void MainWindow::openNextMap() { + openMapFromHistory(false); +} + +// Either open a map/layout from the 'Back' list and put it in the 'Forward' list (i.e., previous == true) or vice versa. +void MainWindow::openMapFromHistory(bool previous) { + if (!this->editor->project) + return; + + MapNavigation* popNavigation = (previous) ? &this->backNavigation : &this->forwardNavigation; + MapNavigation* pushNavigation = (previous) ? &this->forwardNavigation : &this->backNavigation; + if (popNavigation->stack.isEmpty()) + return; + + QString incomingItem = popNavigation->stack.top(); + QString outgoingItem = getActiveItemName(); + + this->ignoreNavigationRecords = true; + + bool success = false; + if (this->editor->project->isKnownMap(incomingItem)) { + success = userSetMap(incomingItem); + } else if (this->editor->project->isKnownLayout(incomingItem)) { + success = userSetLayout(incomingItem); + } + if (success) { + // We were successful in opening the map/layout, so we can remove it from the history. + popNavigation->stack.pop(); + if (popNavigation->stack.isEmpty()) { + popNavigation->button->setEnabled(false); + } + + // Save the map/layout that was previously open. + pushNavigation->stack.push(outgoingItem); + pushNavigation->button->setEnabled(true); + } + + this->ignoreNavigationRecords = false; + +} + +void MainWindow::recordNavigation(const QString &itemName) { + if (this->ignoreNavigationRecords) + return; + + this->backNavigation.stack.push(itemName); + this->backNavigation.button->setEnabled(true); + + this->forwardNavigation.stack.clear(); + this->forwardNavigation.button->setEnabled(false); +} + // setMap, but with a visible error message in case of failure. // Use when the user is specifically requesting a map to open. bool MainWindow::userSetMap(const QString &mapName) { - if (editor->map && editor->map->name() == mapName) - return true; // Already set - if (mapName.isEmpty()) { WarningMessage::show(QStringLiteral("Cannot open map with empty name."), this); return false; @@ -969,10 +1049,15 @@ bool MainWindow::userSetMap(const QString &mapName) { return false; } + QString prevItem = getActiveItemName(); + if (prevItem == mapName) + return true; // Already set + if (!setMap(mapName)) { RecentErrorMessage::show(QString("There was an error opening map '%1'.").arg(mapName), this); return false; } + recordNavigation(prevItem); return true; } @@ -1030,11 +1115,22 @@ void MainWindow::setLayoutOnlyMode(bool layoutOnly) { // setLayout, but with a visible error message in case of failure. // Use when the user is specifically requesting a layout to open. bool MainWindow::userSetLayout(const QString &layoutId) { + if (layoutId.isEmpty()) { + WarningMessage::show(QStringLiteral("Cannot open layout with empty ID."), this); + return false; + } + + QString prevItem = getActiveItemName(); + if (prevItem == layoutId) + return true; // Already set + if (!setLayout(layoutId)) { RecentErrorMessage::show(QString("There was an error opening layout '%1'.").arg(layoutId), this); return false; } + recordNavigation(prevItem); + // Only the Layouts tab of the map list shows Layouts, so if we're not already on that tab we'll open it now. ui->mapListContainer->setCurrentIndex(MapListTab::Layouts); @@ -1698,20 +1794,22 @@ void MainWindow::rebuildMapList_Layouts() { resetMapListFilters(); } +QString MainWindow::getActiveItemName() { + if (this->editor->map) return this->editor->map->name(); + if (this->editor->layout) return this->editor->layout->id; + return QString(); +} + void MainWindow::updateMapList() { - // Get the name of the open map/layout (or clear the relevant selection if there is none). - QString activeItemName; - if (this->editor->map) { - activeItemName = this->editor->map->name(); - } else { + QString activeItemName = getActiveItemName(); + + // Clear relevant selections + if (!this->editor->map) { ui->mapList->clearSelection(); ui->locationList->clearSelection(); - - if (this->editor->layout) { - activeItemName = this->editor->layout->id; - } else { - ui->layoutList->clearSelection(); - } + } + if (!this->editor->layout) { + ui->layoutList->clearSelection(); } this->mapGroupModel->setActiveItem(activeItemName); From 8e141247bb0a310f061218f4bbe77126854c2210 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Mon, 28 Apr 2025 15:13:24 -0400 Subject: [PATCH 3/4] Soften layout load requirements --- include/core/maplayout.h | 4 + include/project.h | 8 +- src/core/maplayout.cpp | 93 +++++++++++++++++- src/mainwindow.cpp | 2 - src/project.cpp | 197 +++++++++++++-------------------------- src/ui/maplistmodels.cpp | 2 - 6 files changed, 166 insertions(+), 140 deletions(-) diff --git a/include/core/maplayout.h b/include/core/maplayout.h index 3e67af18..019a983e 100644 --- a/include/core/maplayout.h +++ b/include/core/maplayout.h @@ -122,6 +122,9 @@ public: bool saveBorder(const QString &root); bool saveBlockdata(const QString &root); + bool loadBorder(const QString &root); + bool loadBlockdata(const QString &root); + bool layoutBlockChanged(int i, const Blockdata &cache); uint16_t getBorderMetatileId(int x, int y); @@ -147,6 +150,7 @@ private: void setNewDimensionsBlockdata(int newWidth, int newHeight); void setNewBorderDimensionsBlockdata(int newWidth, int newHeight); bool writeBlockdata(const QString &path, const Blockdata &blockdata) const; + static Blockdata readBlockdata(const QString &path, QString *error); static int getBorderDrawDistance(int dimension, qreal minimum); diff --git a/include/project.h b/include/project.h index e418c712..30e84abc 100644 --- a/include/project.h +++ b/include/project.h @@ -110,10 +110,6 @@ public: QStringList secondaryTilesetLabels; QStringList tilesetLabelsOrdered; - Blockdata readBlockdata(QString, bool *ok = nullptr); - bool loadBlockdata(Layout *); - bool loadLayoutBorder(Layout *); - bool readMapGroups(); void addNewMapGroup(const QString &groupName); QString mapNameToMapGroup(const QString &mapName) const; @@ -293,6 +289,10 @@ private: QSet loadedMapNames; QSet loadedLayoutIds; + // Data for layouts that failed to load at launch. + // We can't display these layouts to the user, but we want to preserve the data when they save. + QList failedLayoutsData; + const QRegularExpression re_gbapalExtension; const QRegularExpression re_bppExtension; diff --git a/src/core/maplayout.cpp b/src/core/maplayout.cpp index c3e91ba3..f49f58f4 100644 --- a/src/core/maplayout.cpp +++ b/src/core/maplayout.cpp @@ -509,7 +509,7 @@ bool Layout::saveBlockdata(const QString &root) { bool Layout::writeBlockdata(const QString &path, const Blockdata &blockdata) const { QFile file(path); if (!file.open(QIODevice::WriteOnly)) { - logError(QString("Could not open '%1' for writing: %2").arg(path).arg(file.errorString())); + logError(QString("Failed to write '%1' for %2: %3").arg(path).arg(this->name).arg(file.errorString())); return false; } @@ -517,3 +517,94 @@ bool Layout::writeBlockdata(const QString &path, const Blockdata &blockdata) con file.write(data); return true; } + +bool Layout::loadBorder(const QString &root) { + if (this->border_path.isEmpty()) { + logError(QString("Failed to load border for %1: no path specified.").arg(this->name)); + return false; + } + + QString error; + QString path = QString("%1/%2").arg(root).arg(this->border_path); + auto blockdata = readBlockdata(path, &error); + if (!error.isEmpty()) { + logError(QString("Failed to load border for %1 from '%2': %3").arg(this->name).arg(path).arg(error)); + return false; + } + + // 0 is an expected border width/height that should be handled, GF used it for the RS layouts in FRLG + if (this->border_width <= 0) { + this->border_width = DEFAULT_BORDER_WIDTH; + } + if (this->border_height <= 0) { + this->border_height = DEFAULT_BORDER_HEIGHT; + } + + this->border = blockdata; + this->lastCommitBlocks.border = blockdata; + this->lastCommitBlocks.borderDimensions = QSize(this->border_width, this->border_height); + + int expectedSize = this->border_width * this->border_height; + if (this->border.count() != expectedSize) { + logWarn(QString("%1 border blockdata length %2 does not match dimensions %3x%4 (should be %5). Resizing border blockdata.") + .arg(this->name) + .arg(this->border.count()) + .arg(this->border_width) + .arg(this->border_height) + .arg(expectedSize)); + this->border.resize(expectedSize); + } + return true; +} + +bool Layout::loadBlockdata(const QString &root) { + if (this->blockdata_path.isEmpty()) { + logError(QString("Failed to load blockdata for %1: no path specified.").arg(this->name)); + return false; + } + + QString error; + QString path = QString("%1/%2").arg(root).arg(this->blockdata_path); + auto blockdata = readBlockdata(path, &error); + if (!error.isEmpty()) { + logError(QString("Failed to load blockdata for %1 from '%2': %3").arg(this->name).arg(path).arg(error)); + return false; + } + + this->blockdata = blockdata; + this->lastCommitBlocks.blocks = blockdata; + this->lastCommitBlocks.layoutDimensions = QSize(this->width, this->height); + + int expectedSize = this->width * this->height; + if (expectedSize <= 0) { + logError(QString("Failed to load blockdata for %1: invalid dimensions %2x%3").arg(this->name).arg(this->width).arg(this->height)); + return false; + } + if (this->blockdata.count() != expectedSize) { + logWarn(QString("%1 blockdata length %2 does not match dimensions %3x%4 (should be %5). Resizing blockdata.") + .arg(this->name) + .arg(this->blockdata.count()) + .arg(this->width) + .arg(this->height) + .arg(expectedSize)); + this->blockdata.resize(expectedSize); + } + return true; +} + +Blockdata Layout::readBlockdata(const QString &path, QString *error) { + Blockdata blockdata; + + QFile file(path); + if (file.open(QIODevice::ReadOnly)) { + QByteArray data = file.readAll(); + for (int i = 0; (i + 1) < data.length(); i += 2) { + uint16_t word = static_cast((data[i] & 0xff) + ((data[i + 1] & 0xff) << 8)); + blockdata.append(word); + } + } else { + if (error) *error = file.errorString(); + } + + return blockdata; +} diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index ff67e995..268c8aa9 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -452,7 +452,6 @@ void MainWindow::initMapList() { auto backArrow = new QToolButton(navigationFrame); backArrow->setArrowType(Qt::LeftArrow); - backArrow->setSizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed); backArrow->setToolTip("Open previous map"); backArrow->setEnabled(false); connect(backArrow, &QToolButton::clicked, this, &MainWindow::openPreviousMap); @@ -1019,7 +1018,6 @@ void MainWindow::openMapFromHistory(bool previous) { } this->ignoreNavigationRecords = false; - } void MainWindow::recordNavigation(const QString &itemName) { diff --git a/src/project.cpp b/src/project.cpp index 42dd7e20..125f4722 100644 --- a/src/project.cpp +++ b/src/project.cpp @@ -324,8 +324,8 @@ Layout *Project::loadLayout(const QString &layoutId) { // Force these to run even if one fails bool loadedTilesets = loadLayoutTilesets(layout); - bool loadedBlockdata = loadBlockdata(layout); - bool loadedBorder = loadLayoutBorder(layout); + bool loadedBlockdata = layout->loadBlockdata(this->root); + bool loadedBorder = layout->loadBorder(this->root); if (!loadedTilesets || !loadedBlockdata || !loadedBorder) { // Error should already be logged. return nullptr; @@ -597,6 +597,7 @@ void Project::clearMapLayouts() { this->orderedLayoutIdsMaster.clear(); this->loadedLayoutIds.clear(); this->customLayoutsData = QJsonObject(); + this->failedLayoutsData.clear(); } bool Project::readMapLayouts() { @@ -626,80 +627,46 @@ bool Project::readMapLayouts() { QJsonObject layoutObj = layouts[i].toObject(); if (layoutObj.isEmpty()) continue; - Layout *layout = new Layout(); + + QScopedPointer layout(new Layout()); layout->id = ParseUtil::jsonToQString(layoutObj.take("id")); if (layout->id.isEmpty()) { - logError(QString("Missing 'id' value on layout %1 in %2").arg(i).arg(layoutsFilepath)); - delete layout; - return false; + // Use name to identify it in the warning, if available. + QString name = ParseUtil::jsonToQString(layoutObj["name"]); + if (name.isEmpty()) name = QString("Layout %1 (unnamed)").arg(i); + logWarn(QString("Missing 'id' value for %1 in %2").arg(name).arg(layoutsFilepath)); + this->failedLayoutsData.append(layouts[i].toObject()); + continue; } - if (mapLayouts.contains(layout->id)) { + if (this->mapLayouts.contains(layout->id)) { logWarn(QString("Duplicate layout entry for %1 in %2 will be ignored.").arg(layout->id).arg(layoutsFilepath)); - delete layout; + this->failedLayoutsData.append(layouts[i].toObject()); continue; } layout->name = ParseUtil::jsonToQString(layoutObj.take("name")); if (layout->name.isEmpty()) { - logError(QString("Missing 'name' value for %1 in %2").arg(layout->id).arg(layoutsFilepath)); - delete layout; - return false; + logWarn(QString("Missing 'name' value for %1 in %2").arg(layout->id).arg(layoutsFilepath)); + this->failedLayoutsData.append(layouts[i].toObject()); + continue; } - int lwidth = ParseUtil::jsonToInt(layoutObj.take("width")); - if (lwidth <= 0) { - logError(QString("Invalid 'width' value '%1' for %2 in %3. Must be greater than 0.").arg(lwidth).arg(layout->id).arg(layoutsFilepath)); - delete layout; - return false; - } - layout->width = lwidth; - int lheight = ParseUtil::jsonToInt(layoutObj.take("height")); - if (lheight <= 0) { - logError(QString("Invalid 'height' value '%1' for %2 in %3. Must be greater than 0.").arg(lheight).arg(layout->id).arg(layoutsFilepath)); - delete layout; - return false; - } - layout->height = lheight; + + layout->width = ParseUtil::jsonToInt(layoutObj.take("width")); + layout->height = ParseUtil::jsonToInt(layoutObj.take("height")); if (projectConfig.useCustomBorderSize) { - int bwidth = ParseUtil::jsonToInt(layoutObj.take("border_width")); - if (bwidth <= 0) { // 0 is an expected border width/height that should be handled, GF used it for the RS layouts in FRLG - bwidth = DEFAULT_BORDER_WIDTH; - } - layout->border_width = bwidth; - int bheight = ParseUtil::jsonToInt(layoutObj.take("border_height")); - if (bheight <= 0) { - bheight = DEFAULT_BORDER_HEIGHT; - } - layout->border_height = bheight; + layout->border_width = ParseUtil::jsonToInt(layoutObj.take("border_width")); + layout->border_height = ParseUtil::jsonToInt(layoutObj.take("border_height")); } else { layout->border_width = DEFAULT_BORDER_WIDTH; layout->border_height = DEFAULT_BORDER_HEIGHT; } layout->tileset_primary_label = ParseUtil::jsonToQString(layoutObj.take("primary_tileset")); - if (layout->tileset_primary_label.isEmpty()) { - logError(QString("Missing 'primary_tileset' value for %1 in %2").arg(layout->id).arg(layoutsFilepath)); - delete layout; - return false; - } layout->tileset_secondary_label = ParseUtil::jsonToQString(layoutObj.take("secondary_tileset")); - if (layout->tileset_secondary_label.isEmpty()) { - logError(QString("Missing 'secondary_tileset' value for %1 in %2").arg(layout->id).arg(layoutsFilepath)); - delete layout; - return false; - } layout->border_path = ParseUtil::jsonToQString(layoutObj.take("border_filepath")); - if (layout->border_path.isEmpty()) { - logError(QString("Missing 'border_filepath' value for %1 in %2").arg(layout->id).arg(layoutsFilepath)); - delete layout; - return false; - } layout->blockdata_path = ParseUtil::jsonToQString(layoutObj.take("blockdata_filepath")); - if (layout->blockdata_path.isEmpty()) { - logError(QString("Missing 'blockdata_filepath' value for %1 in %2").arg(layout->id).arg(layoutsFilepath)); - delete layout; - return false; - } + layout->customData = layoutObj; - this->mapLayouts.insert(layout->id, layout); + this->mapLayouts.insert(layout->id, layout->copy()); this->mapLayoutsMaster.insert(layout->id, layout->copy()); this->orderedLayoutIds.append(layout->id); this->orderedLayoutIdsMaster.append(layout->id); @@ -748,6 +715,22 @@ bool Project::saveMapLayouts() { OrderedJson::append(&layoutObj, layout->customData); layoutsArr.push_back(layoutObj); } + // Append any layouts that were hidden because we failed to load them at launch. + // We do a little extra work to keep the field order the same as successfully-loaded layouts. + for (QJsonObject failedData : this->failedLayoutsData) { + OrderedJson::object layoutObj; + static const QStringList expectedFields = { + "id", "name", "width", "height", "border_width", "border_height", + "primary_tileset", "secondary_tileset", "border_filepath", "blockdata_filepath" + }; + for (const auto &field : expectedFields) { + if (failedData.contains(field)) { + layoutObj[field] = OrderedJson::fromQJsonValue(failedData.take(field)); + } + } + OrderedJson::append(&layoutObj, failedData); + layoutsArr.push_back(layoutObj); + } layoutsObj["layouts"] = layoutsArr; OrderedJson::append(&layoutsObj, this->customLayoutsData); @@ -1107,6 +1090,18 @@ bool Project::saveTilesetMetatileLabels(Tileset *primaryTileset, Tileset *second } bool Project::loadLayoutTilesets(Layout *layout) { + // Note: Do not replace invalid tileset labels with the default tileset labels here. + // Changing the tilesets like this can require us to load tilesets unnecessarily + // in order to avoid strange behavior (e.g. tile/metatile usage counts changing). + if (layout->tileset_primary_label.isEmpty()) { + logError(QString("Failed to load %1: missing primary tileset label.").arg(layout->name)); + return false; + } + if (layout->tileset_secondary_label.isEmpty()) { + logError(QString("Failed to load %1: missing secondary tileset label.").arg(layout->name)); + return false; + } + layout->tileset_primary = getTileset(layout->tileset_primary_label); layout->tileset_secondary = getTileset(layout->tileset_secondary_label); return layout->tileset_primary && layout->tileset_secondary; @@ -1161,31 +1156,6 @@ Tileset* Project::loadTileset(QString label, Tileset *tileset) { return tileset; } -bool Project::loadBlockdata(Layout *layout) { - bool ok = true; - QString path = QString("%1/%2").arg(root).arg(layout->blockdata_path); - auto blockdata = readBlockdata(path, &ok); - if (!ok) { - logError(QString("Failed to load layout blockdata from '%1'").arg(path)); - return false; - } - - layout->blockdata = blockdata; - layout->lastCommitBlocks.blocks = blockdata; - layout->lastCommitBlocks.layoutDimensions = QSize(layout->getWidth(), layout->getHeight()); - - if (layout->blockdata.count() != layout->getWidth() * layout->getHeight()) { - logWarn(QString("%1 blockdata length %2 does not match dimensions %3x%4 (should be %5). Resizing blockdata.") - .arg(layout->name) - .arg(layout->blockdata.count()) - .arg(layout->getWidth()) - .arg(layout->getHeight()) - .arg(layout->getWidth() * layout->getHeight())); - layout->blockdata.resize(layout->getWidth() * layout->getHeight()); - } - return true; -} - void Project::setNewLayoutBlockdata(Layout *layout) { layout->blockdata.clear(); int width = layout->getWidth(); @@ -1198,30 +1168,6 @@ void Project::setNewLayoutBlockdata(Layout *layout) { layout->lastCommitBlocks.layoutDimensions = QSize(width, height); } -bool Project::loadLayoutBorder(Layout *layout) { - bool ok = true; - QString path = QString("%1/%2").arg(root).arg(layout->border_path); - auto blockdata = readBlockdata(path, &ok); - if (!ok) { - logError(QString("Failed to load layout border from '%1'").arg(path)); - return false; - } - - layout->border = blockdata; - layout->lastCommitBlocks.border = blockdata; - layout->lastCommitBlocks.borderDimensions = QSize(layout->getBorderWidth(), layout->getBorderHeight()); - - int borderLength = layout->getBorderWidth() * layout->getBorderHeight(); - if (layout->border.count() != borderLength) { - logWarn(QString("%1 border blockdata length %2 must be %3. Resizing border blockdata.") - .arg(layout->name) - .arg(layout->border.count()) - .arg(borderLength)); - layout->border.resize(borderLength); - } - return true; -} - void Project::setNewLayoutBorder(Layout *layout) { layout->border.clear(); int width = layout->getBorderWidth(); @@ -1662,24 +1608,6 @@ void Project::loadTilesetMetatileLabels(Tileset* tileset) { } } -Blockdata Project::readBlockdata(QString path, bool *ok) { - Blockdata blockdata; - QFile file(path); - if (file.open(QIODevice::ReadOnly)) { - QByteArray data = file.readAll(); - for (int i = 0; (i + 1) < data.length(); i += 2) { - uint16_t word = static_cast((data[i] & 0xff) + ((data[i + 1] & 0xff) << 8)); - blockdata.append(word); - } - if (ok) *ok = true; - } else { - // Failed - if (ok) *ok = false; - } - - return blockdata; -} - Tileset* Project::getTileset(QString label, bool forceLoad) { Tileset *existingTileset = nullptr; if (tilesetCache.contains(label)) { @@ -1940,23 +1868,30 @@ bool Project::readMapGroups() { // Process the names in this map group for (int j = 0; j < mapNamesJson.size(); j++) { const QString mapName = ParseUtil::jsonToQString(mapNamesJson.at(j)); - - // This list should accept all maps we find, valid or not. - // It will only be used to populate the map list panel. - // Always keeping the name prevents us from deleting it from the list - // if we're unable to load the rest of the necessary map data. - this->groupNameToMapNames[groupName].append(mapName); - + if (mapName.isEmpty()) { + logWarn(QString("Ignoring empty map %1 in map group '%2'.").arg(j).arg(groupName).arg(mapName)); + continue; + } + // We explicitly hide "Dynamic" from the map list, so this entry will be deleted from the file if the user changes the map list order. if (mapName == dynamicMapName) { logWarn(QString("Ignoring map %1 in map group '%2': Cannot use reserved map name '%3'.").arg(j).arg(groupName).arg(mapName)); continue; } + + // Excepting the disallowed map names above, we want to preserve the user's map list data, + // so this list should accept all names we find whether they have valid data or not. + this->groupNameToMapNames[groupName].append(mapName); + + // We log a warning for this, but a repeated name in the map list otherwise functions fine. + // All repeated entries will refer to the same map data. if (this->maps.contains(mapName)) { - logWarn(QString("Ignoring map %1 in map group '%2': Repeated map name '%3'.").arg(j).arg(groupName).arg(mapName)); + logWarn(QString("Map %1 in map group '%2' has repeated map name '%3'.").arg(j).arg(groupName).arg(mapName)); continue; } // Load the map's json file so we can get its ID constant (and two other constants we use for the map list). + // If we fail to get the ID for any reason, we flag the map as 'errored'. It can still appear in the map list, + // but we won't be able to translate the map name to a map constant, so the map name can't appear elsewhere. QString mapJsonError; QJsonDocument mapDoc = readMapJson(mapName, &mapJsonError); if (!mapJsonError.isEmpty()) { diff --git a/src/ui/maplistmodels.cpp b/src/ui/maplistmodels.cpp index bd20ce6f..3bd95d66 100644 --- a/src/ui/maplistmodels.cpp +++ b/src/ui/maplistmodels.cpp @@ -508,8 +508,6 @@ QVariant LayoutTreeModel::data(const QModelIndex &index, int role) const { // Map layouts are used as folders, but we display them with the same icons as maps. if (layoutId == this->activeItemName) return this->mapOpenedIcon; - /*if (this->project->isErroredLayout(layoutId)) - return this->mapErroredIcon;*/ if (this->project->isUnsavedLayout(layoutId)) return this->mapEditedIcon; if (this->project->isLoadedLayout(layoutId)) From 9812f94d122f4fb322b007ab8c2161ec824e884f Mon Sep 17 00:00:00 2001 From: GriffinR Date: Thu, 8 May 2025 16:19:46 -0400 Subject: [PATCH 4/4] Add shortcuts for Back/Forward --- forms/mainwindow.ui | 19 +++++++++++++++++++ src/mainwindow.cpp | 2 ++ 2 files changed, 21 insertions(+) diff --git a/forms/mainwindow.ui b/forms/mainwindow.ui index c75c8328..c605528d 100644 --- a/forms/mainwindow.ui +++ b/forms/mainwindow.ui @@ -2885,6 +2885,9 @@ + + + @@ -3248,6 +3251,22 @@ Duplicate Current Map/Layout... + + + Back + + + Alt+Left + + + + + Forward + + + Alt+Right + + diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 268c8aa9..1a76414e 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -455,6 +455,7 @@ void MainWindow::initMapList() { backArrow->setToolTip("Open previous map"); backArrow->setEnabled(false); connect(backArrow, &QToolButton::clicked, this, &MainWindow::openPreviousMap); + connect(ui->actionBack, &QAction::triggered, this, &MainWindow::openPreviousMap); navigationLayout->addWidget(backArrow); this->backNavigation.button = backArrow; @@ -463,6 +464,7 @@ void MainWindow::initMapList() { forwardArrow->setToolTip("Open next map"); forwardArrow->setEnabled(false); connect(forwardArrow, &QToolButton::clicked, this, &MainWindow::openNextMap); + connect(ui->actionForward, &QAction::triggered, this, &MainWindow::openNextMap); navigationLayout->addWidget(forwardArrow); this->forwardNavigation.button = forwardArrow;