Code cleanup

- Add missing isCommandZoneCounterBlocked check to cmdSetCounterActive
  - Revert accidental deck view layout change from b4057a86
  - Fix duplicate @param in playSelectedCardsImpl doc
  - Add null check for PlayerLogic in CommandZoneMenu constructor
  - Add index bounds check in CommandZone handleDropEvent
  - Add index bounds check in StackZone handleDropEvent
  - Add ownership comment for tax counter widget creation
  - Add command zone to zoneGraphicsItems map
  - Conditionally show command zone menu item based on server support
  - Remove layer-violating includes from player_logic.cpp
  - Fix tax counter increment (1 per cast, not 2)
  - Add getTaxCounterIfActive() helper to PlayerGraphicsItem

cockatrice/src/game/player/player_actions.cpp

@@ -1657,7 +1657,7 @@ void PlayerActions::actPlayAndIncreaseTax(QList<CardItem *> selectedCards)
         if (originalZone == ZoneNames::COMMAND) {
             CounterState *state = player->getCounters().value(CounterIds::CommanderTax, nullptr);
             if (state && state->isActive()) {
-                sendIncCounter(CounterIds::CommanderTax, 2);
+                sendIncCounter(CounterIds::CommanderTax, 1);
             }
         }
     });
@@ -1669,7 +1669,7 @@ void PlayerActions::actPlayAndIncreasePartnerTax(QList<CardItem *> selectedCards
         if (originalZone == ZoneNames::COMMAND) {
             CounterState *state = player->getCounters().value(CounterIds::PartnerTax, nullptr);
             if (state && state->isActive()) {
-                sendIncCounter(CounterIds::PartnerTax, 2);
+                sendIncCounter(CounterIds::PartnerTax, 1);
             }
         }
     });
