[Game] Move state fields out of CardItem (#6904)

* [Game] Move state fields out of CardItem

Took 1 hour 2 minutes

* Move stuff into .cpp

Took 14 minutes

* Signals pass changed values as params

Took 2 minutes

* Comments.

Took 23 minutes

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cockatrice/CMakeLists.txt

@@ -64,6 +64,7 @@ set(cockatrice_SOURCES
     src/game/board/card_drag_item.cpp
     src/game/board/card_item.cpp
     src/game/board/card_list.cpp
+    src/game/board/card_state.cpp
     src/game/board/counter_general.cpp
     src/game/board/translate_counter_name.cpp
     src/game/deckview/deck_view.cpp

cockatrice/src/game/board/card_item.cpp

@@ -21,13 +21,12 @@
 #include <libcockatrice/protocol/pb/serverinfo_card.pb.h>
 
 CardItem::CardItem(Player *_owner, QGraphicsItem *parent, const CardRef &cardRef, int _cardid, CardZoneLogic *_zone)
-    : AbstractCardItem(parent, cardRef, _owner, _cardid), zone(_zone), attacking(false), destroyOnZoneChange(false),
+    : AbstractCardItem(parent, cardRef, _owner, _cardid), state(new CardState(this, _zone)), dragItem(nullptr)
-      doesntUntap(false), dragItem(nullptr), attachedTo(nullptr)
 {
     owner->addCard(this);
 
     connect(&SettingsCache::instance().cardCounters(), &CardCounterSettings::colorChanged, this, [this](int counterId) {
-        if (counters.contains(counterId)) {
+        if (state->getCounters().contains(counterId)) {
             update();
         }
     });
@@ -48,9 +47,9 @@ void CardItem::prepareDelete()
         attachedCards.first()->setAttachedTo(nullptr);
     }
 
-    if (attachedTo != nullptr) {
+    if (state->getAttachedTo() != nullptr) {
-        attachedTo->removeAttachedCard(this);
+        state->getAttachedTo()->removeAttachedCard(this);
-        attachedTo = nullptr;
+        state->setAttachedTo(nullptr);
     }
 }
 
@@ -65,7 +64,7 @@ void CardItem::deleteLater()
 
 void CardItem::setZone(CardZoneLogic *_zone)
 {
-    zone = _zone;
+    state->setZone(_zone);
 }
 
 void CardItem::retranslateUi()
@@ -80,23 +79,23 @@ void CardItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option,
     AbstractCardItem::paint(painter, option, widget);
 
     int i = 0;
-    QMapIterator<int, int> counterIterator(counters);
+    QMapIterator<int, int> counterIterator(state->getCounters());
     while (counterIterator.hasNext()) {
         counterIterator.next();
         QColor _color = cardCounterSettings.color(counterIterator.key());
 
-        paintNumberEllipse(counterIterator.value(), 14, _color, i, counters.size(), painter);
+        paintNumberEllipse(counterIterator.value(), 14, _color, i, state->getCounters().size(), painter);
         ++i;
     }
 
     QSizeF translatedSize = getTranslatedSize(painter);
     qreal scaleFactor = translatedSize.width() / boundingRect().width();
 
-    if (!pt.isEmpty()) {
+    if (!state->getPT().isEmpty()) {
         painter->save();
         transformPainter(painter, translatedSize, tapAngle);
 
-        if (!getFaceDown() && pt == exactCard.getInfo().getPowTough()) {
+        if (!getFaceDown() && state->getPT() == exactCard.getInfo().getPowTough()) {
             painter->setPen(Qt::white);
         } else {
             painter->setPen(QColor(255, 150, 0)); // dark orange
@@ -107,11 +106,11 @@ void CardItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option,
 
         painter->drawText(QRectF(4 * scaleFactor, 4 * scaleFactor, translatedSize.width() - 10 * scaleFactor,
                                  translatedSize.height() - 8 * scaleFactor),
-                          Qt::AlignRight | Qt::AlignBottom, pt);
+                          Qt::AlignRight | Qt::AlignBottom, state->getPT());
         painter->restore();
     }
 
-    if (!annotation.isEmpty()) {
+    if (!state->getAnnotation().isEmpty()) {
         painter->save();
 
         transformPainter(painter, translatedSize, tapAngle);
@@ -121,7 +120,7 @@ void CardItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option,
 
         painter->drawText(QRectF(4 * scaleFactor, 4 * scaleFactor, translatedSize.width() - 8 * scaleFactor,
                                  translatedSize.height() - 8 * scaleFactor),
-                          Qt::AlignCenter | Qt::TextWrapAnywhere, annotation);
+                          Qt::AlignCenter | Qt::TextWrapAnywhere, state->getAnnotation());
         painter->restore();
     }
 
@@ -129,7 +128,7 @@ void CardItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option,
         painter->fillPath(shape(), QBrush(QColor(255, 0, 0, 100)));
     }
 
-    if (doesntUntap) {
+    if (state->getDoesntUntap()) {
         painter->save();
 
         painter->setRenderHint(QPainter::Antialiasing, false);
@@ -148,70 +147,66 @@ void CardItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option,
 
 void CardItem::setAttacking(bool _attacking)
 {
-    attacking = _attacking;
+    state->setAttacking(_attacking);
     update();
 }
 
 void CardItem::setCounter(int _id, int _value)
 {
-    if (_value) {
+    state->setCounter(_id, _value);
-        counters.insert(_id, _value);
-    } else {
-        counters.remove(_id);
-    }
     update();
 }
 
 void CardItem::setAnnotation(const QString &_annotation)
 {
-    annotation = _annotation;
+    state->setAnnotation(_annotation);
     update();
 }
 
 void CardItem::setDoesntUntap(bool _doesntUntap)
 {
-    doesntUntap = _doesntUntap;
+    state->setDoesntUntap(_doesntUntap);
     update();
 }
 
 void CardItem::setPT(const QString &_pt)
 {
-    pt = _pt;
+    state->setPT(_pt);
     update();
 }
 
 void CardItem::setAttachedTo(CardItem *_attachedTo)
 {
-    if (attachedTo != nullptr) {
+    if (state->getAttachedTo() != nullptr) {
-        attachedTo->removeAttachedCard(this);
+        state->getAttachedTo()->removeAttachedCard(this);
     }
 
     gridPoint.setX(-1);
-    attachedTo = _attachedTo;
+    state->setAttachedTo(_attachedTo);
-    if (attachedTo != nullptr) {
+    if (state->getAttachedTo() != nullptr) {
         // If the zone is being torn down, it might already be null by the time a card tries to un-attach all its
         // attached cards
-        if (attachedTo->zone == nullptr) {
+        if (state->getAttachedTo()->getZone() == nullptr) {
             deleteLater();
         } else {
-            emit attachedTo->zone->cardAdded(this);
+            emit state->getAttachedTo()->getZone()->cardAdded(this);
-            attachedTo->addAttachedCard(this);
+            state->getAttachedTo()->addAttachedCard(this);
-            if (zone != attachedTo->getZone()) {
+            if (state->getZone() != state->getAttachedTo()->getZone()) {
-                attachedTo->getZone()->reorganizeCards();
+                state->getAttachedTo()->getZone()->reorganizeCards();
             }
         }
     } else {
         // If the zone is being torn down, it might already be null by the time a card tries to un-attach all its
         // attached cards
-        if (zone == nullptr) {
+        if (state->getZone() == nullptr) {
             deleteLater();
         } else {
-            emit zone->cardAdded(this);
+            emit state->getZone()->cardAdded(this);
         }
     }
 
-    if (zone != nullptr) {
+    if (state->getZone() != nullptr) {
-        zone->reorganizeCards();
+        state->getZone()->reorganizeCards();
     }
 }
 
@@ -220,13 +215,7 @@ void CardItem::setAttachedTo(CardItem *_attachedTo)
  */
 void CardItem::resetState(bool keepAnnotations)
 {
-    attacking = false;
+    state->resetState(keepAnnotations);
-    counters.clear();
-    pt.clear();
-    if (!keepAnnotations) {
-        annotation.clear();
-    }
-    attachedTo = 0;
     attachedCards.clear();
     setTapped(false, false);
     setDoesntUntap(false);
@@ -238,11 +227,11 @@ void CardItem::resetState(bool keepAnnotations)
 
 void CardItem::processCardInfo(const ServerInfo_Card &_info)
 {
-    counters.clear();
+    state->clearCounters();
     const int counterListSize = _info.counter_list_size();
     for (int i = 0; i < counterListSize; ++i) {
         const ServerInfo_CardCounter &counterInfo = _info.counter_list(i);
-        counters.insert(counterInfo.id(), counterInfo.value());
+        state->insertCounter(counterInfo.id(), counterInfo.value());
     }
 
     setId(_info.id());
@@ -299,7 +288,7 @@ void CardItem::drawArrow(const QColor &arrowColor)
         if (card == nullptr || card == this) {
             continue;
         }
-        if (card->getZone() != zone) {
+        if (card->getZone() != state->getZone()) {
             continue;
         }
 
@@ -324,7 +313,7 @@ void CardItem::drawAttachArrow()
         if (card == nullptr) {
             continue;
         }
-        if (card->getZone() != zone) {
+        if (card->getZone() != state->getZone()) {
             continue;
         }
 
@@ -357,7 +346,7 @@ void CardItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
             2 * QApplication::startDragDistance()) {
             return;
         }
-        if (const ZoneViewZoneLogic *view = qobject_cast<const ZoneViewZoneLogic *>(zone)) {
+        if (const ZoneViewZoneLogic *view = qobject_cast<const ZoneViewZoneLogic *>(state->getZone())) {
             if (view->getRevealZone() && !view->getWriteableRevealZone()) {
                 return;
             }
@@ -375,12 +364,12 @@ void CardItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
         int childIndex = 0;
         for (const auto &item : scene()->selectedItems()) {
             CardItem *card = static_cast<CardItem *>(item);
-            if ((card == this) || (card->getZone() != zone)) {
+            if ((card == this) || (card->getZone() != state->getZone())) {
                 continue;
             }
             ++childIndex;
             QPointF childPos;
-            if (zone->getHasCardAttr()) {
+            if (state->getZone()->getHasCardAttr()) {
                 childPos = card->pos() - pos();
             } else {
                 childPos = QPointF(childIndex * CardDimensions::WIDTH_HALF_F, 0);
@@ -401,15 +390,15 @@ void CardItem::playCard(bool faceDown)
         return;
     }
 
-    TableZoneLogic *tz = qobject_cast<TableZoneLogic *>(zone);
+    TableZoneLogic *tz = qobject_cast<TableZoneLogic *>(state->getZone());
     if (tz) {
         emit tz->toggleTapped();
     } else {
         if (SettingsCache::instance().getClickPlaysAllSelected()) {
-            faceDown ? zone->getPlayer()->getPlayerActions()->actPlayFacedown()
+            faceDown ? state->getZone()->getPlayer()->getPlayerActions()->actPlayFacedown()
-                     : zone->getPlayer()->getPlayerActions()->actPlay();
+                     : state->getZone()->getPlayer()->getPlayerActions()->actPlay();
         } else {
-            zone->getPlayer()->getPlayerActions()->playCard(this, faceDown);
+            state->getZone()->getPlayer()->getPlayerActions()->playCard(this, faceDown);
         }
     }
 }
@@ -465,11 +454,11 @@ static bool isUnwritableRevealZone(CardZoneLogic *zone)
  */
 void CardItem::handleClickedToPlay(bool shiftHeld)
 {
-    if (isUnwritableRevealZone(zone)) {
+    if (isUnwritableRevealZone(state->getZone())) {
         if (SettingsCache::instance().getClickPlaysAllSelected()) {
-            zone->getPlayer()->getPlayerActions()->actHide();
+            state->getZone()->getPlayer()->getPlayerActions()->actHide();
         } else {
-            zone->removeCard(this);
+            state->getZone()->removeCard(this);
         }
     } else {
         playCard(shiftHeld);

cockatrice/src/game/board/card_item.h

@@ -9,6 +9,7 @@
 
 #include "../zones/card_zone_logic.h"
 #include "abstract_card_item.h"
+#include "card_state.h"
 
 #include <libcockatrice/network/server/remote/game/server_card.h>
 
@@ -27,16 +28,10 @@ class CardItem : public AbstractCardItem
 {
     Q_OBJECT
 private:
-    CardZoneLogic *zone;
+    CardState *state;
-    bool attacking;
+
-    QMap<int, int> counters;
-    QString annotation;
-    QString pt;
-    bool destroyOnZoneChange;
-    bool doesntUntap;
     QPoint gridPoint;
     CardDragItem *dragItem;
-    CardItem *attachedTo;
     QList<CardItem *> attachedCards;
 
     void prepareDelete();
@@ -62,7 +57,7 @@ public:
     void retranslateUi();
     [[nodiscard]] CardZoneLogic *getZone() const
     {
-        return zone;
+        return state->getZone();
     }
     void setZone(CardZoneLogic *_zone);
     void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) override;
@@ -88,40 +83,40 @@ public:
     }
     [[nodiscard]] bool getAttacking() const
     {
-        return attacking;
+        return state->getAttacking();
     }
     void setAttacking(bool _attacking);
     [[nodiscard]] const QMap<int, int> &getCounters() const
     {
-        return counters;
+        return state->getCounters();
     }
     void setCounter(int _id, int _value);
     [[nodiscard]] QString getAnnotation() const
     {
-        return annotation;
+        return state->getAnnotation();
     }
     void setAnnotation(const QString &_annotation);
     [[nodiscard]] bool getDoesntUntap() const
     {
-        return doesntUntap;
+        return state->getDoesntUntap();
     }
     void setDoesntUntap(bool _doesntUntap);
     [[nodiscard]] QString getPT() const
     {
-        return pt;
+        return state->getPT();
     }
     void setPT(const QString &_pt);
     [[nodiscard]] bool getDestroyOnZoneChange() const
     {
-        return destroyOnZoneChange;
+        return state->getDestroyOnZoneChange();
     }
     void setDestroyOnZoneChange(bool _destroy)
     {
-        destroyOnZoneChange = _destroy;
+        state->setDestroyOnZoneChange(_destroy);
     }
     [[nodiscard]] CardItem *getAttachedTo() const
     {
-        return attachedTo;
+        return state->getAttachedTo();
     }
     void setAttachedTo(CardItem *_attachedTo);
     void addAttachedCard(CardItem *card)

cockatrice/src/game/board/card_state.cpp

@@ -0,0 +1,111 @@
+#include "card_state.h"
+
+void CardState::resetState(bool keepAnnotations)
+{
+    attacking = false;
+    counters.clear();
+    pt.clear();
+    if (!keepAnnotations) {
+        annotation.clear();
+    }
+    attachedTo = nullptr;
+}
+
+void CardState::setZone(CardZoneLogic *_zone)
+{
+    if (zone == _zone) {
+        return;
+    }
+
+    zone = _zone;
+    emit zoneChanged(zone);
+    emit stateChanged();
+}
+
+void CardState::setAttacking(bool _attacking)
+{
+    if (attacking == _attacking) {
+        return;
+    }
+    attacking = _attacking;
+    emit attackingChanged(_attacking);
+    emit stateChanged();
+}
+
+void CardState::insertCounter(int id, int value)
+{
+    counters.insert(id, value);
+
+    emit countersChanged(counters);
+    emit stateChanged();
+}
+
+void CardState::setCounter(int id, int value)
+{
+    if (value) {
+        counters[id] = value;
+    } else {
+        counters.remove(id);
+    }
+
+    emit countersChanged(counters);
+    emit stateChanged();
+}
+
+void CardState::clearCounters()
+{
+    counters.clear();
+    emit countersChanged(counters);
+    emit stateChanged();
+}
+
+void CardState::setAnnotation(const QString &_annotation)
+{
+    if (annotation == _annotation) {
+        return;
+    }
+    annotation = _annotation;
+    emit annotationChanged(annotation);
+    emit stateChanged();
+}
+
+void CardState::setPT(const QString &_pt)
+{
+    if (pt == _pt) {
+        return;
+    }
+    pt = _pt;
+    emit ptChanged(pt);
+    emit stateChanged();
+}
+
+void CardState::setDoesntUntap(bool _doesntUntap)
+{
+    if (doesntUntap == _doesntUntap) {
+        return;
+    }
+    doesntUntap = _doesntUntap;
+    emit doesntUntapChanged(_doesntUntap);
+    emit stateChanged();
+}
+
+void CardState::setDestroyOnZoneChange(bool _destroyOnZoneChange)
+{
+    if (destroyOnZoneChange == _destroyOnZoneChange) {
+        return;
+    }
+
+    destroyOnZoneChange = _destroyOnZoneChange;
+    emit destroyOnZoneChangeChanged(_destroyOnZoneChange);
+    emit stateChanged();
+}
+
+void CardState::setAttachedTo(CardItem *_attachedTo)
+{
+    if (attachedTo == _attachedTo) {
+        return;
+    }
+    attachedTo = _attachedTo;
+    emit attachedToChanged(_attachedTo);
+    emit stateChanged();
+}

cockatrice/src/game/board/card_state.h

@@ -0,0 +1,103 @@
+#ifndef COCKATRICE_CARD_STATE_H
+#define COCKATRICE_CARD_STATE_H
+
+#include <QMap>
+#include <QObject>
+
+class CardZoneLogic;
+class CardItem;
+class CardState : public QObject
+{
+    Q_OBJECT
+
+private:
+    bool attacking = false;
+    QMap<int, int> counters;
+    QString annotation;
+    QString pt;
+    bool doesntUntap = false;
+    bool destroyOnZoneChange = false;
+
+    CardItem *attachedTo = nullptr;
+    CardZoneLogic *zone = nullptr;
+
+signals:
+    void stateChanged();
+
+    void attackingChanged(bool newValue);
+    void countersChanged(const QMap<int, int> &newCounters);
+    void annotationChanged(const QString &newAnnotation);
+    void ptChanged(const QString &newPt);
+    void doesntUntapChanged(bool newValue);
+    void destroyOnZoneChangeChanged(bool newValue);
+    void attachedToChanged(CardItem *newAttachedTo);
+    void zoneChanged(CardZoneLogic *newZone);
+
+public:
+    explicit CardState(QObject *parent, CardZoneLogic *_zone) : QObject(parent), zone(_zone)
+    {
+    }
+
+    void resetState(bool keepAnnotations);
+
+    CardZoneLogic *getZone() const
+    {
+        return zone;
+    }
+
+    void setZone(CardZoneLogic *_zone);
+
+    bool getAttacking() const
+    {
+        return attacking;
+    }
+    void setAttacking(bool _attacking);
+
+    const QMap<int, int> &getCounters() const
+    {
+        return counters;
+    }
+
+    void insertCounter(int id, int value);
+
+    void setCounter(int id, int value);
+
+    void clearCounters();
+
+    QString getAnnotation() const
+    {
+        return annotation;
+    }
+
+    void setAnnotation(const QString &_annotation);
+
+    QString getPT() const
+    {
+        return pt;
+    }
+
+    void setPT(const QString &_pt);
+
+    bool getDoesntUntap() const
+    {
+        return doesntUntap;
+    }
+
+    void setDoesntUntap(bool _doesntUntap);
+
+    bool getDestroyOnZoneChange() const
+    {
+        return destroyOnZoneChange;
+    }
+
+    void setDestroyOnZoneChange(bool _destroyOnZoneChange);
+
+    CardItem *getAttachedTo() const
+    {
+        return attachedTo;
+    }
+
+    void setAttachedTo(CardItem *_attachedTo);
+};
+
+#endif // COCKATRICE_CARD_STATE_H