[Refactor] Give graphics zones a card item cache and own card placement (#7427)

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
BruebachL 2026-10-31 18:44:12 +01:00 • committed by GitHub
parent 4879ddbd0f
commit 47c90cda10
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 111 additions and 74 deletions

View file

@ -14,6 +14,9 @@ void CardState::resetState(bool keepAnnotations)
annotation.clear();
}
attachedTo = nullptr;
setTapped(false, false);
setDoesntUntap(false);
emit stateReset();
}
void CardState::setCardRef(const CardRef &_cardRef)

View file

@ -43,6 +43,7 @@ signals:
void cardPixmapUpdated();
void tappedChanged(bool newTapped, bool canAnimate);
void facedownChanged(bool newFaceDown);
void stateReset();
void attackingChanged(bool newValue);
void countersChanged(const QMap<int, int> &newCounters);

View file

@ -5,9 +5,9 @@ namespace CardZoneAlgorithms
{
/**
* Shared insertion logic for zones where cards become visible on add and follow
* the standard pattern: clamp index, insert, clear identity if contents unknown,
* reset state, show card.
* Shared insertion logic for zones where cards follow the standard pattern:
* clamp index, insert, clear identity if contents unknown, reset state.
* Visibility and parenting are handled by the graphics zone's onCardAdded().
*
* Zones with different post-add behavior (signal connections, positional resets,
* hidden cards, or coordinate-based placement) should NOT use this — implement
@ -18,8 +18,7 @@ namespace CardZoneAlgorithms
*
* @tparam CardList Must provide: size() -> int, insert(int, CardType*),
* getContentsKnown() -> bool
* @tparam CardType Must provide: setId(int), setCardRef(CardRefType),
* resetState(bool), setVisible(bool)
* @tparam CardType Must provide: setId(int), setCardRef(CardRefType), resetState(bool)
* @param keepAnnotations Forwarded to card->resetState(). Stack-like zones preserve
* annotations across zone transitions; hand-like zones clear them.
*/
@ -37,7 +36,6 @@ void addCardToList(CardList &cards, CardType *card, int x, bool keepAnnotations)
}
card->resetState(keepAnnotations);
card->setVisible(true);
}
} // namespace CardZoneAlgorithms

View file

@ -53,6 +53,7 @@ public:
void rawInsertCard(CardItem *card, int index)
{
cards.insert(index, card);
emit cardCountChanged();
}
[[nodiscard]] const CardList &getCards() const
@ -63,6 +64,7 @@ public:
void sortCards(const QList<CardList::SortOption> &options)
{
cards.sortBy(options);
emit cardCountChanged();
}
[[nodiscard]] QString getName() const
{

View file

@ -18,13 +18,12 @@ PileZoneLogic::PileZoneLogic(PlayerLogic *_player,
void PileZoneLogic::addCardImpl(CardItem *card, int x, int /*y*/)
{
connect(card, &CardItem::sigPixmapUpdated, this, &PileZoneLogic::callUpdate);
connect(card->getState(), &CardState::cardPixmapUpdated, this, &PileZoneLogic::callUpdate);
// if x is negative set it to add at end
if (x < 0 || x >= cards.size()) {
x = cards.size();
}
cards.insert(x, card);
card->setPos(0, 0);
if (!contentsKnown()) {
card->setCardRef({});
card->setId(-1);
@ -33,6 +32,5 @@ void PileZoneLogic::addCardImpl(CardItem *card, int x, int /*y*/)
cards.at(x + 1)->setCardRef({});
}
}
card->setVisible(false);
card->resetState();
}

View file

@ -29,7 +29,6 @@ void TableZoneLogic::addCardImpl(CardItem *card, int _x, int _y)
card->setDoesntUntap(true);
}
card->setGridPoint(QPoint(_x, _y));
card->setVisible(true);
}
CardItem *TableZoneLogic::takeCard(int position, int cardId, bool toNewZone)

View file

@ -168,6 +168,7 @@ void ZoneViewZoneLogic::clearCards()
card->deleteLater();
}
cards.clear();
emit cardCountChanged();
}
void ZoneViewZoneLogic::setWriteableRevealZone(bool _writeableRevealZone)

View file

