From f70e77ab66bf6dfb93a2a1ac1ae5af905b2df4d2 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Mon, 19 Aug 2024 21:31:36 -0400 Subject: [PATCH] Final MapConnection testing fixes --- include/core/map.h | 6 ++-- include/core/mapconnection.h | 4 +-- include/editor.h | 15 ++++----- include/ui/connectionslistitem.h | 4 +-- include/ui/mapimageexporter.h | 3 +- src/core/map.cpp | 53 ++++++++++++++++------------- src/core/mapconnection.cpp | 2 +- src/editor.cpp | 57 ++++++++++++-------------------- src/mainwindow.cpp | 4 ++- src/ui/connectionslistitem.cpp | 4 +-- src/ui/mapimageexporter.cpp | 41 +++++++++++++++++------ 11 files changed, 104 insertions(+), 89 deletions(-) diff --git a/include/core/map.h b/include/core/map.h index 27020236..2333c862 100644 --- a/include/core/map.h +++ b/include/core/map.h @@ -99,9 +99,8 @@ public: void addEvent(Event *); void deleteConnections(); QList getConnections() const; - bool takeConnection(MapConnection *); - bool removeConnection(MapConnection *); - bool addConnection(MapConnection *); + void removeConnection(MapConnection *); + void addConnection(MapConnection *); void loadConnection(MapConnection *); QRect getConnectionRect(const QString &direction, MapLayout *fromLayout = nullptr); QPixmap renderConnection(const QString &direction, MapLayout *fromLayout = nullptr); @@ -133,6 +132,7 @@ public: private: void setNewDimensionsBlockdata(int newWidth, int newHeight); void setNewBorderDimensionsBlockdata(int newWidth, int newHeight); + void trackConnection(MapConnection*); // MapConnections in 'ownedConnections' but not 'connections' persist in the edit history. QList connections; diff --git a/include/core/mapconnection.h b/include/core/mapconnection.h index 6ffa85b7..21c00ac6 100644 --- a/include/core/mapconnection.h +++ b/include/core/mapconnection.h @@ -55,8 +55,8 @@ private: signals: void parentMapChanged(Map* before, Map* after); - void targetMapNameChanged(const QString &before, const QString &after); - void directionChanged(const QString &before, const QString &after); + void targetMapNameChanged(QString before, QString after); + void directionChanged(QString before, QString after); void offsetChanged(int before, int after); }; diff --git a/include/editor.h b/include/editor.h index 3b2c9ae6..89b6ccb5 100644 --- a/include/editor.h +++ b/include/editor.h @@ -78,7 +78,6 @@ public: void setMapEditingButtonsEnabled(bool enabled); void setConnectionsVisibility(bool visible); void updateDivingMapsVisibility(); - void displayDivingConnections(); void renderDivingConnections(); void addConnection(MapConnection* connection); void removeConnection(MapConnection* connection); @@ -185,13 +184,14 @@ private: void clearMapGrid(); void clearWildMonTables(); void updateBorderVisibility(); + void disconnectMapConnection(MapConnection *connection); QPoint getConnectionOrigin(MapConnection *connection); - void removeConnectionPixmap(MapConnection* connection); - void updateConnectionPixmap(ConnectionPixmapItem* connectionItem); - void displayConnection(MapConnection* connection); - void displayDivingConnection(MapConnection* connection); + void removeConnectionPixmap(MapConnection *connection); + void updateConnectionPixmap(ConnectionPixmapItem *connectionItem); + void displayConnection(MapConnection *connection); + void displayDivingConnection(MapConnection *connection); void setDivingMapName(QString mapName, QString direction); - void removeDivingMapPixmap(MapConnection* connection); + void removeDivingMapPixmap(MapConnection *connection); void updateEncounterFields(EncounterFields newFields); QString getMovementPermissionText(uint16_t collision, uint16_t elevation); QString getMetatileDisplayMessage(uint16_t metatileId); @@ -207,7 +207,7 @@ private slots: void setStraightPathCursorMode(QGraphicsSceneMouseEvent *event); void mouseEvent_map(QGraphicsSceneMouseEvent *event, MapPixmapItem *item); void mouseEvent_collision(QGraphicsSceneMouseEvent *event, CollisionPixmapItem *item); - void setSelectedConnectionItem(ConnectionPixmapItem* connectionItem); + void setSelectedConnectionItem(ConnectionPixmapItem *connectionItem); void onHoveredMovementPermissionChanged(uint16_t, uint16_t); void onHoveredMovementPermissionCleared(); void onHoveredMetatileSelectionChanged(uint16_t); @@ -219,7 +219,6 @@ private slots: void onSelectedMetatilesChanged(); void onWheelZoom(int); void onToggleGridClicked(bool); - void onMapConnectionDoubleClicked(MapConnection*); signals: void objectsChanged(); diff --git a/include/ui/connectionslistitem.h b/include/ui/connectionslistitem.h index 18c3434c..bbe0f2d3 100644 --- a/include/ui/connectionslistitem.h +++ b/include/ui/connectionslistitem.h @@ -42,8 +42,8 @@ signals: void openMapClicked(MapConnection*); private slots: - void on_comboBox_Direction_currentTextChanged(const QString &direction); - void on_comboBox_Map_currentTextChanged(const QString &mapName); + void on_comboBox_Direction_currentTextChanged(QString direction); + void on_comboBox_Map_currentTextChanged(QString mapName); void on_spinBox_Offset_valueChanged(int offset); void on_button_Delete_clicked(); void on_button_OpenMap_clicked(); diff --git a/include/ui/mapimageexporter.h b/include/ui/mapimageexporter.h index 6d8ae643..b9cc20bd 100644 --- a/include/ui/mapimageexporter.h +++ b/include/ui/mapimageexporter.h @@ -50,9 +50,10 @@ private: ImageExporterMode mode = ImageExporterMode::Normal; void updatePreview(); + void updateShowBorderState(); void saveImage(); QPixmap getStitchedImage(QProgressDialog *progress, bool includeBorder); - QPixmap getFormattedMapPixmap(Map *map, bool ignoreBorder); + QPixmap getFormattedMapPixmap(Map *map, bool ignoreBorder = false); bool historyItemAppliesToFrame(const QUndoCommand *command); private slots: diff --git a/src/core/map.cpp b/src/core/map.cpp index 91e196fe..86df7947 100644 --- a/src/core/map.cpp +++ b/src/core/map.cpp @@ -537,12 +537,12 @@ void Map::deleteConnections() { } QList Map::getConnections() const { - return connections; + return this->connections; } -bool Map::addConnection(MapConnection *connection) { +void Map::addConnection(MapConnection *connection) { if (!connection || this->connections.contains(connection)) - return false; + return; // Maps should only have one Dive/Emerge connection at a time. // (Users can technically have more by editing their data manually, but we will only display one at a time) @@ -550,9 +550,8 @@ bool Map::addConnection(MapConnection *connection) { if (MapConnection::isDiving(connection->direction())) { for (auto i : this->connections) { if (i->direction() == connection->direction()) { - this->ownedConnections.insert(connection); - connection->setParentMap(this, false); - return true; + trackConnection(connection); + return; } } } @@ -560,33 +559,41 @@ bool Map::addConnection(MapConnection *connection) { loadConnection(connection); modify(); emit connectionAdded(connection); - return true; + return; } void Map::loadConnection(MapConnection *connection) { if (!connection) return; - this->connections.append(connection); - this->ownedConnections.insert(connection); - connection->setParentMap(this, false); + + if (!this->connections.contains(connection)) + this->connections.append(connection); + + trackConnection(connection); } -// connection should not be deleted here, a pointer to it is allowed to persist in the edit history -bool Map::removeConnection(MapConnection *connection) { +void Map::trackConnection(MapConnection *connection) { + connection->setParentMap(this, false); + + if (!this->ownedConnections.contains(connection)) { + this->ownedConnections.insert(connection); + connect(connection, &MapConnection::parentMapChanged, [=](Map *, Map *after) { + if (after != this && after != nullptr) { + // MapConnection's parent has been reassigned, it's no longer our responsibility + this->ownedConnections.remove(connection); + QObject::disconnect(connection, &MapConnection::parentMapChanged, this, nullptr); + } + }); + } +} + +// We retain ownership of this MapConnection until it's assigned to a new parent map. +void Map::removeConnection(MapConnection *connection) { if (!this->connections.removeOne(connection)) - return false; + return; + connection->setParentMap(nullptr, false); modify(); emit connectionRemoved(connection); - return true; -} - -// Same as Map::removeConnection but caller takes ownership of connection. -bool Map::takeConnection(MapConnection *connection) { - if (!this->removeConnection(connection)) - return false; - connection->setParentMap(nullptr, false); - this->ownedConnections.remove(connection); - return true; } void Map::modify() { diff --git a/src/core/mapconnection.cpp b/src/core/mapconnection.cpp index 022158a0..b80b8d09 100644 --- a/src/core/mapconnection.cpp +++ b/src/core/mapconnection.cpp @@ -79,7 +79,7 @@ void MapConnection::setParentMap(Map* map, bool mirror) { } if (m_parentMap) - m_parentMap->takeConnection(this); + m_parentMap->removeConnection(this); auto before = m_parentMap; m_parentMap = map; diff --git a/src/editor.cpp b/src/editor.cpp index de1caa9a..586765f4 100644 --- a/src/editor.cpp +++ b/src/editor.cpp @@ -733,7 +733,15 @@ void Editor::updateEncounterFields(EncounterFields newFields) { project->wildMonFields = newFields; } -void Editor::displayConnection(MapConnection* connection) { +void Editor::disconnectMapConnection(MapConnection *connection) { + // Disconnect MapConnection's signals used by the display. + // It'd be nice if we could just 'connection->disconnect(this)' but that doesn't account for lambda functions. + QObject::disconnect(connection, &MapConnection::targetMapNameChanged, nullptr, nullptr); + QObject::disconnect(connection, &MapConnection::directionChanged, nullptr, nullptr); + QObject::disconnect(connection, &MapConnection::offsetChanged, nullptr, nullptr); +} + +void Editor::displayConnection(MapConnection *connection) { if (!connection) return; @@ -752,15 +760,9 @@ void Editor::displayConnection(MapConnection* connection) { ConnectionsListItem *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 - // It's possible for a map connection to be displayed repeatedly if it's being - // updated by mirroring and the target map is changing to/from the current map. - QObject::disconnect(connection, &MapConnection::targetMapNameChanged, nullptr, nullptr); - QObject::disconnect(connection, &MapConnection::directionChanged, nullptr, nullptr); - QObject::disconnect(connection, &MapConnection::offsetChanged, nullptr, nullptr); - // Double clicking the pixmap or clicking the list item's map button opens the connected map - connect(listItem, &ConnectionsListItem::openMapClicked, this, &Editor::onMapConnectionDoubleClicked); - connect(pixmapItem, &ConnectionPixmapItem::connectionItemDoubleClicked, this, &Editor::onMapConnectionDoubleClicked); + connect(listItem, &ConnectionsListItem::openMapClicked, this, &Editor::openConnectedMap); + connect(pixmapItem, &ConnectionPixmapItem::connectionItemDoubleClicked, this, &Editor::openConnectedMap); // Sync the selection highlight between the list UI and the pixmap connect(pixmapItem, &ConnectionPixmapItem::selectionChanged, [=](bool selected) { @@ -803,7 +805,7 @@ void Editor::displayConnection(MapConnection* connection) { } } -void Editor::addConnection(MapConnection* connection) { +void Editor::addConnection(MapConnection *connection) { if (!connection) return; @@ -814,7 +816,7 @@ void Editor::addConnection(MapConnection* connection) { this->map->editHistory.push(new MapConnectionAdd(this->map, connection)); } -void Editor::removeConnection(MapConnection* connection) { +void Editor::removeConnection(MapConnection *connection) { if (!connection) return; this->map->editHistory.push(new MapConnectionRemove(this->map, connection)); @@ -825,10 +827,12 @@ void Editor::removeSelectedConnection() { removeConnection(selected_connection_item->connection); } -void Editor::removeConnectionPixmap(MapConnection* connection) { +void Editor::removeConnectionPixmap(MapConnection *connection) { if (!connection) return; + disconnectMapConnection(connection); + if (MapConnection::isDiving(connection->direction())) { removeDivingMapPixmap(connection); return; @@ -857,15 +861,7 @@ void Editor::removeConnectionPixmap(MapConnection* connection) { delete pixmapItem; } -void Editor::displayDivingConnections() { - if (!this->map) - return; - - for (auto connection : this->map->getConnections()) - displayDivingConnection(connection); -} - -void Editor::displayDivingConnection(MapConnection* connection) { +void Editor::displayDivingConnection(MapConnection *connection) { if (!connection) return; @@ -994,7 +990,7 @@ QPoint Editor::getConnectionOrigin(MapConnection *connection) { return QPoint(x * 16, y * 16); } -void Editor::updateConnectionPixmap(ConnectionPixmapItem* pixmapItem) { +void Editor::updateConnectionPixmap(ConnectionPixmapItem *pixmapItem) { if (!pixmapItem) return; @@ -1004,7 +1000,7 @@ void Editor::updateConnectionPixmap(ConnectionPixmapItem* pixmapItem) { maskNonVisibleConnectionTiles(); } -void Editor::setSelectedConnectionItem(ConnectionPixmapItem* pixmapItem) { +void Editor::setSelectedConnectionItem(ConnectionPixmapItem *pixmapItem) { if (!pixmapItem || pixmapItem == selected_connection_item) return; @@ -1025,14 +1021,9 @@ void Editor::setSelectedConnection(MapConnection *connection) { } } -void Editor::onMapConnectionDoubleClicked(MapConnection* connection) { - if (connection) - emit openConnectedMap(connection); -} - void Editor::onBorderMetatilesChanged() { displayMapBorder(); - updateBorderVisibility(); // TODO: Why do we need to call this here + updateBorderVisibility(); } void Editor::onHoveredMovementPermissionChanged(uint16_t collision, uint16_t elevation) { @@ -1214,12 +1205,8 @@ bool Editor::setMap(QString map_name) { if (map) { map->pruneEditHistory(); map->disconnect(this); - for (auto connection : map->getConnections()) { - // Disconnect signals used by the display - QObject::disconnect(connection, &MapConnection::targetMapNameChanged, nullptr, nullptr); - QObject::disconnect(connection, &MapConnection::directionChanged, nullptr, nullptr); - QObject::disconnect(connection, &MapConnection::offsetChanged, nullptr, nullptr); - } + for (auto connection : map->getConnections()) + disconnectMapConnection(connection); } if (project) { diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index cc2b5f7f..1d60c9ad 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1892,7 +1892,7 @@ void MainWindow::on_mainTabBar_tabBarClicked(int index) } else if (index == MainTab::Connections) { editor->setEditingConnections(); // Stop the Dive/Emerge combo boxes from getting the initial focus - ui->graphicsView_Map->setFocus(); + ui->graphicsView_Connections->setFocus(); } if (index != MainTab::WildPokemon) { if (editor->project && editor->project->wildEncountersLoaded) @@ -2542,6 +2542,8 @@ void MainWindow::clickToolButtonFromEditMode(QString editMode) { } void MainWindow::onOpenConnectedMap(MapConnection *connection) { + if (!connection) + return; if (userSetMap(connection->targetMapName(), true)) editor->setSelectedConnection(connection->findMirror()); } diff --git a/src/ui/connectionslistitem.cpp b/src/ui/connectionslistitem.cpp index 337623b4..a5b8759a 100644 --- a/src/ui/connectionslistitem.cpp +++ b/src/ui/connectionslistitem.cpp @@ -75,13 +75,13 @@ void ConnectionsListItem::mousePressEvent(QMouseEvent *) { this->setSelected(true); } -void ConnectionsListItem::on_comboBox_Direction_currentTextChanged(const QString &direction) { +void ConnectionsListItem::on_comboBox_Direction_currentTextChanged(QString direction) { this->setSelected(true); if (this->map) this->map->editHistory.push(new MapConnectionChangeDirection(this->connection, direction)); } -void ConnectionsListItem::on_comboBox_Map_currentTextChanged(const QString &mapName) { +void ConnectionsListItem::on_comboBox_Map_currentTextChanged(QString mapName) { this->setSelected(true); if (this->map && ui->comboBox_Map->findText(mapName) >= 0) this->map->editHistory.push(new MapConnectionChangeMap(this->connection, mapName)); diff --git a/src/ui/mapimageexporter.cpp b/src/ui/mapimageexporter.cpp index b61d3738..65811474 100644 --- a/src/ui/mapimageexporter.cpp +++ b/src/ui/mapimageexporter.cpp @@ -33,7 +33,7 @@ MapImageExporter::MapImageExporter(QWidget *parent_, Editor *editor_, ImageExpor this->editor = editor_; this->mode = mode; this->setWindowTitle(getTitle(this->mode)); - this->ui->groupBox_Connections->setVisible(this->mode == ImageExporterMode::Normal); + this->ui->groupBox_Connections->setVisible(this->mode != ImageExporterMode::Stitch); this->ui->groupBox_Timelapse->setVisible(this->mode == ImageExporterMode::Timelapse); this->ui->comboBox_MapSelection->addItems(editor->project->mapNames); @@ -144,7 +144,7 @@ void MapImageExporter::saveImage() { this->map->editHistory.redo(); } progress.setValue(progress.maximum() - i); - QPixmap pixmap = this->getFormattedMapPixmap(this->map, !this->showBorder); + QPixmap pixmap = this->getFormattedMapPixmap(this->map); if (pixmap.width() < maxWidth || pixmap.height() < maxHeight) { QPixmap pixmap2 = QPixmap(maxWidth, maxHeight); QPainter painter(&pixmap2); @@ -167,7 +167,7 @@ void MapImageExporter::saveImage() { } } // The latest map state is the last animated frame. - QPixmap pixmap = this->getFormattedMapPixmap(this->map, !this->showBorder); + QPixmap pixmap = this->getFormattedMapPixmap(this->map); timelapseImg.addFrame(pixmap.toImage()); timelapseImg.save(filepath); progress.close(); @@ -178,6 +178,9 @@ void MapImageExporter::saveImage() { } bool MapImageExporter::historyItemAppliesToFrame(const QUndoCommand *command) { + if (command->isObsolete()) + return false; + switch (command->id() & 0xFF) { case CommandId::ID_PaintMetatile: case CommandId::ID_BucketFillMetatile: @@ -249,7 +252,9 @@ QPixmap MapImageExporter::getStitchedImage(QProgressDialog *progress, bool inclu int x = cur.x; int y = cur.y; int offset = connection->offset(); - Map *connectionMap = this->editor->project->loadMap(connection->targetMapName()); + Map *connectionMap = connection->targetMap(); + if (!connectionMap) + continue; if (direction == "up") { x += offset; y -= connectionMap->getHeight(); @@ -318,7 +323,7 @@ QPixmap MapImageExporter::getStitchedImage(QProgressDialog *progress, bool inclu pixelX -= STITCH_MODE_BORDER_DISTANCE * 16; pixelY -= STITCH_MODE_BORDER_DISTANCE * 16; } - QPixmap pixmap = this->getFormattedMapPixmap(map.map, false); + QPixmap pixmap = this->getFormattedMapPixmap(map.map); painter.drawPixmap(pixelX, pixelY, pixmap); } @@ -353,7 +358,7 @@ void MapImageExporter::updatePreview() { scene = nullptr; } - preview = getFormattedMapPixmap(this->map, false); + preview = getFormattedMapPixmap(this->map); scene = new QGraphicsScene; scene->addPixmap(preview); this->scene->setSceneRect(this->scene->itemsBoundingRect()); @@ -381,8 +386,7 @@ QPixmap MapImageExporter::getFormattedMapPixmap(Map *map, bool ignoreBorder) { // draw map border // note: this will break when allowing map to be selected from drop down maybe int borderHeight = 0, borderWidth = 0; - bool forceDrawBorder = showUpConnections || showDownConnections || showLeftConnections || showRightConnections; - if (!ignoreBorder && (showBorder || forceDrawBorder)) { + if (!ignoreBorder && this->showBorder) { int borderDistance = this->mode ? STITCH_MODE_BORDER_DISTANCE : BORDER_DISTANCE; map->renderBorder(); int borderHorzDist = editor->getBorderDrawDistance(map->getBorderWidth()); @@ -401,10 +405,9 @@ QPixmap MapImageExporter::getFormattedMapPixmap(Map *map, bool ignoreBorder) { pixmap = newPixmap; } - if (!this->mode) { + if (!ignoreBorder && (this->showUpConnections || this->showDownConnections || this->showLeftConnections || this->showRightConnections)) { // if showing connections, draw on outside of image QPainter connectionPainter(&pixmap); - // TODO: Is this still the most sensible way to do this (esp. rendering pixmap) for (auto connectionItem : editor->connection_items) { const QString direction = connectionItem->connection->direction(); if ((showUpConnections && direction == "up") @@ -421,7 +424,7 @@ QPixmap MapImageExporter::getFormattedMapPixmap(Map *map, bool ignoreBorder) { QPainter eventPainter(&pixmap); QList events = map->getAllEvents(); int pixelOffset = 0; - if (!ignoreBorder && showBorder) { + if (!ignoreBorder && this->showBorder) { pixelOffset = this->mode == ImageExporterMode::Normal ? BORDER_DISTANCE * 16 : STITCH_MODE_BORDER_DISTANCE * 16; } for (Event *event : events) { @@ -459,6 +462,18 @@ QPixmap MapImageExporter::getFormattedMapPixmap(Map *map, bool ignoreBorder) { return pixmap; } +void MapImageExporter::updateShowBorderState() { + // If any of the Connections settings are enabled then this setting is locked (it's implicitly enabled) + const QSignalBlocker blocker(ui->checkBox_Border); + if (showUpConnections || showDownConnections || showLeftConnections || showRightConnections) { + ui->checkBox_Border->setChecked(true); + ui->checkBox_Border->setDisabled(true); + showBorder = true; + } else { + ui->checkBox_Border->setDisabled(false); + } +} + void MapImageExporter::on_checkBox_Elevation_stateChanged(int state) { showCollision = (state == Qt::Checked); updatePreview(); @@ -501,21 +516,25 @@ void MapImageExporter::on_checkBox_HealSpots_stateChanged(int state) { void MapImageExporter::on_checkBox_ConnectionUp_stateChanged(int state) { showUpConnections = (state == Qt::Checked); + updateShowBorderState(); updatePreview(); } void MapImageExporter::on_checkBox_ConnectionDown_stateChanged(int state) { showDownConnections = (state == Qt::Checked); + updateShowBorderState(); updatePreview(); } void MapImageExporter::on_checkBox_ConnectionLeft_stateChanged(int state) { showLeftConnections = (state == Qt::Checked); + updateShowBorderState(); updatePreview(); } void MapImageExporter::on_checkBox_ConnectionRight_stateChanged(int state) { showRightConnections = (state == Qt::Checked); + updateShowBorderState(); updatePreview(); }