[Game][Counters] Split counters into AbstractCounter (graphics) and CounterState (logic) (#6917)

* [Counters] Split counters into graphics and logic states

Took 22 minutes

* Don't have widget hold pointer to state -> Copy what we need and subscribe to changes.

Took 12 minutes

Took 5 seconds

* Sync value too.

Took 3 minutes

---------

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

cockatrice/src/game/player/menu/player_menu.cpp

@@ -97,10 +97,7 @@ void PlayerMenu::retranslateUi()
         countersMenu->setTitle(tr("&Counters"));
     }
 
-    QMapIterator<int, AbstractCounter *> counterIterator(player->getCounters());
-    while (counterIterator.hasNext()) {
-        counterIterator.next().value()->retranslateUi();
-    }
+    emit retranslateRequested();
 }
 
 void PlayerMenu::refreshShortcuts()
@@ -120,30 +117,17 @@ void PlayerMenu::refreshShortcuts()
 void PlayerMenu::setShortcutsActive()
 {
     shortcutsActive = true;
-
-    for (auto *component : managedComponents) {
-        component->setShortcutsActive();
-    }
-
-    // Counters implement AbstractPlayerComponent but are iterated via Player::counters
-    // (the authoritative source) rather than managedComponents to avoid a redundant
-    // list that must stay in sync with the map.
-    QMapIterator<int, AbstractCounter *> counterIterator(player->getCounters());
-    while (counterIterator.hasNext()) {
-        counterIterator.next().value()->setShortcutsActive();
+    for (auto *c : managedComponents) {
+        c->setShortcutsActive();
     }
+    emit shortcutsActivated();
 }
 
 void PlayerMenu::setShortcutsInactive()
 {
     shortcutsActive = false;
-
-    for (auto *component : managedComponents) {
-        component->setShortcutsInactive();
-    }
-
-    QMapIterator<int, AbstractCounter *> counterIterator(player->getCounters());
-    while (counterIterator.hasNext()) {
-        counterIterator.next().value()->setShortcutsInactive();
+    for (auto *c : managedComponents) {
+        c->setShortcutsInactive();
     }
+    emit shortcutsDeactivated();
 }

cockatrice/src/game/player/menu/player_menu.h

@@ -29,6 +29,9 @@ class PlayerMenu : public QObject
 
 signals:
     void cardMenuUpdated(QMenu *cardMenu);
+    void shortcutsActivated();
+    void shortcutsDeactivated();
+    void retranslateRequested();
 
 public slots:
     void setMenusForGraphicItems();

cockatrice/src/game/player/player_event_handler.cpp

@@ -246,7 +246,7 @@ void PlayerEventHandler::eventCreateCounter(const Event_CreateCounter &event)
 
 void PlayerEventHandler::eventSetCounter(const Event_SetCounter &event)
 {
-    AbstractCounter *ctr = player->getCounters().value(event.counter_id(), 0);
+    CounterState *ctr = player->getCounters().value(event.counter_id(), nullptr);
     if (!ctr) {
         return;
     }

cockatrice/src/game/player/player_graphics_item.cpp

@@ -6,6 +6,7 @@
 #include "../../game_graphics/zones/table_zone.h"
 #include "../../interface/widgets/tabs/tab_game.h"
 #include "../board/abstract_card_item.h"
+#include "../board/counter_general.h"
 #include "../hand_counter.h"
 
 PlayerGraphicsItem::PlayerGraphicsItem(PlayerLogic *_player) : player(_player)
@@ -18,6 +19,25 @@ PlayerGraphicsItem::PlayerGraphicsItem(PlayerLogic *_player) : player(_player)
     connect(player, &PlayerLogic::concededChanged, this, [this](int, bool c) { setVisible(!c); });
     connect(player, &PlayerLogic::zoneIdChanged, this, [this](int id) { playerArea->setPlayerZoneId(id); });
 
+    connect(player, &PlayerLogic::counterAdded, this, &PlayerGraphicsItem::onCounterAdded);
+    connect(player, &PlayerLogic::counterRemoved, this, &PlayerGraphicsItem::onCounterRemoved);
+
+    connect(player->getPlayerMenu(), &PlayerMenu::shortcutsActivated, this, [this]() {
+        for (auto *ctr : counterWidgets) {
+            ctr->setShortcutsActive();
+        }
+    });
+    connect(player->getPlayerMenu(), &PlayerMenu::shortcutsDeactivated, this, [this]() {
+        for (auto *ctr : counterWidgets) {
+            ctr->setShortcutsInactive();
+        }
+    });
+    connect(player->getPlayerMenu(), &PlayerMenu::retranslateRequested, this, [this]() {
+        for (auto *ctr : counterWidgets) {
+            ctr->retranslateUi();
+        }
+    });
+
     playerArea = new PlayerArea(this);
 
     playerTarget = new PlayerTarget(player, playerArea);
@@ -43,11 +63,6 @@ void PlayerGraphicsItem::retranslateUi()
     while (zoneIterator.hasNext()) {
         emit zoneIterator.next().value()->retranslateUi();
     }
-
-    QMapIterator<int, AbstractCounter *> counterIterator(player->getCounters());
-    while (counterIterator.hasNext()) {
-        counterIterator.next().value()->retranslateUi();
-    }
 }
 
 void PlayerGraphicsItem::onPlayerActiveChanged(bool _active)
@@ -134,20 +149,48 @@ void PlayerGraphicsItem::setMirrored(bool _mirrored)
     }
 }
 