@@ -1695,6 +1695,7 @@ void PlayerActions::actModifyTaxCounter(int counterId, int delta)
 void PlayerActions::actToggleTaxCounter(int counterId)
 {
     CounterState *state = player->getCounters().value(counterId, nullptr);
+    // Prevent disabling a counter with tax accumulated; player must reset to 0 first
     if (!state || (state->isActive() && state->getValue() != 0)) {
         return;
     }

cockatrice/src/game/player/player_actions.h

@@ -257,9 +257,8 @@ private:
 
     /**
      * @brief Shared implementation for playing selected cards with an optional post-play callback.
-     * @param selectedCards
+     * @param selectedCards Cards to play
-     * @param selectedCards
+     * @param faceDown Whether to play cards face-down
-     * @param selectedCards
      * @param postPlayCallback Called after each card is played, receiving the card and its *original* zone name
      *        (captured before playCard, since playCard sends a move command that may change the card's zone).
      */

cockatrice/src/game/player/player_logic.cpp

@@ -2,11 +2,9 @@
 
 #include "../../game_graphics/board/arrow_item.h"
 #include "../../game_graphics/board/card_item.h"
-#include "../../game_graphics/board/commander_tax_counter.h"
 #include "../../game_graphics/board/counter_general.h"
 #include "../../game_graphics/game_scene.h"
 #include "../../game_graphics/player/player_target.h"
-#include "../../game_graphics/zones/command_zone.h"
 #include "../../game_graphics/zones/hand_zone.h"
 #include "../../game_graphics/zones/pile_zone.h"
 #include "../../game_graphics/zones/stack_zone.h"

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

@@ -165,13 +165,11 @@ CardMenu::CardMenu(PlayerGraphicsItem *_player, const CardItem *_card, bool _sho
                 if (writeableCard) {
                     addAction(aPlay);
 
-                    AbstractCounter *cmdTax = player->getCounterWidget(CounterIds::CommanderTax);
+                    if (player->getTaxCounterIfActive(CounterIds::CommanderTax)) {
-                    if (cmdTax && cmdTax->isActive()) {
                         addAction(aPlayAndIncreaseTax);
                     }
 
-                    AbstractCounter *partnerTax = player->getCounterWidget(CounterIds::PartnerTax);
+                    if (player->getTaxCounterIfActive(CounterIds::PartnerTax)) {
-                    if (partnerTax && partnerTax->isActive()) {
                         addAction(aPlayAndIncreasePartnerTax);
                     }
 

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

@@ -26,7 +26,8 @@ CommandZoneMenu::CommandZoneMenu(PlayerGraphicsItem *_player, QMenu *playerMenu)
         }
     });
 
-    if (player->getLogic()->getPlayerInfo()->getLocalOrJudge()) {
+    PlayerLogic *logic = player->getLogic();
+    if (logic && logic->getPlayerInfo()->getLocalOrJudge()) {
         addAction(aViewZone);
         addSeparator();
 
@@ -117,32 +118,29 @@ void CommandZoneMenu::actToggleMinimized()
 
 void CommandZoneMenu::updateTaxCounterActionStates()
 {
-    AbstractCounter *cmdTax = player->getCounterWidget(CounterIds::CommanderTax);
+    AbstractCounter *cmdTax = player->getTaxCounterIfActive(CounterIds::CommanderTax);
-    bool cmdActive = cmdTax != nullptr && cmdTax->isActive();
+    AbstractCounter *partnerTax = player->getTaxCounterIfActive(CounterIds::PartnerTax);
-
-    AbstractCounter *partnerTax = player->getCounterWidget(CounterIds::PartnerTax);
-    bool partnerActive = partnerTax != nullptr && partnerTax->isActive();
 
     if (aIncreaseCommanderTax) {
-        aIncreaseCommanderTax->setVisible(cmdActive);
+        aIncreaseCommanderTax->setVisible(cmdTax != nullptr);
     }
     if (aDecreaseCommanderTax) {
-        aDecreaseCommanderTax->setVisible(cmdActive);
+        aDecreaseCommanderTax->setVisible(cmdTax != nullptr);
     }
     if (aToggleCommanderTaxCounter) {
-        aToggleCommanderTaxCounter->setText(cmdActive ? tr("&Remove Commander Tax") : tr("&Add Commander Tax"));
+        aToggleCommanderTaxCounter->setText(cmdTax ? tr("&Remove Commander Tax") : tr("&Add Commander Tax"));
-        aToggleCommanderTaxCounter->setVisible(!cmdActive || (cmdTax && cmdTax->getValue() == 0));
+        aToggleCommanderTaxCounter->setVisible(!cmdTax || cmdTax->getValue() == 0);
     }
 
     if (aIncreasePartnerTax) {
-        aIncreasePartnerTax->setVisible(partnerActive);
+        aIncreasePartnerTax->setVisible(partnerTax != nullptr);
     }
     if (aDecreasePartnerTax) {
-        aDecreasePartnerTax->setVisible(partnerActive);
+        aDecreasePartnerTax->setVisible(partnerTax != nullptr);
     }
     if (aTogglePartnerTaxCounter) {
-        aTogglePartnerTaxCounter->setText(partnerActive ? tr("R&emove Partner Tax") : tr("&Add Partner Tax"));
+        aTogglePartnerTaxCounter->setText(partnerTax ? tr("R&emove Partner Tax") : tr("&Add Partner Tax"));
-        aTogglePartnerTaxCounter->setVisible(!partnerActive || (partnerTax && partnerTax->getValue() == 0));
+        aTogglePartnerTaxCounter->setVisible(!partnerTax || partnerTax->getValue() == 0);
     }
 }
 

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

@@ -54,6 +54,11 @@ MoveMenu::MoveMenu(PlayerGraphicsItem *player) : QMenu(tr("Move to"))
     addSeparator();
     addAction(aMoveToCommandZone);
 
+    auto *playerLogic = player->getLogic();
+    auto updateCommandZoneVisibility = [this](bool has) { aMoveToCommandZone->setVisible(has); };
+    connect(playerLogic, &PlayerLogic::commandZoneSupportChanged, this, updateCommandZoneVisibility);
+    updateCommandZoneVisibility(playerLogic->hasServerCommandZone());
+
     setShortcutsActive();
 
     retranslateUi();

cockatrice/src/game_graphics/player/player_graphics_item.cpp

@@ -143,6 +143,7 @@ void PlayerGraphicsItem::initializeZones()
     zoneGraphicsItems.insert(player->getTableZone()->getName(), tableZoneGraphicsItem);
     zoneGraphicsItems.insert(player->getStackZone()->getName(), stackZoneGraphicsItem);
     zoneGraphicsItems.insert(player->getHandZone()->getName(), handZoneGraphicsItem);
+    zoneGraphicsItems.insert(player->getCommandZone()->getName(), commandZoneGraphicsItem);
 }
 
 void PlayerGraphicsItem::onCustomZoneAdded(QString customZoneName)
@@ -203,6 +204,8 @@ void PlayerGraphicsItem::onCounterAdded(CounterState *state)
             qWarning() << "Cannot create tax counter" << state->getName() << "- command zone not available";
             return;
         }
+        // Qt parent (commandZoneGraphicsItem) owns widget; counterWidgets map holds reference
+        // for lookup; CommandZone::registerTaxCounter connects QObject::destroyed for cleanup
         widget = new CommanderTaxCounter(state, player, commandZoneGraphicsItem);
         widget->setActive(state->isActive());
         commandZoneGraphicsItem->registerTaxCounter(widget);
@@ -267,6 +270,12 @@ QList<AbstractCounter *> PlayerGraphicsItem::getTaxCounterWidgets() const
     return result;
 }
 
+AbstractCounter *PlayerGraphicsItem::getTaxCounterIfActive(int counterId) const
+{
+    AbstractCounter *counter = getCounterWidget(counterId);
+    return (counter && counter->isActive()) ? counter : nullptr;
+}
+
 void PlayerGraphicsItem::rearrangeZones()
 {
     auto base = QPointF(CardDimensions::HEIGHT_F + counterAreaWidth + 15, 0);

cockatrice/src/game_graphics/player/player_graphics_item.h

@@ -125,6 +125,8 @@ public:
     }
     /** @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;
 
 public slots:
     void onPlayerActiveChanged(bool _active);

cockatrice/src/game_graphics/zones/command_zone.cpp

@@ -96,7 +96,8 @@ void CommandZone::handleDropEvent(const QList<CardDragItem *> &dragItems,
 
     // Same-zone no-op: don't move a card onto itself
     const auto &cards = getLogic()->getCards();
-    if (!cards.isEmpty() && startZone == getLogic() && cards.at(index)->getId() == dragItems.at(0)->getId()) {
+    if (!cards.isEmpty() && index < cards.size() && startZone == getLogic() &&
+        cards.at(index)->getId() == dragItems.at(0)->getId()) {
         return;
     }
 

cockatrice/src/game_graphics/zones/stack_zone.cpp

@@ -51,7 +51,8 @@ void StackZone::handleDropEvent(const QList<CardDragItem *> &dragItems,
 
     // Same-zone no-op: don't move a card onto itself
     const auto &cards = getLogic()->getCards();
-    if (!cards.isEmpty() && startZone == getLogic() && cards.at(index)->getId() == dragItems.at(0)->getId()) {
+    if (!cards.isEmpty() && index < cards.size() && startZone == getLogic() &&
+        cards.at(index)->getId() == dragItems.at(0)->getId()) {
         return;
     }
 

cockatrice/src/interface/widgets/tabs/tab_game.cpp

@@ -690,7 +690,7 @@ void TabGame::addLocalPlayer(PlayerLogic *newPlayer, int playerId)
     auto *deckView = new TabbedDeckViewContainer(playerId, this);
     connect(deckView->playerDeckView, &DeckViewContainer::newCardAdded, this, &TabGame::newCardAdded);
     deckViewContainers.insert(playerId, deckView);
-    deckViewContainerLayout->insertWidget(0, deckView, 1);
+    deckViewContainerLayout->addWidget(deckView);
 
     // auto load deck for player if that debug setting is enabled
     QString deckPath = SettingsCache::instance().debug().getDeckPathForPlayer(newPlayer->getPlayerInfo()->getName());

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

@@ -578,6 +578,10 @@ Response::ResponseCode Server_Player::cmdSetCounterActive(const Command_SetCount
     }
 
     const int counterId = cmd.counter_id();
+    if (isCommandZoneCounterBlocked(counterId)) {
+        return Response::RespContextError;
+    }
+
     Server_Counter *c = counters.value(counterId, nullptr);
     if (!c) {
         return Response::RespNameNotFound;