Introduce generic IAnimatedItem interface for scene animations

GameScene's shared 10ms animation timer previously only knew about
CardItems (cardsToAnimate). Generalize it so any scene item can tick on
the shared timer instead of owning its own QTimer:

- New IAnimatedItem interface with a single animationEvent() tick.
- GameScene tracks animated items in a QHash keyed by QObject and
  auto-unregisters items when they are destroyed, so an item deleted
  mid-animation (concede, removePlayer, deleteLater) can never leave a
  dangling pointer in the set.
- GameScene::~GameScene disconnects incoming connections before the
  animation timer is deleted; all timer stops are null-guarded so
  destruction ordering no longer matters.
- AbstractCardItem implements IAnimatedItem with a no-op tick so the
  existing tap-animation registration path keeps working; CardItem
  overrides it with the real rotate animation.

Took 5 minutes

cockatrice/src/game_graphics/animated_item.h

@@ -0,0 +1,26 @@
+#ifndef ANIMATED_ITEM_H
+#define ANIMATED_ITEM_H
+
+/**
+ * @file animated_item.h
+ * @ingroup GameGraphics
+ * @brief Interface for scene items driven by GameScene's shared animation timer.
+ *
+ * Items that want per-tick animation while a single QBasicTimer runs (instead of
+ * owning their own QTimer) implement this interface and register with the scene
+ * via GameScene::registerAnimationItem.
+ */
+
+class IAnimatedItem
+{
+public:
+    virtual ~IAnimatedItem() = default;
+
+    /**
+     * @brief Advances the item's animation by one timer tick.
+     * @return true while the animation is still running, false once it has finished.
+     */
+    virtual bool animationEvent() = 0;
+};
+
+#endif

cockatrice/src/game_graphics/board/abstract_card_item.cpp

@@ -305,6 +305,11 @@ void AbstractCardItem::setTapped(bool _tapped, bool canAnimate)
     }
 }
 