+void PlayerGraphicsItem::onCounterAdded(CounterState *state)
+{
+    AbstractCounter *widget;
+    if (state->getName() == "life") {
+        widget = playerTarget->addCounter(state);
+    } else {
+        widget = new GeneralCounter(state, player, true, this);
+    }
+    counterWidgets.insert(state->getId(), widget);
+
+    if (player->getPlayerMenu()->getCountersMenu() && widget->getMenu()) {
+        player->getPlayerMenu()->getCountersMenu()->addMenu(widget->getMenu());
+    }
+
+    if (player->getPlayerMenu()->getShortcutsActive()) {
+        widget->setShortcutsActive();
+    }
+
+    rearrangeCounters();
+}
+
+void PlayerGraphicsItem::onCounterRemoved(int counterId)
+{
+    auto *widget = counterWidgets.take(counterId);
+    if (!widget) {
+        return;
+    }
+    if (player->getPlayerMenu()->getCountersMenu() && widget->getMenu()) {
+        player->getPlayerMenu()->getCountersMenu()->removeAction(widget->getMenu()->menuAction());
+    }
+    widget->delCounter();
+    rearrangeCounters();
+}
+
 void PlayerGraphicsItem::rearrangeCounters()
 {
-    qreal marginTop = 80;
-    const qreal padding = 5;
-    qreal ySize = boundingRect().y() + marginTop;
-
-    // Place objects
-    for (const auto &counter : player->getCounters()) {
-        AbstractCounter *ctr = counter;
-
+    qreal ySize = boundingRect().y() + 80;
+    constexpr qreal padding = 5;
+    for (auto *ctr : counterWidgets.values()) {
         if (!ctr->getShownInCounterArea()) {
             continue;
         }
-
         QRectF br = ctr->boundingRect();
         ctr->setPos((counterAreaWidth - br.width()) / 2, ySize);
         ySize += br.height() + padding;

cockatrice/src/game/player/player_graphics_item.h

@@ -6,6 +6,7 @@
 
 #ifndef COCKATRICE_PLAYER_GRAPHICS_ITEM_H
 #define COCKATRICE_PLAYER_GRAPHICS_ITEM_H
+#include "../board/abstract_counter.h"
 #include "../game_scene.h"
 #include "player_logic.h"
 
@@ -102,6 +103,9 @@ public:
 
 public slots:
     void onPlayerActiveChanged(bool _active);
+    void onCounterAdded(CounterState *state);
+    void onCounterRemoved(int counterId);
+    void rearrangeCounters();
     void retranslateUi();
 
 signals:
@@ -112,6 +116,7 @@ private:
     PlayerLogic *player;
     PlayerArea *playerArea;
     PlayerTarget *playerTarget;
+    QMap<int, AbstractCounter *> counterWidgets;
     PileZone *deckZoneGraphicsItem;
     PileZone *sideboardGraphicsItem;
     PileZone *graveyardZoneGraphicsItem;
@@ -126,7 +131,6 @@ private:
 private slots:
     void updateBoundingRect();
     void rearrangeZones();
-    void rearrangeCounters();
 };
 
 #endif // COCKATRICE_PLAYER_GRAPHICS_ITEM_H

cockatrice/src/game/player/player_logic.cpp

@@ -258,57 +258,52 @@ void PlayerLogic::setDeck(const DeckList &_deck)
     emit deckChanged();
 }
 
-AbstractCounter *PlayerLogic::addCounter(const ServerInfo_Counter &counter)
+CounterState *PlayerLogic::addCounter(const ServerInfo_Counter &counter)
 {
     return addCounter(counter.id(), QString::fromStdString(counter.name()),
                       convertColorToQColor(counter.counter_color()), counter.radius(), counter.count());
 }
 
-AbstractCounter *PlayerLogic::addCounter(int counterId, const QString &name, QColor color, int radius, int value)
+CounterState *PlayerLogic::addCounter(int id, const QString &name, const QColor &color, int radius, int value)
 {
-    if (counters.contains(counterId)) {
+    if (counters.contains(id)) {
         return nullptr;
     }
-
-    AbstractCounter *ctr;
-    if (name == "life") {
-        ctr = getGraphicsItem()->getPlayerTarget()->addCounter(counterId, name, value);
-    } else {
-        ctr = new GeneralCounter(this, counterId, name, color, radius, value, true, graphicsItem);
-    }
-    counters.insert(counterId, ctr);
-
-    if (playerMenu->getCountersMenu() && ctr->getMenu()) {
-        playerMenu->getCountersMenu()->addMenu(ctr->getMenu());
-    }
-    if (playerMenu->getShortcutsActive()) {
-        ctr->setShortcutsActive();
-    }
-    emit rearrangeCounters();
-    return ctr;
+    auto *state = new CounterState(id, name, color, radius, value, this);
+    counters.insert(id, state);
+    emit counterAdded(state);
+    return state;
 }
 
-void PlayerLogic::delCounter(int counterId)
+void PlayerLogic::delCounter(int id)
 {
-    AbstractCounter *ctr = counters.value(counterId, 0);
-    if (!ctr) {
+    auto *state = counters.take(id);
+    if (!state) {
         return;
     }
-
-    ctr->delCounter();
-    counters.remove(counterId);
-    emit rearrangeCounters();
+    emit counterRemoved(id);
+    state->deleteLater();
 }
 
 void PlayerLogic::clearCounters()
 {
-    QMapIterator<int, AbstractCounter *> counterIterator(counters);
-    while (counterIterator.hasNext()) {
-        counterIterator.next().value()->delCounter();
+    for (int id : counters.keys()) {
+        emit counterRemoved(id);
     }
+    qDeleteAll(counters);
     counters.clear();
 }
 
+CounterState *PlayerLogic::getLifeCounter() const
+{
+    for (auto *s : counters.values()) {
+        if (s->getName() == "life") {
+            return s;
+        }
+    }
+    return nullptr;
+}
+
 void PlayerLogic::incrementAllCardCounters()
 {
     auto cardsToUpdate = getGameScene()->selectedCards();
@@ -345,16 +340,6 @@ void PlayerLogic::incrementAllCardCounters()
     }
 }
 
-AbstractCounter *PlayerLogic::getLifeCounter() const
-{
-    for (auto counter : counters.values()) {
-        if (counter->getName() == "life") {
-            return counter;
-        }
-    }
-    return nullptr;
-}
-
 ArrowItem *PlayerLogic::addArrow(const ServerInfo_Arrow &arrow)
 {
     const QMap<int, PlayerLogic *> &playerList = game->getPlayerManager()->getPlayers();

cockatrice/src/game/player/player_logic.h

@@ -7,7 +7,6 @@
 #ifndef PLAYER_H
 #define PLAYER_H
 
-#include "../../game_graphics/board/abstract_graphics_item.h"
 #include "../../interface/widgets/menus/tearoff_menu.h"
 #include "../interface/deck_loader/loaded_deck.h"
 #include "../zones/hand_zone_logic.h"
@@ -39,7 +38,6 @@ class Message;
 }
 } // namespace google
 class AbstractCardItem;
-class AbstractCounter;
 class AbstractGame;
 class ArrowItem;
 class ArrowTarget;
@@ -70,6 +68,8 @@ signals:
     void openDeckEditor(const LoadedDeck &deck);
     void deckChanged();
     void newCardAdded(AbstractCardItem *card);
+    void counterAdded(CounterState *state);
+    void counterRemoved(int counterId);
     void rearrangeCounters();
     void activeChanged(bool active);
     void zoneIdChanged(int zoneId);
@@ -189,13 +189,13 @@ public:
         return qobject_cast<HandZoneLogic *>(zones.value(ZoneNames::HAND));
     }
 
-    AbstractCounter *addCounter(const ServerInfo_Counter &counter);
-    AbstractCounter *addCounter(int counterId, const QString &name, QColor color, int radius, int value);
+    CounterState *addCounter(const ServerInfo_Counter &counter);
+    CounterState *addCounter(int id, const QString &name, const QColor &color, int radius, int value);
     void delCounter(int counterId);
     void clearCounters();
     void incrementAllCardCounters();
 
-    QMap<int, AbstractCounter *> getCounters()
+    QMap<int, CounterState *> getCounters() const
     {
         return counters;
     }
@@ -203,7 +203,7 @@ public:
     /**
      * Gets the counter that represents the life total.
      */
-    AbstractCounter *getLifeCounter() const;
+    CounterState *getLifeCounter() const;
 
     ArrowItem *addArrow(const ServerInfo_Arrow &arrow);
     ArrowItem *addArrow(int arrowId, CardItem *startCard, ArrowTarget *targetItem, const QColor &color);
@@ -251,7 +251,7 @@ private:
 
     int zoneId;
     QMap<QString, CardZoneLogic *> zones;
-    QMap<int, AbstractCounter *> counters;
+    QMap<int, CounterState *> counters;
     QMap<int, ArrowItem *> arrows;
 
     bool dialogSemaphore;

cockatrice/src/game/player/player_target.cpp

@@ -9,8 +9,8 @@
 #include <QtMath>
 #include <libcockatrice/protocol/pb/serverinfo_user.pb.h>
 
-PlayerCounter::PlayerCounter(PlayerLogic *_player, int _id, const QString &_name, int _value, QGraphicsItem *parent)
-    : AbstractCounter(_player, _id, _name, false, _value, false, parent)
+PlayerCounter::PlayerCounter(CounterState *state, PlayerLogic *player, QGraphicsItem *parent)
+    : AbstractCounter(state, player, false, false, parent)
 {
 }
 
@@ -150,18 +150,16 @@ void PlayerTarget::paint(QPainter *painter, const QStyleOptionGraphicsItem * /*o
     }
 }
 
-AbstractCounter *PlayerTarget::addCounter(int _counterId, const QString &_name, int _value)
+AbstractCounter *PlayerTarget::addCounter(CounterState *state)
 {
     if (playerCounter) {
         disconnect(playerCounter, nullptr, this, nullptr);
         playerCounter->delCounter();
     }
-
-    playerCounter = new PlayerCounter(owner, _counterId, _name, _value, this);
+    playerCounter = new PlayerCounter(state, owner, this);
     playerCounter->setPos(boundingRect().width() - playerCounter->boundingRect().width(),
                           boundingRect().height() - playerCounter->boundingRect().height());
     connect(playerCounter, &PlayerCounter::destroyed, this, &PlayerTarget::counterDeleted);
-
     return playerCounter;
 }
 

cockatrice/src/game/player/player_target.h

@@ -19,7 +19,7 @@ class PlayerCounter : public AbstractCounter
 {
     Q_OBJECT
 public:
-    PlayerCounter(PlayerLogic *_player, int _id, const QString &_name, int _value, QGraphicsItem *parent = nullptr);
+    PlayerCounter(CounterState *state, PlayerLogic *player, QGraphicsItem *parent);
     QRectF boundingRect() const override;
     void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) override;
 };
@@ -48,7 +48,7 @@ public:
     QRectF boundingRect() const override;
     void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) override;
 
-    AbstractCounter *addCounter(int _counterId, const QString &_name, int _value);
+    AbstractCounter *addCounter(CounterState *state);
 };
 
 #endif