Remove inactive counters submenus from the player counters menu

cockatrice/src/game_graphics/board/abstract_counter.cpp

@@ -94,6 +94,12 @@ void AbstractCounter::setActive(bool _active)
 {
     active = _active;
     setVisible(_active);
+    if (menu) {
+        menu->setEnabled(_active);
+        if (!_active) {
+            menu->hideTearOffMenu();
+        }
+    }
     update();
 }
 
@@ -176,6 +182,10 @@ void AbstractCounter::hoverLeaveEvent(QGraphicsSceneHoverEvent *)
 
 void AbstractCounter::incrementCounter()
 {
+    if (!active) {
+        return;
+    }
+
     Command_IncCounter cmd;
     cmd.set_counter_id(id);
     cmd.set_delta(static_cast<QAction *>(sender())->data().toInt());
@@ -184,6 +194,10 @@ void AbstractCounter::incrementCounter()
 
 void AbstractCounter::setCounter()
 {
+    if (!active) {
+        return;
+    }
+
     QWidget *parent = nullptr;
     if (auto *view = scene() ? scene()->views().value(0) : nullptr) {
         parent = view->window();
@@ -198,7 +212,8 @@ void AbstractCounter::setCounter()
         deleteLater();
         return;
     }
-    if (!ok) {
+    // Re-check active: exec() spins the event loop in case the counter was deactivated.
+    if (!ok || !active) {
         return;
     }
 

cockatrice/src/game_graphics/board/abstract_counter.h

@@ -102,25 +102,20 @@ public:
         return shownInCounterArea;
     }
 
-    /**
+    /** @brief Returns whether this counter is shown and can be modified. */
-     * @brief Returns whether this counter is active (visible and interactable).
-     * Inactive counters are hidden and their menu actions should be disabled.
-     */
     [[nodiscard]] bool isActive() const
     {
         return active;
     }
 
     /**
-     * @brief Sets the active state of this counter.
+     * @brief Shows or hides the counter and enables or disables its menu.
      *
-     * This is the sole owner of the counter's own visibility flag: active maps directly to
+     * Sole owner of both, so container layout code only positions counters and may read
-     * setVisible(_active). For counters nested in another item (e.g. tax counters parented to
+     * isActive(). Qt AND-s visibility with the parent item's, so an active counter nested in a
-     * the command zone), Qt AND-s this flag with the parent's visibility, so an active counter
+     * hidden zone (e.g. a tax counter in the command zone) still does not render.
-     * inside a hidden zone still does not render. Container layout code must therefore not set
-     * visibility itself; it only positions counters and may read isActive().
      *
-     * @param _active True to show and enable the counter, false to hide it
+     * @param _active True to show and enable the counter, false to hide and disable it
      */
     virtual void setActive(bool _active);
 

cockatrice/src/game_graphics/player/player_graphics_item.cpp

@@ -202,6 +202,36 @@ void PlayerGraphicsItem::setMirrored(bool _mirrored)
     }
 }
 
+QAction *PlayerGraphicsItem::counterMenuInsertAnchor(int counterId) const
+{
+    QMenu *countersMenu = playerMenu->getCountersMenu();
+    if (!countersMenu) {
+        return nullptr;
+    }
+    const QList<QAction *> inMenu = countersMenu->actions();
+    // QMap iterates in ascending key order, so this finds the lowest id above counterId.
+    for (auto it = counterWidgets.upperBound(counterId); it != counterWidgets.constEnd(); ++it) {
+        QMenu *menu = it.value()->getMenu();
+        if (menu && inMenu.contains(menu->menuAction())) {
+            return menu->menuAction();
+        }
+    }
+    return nullptr;
+}
+
+void PlayerGraphicsItem::setCounterMenuRegistered(AbstractCounter *widget, bool registered)
+{
+    QMenu *countersMenu = playerMenu->getCountersMenu();
+    if (!widget || !countersMenu || !widget->getMenu()) {
+        return;
+    }
+    if (registered) {
+        countersMenu->insertMenu(counterMenuInsertAnchor(widget->getId()), widget->getMenu());
+    } else {
+        countersMenu->removeAction(widget->getMenu()->menuAction());
+    }
+}
+
 void PlayerGraphicsItem::onCounterAdded(CounterState *state)
 {
     AbstractCounter *widget;
@@ -222,9 +252,14 @@ void PlayerGraphicsItem::onCounterAdded(CounterState *state)
     }
     counterWidgets.insert(state->getId(), widget);
 
-    if (playerMenu->getCountersMenu() && widget->getMenu()) {
+    // A counter's submenu follows its isActive() state: while inactive the counter is hidden and
-        playerMenu->getCountersMenu()->addMenu(widget->getMenu());
+    // the server rejects every modification. Only tax counters go inactive today.
-    }
+    setCounterMenuRegistered(widget, state->isActive());
+    connect(state, &CounterState::activeChanged, this, [this, counterId = state->getId()](bool newActive) {
+        if (AbstractCounter *counter = getCounterWidget(counterId)) {
+            setCounterMenuRegistered(counter, newActive);
+        }
+    });
 
     if (playerMenu->getShortcutsActive()) {
         widget->setShortcutsActive();
@@ -239,9 +274,7 @@ void PlayerGraphicsItem::onCounterRemoved(int counterId)
     if (!widget) {
         return;
     }
-    if (playerMenu->getCountersMenu() && widget->getMenu()) {
+    setCounterMenuRegistered(widget, false);
-        playerMenu->getCountersMenu()->removeAction(widget->getMenu()->menuAction());
-    }
     if (commandZoneGraphicsItem && CounterNames::isTaxCounter(widget->getName())) {
         commandZoneGraphicsItem->unregisterTaxCounter(widget);
     }

cockatrice/src/game_graphics/player/player_graphics_item.h

@@ -162,6 +162,13 @@ private:
     bool mirrored;
     bool handVisible = false;
 
+    /**
+     * @brief Returns the menu action a counter's submenu is inserted before to keep the
+     * Player -> Counters menu ordered by counter id, or nullptr to append.
+     */
+    [[nodiscard]] QAction *counterMenuInsertAnchor(int counterId) const;
+    /** @brief Adds or removes a counter's submenu in the Player -> Counters menu. */
+    void setCounterMenuRegistered(AbstractCounter *widget, bool registered);
     /** @brief Returns the command zone's display height, or 0 if hidden. */
     [[nodiscard]] qreal totalCommandZoneHeight() const;
     /** @brief Positions the command and stack zones vertically starting from base, updating base.y. */