Harden command zone tax counter handling and more clean up

- Initialize commandZoneGraphicsItem to nullptr so the pervasive null
    checks have defined behavior before initializeZones() runs
  - Remove unused getTaxCounterWidgets()
  - Enforce the "reset tax to 0 before deactivating" invariant server-side
    in cmdSetCounterActive, not just in the UI
  - Hide tax increment/decrement menu actions at their bounds (0 and
    MAX_COUNTER_VALUE) to avoid sending no-op commands
  - Drop redundant/inconsistent toggle labels from CommandZoneMenu::
    retranslateUi(); updateTaxCounterActionStates() owns them
  - Add default=-1 to Event_SetCounterActive.counter_id for parity with
    Command_SetCounterActive

cockatrice/src/game_graphics/player/menu/command_zone_menu.cpp

@@ -9,6 +9,7 @@
 #include "../player_graphics_item.h"
 
 #include <libcockatrice/utility/counter_ids.h>
+#include <libcockatrice/utility/trice_limits.h>
 #include <libcockatrice/utility/zone_names.h>
 
 CommandZoneMenu::CommandZoneMenu(PlayerGraphicsItem *_player, QMenu *playerMenu) : QMenu(playerMenu), player(_player)
@@ -107,18 +108,13 @@ void CommandZoneMenu::retranslateUi()
     if (aDecreaseCommanderTax) {
         aDecreaseCommanderTax->setText(tr("&Decrease Commander Tax (-1)"));
     }
-    if (aToggleCommanderTaxCounter) {
-        aToggleCommanderTaxCounter->setText(tr("&Remove Commander Tax"));
-    }
     if (aIncreasePartnerTax) {
         aIncreasePartnerTax->setText(tr("Increase &Partner Tax (+1)"));
     }
     if (aDecreasePartnerTax) {
         aDecreasePartnerTax->setText(tr("Decrease P&artner Tax (-1)"));
     }
-    if (aTogglePartnerTaxCounter) {
+    // Toggle action labels are derived dynamically in updateTaxCounterActionStates()
-        aTogglePartnerTaxCounter->setText(tr("&Add Partner Tax"));
-    }
     if (aToggleMinimized) {
         aToggleMinimized->setText(tr("&Minimize"));
     }
@@ -138,10 +134,10 @@ void CommandZoneMenu::updateTaxCounterActionStates()
     AbstractCounter *partnerTax = player->getTaxCounterIfActive(CounterIds::PartnerTax);
 
     if (aIncreaseCommanderTax) {
-        aIncreaseCommanderTax->setVisible(cmdTax != nullptr);
+        aIncreaseCommanderTax->setVisible(cmdTax && cmdTax->getValue() < MAX_COUNTER_VALUE);
     }
     if (aDecreaseCommanderTax) {
-        aDecreaseCommanderTax->setVisible(cmdTax != nullptr);
+        aDecreaseCommanderTax->setVisible(cmdTax && cmdTax->getValue() > 0);
     }
     if (aToggleCommanderTaxCounter) {
         aToggleCommanderTaxCounter->setText(cmdTax ? tr("&Remove Commander Tax") : tr("&Add Commander Tax"));
@@ -149,10 +145,10 @@ void CommandZoneMenu::updateTaxCounterActionStates()
     }
 
     if (aIncreasePartnerTax) {
-        aIncreasePartnerTax->setVisible(partnerTax != nullptr);
+        aIncreasePartnerTax->setVisible(partnerTax && partnerTax->getValue() < MAX_COUNTER_VALUE);
     }
     if (aDecreasePartnerTax) {
-        aDecreasePartnerTax->setVisible(partnerTax != nullptr);
+        aDecreasePartnerTax->setVisible(partnerTax && partnerTax->getValue() > 0);
     }
     if (aTogglePartnerTaxCounter) {
         aTogglePartnerTaxCounter->setText(partnerTax ? tr("R&emove Partner Tax") : tr("&Add Partner Tax"));

cockatrice/src/game_graphics/player/player_graphics_item.cpp

@@ -259,17 +259,6 @@ void PlayerGraphicsItem::rearrangeCounters()
     }
 }
 
-QList<AbstractCounter *> PlayerGraphicsItem::getTaxCounterWidgets() const
-{
-    QList<AbstractCounter *> result;
-    for (AbstractCounter *ctr : counterWidgets.values()) {
-        if (CounterNames::isTaxCounter(ctr->getName())) {
-            result.append(ctr);
-        }
-    }
-    return result;
-}
-
 AbstractCounter *PlayerGraphicsItem::getTaxCounterIfActive(int counterId) const
 {
     AbstractCounter *counter = getCounterWidget(counterId);

cockatrice/src/game_graphics/player/player_graphics_item.h

@@ -123,8 +123,6 @@ public:
     {
         return counterWidgets.value(counterId, nullptr);
     }
-    /** @brief Returns all tax counter widgets (commander tax and partner tax). */
-    [[nodiscard]] QList<AbstractCounter *> getTaxCounterWidgets() const;
     /** @brief Returns the tax counter if it exists and is active, or nullptr otherwise. */
     [[nodiscard]] AbstractCounter *getTaxCounterIfActive(int counterId) const;
 
@@ -159,7 +157,7 @@ private:
     TableZone *tableZoneGraphicsItem;
     StackZone *stackZoneGraphicsItem;
     HandZone *handZoneGraphicsItem;
-    CommandZone *commandZoneGraphicsItem;
+    CommandZone *commandZoneGraphicsItem = nullptr;
     QRectF bRect;
     bool mirrored;
     bool handVisible = false;

libcockatrice_network/libcockatrice/network/server/remote/game/server_player.cpp

@@ -591,6 +591,11 @@ Response::ResponseCode Server_Player::cmdSetCounterActive(const Command_SetCount
         return Response::RespNameNotFound;
     }
 
+    // Prevent disabling a counter with tax accumulated; player must reset to 0 first
+    if (!cmd.active() && c->getCount() != 0) {
+        return Response::RespContextError;
+    }
+
     bool didChange = c->setActive(cmd.active());
 
     if (didChange) {

libcockatrice_protocol/libcockatrice/protocol/pb/event_set_counter_active.proto

@@ -5,6 +5,6 @@ message Event_SetCounterActive {
     extend GameEvent {
         optional Event_SetCounterActive ext = 2023;
     }
-    optional sint32 counter_id = 1;
+    optional sint32 counter_id = 1 [default = -1];
     optional bool active = 2 [default = true];
 }