+bool AbstractCardItem::animationEvent()
+{
+    return false;
+}
+
 void AbstractCardItem::setFaceDown(bool _facedown)
 {
     facedown = _facedown;

cockatrice/src/game_graphics/board/abstract_card_item.h

@@ -7,6 +7,7 @@
 #ifndef ABSTRACTCARDITEM_H
 #define ABSTRACTCARDITEM_H
 
+#include "../animated_item.h"
 #include "../card_dimensions.h"
 #include "arrow_target.h"
 #include "graphics_item_type.h"
@@ -16,7 +17,7 @@
 
 class PlayerLogic;
 
-class AbstractCardItem : public ArrowTarget
+class AbstractCardItem : public ArrowTarget, public IAnimatedItem
 {
     Q_OBJECT
 protected:
@@ -126,6 +127,9 @@ public:
         emit deleteCardInfoPopup(cardRef.name);
     }
 
+    /** @brief Default: no per-tick animation. Subclasses override to animate. */
+    bool animationEvent() override;
+
 protected:
     void transformPainter(QPainter *painter, const QSizeF &translatedSize, int angle);
     void mousePressEvent(QGraphicsSceneMouseEvent *event) override;

cockatrice/src/game_graphics/board/card_item.h

@@ -137,7 +137,7 @@ public:
     void resetState(bool keepAnnotations = false);
     void processCardInfo(const ServerInfo_Card &_info);
 
-    bool animationEvent();
+    bool animationEvent() override;
     CardDragItem *createDragItem(int _id, const QPointF &_pos, const QPointF &_scenePos, bool forceFaceDown);
     void deleteDragItem();
     void drawArrow(const QColor &arrowColor);

cockatrice/src/game_graphics/game_scene.cpp

@@ -17,7 +17,6 @@
 #include <QDebug>
 #include <QGraphicsSceneMouseEvent>
 #include <QGraphicsView>
-#include <QSet>
 #include <QtMath>
 #include <libcockatrice/settings/interface_settings.h>
 #include <libcockatrice/utility/zone_names.h>
@@ -45,7 +44,14 @@ GameScene::GameScene(PhasesToolbar *_phasesToolbar, QObject *parent)
 
 GameScene::~GameScene()
 {
+    // Sever all incoming connections (animated item destroy-tracking) before the
+    // members below are destroyed: the base QGraphicsScene destructor destroys the
+    // remaining items, and their destroyed() signals must not reach slots that
+    // reference members that no longer exist.
+    disconnect(this);
+
     delete animationTimer;
+    animationTimer = nullptr;
 
     // Delete all ArrowItems before QGraphicsScene's base destructor runs.
     // QGraphicsScene::~QGraphicsScene() destroys items in arbitrary order.
@@ -736,30 +742,45 @@ bool GameScene::event(QEvent *event)
 
 void GameScene::timerEvent(QTimerEvent * /*event*/)
 {
-    QMutableSetIterator<CardItem *> i(cardsToAnimate);
+    QMutableHashIterator<QObject *, IAnimatedItem *> i(animatedItems);
     while (i.hasNext()) {
         i.next();
         if (!i.value()->animationEvent()) {
             i.remove();
         }
     }
-    if (cardsToAnimate.isEmpty()) {
+    if (animatedItems.isEmpty()) {
         animationTimer->stop();
     }
 }
 
-void GameScene::registerAnimationItem(AbstractCardItem *card)
+void GameScene::registerAnimationItem(IAnimatedItem *item)
 {
-    cardsToAnimate.insert(static_cast<CardItem *>(card));
-    if (!animationTimer->isActive()) {
+    auto *object = dynamic_cast<QObject *>(item);
+    if (!object) {
+        return;
+    }
+    if (!animatedItems.contains(object)) {
+        connect(object, &QObject::destroyed, this, &GameScene::removeAnimatedItem);
+    }
+    animatedItems.insert(object, item);
+    if (animationTimer && !animationTimer->isActive()) {
         animationTimer->start(10, this);
     }
 }
 
-void GameScene::unregisterAnimationItem(AbstractCardItem *card)
+void GameScene::unregisterAnimationItem(IAnimatedItem *item)
 {
-    cardsToAnimate.remove(static_cast<CardItem *>(card));
-    if (cardsToAnimate.isEmpty()) {
+    animatedItems.remove(dynamic_cast<QObject *>(item));
+    if (animationTimer && animatedItems.isEmpty()) {
+        animationTimer->stop();
+    }
+}
+
+void GameScene::removeAnimatedItem(QObject *item)
+{
+    animatedItems.remove(item);
+    if (animationTimer && animatedItems.isEmpty()) {
         animationTimer->stop();
     }
 }

cockatrice/src/game_graphics/game_scene.h

@@ -4,13 +4,14 @@
 #include "../game/arrow_registry.h"
 #include "../game/board/arrow_data.h"
 #include "../game/zones/card_zone_logic.h"
+#include "animated_item.h"
 #include "board/arrow_item.h"
 
 #include <QGraphicsScene>
+#include <QHash>
 #include <QList>
 #include <QLoggingCategory>
 #include <QPointer>
-#include <QSet>
 
 inline Q_LOGGING_CATEGORY(GameSceneLog, "game_scene");
 inline Q_LOGGING_CATEGORY(GameScenePlayerAdditionRemovalLog, "game_scene.player_addition_removal");
@@ -24,6 +25,7 @@ class CardItem;
 class ServerInfo_Card;
 class PhasesToolbar;
 class QBasicTimer;
+class QObject;
 
 /**
  * @class GameScene
@@ -50,8 +52,8 @@ private:
     QList<ZoneViewWidget *> zoneViews;                  ///< Active zone view widgets
     QSize viewSize;                                     ///< Current view size
     QPointer<CardItem> hoveredCard;                     ///< Currently hovered card
-    QBasicTimer *animationTimer;                        ///< Timer for card animations
-    QSet<CardItem *> cardsToAnimate;                    ///< Cards currently animating
+    QBasicTimer *animationTimer;                        ///< Timer for scene animations
+    QHash<QObject *, IAnimatedItem *> animatedItems;    ///< Items currently animating
     int playerRotation;                                 ///< Rotation offset for player layout
 
     /**
@@ -182,15 +184,24 @@ public:
     /** @brief Updates hovered card highlighting. */
     void updateHoveredCard(CardItem *newCard);
 
-    /** @brief Registers a card for animation updates. */
-    void registerAnimationItem(AbstractCardItem *card);
+    /**
+     * @brief Registers an item for animation updates with the shared scene timer.
+     *
+     * The item must inherit QObject; it is unregistered automatically when it is
+     * destroyed, so it may be deleted mid-animation without a dangling pointer.
+     */
+    void registerAnimationItem(IAnimatedItem *item);
 
-    /** @brief Unregisters a card from animation updates. */
-    void unregisterAnimationItem(AbstractCardItem *card);
+    /** @brief Unregisters an item from animation updates. */
+    void unregisterAnimationItem(IAnimatedItem *item);
     void startRubberBand(const QPointF &selectionOrigin);
     void resizeRubberBand(const QPointF &cursorPoint, int selectedCount);
     void stopRubberBand();
 
+private slots:
+    /** @brief Removes a destroyed item from the animation set. */
+    void removeAnimatedItem(QObject *item);
+
 public slots:
     void onCardSelectionChanged(AbstractCardItem *card, bool selected);
     void onCardRightClicked(AbstractCardItem *card, QPoint screenPos);