diff --git a/cockatrice/src/game_graphics/board/arrow_item.cpp b/cockatrice/src/game_graphics/board/arrow_item.cpp index f7df98ab5..b07a156bd 100644 --- a/cockatrice/src/game_graphics/board/arrow_item.cpp +++ b/cockatrice/src/game_graphics/board/arrow_item.cpp @@ -88,7 +88,7 @@ void ArrowItem::onTargetDestroyed() // Drag and attach arrows are never inserted into the arrow registry and // have no server-side counterpart, so no deletion event can clean them // up. Delete them locally when either endpoint is destroyed. - delArrow(); + cancel(); return; } @@ -103,6 +103,12 @@ void ArrowItem::delArrow() deleteLater(); } +void ArrowItem::cancel() +{ + ungrabMouse(); + delArrow(); +} + void ArrowItem::updatePath() { if (!targetItem) { @@ -350,6 +356,17 @@ void ArrowDragItem::addChildArrow(ArrowDragItem *child) childArrows.append(child); } +void ArrowDragItem::cancel() +{ + for (const auto &child : childArrows) { + if (child) { + child->cancel(); + } + } + childArrows.clear(); + ArrowItem::cancel(); +} + void ArrowDragItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) { if (targetLocked || !startItem) { @@ -393,8 +410,10 @@ void ArrowDragItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) targetItem ? updatePath() : updatePath(endPos); update(); - for (auto *child : childArrows) { - child->mouseMoveEvent(event); + for (const auto &child : childArrows) { + if (child) { + child->mouseMoveEvent(event); + } } } @@ -403,10 +422,7 @@ void ArrowDragItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) if (!startItem) { // The source card was destroyed while the arrow was being drawn. // Clean up the arrow and its children instead of leaking them. - delArrow(); - for (auto *child : childArrows) { - child->mouseReleaseEvent(event); - } + cancel(); return; } @@ -415,7 +431,7 @@ void ArrowDragItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) // For now, we can safely assume that the start item is always a card. // The target item can be a player as well. if (!startCard) { - delArrow(); + cancel(); return; } @@ -435,7 +451,7 @@ void ArrowDragItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) } else if (auto *targetPlayer = qgraphicsitem_cast(targetItem)) { arrow.targetPlayerId = targetPlayer->getOwner()->getPlayerInfo()->getId(); } else { - delArrow(); + cancel(); return; } @@ -448,9 +464,12 @@ void ArrowDragItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) } delArrow(); - for (auto *child : childArrows) { - child->mouseReleaseEvent(event); + for (const auto &child : childArrows) { + if (child) { + child->mouseReleaseEvent(event); + } } + childArrows.clear(); } // ArrowAttachItem @@ -471,6 +490,17 @@ void ArrowAttachItem::addChildArrow(ArrowAttachItem *child) childArrows.append(child); } +void ArrowAttachItem::cancel() +{ + for (const auto &child : childArrows) { + if (child) { + child->cancel(); + } + } + childArrows.clear(); + ArrowItem::cancel(); +} + void ArrowAttachItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) { if (targetLocked || !startItem) { @@ -509,8 +539,10 @@ void ArrowAttachItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) targetItem ? updatePath() : updatePath(endPos); update(); - for (auto *child : childArrows) { - child->mouseMoveEvent(event); + for (const auto &child : childArrows) { + if (child) { + child->mouseMoveEvent(event); + } } } @@ -519,17 +551,16 @@ void ArrowAttachItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) if (!startItem) { // The source card was destroyed while the arrow was being drawn. // Clean up the arrow and its children instead of leaking them. - delArrow(); - for (auto *child : childArrows) { - child->mouseReleaseEvent(event); - } + cancel(); return; } // Attaching could move startItem under the current cursor position, causing all children to retarget to it right // before they are processed. Prevent that. - for (auto *child : childArrows) { - child->setTargetLocked(true); + for (const auto &child : childArrows) { + if (child) { + child->setTargetLocked(true); + } } if (targetItem && targetItem != startItem) { @@ -541,7 +572,10 @@ void ArrowAttachItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) } delArrow(); - for (auto *child : childArrows) { - child->mouseReleaseEvent(event); + for (const auto &child : childArrows) { + if (child) { + child->mouseReleaseEvent(event); + } } + childArrows.clear(); } diff --git a/cockatrice/src/game_graphics/board/arrow_item.h b/cockatrice/src/game_graphics/board/arrow_item.h index 64a48cd14..c70a1030b 100644 --- a/cockatrice/src/game_graphics/board/arrow_item.h +++ b/cockatrice/src/game_graphics/board/arrow_item.h @@ -70,6 +70,8 @@ public: void onTargetDestroyed(); void delArrow(); + /// Abandons an in-progress drag arrow, discarding any child arrows with it. + virtual void cancel(); void updatePath(); void updatePath(const QPointF &endPoint); void startDrawAnimation(); @@ -112,12 +114,13 @@ class ArrowDragItem : public ArrowItem private: PlayerLogic *player; int deleteInPhase; - QList childArrows; + QList> childArrows; QMetaObject::Connection positionConnection; public: ArrowDragItem(PlayerLogic *_owner, ArrowTarget *_startItem, const QColor &_color, int _deleteInPhase); void addChildArrow(ArrowDragItem *child); + void cancel() override; protected: void mouseMoveEvent(QGraphicsSceneMouseEvent *event) override; @@ -129,12 +132,13 @@ class ArrowAttachItem : public ArrowItem Q_OBJECT private: PlayerLogic *player; - QList childArrows; + QList> childArrows; QMetaObject::Connection positionConnection; public: explicit ArrowAttachItem(ArrowTarget *_startItem); void addChildArrow(ArrowAttachItem *child); + void cancel() override; protected: void mouseMoveEvent(QGraphicsSceneMouseEvent *event) override; diff --git a/cockatrice/src/game_graphics/board/card_item.cpp b/cockatrice/src/game_graphics/board/card_item.cpp index acd0764c9..02c0427d7 100644 --- a/cockatrice/src/game_graphics/board/card_item.cpp +++ b/cockatrice/src/game_graphics/board/card_item.cpp @@ -294,6 +294,13 @@ void CardItem::drawArrow(const QColor &arrowColor) return; } + // A held shortcut repeats the action before the previous arrow is placed. + // That arrow has lost the mouse grab to the new one, so it would never + // receive the release that deletes it; discard it (and its children). + if (pendingArrow) { + pendingArrow->cancel(); + } + auto *game = owner->getGame(); PlayerLogic *arrowOwner = game->getPlayerManager()->getActiveLocalPlayer(game->getGameState()->getActivePlayer()); int phase = 0; // 0 means to not set the phase @@ -303,6 +310,7 @@ void CardItem::drawArrow(const QColor &arrowColor) } ArrowDragItem *arrow = new ArrowDragItem(arrowOwner, this, arrowColor, phase); scene()->addItem(arrow); + pendingArrow = arrow; arrow->grabMouse(); for (const auto &item : scene()->selectedItems()) { @@ -326,8 +334,14 @@ void CardItem::drawAttachArrow() return; } + // See drawArrow(): a repeated trigger must not orphan the previous arrow. + if (pendingArrow) { + pendingArrow->cancel(); + } + auto *arrow = new ArrowAttachItem(this); scene()->addItem(arrow); + pendingArrow = arrow; arrow->grabMouse(); for (const auto &item : scene()->selectedItems()) { diff --git a/cockatrice/src/game_graphics/board/card_item.h b/cockatrice/src/game_graphics/board/card_item.h index 01dacc021..ae22d9493 100644 --- a/cockatrice/src/game_graphics/board/card_item.h +++ b/cockatrice/src/game_graphics/board/card_item.h @@ -13,11 +13,13 @@ #include #include #include +#include #include #include #include #include +class ArrowItem; class CardDragItem; class ServerInfo_Card; class PlayerLogic; @@ -34,6 +36,7 @@ class CardItem : public AbstractCardItem Q_OBJECT private: CardDragItem *dragItem; + QPointer pendingArrow; void prepareDelete(); void handleClickedToPlay(bool shiftHeld);