@ -63,6 +63,14 @@ CardItem::CardItem(PlayerLogic *_owner,
update();
}
});
connect(state, &CardState::stateReset, this, [this] {
attachedCards.clear();
if (scene()) {
static_cast<GameScene *>(scene())->unregisterAnimationItem(this);
}
update();
});
}
void CardItem::prepareDelete()
@ -250,13 +258,6 @@ void CardItem::setAttachedTo(CardItem *_attachedTo)
void CardItem::resetState(bool keepAnnotations)
{
state->resetState(keepAnnotations);
attachedCards.clear();
setTapped(false, false);
setDoesntUntap(false);
if (scene()) {
static_cast<GameScene *>(scene())->unregisterAnimationItem(this);
}
update();
}
void CardItem::processCardInfo(const ServerInfo_Card &_info)

View file

@ -18,12 +18,31 @@ CardZone::CardZone(CardZoneLogic *_logic, QGraphicsItem *parent)
: AbstractGraphicsItem(parent), menu(nullptr), doubleClickAction(0), logic(_logic)
{
connect(logic, &CardZoneLogic::retranslateUi, this, &CardZone::retranslateUi);
// Invalidation is connected before any consumer so a handler that reads cardItems()
// during the same signal emission always sees a stale flag and rebuilds.
connect(logic, &CardZoneLogic::cardAdded, this, &CardZone::invalidateItems);
connect(logic, &CardZoneLogic::cardCountChanged, this, &CardZone::invalidateItems);
connect(logic, &CardZoneLogic::reorganizeCards, this, &CardZone::invalidateItems);
connect(logic, &CardZoneLogic::cardAdded, this, &CardZone::onCardAdded);
connect(logic, &CardZoneLogic::setGraphicsVisibility, this, [this](bool v) { this->setVisible(v); });
connect(logic, &CardZoneLogic::updateGraphics, this, [this]() { update(); });
connect(logic, &CardZoneLogic::reorganizeCards, this, &CardZone::reorganizeCards);
}
void CardZone::invalidateItems()
{
itemsDirty = true;
}
const QList<CardItem *> &CardZone::cardItems() const
{
if (itemsDirty) {
items = getLogic()->getCards();
itemsDirty = false;
}
return items;
}
void CardZone::onCardAdded(CardItem *addedCard)
{
addedCard->setParentItem(this);
@ -33,8 +52,8 @@ void CardZone::onCardAdded(CardItem *addedCard)
void CardZone::retranslateUi()
{
for (int i = 0; i < getLogic()->getCards().size(); ++i) {
getLogic()->getCards()[i]->retranslateUi();
for (auto *card : cardItems()) {
card->retranslateUi();
}
}

View file

@ -76,6 +76,15 @@ public:
return logic;
}
/**
* @brief The CardItems backing getLogic()->getCards(), in zone order.
*
* Game logic talks about cards as states; this is the graphics side's view of the same list.
* It is rebuilt on demand after any content change, so views never reach into the logic's
* container themselves and never observe a stale one.
*/
[[nodiscard]] const QList<CardItem *> &cardItems() const;
void setMenu(QMenu *_menu, QAction *_doubleClickAction = 0)
{
menu = _menu;
@ -83,7 +92,12 @@ public:
}
private:
/** @brief Marks the cached card items stale. Connected to every content-changing signal. */
void invalidateItems();
CardZoneLogic *logic;
mutable QList<CardItem *> items;
mutable bool itemsDirty = true;
};
#endif

View file

