From 0b4f02779b073b3578b1ed8a2b4df762224621c1 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Fri, 30 Aug 2024 13:15:39 -0400 Subject: [PATCH] Fix regression for dragging multiple events --- include/editor.h | 3 +- include/ui/draggablepixmapitem.h | 12 +++--- src/core/editcommands.cpp | 2 +- src/editor.cpp | 12 ++---- src/mainwindow.cpp | 2 +- src/ui/draggablepixmapitem.cpp | 70 ++++++++++++++++++++++---------- 6 files changed, 60 insertions(+), 41 deletions(-) diff --git a/include/editor.h b/include/editor.h index 89b6ccb59..3e5cefd1b 100644 --- a/include/editor.h +++ b/include/editor.h @@ -95,8 +95,7 @@ class Editor : public QObject Tileset *getCurrentMapPrimaryTileset(); DraggablePixmapItem *addMapEvent(Event *event); - void selectMapEvent(DraggablePixmapItem *object); - void selectMapEvent(DraggablePixmapItem *object, bool toggle); + void selectMapEvent(DraggablePixmapItem *object, bool toggle = false); DraggablePixmapItem *addNewEvent(Event::Type type); void updateSelectedEvents(); void duplicateSelectedEvents(); diff --git a/include/ui/draggablepixmapitem.h b/include/ui/draggablepixmapitem.h index 9c3189621..aeda4daf2 100644 --- a/include/ui/draggablepixmapitem.h +++ b/include/ui/draggablepixmapitem.h @@ -24,13 +24,7 @@ class DraggablePixmapItem : public QObject, public QGraphicsPixmapItem { updatePosition(); } - Editor *editor = nullptr; Event *event = nullptr; - QGraphicsItemAnimation *pos_anim = nullptr; - - bool active; - int last_x; - int last_y; void updatePosition(); void move(int dx, int dy); @@ -38,6 +32,12 @@ class DraggablePixmapItem : public QObject, public QGraphicsPixmapItem { void emitPositionChanged(); void updatePixmap(); +private: + Editor *editor = nullptr; + QPoint lastPos; + bool active = false; + bool releaseSelectionQueued = false; + signals: void positionChanged(Event *event); void xChanged(int); diff --git a/src/core/editcommands.cpp b/src/core/editcommands.cpp index 640cb43fc..f557ddddb 100644 --- a/src/core/editcommands.cpp +++ b/src/core/editcommands.cpp @@ -331,7 +331,7 @@ void EventCreate::redo() { // select this event editor->selected_events->clear(); - editor->selectMapEvent(event->getPixmapItem(), false); + editor->selectMapEvent(event->getPixmapItem()); } void EventCreate::undo() { diff --git a/src/editor.cpp b/src/editor.cpp index 229577907..fc0c79ae7 100644 --- a/src/editor.cpp +++ b/src/editor.cpp @@ -1358,7 +1358,7 @@ void Editor::mouseEvent_map(QGraphicsSceneMouseEvent *event, MapPixmapItem *item if (newEvent) { newEvent->move(pos.x(), pos.y()); emit objectsChanged(); - selectMapEvent(newEvent, false); + selectMapEvent(newEvent); } } } @@ -1994,10 +1994,6 @@ void Editor::updateSelectedEvents() { emit objectsChanged(); } -void Editor::selectMapEvent(DraggablePixmapItem *object) { - selectMapEvent(object, false); -} - void Editor::selectMapEvent(DraggablePixmapItem *object, bool toggle) { if (!selected_events || !object) return; @@ -2228,10 +2224,8 @@ void Editor::objectsView_onMousePress(QMouseEvent *event) { bool multiSelect = event->modifiers() & Qt::ControlModifier; if (!selectingEvent && !multiSelect && selected_events->length() > 1) { - DraggablePixmapItem *first = selected_events->first(); - selected_events->clear(); - selected_events->append(first); - updateSelectedEvents(); + // User is clearing group selection by clicking on the background + this->selectMapEvent(selected_events->first()); } selectingEvent = false; } diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 23b7eb064..e441fa19b 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -2014,7 +2014,7 @@ void MainWindow::addNewEvent(Event::Type type) { auto centerPos = ui->graphicsView_Map->mapToScene(halfSize.width(), halfSize.height()); object->moveTo(Metatile::coordFromPixmapCoord(centerPos)); updateObjects(); - editor->selectMapEvent(object, false); + editor->selectMapEvent(object); } else { QMessageBox msgBox(this); msgBox.setText("Failed to add new event"); diff --git a/src/ui/draggablepixmapitem.cpp b/src/ui/draggablepixmapitem.cpp index 225116435..59b069ddf 100644 --- a/src/ui/draggablepixmapitem.cpp +++ b/src/ui/draggablepixmapitem.cpp @@ -34,11 +34,26 @@ void DraggablePixmapItem::updatePixmap() { } void DraggablePixmapItem::mousePressEvent(QGraphicsSceneMouseEvent *mouse) { - active = true; - QPoint pos = Metatile::coordFromPixmapCoord(mouse->scenePos()); - last_x = pos.x(); - last_y = pos.y(); - this->editor->selectMapEvent(this, mouse->modifiers() & Qt::ControlModifier); + if (this->active) + return; + this->active = true; + this->lastPos = Metatile::coordFromPixmapCoord(mouse->scenePos()); + + bool selectionToggle = mouse->modifiers() & Qt::ControlModifier; + if (selectionToggle || !editor->selected_events->contains(this)) { + // User is either toggling this selection on/off as part of a group selection, + // or they're newly selecting just this item. + this->editor->selectMapEvent(this, 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. + // 2. This is the only selected event, and they want to drag the item around. + // 3. There's a group selection, and they want to start a new selection with just this item. + // 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; + } this->editor->selectingEvent = true; } @@ -57,28 +72,39 @@ void DraggablePixmapItem::moveTo(const QPoint &pos) { } void DraggablePixmapItem::mouseMoveEvent(QGraphicsSceneMouseEvent *mouse) { - if (active) { - QPoint pos = Metatile::coordFromPixmapCoord(mouse->scenePos()); - if (pos.x() != last_x || pos.y() != last_y) { - emit this->editor->map_item->hoveredMapMetatileChanged(pos); - QList selectedEvents; - if (editor->selected_events->contains(this)) { - for (DraggablePixmapItem *item : *editor->selected_events) { - selectedEvents.append(item->event); - } - } else { - selectedEvents.append(this->event); - } - editor->map->editHistory.push(new EventMove(selectedEvents, pos.x() - last_x, pos.y() - last_y, currentActionId)); - last_x = pos.x(); - last_y = pos.y(); + if (!this->active) + return; + + QPoint pos = Metatile::coordFromPixmapCoord(mouse->scenePos()); + if (pos == this->lastPos) + return; + + QPoint moveDistance = pos - this->lastPos; + this->lastPos = pos; + emit this->editor->map_item->hoveredMapMetatileChanged(pos); + + QList selectedEvents; + if (editor->selected_events->contains(this)) { + for (DraggablePixmapItem *item : *editor->selected_events) { + selectedEvents.append(item->event); } + } else { + selectedEvents.append(this->event); } + editor->map->editHistory.push(new EventMove(selectedEvents, moveDistance.x(), moveDistance.y(), currentActionId)); + this->releaseSelectionQueued = false; } -void DraggablePixmapItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *) { - active = false; +void DraggablePixmapItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *mouse) { + if (!this->active) + return; + this->active = false; currentActionId++; + if (this->releaseSelectionQueued) { + this->releaseSelectionQueued = false; + if (Metatile::coordFromPixmapCoord(mouse->scenePos()) == this->lastPos) + this->editor->selectMapEvent(this); + } } void DraggablePixmapItem::mouseDoubleClickEvent(QGraphicsSceneMouseEvent *) {