Fix z value for events, separate EventPixmapItem from Editor

include/core/events.h

@@ -153,7 +153,7 @@ public:
     QJsonObject getCustomAttributes() const { return this->customAttributes; }
     void setCustomAttributes(const QJsonObject &newCustomAttributes) { this->customAttributes = newCustomAttributes; }
 
-    virtual void loadPixmap(Project *project);
+    virtual QPixmap loadPixmap(Project *project);
 
     void setPixmap(QPixmap newPixmap) { this->pixmap = newPixmap; }
     QPixmap getPixmap() const { return this->pixmap; }
@@ -233,7 +233,7 @@ public:
 
     virtual QSet<QString> getExpectedFields() override;
 
-    virtual void loadPixmap(Project *project) override;
+    virtual QPixmap loadPixmap(Project *project) override;
 
     void setGfx(QString newGfx) { this->gfx = newGfx; }
     QString getGfx() const { return this->gfx; }
@@ -300,7 +300,7 @@ public:
 
     virtual QSet<QString> getExpectedFields() override;
 
-    virtual void loadPixmap(Project *project) override;
+    virtual QPixmap loadPixmap(Project *project) override;
 
     void setTargetMap(QString newTargetMap) { this->targetMap = newTargetMap; }
     QString getTargetMap() const { return this->targetMap; }

include/editor.h

@@ -117,6 +117,7 @@ public:
     void redrawAllEvents();
     void redrawEvents(const QList<Event*> &events);
     void redrawEventPixmapItem(EventPixmapItem *item);
+    void updateEventPixmapItemZValue(EventPixmapItem *item);
     qreal getEventOpacity(const Event *event) const;
 
     void updateCursorRectPos(int x, int y);
@@ -182,6 +183,22 @@ public:
     static void openInTextEditor(const QString &path, int lineNum = 0);
     void setCollisionGraphics();
 
+    enum ZValue {
+        MapBorder = -4,
+        MapConnectionInactive = -3,
+        MapConnectionActive = -2,
+        MapConnectionMask = -1,
+
+        // Event pixmaps set their z value to be their y position on the map.
+        // Their y value is int16_t, so we have enough space to allocate the
+        // full range + 1 for the selected event (which should always be on top).
+        EventMinimum = 1,
+        EventMaximum = EventMinimum + 0x10000,
+
+        Ruler,
+        ResizeLayoutPopup
+    };
+
 public slots:
     void openMapScripts() const;
     void openScript(const QString &scriptLabel) const;

include/ui/eventpixmapitem.h

@@ -10,38 +10,39 @@
 
 #include "events.h"
 
-class Editor;
+class Project;
 
 class EventPixmapItem : public QObject, public QGraphicsPixmapItem {
     Q_OBJECT
 public:
-    EventPixmapItem(QPixmap pixmap): QGraphicsPixmapItem(pixmap) {}
-    
-    EventPixmapItem(Event *event, Editor *editor) : QGraphicsPixmapItem(event->getPixmap()) {
-        this->event = event;
-        event->setPixmapItem(this);
-        this->editor = editor;
-        updatePosition();
-    }
+    explicit EventPixmapItem(Event *event);
 
-    Event *event = nullptr;
+    void render(Project *project);
+
+    bool isSelected() const { return m_selected; }
+    void setSelected(bool selected) { m_selected = selected; }
+
+    Event * getEvent() const { return m_event; }
 
-    void updatePosition();
     void move(int dx, int dy);
+    void moveTo(int x, int y);
     void moveTo(const QPoint &pos);
-    void emitPositionChanged();
-    void updatePixmap();
 
 private:
-    Editor *editor = nullptr;
-    QPoint lastPos;
-    bool active = false;
-    bool releaseSelectionQueued = false;
+    QPixmap m_basePixmap;
+    Event *const m_event = nullptr;
+    QPoint m_lastPos;
+    bool m_active = false;
+    bool m_selected = false;
+    bool m_releaseSelectionQueued = false;
+
+    void updatePixelPosition();
 
 signals:
-    void xChanged(int);
-    void yChanged(int);
-    void spriteChanged(const QPixmap &pixmap);
+    void xChanged(int x);
+    void yChanged(int y);
+    void posChanged(int x, int y);
+    void rendered(const QPixmap &pixmap);
     void selected(Event *event, bool toggle);
     void dragged(Event *event, const QPoint &oldPosition, const QPoint &newPosition);
     void released(Event *event, const QPoint &position);
@@ -51,7 +52,7 @@ protected:
     virtual void mousePressEvent(QGraphicsSceneMouseEvent*) override;
     virtual void mouseMoveEvent(QGraphicsSceneMouseEvent*) override;
     virtual void mouseReleaseEvent(QGraphicsSceneMouseEvent*) override;
-    virtual void mouseDoubleClickEvent(QGraphicsSceneMouseEvent*) override { emit doubleClicked(this->event); }
+    virtual void mouseDoubleClickEvent(QGraphicsSceneMouseEvent*) override { emit doubleClicked(m_event); }
 };
 
 #endif // EVENTPIXMAPITEM_H

