From 1409dcc2e860b27d9b19f324e90f83fa8986e687 Mon Sep 17 00:00:00 2001 From: Basile Clement Date: Sun, 27 Apr 2025 01:55:54 +0200 Subject: [PATCH] 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 | 9 +++---- cockatrice/src/game/player/player.cpp | 32 +++++++++-------------- cockatrice/src/game/player/player.h | 7 ++++- cockatrice/src/game/zones/card_zone.cpp | 11 +++----- cockatrice/src/game/zones/card_zone.h | 8 +----- cockatrice/src/game/zones/select_zone.cpp | 5 ++-- cockatrice/src/game/zones/select_zone.h | 3 +-- cockatrice/src/game/zones/view_zone.cpp | 2 +- 8 files changed, 30 insertions(+), 47 deletions(-) diff --git a/cockatrice/src/game/board/card_item.cpp b/cockatrice/src/game/board/card_item.cpp index e9a345e86..55df7a477 100644 --- a/cockatrice/src/game/board/card_item.cpp +++ b/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()) { - const ZoneViewZone *view = static_cast(zone); + if (const ZoneViewZone *view = qobject_cast(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 = static_cast(zone)) { - return view->getRevealZone() && !view->getWriteableRevealZone(); - } + if (auto *view = qobject_cast(zone)) { + return view->getRevealZone() && !view->getWriteableRevealZone(); } return false; } diff --git a/cockatrice/src/game/player/player.cpp b/cockatrice/src/game/player/player.cpp index e4d319633..c893e27af 100644 --- a/cockatrice/src/game/player/player.cpp +++ b/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()), - (int)table->boundingRect().height(), this); + hand = addZone(new HandZone(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()) { - auto *view = static_cast(zone); - return view && view->getRevealZone() && !view->getWriteableRevealZone(); + if (auto *view = qobject_cast(zone)) { + return 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()) { - auto *view = dynamic_cast(card->getZone()); + if (auto *view = qobject_cast(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(card->getZone())) { cardMenu->addAction(aSelectColumn); } diff --git a/cockatrice/src/game/player/player.h b/cockatrice/src/game/player/player.h index 76365d50d..027a7658b 100644 --- a/cockatrice/src/game/player/player.h +++ b/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 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); diff --git a/cockatrice/src/game/zones/card_zone.cpp b/cockatrice/src/game/zones/card_zone.cpp index 24eaa4f52..05b4c35b9 100644 --- a/cockatrice/src/game/zones/card_zone.cpp +++ b/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, - bool _isView) + QGraphicsItem *parent) : 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; -} \ No newline at end of file +} diff --git a/cockatrice/src/game/zones/card_zone.h b/cockatrice/src/game/zones/card_zone.h index 2b9114a6d..8ce31e885 100644 --- a/cockatrice/src/game/zones/card_zone.h +++ b/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, - bool _isView = false); + QGraphicsItem *parent = nullptr); 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; diff --git a/cockatrice/src/game/zones/select_zone.cpp b/cockatrice/src/game/zones/select_zone.cpp index bb6c40683..b26aa23ff 100644 --- a/cockatrice/src/game/zones/select_zone.cpp +++ b/cockatrice/src/game/zones/select_zone.cpp @@ -37,9 +37,8 @@ SelectZone::SelectZone(Player *_player, bool _hasCardAttr, bool _isShufflable, bool _contentsKnown, - QGraphicsItem *parent, - bool isView) - : CardZone(_player, _name, _hasCardAttr, _isShufflable, _contentsKnown, parent, isView) + QGraphicsItem *parent) + : CardZone(_player, _name, _hasCardAttr, _isShufflable, _contentsKnown, parent) { } diff --git a/cockatrice/src/game/zones/select_zone.h b/cockatrice/src/game/zones/select_zone.h index c8c88b450..6aa56a1c6 100644 --- a/cockatrice/src/game/zones/select_zone.h +++ b/cockatrice/src/game/zones/select_zone.h @@ -26,8 +26,7 @@ public: bool _hasCardAttr, bool _isShufflable, bool _contentsKnown, - QGraphicsItem *parent = nullptr, - bool isView = false); + QGraphicsItem *parent = nullptr); }; qreal divideCardSpaceInZone(qreal index, int cardCount, qreal totalHeight, qreal cardHeight, bool reverse = false); diff --git a/cockatrice/src/game/zones/view_zone.cpp b/cockatrice/src/game/zones/view_zone.cpp index 1c35b6613..fd9387064 100644 --- a/cockatrice/src/game/zones/view_zone.cpp +++ b/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)