@ -47,8 +47,8 @@ void HandZone::handleDropEvent(const QList<CardDragItem *> &dragItems,
QPoint point = dropPoint + scenePos().toPoint();
int x = -1;
if (SettingsCache::instance().userInterface().getHorizontalHand()) {
for (x = 0; x < getLogic()->getCards().size(); x++) {
if (point.x() < static_cast<CardItem *>(getLogic()->getCards().at(x))->scenePos().x()) {
for (x = 0; x < cardItems().size(); x++) {
if (point.x() < cardItems().at(x)->scenePos().x()) {
break;
}
}
@ -83,23 +83,23 @@ void HandZone::paint(QPainter *painter, const QStyleOptionGraphicsItem * /*optio
void HandZone::reorganizeCards()
{
if (!getLogic()->getCards().isEmpty()) {
const int cardCount = getLogic()->getCards().size();
if (!cardItems().isEmpty()) {
const int cardCount = cardItems().size();
if (SettingsCache::instance().userInterface().getHorizontalHand()) {
bool leftJustified = SettingsCache::instance().userInterface().getLeftJustified();
qreal cardWidth = getLogic()->getCards().at(0)->boundingRect().width();
qreal cardWidth = cardItems().at(0)->boundingRect().width();
const int xPadding = leftJustified ? cardWidth * 1.4 : 5;
qreal totalWidth =
leftJustified ? boundingRect().width() - (1 * xPadding) - 5 : boundingRect().width() - 2 * xPadding;
if (cardCount == 1) {
CardItem *c = getLogic()->getCards().at(0);
CardItem *c = cardItems().at(0);
qreal xPosition = leftJustified ? xPadding : xPadding + (totalWidth - cardWidth) / 2;
c->setPos(xPosition, 5);
c->setRealZValue(0);
} else {
for (int i = 0; i < cardCount; i++) {
CardItem *c = getLogic()->getCards().at(i);
CardItem *c = cardItems().at(i);
// If the total width of the cards is smaller than the available width,
// the cards do not need to overlap and are displayed in the center of the area.
if (cardWidth * cardCount > totalWidth) {
@ -124,7 +124,7 @@ void HandZone::reorganizeCards()
void HandZone::sortHand(const QList<CardList::SortOption> &options)
{
if (getLogic()->getCards().isEmpty()) {
if (cardItems().isEmpty()) {
return;
}

View file

@ -66,15 +66,14 @@ void PileZone::paint(QPainter *painter, const QStyleOptionGraphicsItem * /*optio
{
painter->drawPath(shape());
if (!getLogic()->getCards().isEmpty()) {
getLogic()->getCards().at(0)->paintPicture(painter, getLogic()->getCards().at(0)->getTranslatedSize(painter),
90);
if (!cardItems().isEmpty()) {
cardItems().at(0)->paintPicture(painter, cardItems().at(0)->getTranslatedSize(painter), 90);
}
painter->translate(CardDimensions::WIDTH_HALF_F, CardDimensions::HEIGHT_HALF_F);
painter->rotate(-90);
painter->translate(-CardDimensions::WIDTH_HALF_F, -CardDimensions::HEIGHT_HALF_F);
paintNumberEllipse(getLogic()->getCards().size(), 28, Qt::white, -1, -1, painter);
paintNumberEllipse(cardItems().size(), 28, Qt::white, -1, -1, painter);
}
void PileZone::handleDropEvent(const QList<CardDragItem *> &dragItems, CardZoneLogic *startZone, const QPoint &)
@ -88,6 +87,15 @@ void PileZone::handleDropEvent(const QList<CardDragItem *> &dragItems, CardZoneL
getLogic()->getPlayer()->getPlayerActions()->moveCards(startZone, getLogic(), 0, 0, cards);
}
void PileZone::onCardAdded(CardItem *addedCard)
{
CardZone::onCardAdded(addedCard);
if (addedCard) {
addedCard->setPos(0, 0);
addedCard->setVisible(false);
}
}
void PileZone::reorganizeCards()
{
update();
@ -115,15 +123,14 @@ void PileZone::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
return;
}
if (getLogic()->getCards().isEmpty()) {
if (cardItems().isEmpty()) {
return;
}
bool forceFaceDown = event->modifiers().testFlag(Qt::ShiftModifier);
bool bottomCard = event->modifiers().testFlag(Qt::ControlModifier);
CardItem *card = bottomCard ? getLogic()->getCards().last() : getLogic()->getCards().first();
const int cardid =
getLogic()->contentsKnown() ? card->getId() : (bottomCard ? getLogic()->getCards().size() - 1 : 0);
CardItem *card = bottomCard ? cardItems().last() : cardItems().first();
const int cardid = getLogic()->contentsKnown() ? card->getId() : (bottomCard ? cardItems().size() - 1 : 0);
CardDragItem *drag = card->createDragItem(cardid, event->pos(), event->scenePos(), forceFaceDown);
drag->grabMouse();
setCursor(Qt::OpenHandCursor);
@ -136,8 +143,8 @@ void PileZone::mouseReleaseEvent(QGraphicsSceneMouseEvent * /*event*/)
void PileZone::hoverEnterEvent(QGraphicsSceneHoverEvent *event)
{
if (!getLogic()->getCards().isEmpty()) {
getLogic()->getCards()[0]->processHoverEvent();
if (!cardItems().isEmpty()) {
cardItems()[0]->processHoverEvent();
}
QGraphicsItem::hoverEnterEvent(event);
}

View file

@ -33,6 +33,7 @@ public:
[[nodiscard]] QPainterPath shape() const override;
void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) override;
void reorganizeCards() override;
void onCardAdded(CardItem *addedCard) override;
void
handleDropEvent(const QList<CardDragItem *> &dragItems, CardZoneLogic *startZone, const QPoint &dropPoint) override;

View file

@ -85,7 +85,7 @@ SelectZone *SelectZone::findOwningSelectZone(const QGraphicsItem *card)
SelectZone::StackLayoutParams SelectZone::buildStackParams(qreal minOffset) const
{
const auto &cards = getLogic()->getCards();
const auto &cards = cardItems();
if (cards.isEmpty()) {
return {0, boundingRect().height(), 0.0, 0.0, minOffset};
}
@ -97,7 +97,7 @@ SelectZone::StackLayoutParams SelectZone::buildStackParams(qreal minOffset) cons
int SelectZone::calcDropIndexFromY(qreal dropY, bool allowCountExpand, qreal minOffset) const
{
const auto &cards = getLogic()->getCards();
const auto &cards = cardItems();
if (cards.isEmpty()) {
return 0;
}
@ -115,7 +115,7 @@ void SelectZone::restoreStaleEscapedCards()
if (!cardClipContainer) {
return;
}
for (auto *card : getLogic()->getCards()) {
for (auto *card : cardItems()) {
// A card parented to the zone (instead of the clip container) should
// only occur while it is actively hovered. If hover cleanup was
// missed, reparent it back so clipping resumes.
@ -127,7 +127,7 @@ void SelectZone::restoreStaleEscapedCards()
void SelectZone::layoutCardsVertically(const StackLayoutParams &params)
{
const auto &cards = getLogic()->getCards();
const auto &cards = cardItems();
if (cards.isEmpty() || params.cardCount <= 0) {
return;
}
@ -162,9 +162,9 @@ SelectZone::~SelectZone()
if (cardClipContainer) {
// Reparent any hover-escaped cards back to the clip container so Qt's
// parent-child tree is consistent for destruction. setParentItem() does
// not invalidate getLogic()->getCards() (it modifies the graphics tree,
// not the zone's logical card list).
for (auto *card : getLogic()->getCards()) {
// not invalidate cardItems() — it only moves the item in the graphics
// tree, it never changes the zone's contents.
for (auto *card : cardItems()) {
if (card && card->parentItem() == this) {
card->setParentItem(cardClipContainer);
}
@ -247,7 +247,7 @@ void SelectZone::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
}
QRectF selectionRect = QRectF(selectionOrigin, pos).normalized();
for (auto card : getLogic()->getCards()) {
for (auto card : cardItems()) {
if (card->getAttachedTo() && card->getAttachedTo()->getZone() != getLogic()) {
continue;
}

View file

@ -72,7 +72,7 @@ void StackZone::handleDropEvent(const QList<CardDragItem *> &dragItems,
int index = calcDropIndexFromY(dropPoint.y(), !sameZone, MIN_CARD_VISIBLE);
if (sameZone) {
// Same-zone no-op: don't move a card onto itself
const auto &cards = getLogic()->getCards();
const auto &cards = cardItems();
if (!cards.isEmpty() && cards.at(index)->getId() == dragItems.at(0)->getId()) {
return;
}
@ -102,7 +102,7 @@ void StackZone::setHeight(qreal newHeight)
void StackZone::reorganizeCards()
{
if (!getLogic()->getCards().isEmpty()) {
if (!cardItems().isEmpty()) {
const auto params = buildStackParams(MIN_CARD_VISIBLE);
layoutCardsVertically(params);
}

View file

@ -228,8 +228,8 @@ void TableZone::reorganizeCards()
// Calculate card stack widths so mapping functions work properly
computeCardStackWidths();
for (int i = 0; i < getLogic()->getCards().size(); ++i) {
QPoint gridPoint = getLogic()->getCards()[i]->getGridPos();
for (int i = 0; i < cardItems().size(); ++i) {
QPoint gridPoint = cardItems()[i]->getGridPos();
if (gridPoint.x() == -1) {
continue;
}
@ -238,17 +238,17 @@ void TableZone::reorganizeCards()
qreal x = mapPoint.x();
qreal y = mapPoint.y();
int numberAttachedCards = getLogic()->getCards()[i]->getAttachedCards().size();
int numberAttachedCards = cardItems()[i]->getAttachedCards().size();
qreal actualX = x + numberAttachedCards * STACKED_CARD_OFFSET_X;
qreal actualY = y;
if (numberAttachedCards) {
actualY += 15;
}
getLogic()->getCards()[i]->setPos(actualX, actualY);
getLogic()->getCards()[i]->setRealZValue(ZValues::tableCardZValue(actualX, actualY));
cardItems()[i]->setPos(actualX, actualY);
cardItems()[i]->setRealZValue(ZValues::tableCardZValue(actualX, actualY));
QListIterator<CardItem *> attachedCardIterator(getLogic()->getCards()[i]->getAttachedCards());
QListIterator<CardItem *> attachedCardIterator(cardItems()[i]->getAttachedCards());
int j = 0;
while (attachedCardIterator.hasNext()) {
++j;
@ -285,9 +285,9 @@ void TableZone::resizeToContents()
int xMax = 0;
// Find rightmost card position, which includes the left margin amount.
for (int i = 0; i < getLogic()->getCards().size(); ++i) {
if (getLogic()->getCards()[i]->pos().x() > xMax) {
xMax = (int)getLogic()->getCards()[i]->pos().x();
for (int i = 0; i < cardItems().size(); ++i) {
if (cardItems()[i]->pos().x() > xMax) {
xMax = (int)cardItems()[i]->pos().x();
}
}
@ -308,9 +308,9 @@ void TableZone::resizeToContents()
CardItem *TableZone::getCardFromGrid(const QPoint &gridPoint) const
{
for (int i = 0; i < getLogic()->getCards().size(); i++) {
if (getLogic()->getCards().at(i)->getGridPoint() == gridPoint) {
return getLogic()->getCards().at(i);
for (int i = 0; i < cardItems().size(); i++) {
if (cardItems().at(i)->getGridPoint() == gridPoint) {
return cardItems().at(i);
}
}
return 0;
@ -327,8 +327,8 @@ void TableZone::computeCardStackWidths()
// Each card stack is three grid points worth of card locations.
// First pass: compute the number of cards at each card stack.
QMap<int, int> cardStackCount;
for (int i = 0; i < getLogic()->getCards().size(); ++i) {
const QPoint &gridPoint = getLogic()->getCards()[i]->getGridPos();
for (int i = 0; i < cardItems().size(); ++i) {
const QPoint &gridPoint = cardItems()[i]->getGridPos();
if (gridPoint.x() == -1) {
continue;
}
@ -339,8 +339,8 @@ void TableZone::computeCardStackWidths()
// Second pass: compute the width at each card stack.
cardStackWidth.clear();
for (int i = 0; i < getLogic()->getCards().size(); ++i) {
const QPoint &gridPoint = getLogic()->getCards()[i]->getGridPos();
for (int i = 0; i < cardItems().size(); ++i) {
const QPoint &gridPoint = cardItems()[i]->getGridPos();
if (gridPoint.x() == -1) {
continue;
}
@ -348,8 +348,8 @@ void TableZone::computeCardStackWidths()
const int key = getCardStackMapKey(gridPoint.x() / 3, gridPoint.y());
const int stackCount = cardStackCount.value(key, 0);
if (stackCount == 1) {
cardStackWidth.insert(key, CardDimensions::WIDTH + getLogic()->getCards()[i]->getAttachedCards().size() *
STACKED_CARD_OFFSET_X);
cardStackWidth.insert(key, CardDimensions::WIDTH +
cardItems()[i]->getAttachedCards().size() * STACKED_CARD_OFFSET_X);
} else {
cardStackWidth.insert(key, CardDimensions::WIDTH + (stackCount - 1) * STACKED_CARD_OFFSET_X);
}

View file

@ -139,8 +139,8 @@ void ZoneViewZone::zoneDumpReceived(const Response &r)
void ZoneViewZone::reorganizeCards()
{
// filter cards
CardList cardsToDisplay = CardList(getLogic()->getCards().getContentsKnown());
for (auto card : getLogic()->getCards()) {
CardList cardsToDisplay = CardList(getLogic()->contentsKnown());
for (auto card : cardItems()) {
if (filterString.check(card->getCard().getCardPtr())) {
card->show();
cardsToDisplay.append(card);

View file

@ -16,7 +16,6 @@ struct MockCard
bool cardRefWasCalled = false;
bool resetStateCalled = false;
bool resetStateKeepAnnotations = false;
bool visibleSet = false;
void setId(int id)
{
@ -35,11 +34,6 @@ struct MockCard
resetStateCalled = true;
resetStateKeepAnnotations = keepAnnotations;
}
void setVisible(bool visible)
{
visibleSet = visible;
}
};
class MockCardList
@ -109,7 +103,6 @@ TEST_F(AddCardAlgorithmTest, ContentsKnownPreservesIdentity)
EXPECT_FALSE(card.idWasCalled);
EXPECT_FALSE(card.cardRefWasCalled);
EXPECT_TRUE(card.visibleSet);
}
TEST_F(AddCardAlgorithmTest, ContentsUnknownClearsIdentity)