src/core/events.cpp

@@ -114,9 +114,10 @@ QString Event::typeToString(Event::Type type) {
     return typeToStringMap.value(type);
 }
 
-void Event::loadPixmap(Project *project) {
+QPixmap Event::loadPixmap(Project *project) {
     this->pixmap = project->getEventPixmap(this->getEventGroup());
     this->usesDefaultPixmap = true;
+    return this->pixmap;
 }
 
 
@@ -225,13 +226,13 @@ QSet<QString> ObjectEvent::getExpectedFields() {
     return expectedFields;
 }
 
-void ObjectEvent::loadPixmap(Project *project) {
+QPixmap ObjectEvent::loadPixmap(Project *project) {
     this->pixmap = project->getEventPixmap(this->gfx, this->movement);
     if (!this->pixmap.isNull()) {
         this->usesDefaultPixmap = false;
-    } else {
-        Event::loadPixmap(project);
+        return this->pixmap;
     }
+    return Event::loadPixmap(project);
 }
 
 
@@ -314,7 +315,7 @@ QSet<QString> CloneObjectEvent::getExpectedFields() {
     return expectedFields;
 }
 
-void CloneObjectEvent::loadPixmap(Project *project) {
+QPixmap CloneObjectEvent::loadPixmap(Project *project) {
     // Try to get the targeted object to clone
     Map *clonedMap = project->loadMap(this->targetMap);
     Event *clonedEvent = clonedMap ? clonedMap->getEvent(Event::Group::Object, this->targetID) : nullptr;
@@ -329,7 +330,7 @@ void CloneObjectEvent::loadPixmap(Project *project) {
         this->gfx = project->gfxDefines.key(0, "0");
         this->movement = project->movementTypes.value(0, "0");
     }
-    ObjectEvent::loadPixmap(project);
+    return ObjectEvent::loadPixmap(project);
 }
 
 

src/editor.cpp

@@ -1484,7 +1484,7 @@ bool Editor::displayLayout() {
         scene->installEventFilter(filter);
         connect(filter, &MapSceneEventFilter::wheelZoom, this, &Editor::onWheelZoom);
         scene->installEventFilter(this->map_ruler);
-        this->map_ruler->setZValue(1000);
+        this->map_ruler->setZValue(ZValue::Ruler);
         scene->addItem(this->map_ruler);
     }
 
@@ -1696,11 +1696,13 @@ void Editor::displayMapEvents() {
 
 EventPixmapItem *Editor::addEventPixmapItem(Event *event) {
     this->project->loadEventPixmap(event);
-    auto item = new EventPixmapItem(event, this);
+    auto item = new EventPixmapItem(event);
     connect(item, &EventPixmapItem::doubleClicked, this, &Editor::openEventMap);
     connect(item, &EventPixmapItem::dragged, this, &Editor::onEventDragged);
     connect(item, &EventPixmapItem::released, this, &Editor::onEventReleased);
     connect(item, &EventPixmapItem::selected, this, &Editor::selectMapEvent);
+    connect(item, &EventPixmapItem::posChanged, [this, event] { updateWarpEventWarning(event); });
+    connect(item, &EventPixmapItem::yChanged, [this, item] { updateEventPixmapItemZValue(item); });
     redrawEventPixmapItem(item);
     this->events_group->addToGroup(item);
     return item;
@@ -1781,6 +1783,7 @@ void Editor::maskNonVisibleConnectionTiles() {
     QBrush brush(ui->graphicsView_Map->palette().color(QPalette::Active, QPalette::Base));
 
     connection_mask = scene->addPath(mask, pen, brush);
+    connection_mask->setZValue(ZValue::MapConnectionMask);
 }
 
 void Editor::clearMapBorder() {
@@ -1806,7 +1809,7 @@ void Editor::displayMapBorder() {
         QGraphicsPixmapItem *item = new QGraphicsPixmapItem(pixmap);
         item->setX(x * 16);
         item->setY(y * 16);
-        item->setZValue(-3);
+        item->setZValue(ZValue::MapBorder);
         scene->addItem(item);
         borderItems.append(item);
     }
@@ -1977,36 +1980,36 @@ qreal Editor::getEventOpacity(const Event *event) const {
 }
 
 void Editor::redrawEventPixmapItem(EventPixmapItem *item) {
-    if (!item || !item->event)
-        return;
+    if (!item) return;
+    Event *event = item->getEvent();
+    if (!event) return;
 
-    project->loadEventPixmap(item->event, true);
-
-    QPixmap pixmap = item->event->getPixmap();
-    if (pixmap.isNull())
-        return;
-
-    qreal zValue = item->event->getY();
     if (this->editMode == EditMode::Events) {
-        if (this->selectedEvents.contains(item->event)) {
-            // Draw the selection rectangle
-            QPainter painter(&pixmap);
-            painter.setPen(Qt::magenta);
-            painter.drawRect(0, 0, pixmap.width() - 1, pixmap.height() - 1);
-            zValue++;
-        }
         item->setAcceptedMouseButtons(Qt::AllButtons);
+        item->setSelected(this->selectedEvents.contains(event));
     } else {
         // Can't interact with event pixmaps outside of event editing mode.
         // We could do setEnabled(false), but rather than ignoring the mouse events this
         // would reject them, which would prevent painting on the map behind the events.
         item->setAcceptedMouseButtons(Qt::NoButton);
+        item->setSelected(false);
     }
-    item->setPixmap(pixmap);
-    item->setZValue(zValue);
-    item->setOpacity(getEventOpacity(item->event));
+    updateEventPixmapItemZValue(item);
+    item->setOpacity(getEventOpacity(event));
     item->setShapeMode(porymapConfig.eventSelectionShapeMode);
-    item->updatePosition();
+    item->render(project);
+}
+
+void Editor::updateEventPixmapItemZValue(EventPixmapItem *item) {
+    if (!item) return;
+    Event *event = item->getEvent();
+    if (!event) return;
+
+    if (item->isSelected()) {
+        item->setZValue(ZValue::EventMaximum);
+    } else {
+        item->setZValue(event->getY() + ((ZValue::EventMaximum - ZValue::EventMinimum) / 2));
+    }
 }
 
 void Editor::onEventDragged(Event *event, const QPoint &oldPosition, const QPoint &newPosition) {

src/ui/connectionpixmapitem.cpp

@@ -1,6 +1,7 @@
 #include "connectionpixmapitem.h"
 #include "editcommands.h"
 #include "map.h"
+#include "editor.h"
 
 #include <math.h>
 
@@ -31,7 +32,7 @@ void ConnectionPixmapItem::render(bool ignoreCache) {
         this->basePixmap = this->connection->render();
 
     QPixmap pixmap = this->basePixmap.copy(0, 0, this->basePixmap.width(), this->basePixmap.height());
-    this->setZValue(-1);
+    this->setZValue(Editor::ZValue::MapConnectionActive);
 
     // When editing is inactive the current selection is ignored, all connections should appear normal.
     if (this->getEditable()) {
@@ -43,7 +44,7 @@ void ConnectionPixmapItem::render(bool ignoreCache) {
             painter.end();
         } else {
             // Darken the image
-            this->setZValue(-2);
+            this->setZValue(Editor::ZValue::MapConnectionInactive);
             QPainter painter(&pixmap);
             int alpha = static_cast<int>(255 * 0.25);
             painter.fillRect(0, 0, pixmap.width(), pixmap.height(), QColor(0, 0, 0, alpha));

src/ui/eventframes.cpp

@@ -320,6 +320,7 @@ void ObjectFrame::connectSignals(MainWindow *window) {
     if (this->connected) return;
 
     EventFrame::connectSignals(window);
+    Project *project = window->editor->project;
 
     // local id
     this->line_edit_local_id->disconnect();
@@ -330,18 +331,18 @@ void ObjectFrame::connectSignals(MainWindow *window) {
 
     // sprite update
     this->combo_sprite->disconnect();
-    connect(this->combo_sprite, &QComboBox::currentTextChanged, [this](const QString &text) {
+    connect(this->combo_sprite, &QComboBox::currentTextChanged, [this, project](const QString &text) {
         this->object->setGfx(text);
-        this->object->getPixmapItem()->updatePixmap();
+        this->object->getPixmapItem()->render(project);
         this->object->modify();
     });
-    connect(this->object->getPixmapItem(), &EventPixmapItem::spriteChanged, this->label_icon, &QLabel::setPixmap);
+    connect(this->object->getPixmapItem(), &EventPixmapItem::rendered, this->label_icon, &QLabel::setPixmap);
 
     // movement
     this->combo_movement->disconnect();
-    connect(this->combo_movement, &QComboBox::currentTextChanged, [this](const QString &text) {
+    connect(this->combo_movement, &QComboBox::currentTextChanged, [this, project](const QString &text) {
         this->object->setMovement(text);
-        this->object->getPixmapItem()->updatePixmap();
+        this->object->getPixmapItem()->render(project);
         this->object->modify();
     });
 
@@ -498,13 +499,13 @@ void CloneObjectFrame::connectSignals(MainWindow *window) {
     });
 
     // update icon displayed in frame with target
-    connect(this->clone->getPixmapItem(), &EventPixmapItem::spriteChanged, this->label_icon, &QLabel::setPixmap);
+    connect(this->clone->getPixmapItem(), &EventPixmapItem::rendered, this->label_icon, &QLabel::setPixmap);
 
     // target map
     this->combo_target_map->disconnect();
     connect(this->combo_target_map, &QComboBox::currentTextChanged, [this, project](const QString &mapName) {
         this->clone->setTargetMap(mapName);
-        this->clone->getPixmapItem()->updatePixmap();
+        this->clone->getPixmapItem()->render(project);
         this->combo_sprite->setCurrentText(this->clone->getGfx());
         this->clone->modify();
         populateIdNameDropdown(this->combo_target_id, project, mapName, Event::Group::Object);
@@ -513,9 +514,9 @@ void CloneObjectFrame::connectSignals(MainWindow *window) {
 
     // target id
     this->combo_target_id->disconnect();
-    connect(this->combo_target_id, &QComboBox::currentTextChanged, [this](const QString &text) {
+    connect(this->combo_target_id, &QComboBox::currentTextChanged, [this, project](const QString &text) {
         this->clone->setTargetID(text);
-        this->clone->getPixmapItem()->updatePixmap();
+        this->clone->getPixmapItem()->render(project);
         this->combo_sprite->setCurrentText(this->clone->getGfx());
         this->clone->modify();
     });

src/ui/eventpixmapitem.cpp

@@ -1,52 +1,80 @@
 #include "eventpixmapitem.h"
-#include "editor.h"
+#include "project.h"
 #include "editcommands.h"
 #include "mapruler.h"
 #include "metatile.h"
 
+EventPixmapItem::EventPixmapItem(Event *event)
+  : QGraphicsPixmapItem(event->getPixmap()),
+    m_basePixmap(pixmap()),
+    m_event(event)
+{
+    m_event->setPixmapItem(this);
+    updatePixelPosition();
+}
+
+void EventPixmapItem::render(Project *project) {
+    if (!m_event)
+        return;
+
+    m_basePixmap = m_event->loadPixmap(project);
+
+    // If the base pixmap changes, the event's pixel position may change.
+    updatePixelPosition();
+
+    QPixmap pixmap = m_basePixmap;
+    if (m_selected) {
+        // Draw the selection rectangle
+        QPainter painter(&pixmap);
+        painter.setPen(Qt::magenta);
+        painter.drawRect(0, 0, pixmap.width() - 1, pixmap.height() - 1);
+    }
+    setPixmap(pixmap);
+    emit rendered(m_basePixmap);
+}
+
 void EventPixmapItem::move(int dx, int dy) {
-    event->setX(event->getX() + dx);
-    event->setY(event->getY() + dy);
-    updatePosition();
-    emitPositionChanged();
+    moveTo(m_event->getX() + dx,
+           m_event->getY() + dy);
 }
 
 void EventPixmapItem::moveTo(const QPoint &pos) {
-    event->setX(pos.x());
-    event->setY(pos.y());
-    updatePosition();
-    emitPositionChanged();
+    moveTo(pos.x(), pos.y());
 }
 
-void EventPixmapItem::updatePosition() {
-    int x = this->event->getPixelX();
-    int y = this->event->getPixelY();
-    setX(x);
-    setY(y);
-    editor->updateWarpEventWarning(event);
+void EventPixmapItem::moveTo(int x, int y) {
+    bool changed = false;
+    if (m_event->getX() != x) {
+        m_event->setX(x);
+        emit xChanged(x);
+        changed = true;
+    }
+    if (m_event->getY() != y) {
+        m_event->setY(y);
+        emit yChanged(y);
+        changed = true;
+    }
+    if (changed) {
+        updatePixelPosition();
+        emit posChanged(x, y);
+    }
 }
 
-void EventPixmapItem::emitPositionChanged() {
-    emit xChanged(event->getX());
-    emit yChanged(event->getY());
-}
-
-void EventPixmapItem::updatePixmap() {
-    editor->redrawEventPixmapItem(this);
-    emit spriteChanged(event->getPixmap());
+void EventPixmapItem::updatePixelPosition() {
+    setPos(m_event->getPixelX(), m_event->getPixelY());
 }
 
 void EventPixmapItem::mousePressEvent(QGraphicsSceneMouseEvent *mouseEvent) {
-    if (this->active)
+    if (m_active)
         return;
-    this->active = true;
-    this->lastPos = Metatile::coordFromPixmapCoord(mouseEvent->scenePos());
+    m_active = true;
+    m_lastPos = Metatile::coordFromPixmapCoord(mouseEvent->scenePos());
 
     bool selectionToggle = mouseEvent->modifiers() & Qt::ControlModifier;
-    if (selectionToggle || !this->editor->selectedEvents.contains(this->event)) {
+    if (selectionToggle || !m_selected) {
         // User is either toggling this selection on/off as part of a group selection,
         // or they're newly selecting just this item.
-        emit selected(this->event, selectionToggle);
+        emit selected(m_event, selectionToggle);
     } else {
         // This item is already selected and the user isn't toggling the selection, so there are 4 possibilities:
         // 1. This is the only selected event, and the selection is pointless.
@@ -55,32 +83,32 @@ void EventPixmapItem::mousePressEvent(QGraphicsSceneMouseEvent *mouseEvent) {
         // 4. There's a group selection, and they want to drag the group around.
         // 'selectMapEvent' will immediately clear the rest of the selection, which supports #1-3 but prevents #4.
         // To support #4 we set the flag below, and we only call 'selectMapEvent' on mouse release if no move occurred.
-        this->releaseSelectionQueued = true;
+        m_releaseSelectionQueued = true;
     }
     mouseEvent->accept();
 }
 
 void EventPixmapItem::mouseMoveEvent(QGraphicsSceneMouseEvent *mouseEvent) {
-    if (!this->active)
+    if (!m_active)
         return;
 
     QPoint pos = Metatile::coordFromPixmapCoord(mouseEvent->scenePos());
-    if (pos == this->lastPos)
+    if (pos == m_lastPos)
         return;
 
-    this->releaseSelectionQueued = false;
-    emit dragged(this->event, this->lastPos, pos);
-    this->lastPos = pos;
+    m_releaseSelectionQueued = false;
+    emit dragged(m_event, m_lastPos, pos);
+    m_lastPos = pos;
 }
 
 void EventPixmapItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *mouseEvent) {
-    if (!this->active)
+    if (!m_active)
         return;
-    this->active = false;
-    if (this->releaseSelectionQueued) {
-        this->releaseSelectionQueued = false;
-        if (Metatile::coordFromPixmapCoord(mouseEvent->scenePos()) == this->lastPos)
-            emit selected(this->event, false);
+    m_active = false;
+    if (m_releaseSelectionQueued) {
+        m_releaseSelectionQueued = false;
+        if (Metatile::coordFromPixmapCoord(mouseEvent->scenePos()) == m_lastPos)
+            emit selected(m_event, false);
     }
-    emit released(this->event, this->lastPos);
+    emit released(m_event, m_lastPos);
 }

src/ui/mapimageexporter.cpp

@@ -113,8 +113,7 @@ void MapImageExporter::setModeSpecificUi() {
     }
 
     if (m_mode == ImageExporterMode::Timelapse) {
-        // TODO: At the moment edit history for events (and the EventPixmapItem class)
-        // explicitly depend on the editor and assume their map is currently open.
+        // TODO: At the moment edit history for events explicitly depend on the editor and assume their map is currently open.
         // Other edit commands rely on this more subtly, like triggering API callbacks or
         // spending time rendering their layout (which can make creating timelapses very slow).
         // Until this is resolved, the selected map/layout must remain the same as in the editor.

src/ui/movablerect.cpp

@@ -27,7 +27,6 @@ ResizableRect::ResizableRect(QObject *parent, bool *enabled, int width, int heig
   : QObject(parent),
     MovableRect(enabled, width * 16, height * 16, color)
 {
-        setZValue(0xFFFFFFFF); // ensure on top of view
         setAcceptHoverEvents(true);
         setFlags(this->flags() | QGraphicsItem::ItemIsMovable);
 }

src/ui/resizelayoutpopup.cpp

@@ -139,6 +139,7 @@ void ResizeLayoutPopup::setupLayoutView() {
     static bool layoutSizeRectVisible = true;
 
     this->outline = new ResizableRect(this, &layoutSizeRectVisible, this->editor->layout->getWidth(), this->editor->layout->getHeight(), qRgb(255, 0, 255));
+    this->outline->setZValue(Editor::ZValue::ResizeLayoutPopup); // Ensure on top of view
     this->outline->setLimit(cover->rect().toAlignedRect());
     connect(outline, &ResizableRect::rectUpdated, [=](QRect rect){
         // Note: this extra limit check needs access to the project values, so it is done here and not ResizableRect::mouseMoveEvent