Remove isView flag from CardZone (#5728)

* Remove `isView` flag from CardZone

This flag is used for two purposes:

 1. It is used as a check for casting to a zone to a `ZoneViewZone`;

 2. Non-view zones are added to the player's zones on construction

This patch removes the `isView` flag and instead:

 1. We directly cast zones to `ZoneViewZone` using a dynamic (qobject)
    cast and use the result of the cast instead of the `isView` flag to
    detect if we are a view zone or not;

 2. The player records its own zones when they are created, simplifying
    control flow.

* Review

cockatrice/src/game/board/card_item.cpp

@@ -336,8 +336,7 @@ void CardItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
         if ((event->screenPos() - event->buttonDownScreenPos(Qt::LeftButton)).manhattanLength() <
             2 * QApplication::startDragDistance())
             return;
-        if (zone->getIsView()) {
+        if (const ZoneViewZone *view = qobject_cast<const ZoneViewZone *>(zone)) {
-            const ZoneViewZone *view = static_cast<const ZoneViewZone *>(zone);
             if (view->getRevealZone() && !view->getWriteableRevealZone())
                 return;
         } else if (!owner->getLocalOrJudge())
@@ -394,10 +393,8 @@ void CardItem::playCard(bool faceDown)
  */
 static bool isUnwritableRevealZone(CardZone *zone)
 {
-    if (zone && zone->getIsView()) {
+    if (auto *view = qobject_cast<ZoneViewZone *>(zone)) {
-        if (auto *view = static_cast<ZoneViewZone *>(zone)) {
+        return view->getRevealZone() && !view->getWriteableRevealZone();
-            return view->getRevealZone() && !view->getWriteableRevealZone();
-        }
     }
     return false;
 }

cockatrice/src/game/player/player.cpp

@@ -124,7 +124,7 @@ Player::Player(const ServerInfo_User &info, int _id, bool _local, bool _judge, T
     qreal avatarMargin = (counterAreaWidth + CARD_HEIGHT + 15 - playerTarget->boundingRect().width()) / 2.0;
     playerTarget->setPos(QPointF(avatarMargin, avatarMargin));
 
-    auto *_deck = new PileZone(this, "deck", true, false, playerArea);
+    auto *_deck = addZone(new PileZone(this, "deck", true, false, playerArea));
     QPointF base = QPointF(counterAreaWidth + (CARD_HEIGHT - CARD_WIDTH + 15) / 2.0,
                            10 + playerTarget->boundingRect().height() + 5 - (CARD_HEIGHT - CARD_WIDTH) / 2.0);
     _deck->setPos(base);
@@ -135,22 +135,23 @@ Player::Player(const ServerInfo_User &info, int _id, bool _local, bool _judge, T
     handCounter->setPos(base + QPointF(0, h + 10));
     qreal h2 = handCounter->boundingRect().height();
 
-    PileZone *grave = new PileZone(this, "grave", false, true, playerArea);
+    PileZone *grave = addZone(new PileZone(this, "grave", false, true, playerArea));
     grave->setPos(base + QPointF(0, h + h2 + 10));
 
-    PileZone *rfg = new PileZone(this, "rfg", false, true, playerArea);
+    PileZone *rfg = addZone(new PileZone(this, "rfg", false, true, playerArea));
     rfg->setPos(base + QPointF(0, 2 * h + h2 + 10));
 
-    PileZone *sb = new PileZone(this, "sb", false, false, playerArea);
+    PileZone *sb = addZone(new PileZone(this, "sb", false, false, playerArea));
     sb->setVisible(false);
 
-    table = new TableZone(this, this);
+    table = addZone(new TableZone(this, this));
     connect(table, &TableZone::sizeChanged, this, &Player::updateBoundingRect);
 
-    stack = new StackZone(this, (int)table->boundingRect().height(), this);
+    stack = addZone(new StackZone(this, (int)table->boundingRect().height(), this));
 
-    hand = new HandZone(this, _local || _judge || (_parent->getSpectator() && _parent->getSpectatorsSeeEverything()),
+    hand = addZone(new HandZone(this,
-                        (int)table->boundingRect().height(), this);
+                                _local || _judge || (_parent->getSpectator() && _parent->getSpectatorsSeeEverything()),
+                                (int)table->boundingRect().height(), this));
     connect(hand, &HandZone::cardCountChanged, handCounter, &HandCounter::updateNumber);
     connect(handCounter, &HandCounter::showContextMenu, hand, &HandZone::showContextMenu);
 
@@ -2889,11 +2890,6 @@ void Player::deleteCard(CardItem *card)
     }
 }
 
-void Player::addZone(CardZone *zone)
-{
-    zones.insert(zone->getName(), zone);
-}
-
 AbstractCounter *Player::addCounter(const ServerInfo_Counter &counter)
 {
     return addCounter(counter.id(), QString::fromStdString(counter.name()),
@@ -3692,9 +3688,8 @@ void Player::actCardCounterTrigger()
  */
 static bool isUnwritableRevealZone(CardZone *zone)
 {
-    if (zone && zone->getIsView()) {
+    if (auto *view = qobject_cast<ZoneViewZone *>(zone)) {
-        auto *view = static_cast<ZoneViewZone *>(zone);
+        return view->getRevealZone() && !view->getWriteableRevealZone();
-        return view && view->getRevealZone() && !view->getWriteableRevealZone();
     }
     return false;
 }
@@ -3784,8 +3779,7 @@ void Player::updateCardMenu(const CardItem *card)
 
     bool revealedCard = false;
     bool writeableCard = getLocalOrJudge();
-    if (card->getZone() && card->getZone()->getIsView()) {
+    if (auto *view = qobject_cast<ZoneViewZone *>(card->getZone())) {
-        auto *view = dynamic_cast<ZoneViewZone *>(card->getZone());
         if (view->getRevealZone()) {
             if (view->getWriteableRevealZone()) {
                 writeableCard = true;
@@ -3955,7 +3949,7 @@ void Player::updateCardMenu(const CardItem *card)
 
                 cardMenu->addSeparator();
                 cardMenu->addAction(aSelectAll);
-                if (card->getZone()->getIsView()) {
+                if (qobject_cast<ZoneViewZone *>(card->getZone())) {
                     cardMenu->addAction(aSelectColumn);
                 }
 

cockatrice/src/game/player/player.h

@@ -409,7 +409,12 @@ public:
     void playCardToTable(const CardItem *c, bool faceDown);
     void addCard(CardItem *c);
     void deleteCard(CardItem *c);
-    void addZone(CardZone *z);
+
+    template <typename T> T *addZone(T *zone)
+    {
+        zones.insert(zone->getName(), zone);
+        return zone;
+    }
 
     AbstractCounter *addCounter(const ServerInfo_Counter &counter);
     AbstractCounter *addCounter(int counterId, const QString &name, QColor color, int radius, int value);

cockatrice/src/game/zones/card_zone.cpp

@@ -19,21 +19,16 @@
  * @param _isShufflable whether it makes sense to shuffle this zone by default after viewing it
  * @param _contentsKnown whether the cards in the zone are known to the client
  * @param parent the parent graphics object.
- * @param _isView whether this zone is a view of another zone. Modifications to a view should modify the original
  */
 CardZone::CardZone(Player *_p,
                    const QString &_name,
                    bool _hasCardAttr,
                    bool _isShufflable,
                    bool _contentsKnown,
-                   QGraphicsItem *parent,
+                   QGraphicsItem *parent)
-                   bool _isView)
     : AbstractGraphicsItem(parent), player(_p), name(_name), cards(_contentsKnown), views{}, menu(nullptr),
-      doubleClickAction(0), hasCardAttr(_hasCardAttr), isShufflable(_isShufflable), isView(_isView)
+      doubleClickAction(0), hasCardAttr(_hasCardAttr), isShufflable(_isShufflable)
 {
-    if (!isView)
-        player->addZone(this);
-
     // If we join a game before the card db finishes loading, the cards might have the wrong printings.
     // Force refresh all cards in the zone when db finishes loading to fix that.
     connect(CardDatabaseManager::getInstance(), &CardDatabase::cardDatabaseLoadingFinished, this,
@@ -236,4 +231,4 @@ void CardZone::moveAllToZone()
 QPointF CardZone::closestGridPoint(const QPointF &point)
 {
     return point;
-}
+}

cockatrice/src/game/zones/card_zone.h

@@ -35,7 +35,6 @@ protected:
     QAction *doubleClickAction;
     bool hasCardAttr;
     bool isShufflable;
-    bool isView;
     bool alwaysRevealTopCard;
     void mouseDoubleClickEvent(QGraphicsSceneMouseEvent *event) override;
     void mousePressEvent(QGraphicsSceneMouseEvent *event) override;
@@ -65,8 +64,7 @@ public:
              bool _hasCardAttr,
              bool _isShufflable,
              bool _contentsKnown,
-             QGraphicsItem *parent = nullptr,
+             QGraphicsItem *parent = nullptr);
-             bool _isView = false);
     void retranslateUi();
     void clearContents();
     bool getHasCardAttr() const
@@ -115,10 +113,6 @@ public:
     }
     virtual void reorganizeCards() = 0;
     virtual QPointF closestGridPoint(const QPointF &point);
-    bool getIsView() const
-    {
-        return isView;
-    }
     bool getAlwaysRevealTopCard() const
     {
         return alwaysRevealTopCard;

cockatrice/src/game/zones/select_zone.cpp

@@ -37,9 +37,8 @@ SelectZone::SelectZone(Player *_player,
                        bool _hasCardAttr,
                        bool _isShufflable,
                        bool _contentsKnown,
-                       QGraphicsItem *parent,
+                       QGraphicsItem *parent)
-                       bool isView)
+    : CardZone(_player, _name, _hasCardAttr, _isShufflable, _contentsKnown, parent)
-    : CardZone(_player, _name, _hasCardAttr, _isShufflable, _contentsKnown, parent, isView)
 {
 }
 

cockatrice/src/game/zones/select_zone.h

@@ -26,8 +26,7 @@ public:
                bool _hasCardAttr,
                bool _isShufflable,
                bool _contentsKnown,
-               QGraphicsItem *parent = nullptr,
+               QGraphicsItem *parent = nullptr);
-               bool isView = false);
 };
 
 qreal divideCardSpaceInZone(qreal index, int cardCount, qreal totalHeight, qreal cardHeight, bool reverse = false);

cockatrice/src/game/zones/view_zone.cpp

@@ -30,7 +30,7 @@ ZoneViewZone::ZoneViewZone(Player *_p,
                            bool _writeableRevealZone,
                            QGraphicsItem *parent,
                            bool _isReversed)
-    : SelectZone(_p, _origZone->getName(), false, false, true, parent, true), bRect(QRectF()), minRows(0),
+    : SelectZone(_p, _origZone->getName(), false, false, true, parent), bRect(QRectF()), minRows(0),
       numberCards(_numberCards), origZone(_origZone), revealZone(_revealZone),
       writeableRevealZone(_writeableRevealZone), groupBy(CardList::NoSort), sortBy(CardList::NoSort),
       isReversed(_isReversed)