Fix stale documentation and align doc comments with CONTRIBUTING guidelines

cockatrice/src/game/board/counter_state.h

@@ -40,16 +40,19 @@ public:
     {
         return value;
     }
+    /** @brief Returns whether this counter is active (visible and modifiable). */
     bool isActive() const
     {
         return active;
     }
 
     void setValue(int newValue);
+    /** @brief Sets the active (visible) state and emits activeChanged if it changed. */
     void setActive(bool newActive);
 
 signals:
     void valueChanged(int oldValue, int newValue);
+    /** @brief Emitted when the counter's active state changes. */
     void activeChanged(bool newActive);
 
 private:
@@ -58,7 +61,7 @@ private:
     QColor color;
     int radius;
     int value;
-    bool active;
+    bool active; ///< Inactive counters are hidden; server rejects modification attempts
 };
 
 #endif // COCKATRICE_COUNTER_STATE_H

cockatrice/src/game/player/player_actions.h

@@ -228,6 +228,7 @@ public slots:
     void cardMenuAction(QList<CardItem *> selectedCards, CardMenuActionType type);
 
 private:
+    /** @brief Sends an increment command for the specified counter. */
     void sendIncCounter(int counterId, int delta);
 
     PlayerLogic *player;
@@ -258,6 +259,8 @@ private:
 
     /**
      * @brief Builds the move command for playing a card, returning the prepared (unsent) PendingCommand.
+     * @param card The card to play
+     * @param faceDown Whether to play the card face-down
      * @return The prepared command, or nullptr if the card cannot be played.
      */
     PendingCommand *prepareCardMove(CardItem *card, bool faceDown);

cockatrice/src/game/player/player_event_handler.h

@@ -156,7 +156,7 @@ public:
     /// Set a player-level counter value.
     void eventSetCounter(const Event_SetCounter &event);
 
-    /// Show or hide a player-level counter without deleting it.
+    /** @brief Show or hide a player-level counter without deleting it. */
     void eventSetCounterActive(const Event_SetCounterActive &event);
 
     /// Delete a player-level counter.

cockatrice/src/game/player/player_logic.h

@@ -89,6 +89,7 @@ signals:
     void arrowDeleteRequested(int creatorId, int arrowId);
     void arrowDeleted(int creatorId, int arrowId);
     void arrowsClearedLocally(); // fires on clear() and processPlayerInfo
+    /** @brief Emitted when server command zone support is detected or lost (e.g. on game join or reconnect). */
     void commandZoneSupportChanged(bool hasCommandZone);
 
 public slots:

cockatrice/src/game_graphics/board/abstract_counter.h

@@ -3,7 +3,6 @@
  * @ingroup GameGraphicsPlayers
  * @brief Abstract base for player counters displayed on the game board.
  */
-//! \todo Document this file.
 
 #ifndef COUNTER_H
 #define COUNTER_H
@@ -21,6 +20,7 @@ class QKeyEvent;
 class QMenu;
 class QString;
 
+//! \todo Document AbstractCounter class members.
 class AbstractCounter : public QObject, public QGraphicsItem, public AbstractPlayerComponent
 {
     Q_OBJECT
@@ -65,8 +65,11 @@ public:
 
     /**
      * @brief Sets the counter value and triggers a visual update.
+     *
      * Virtual to allow subclass display customization (e.g., CommanderTaxCounter tooltip updates).
      * Overflow protection is handled server-side, not in client counter classes.
+     *
+     * @param _value The new counter value
      */
     virtual void setValue(int _value);
     void setShortcutsActive() override;
@@ -120,7 +123,7 @@ public:
     virtual void setActive(bool _active);
 
 private:
-    bool active = true;
+    bool active = true; ///< Whether the counter is shown and modifiable
 };
 
 class AbstractCounterDialog : public QInputDialog

cockatrice/src/game_graphics/board/commander_tax_counter.h

@@ -28,9 +28,10 @@ constexpr int TAX_COUNTER_MARGIN = 2;
  * @class CommanderTaxCounter
  * @brief Counter for tracking commander tax in Commander format.
  *
- * Displays cumulative cost increase for casting a commander. The counter
+ * Displays the number of times the commander has been cast from the command
- * is manually adjusted by the player to track their commander tax. Values
+ * zone. Can be adjusted manually via +1/-1 menu actions, or automatically
- * are clamped to >= 0.
+ * incremented when using "Play and Increase Tax" on an accepted cast from
+ * the command zone. Values are clamped to >= 0.
  *
  * Appearance: square with rounded corners, semi-transparent background,
  * positioned at top-left of command zone.
@@ -48,7 +49,7 @@ class CommanderTaxCounter : public AbstractCounter
 {
     Q_OBJECT
 private:
-    int size;
+    int size; ///< Width and height of the counter in pixels
 
 public:
     /**

cockatrice/src/game_graphics/board/translate_counter_name.cpp

@@ -1,5 +1,7 @@
 #include "translate_counter_name.h"
 
+#include <libcockatrice/utility/counter_ids.h>
+
 const QMap<QString, QString> TranslateCounterName::translated = {
     {"life", QT_TRANSLATE_NOOP("TranslateCounterName", "Life")},
     {"w", QT_TRANSLATE_NOOP("TranslateCounterName", "White")},
@@ -9,5 +11,5 @@ const QMap<QString, QString> TranslateCounterName::translated = {
     {"g", QT_TRANSLATE_NOOP("TranslateCounterName", "Green")},
     {"x", QT_TRANSLATE_NOOP("TranslateCounterName", "Colorless")},
     {"storm", QT_TRANSLATE_NOOP("TranslateCounterName", "Other")},
-    {"commander_tax_counter", QT_TRANSLATE_NOOP("TranslateCounterName", "Commander Tax")},
+    {CounterNames::CommanderTax, QT_TRANSLATE_NOOP("TranslateCounterName", "Commander Tax")},
-    {"partner_tax_counter", QT_TRANSLATE_NOOP("TranslateCounterName", "Partner Tax")}};
+    {CounterNames::PartnerTax, QT_TRANSLATE_NOOP("TranslateCounterName", "Partner Tax")}};

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

@@ -32,9 +32,8 @@ public:
     QMenu *mCardCounters;
 
     QAction *aPlay, *aPlayFacedown;
-    QAction *
+    /** @brief Play actions that also increment the corresponding tax counter. */
-        aPlayAndIncreaseTax; ///< Plays card and increments the primary commander tax counter (CounterIds::CommanderTax)
+    QAction *aPlayAndIncreaseTax, *aPlayAndIncreasePartnerTax;
-    QAction *aPlayAndIncreasePartnerTax;
     QAction *aRevealToAll;
     QAction *aHide;
     QAction *aClone;

cockatrice/src/game_graphics/player/player_graphics_item.h

@@ -171,7 +171,7 @@ private:
     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. */
+    /** @brief Positions the command and stack zones vertically starting from base. */
     void positionCommandAndStackZones(const QPointF &base);
 private slots:
     void updateBoundingRect();

cockatrice/src/game_graphics/zones/command_zone.h

@@ -36,11 +36,11 @@ constexpr qreal COMMAND_ZONE_WIDTH = CardDimensions::WIDTH_F * 1.5;
  * @class CommandZone
  * @brief Graphics layer for the command zone in Commander format games.
  *
- * Always visible when enabled. Supports multiple cards using a zigzag
+ * Always visible when enabled. Uses the generic vertical stacking layout
- * horizontal stacking pattern: single cards display centered, multiple
+ * with bottom overflow enabled. Can be minimized via double-click (25% height,
- * cards alternate left-right with vertical overlap compression.
+ * or the tax-counter floor if higher).
- * Can be minimized to 25% height via double-click.
  *
+ * @see SelectZone::layoutCardsVertically for the stacking algorithm
  * @see CommandZoneLogic for card data management
  * @see CommanderTaxCounter for the tax counter overlay
  */
@@ -50,7 +50,7 @@ class CommandZone : public SelectZone
 private:
     static constexpr double MINIMIZED_HEIGHT_RATIO = 0.25;
     int zoneHeight;                       ///< Full height in pixels when expanded
-    bool minimized = false;               ///< Whether zone is at 25% height
+    bool minimized = false;               ///< Whether zone is collapsed (25% height, or the tax-counter floor)
     int minimumHeight = 0;                ///< Floor for minimized height (e.g. to fit tax counters)
     QList<AbstractCounter *> taxCounters; ///< Registered tax counter widgets
 
@@ -77,11 +77,12 @@ public:
     [[nodiscard]] QRectF boundingRect() const override;
     /** @brief Paints the zone background using the Commander theme brush. */
     void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) override;
-    /** @brief Repositions cards using zigzag horizontal stacking with overlap compression. */
+    /** @brief Repositions cards using vertical stacking with bottom overflow. */
     void reorganizeCards() override;
 
-    /** @brief Toggles between full and 25% minimized height. */
+    /** @brief Toggles between full and minimized height. */
     void toggleMinimized();
+    /** @brief Returns whether the zone is currently minimized. */
     [[nodiscard]] bool isMinimized() const;
     /** @brief Returns the current display height (full or minimized). */
     [[nodiscard]] qreal currentHeight() const;
@@ -100,9 +101,10 @@ public:
     void rearrangeTaxCounters();
 
 signals:
+    /** @brief Emitted when the zone's minimized state changes. */
     void minimizedChanged(bool isMinimized);
-    // Displayed height changed without a minimized-state change (e.g. tax counter toggled
+    /** @brief Emitted when display height changes without a minimized-state change (e.g. tax counter toggled while
-    // while minimized); lets neighbouring zones reposition.
+     * minimized). */
     void effectiveHeightChanged();
 
 protected:

doc/doxygen/extra-pages/developer_documentation/network/protocol/protocol_game_command.md

@@ -268,7 +268,10 @@ Client
 **Server:**
 - `Server_Player::cmdIncCounter`
 - Rejects if the game has not started or the player has conceded
-- Updates the counter value
+- Rejects tax counters when command zone is disabled (`RespContextError`)
+- Rejects inactive tax counters (`RespContextError`)
+- Rejects if counter doesn't exist (`RespNameNotFound`)
+- Updates the counter value (clamped to `[minValue, maxValue]`)
 - Emits `Event_SetCounter` only if the value changed
 
 **Client:**
@@ -285,7 +288,8 @@ Client
 **Server:**
 - `Server_Player::cmdCreateCounter`
 - Rejects if the game has not started or the player has conceded
-- Allocates a new counter ID
+- Rejects reserved tax counter names (`RespFunctionNotAllowed`)
+- Allocates a new counter ID (starting at `CounterIds::FirstUserId`)
 - Creates the counter
 - Emits `Event_CreateCounter`
 
@@ -302,7 +306,10 @@ Client
 **Server:**
 - `Server_Player::cmdSetCounter`
 - Rejects if the game has not started or the player has conceded
-- Updates the counter value
+- Rejects tax counters when command zone is disabled (`RespContextError`)
+- Rejects inactive tax counters (`RespContextError`)
+- Rejects if counter doesn't exist (`RespNameNotFound`)
+- Updates the counter value (clamped to `[minValue, maxValue]`)
 - Emits `Event_SetCounter` only if the value changed
 
 **Client:**
@@ -319,6 +326,8 @@ Client
 **Server:**
 - `Server_Player::cmdDelCounter`
 - Rejects if the game has not started or the player has conceded
+- Rejects tax counters (`RespFunctionNotAllowed`)
+- Rejects if counter doesn't exist (`RespNameNotFound`)
 - Deletes the counter
 - Emits `Event_DelCounter`
 
@@ -521,6 +530,26 @@ Client
 
 ---
 
+### `SET_COUNTER_ACTIVE` (1035)
+
+**Purpose:** Show or hide a reserved tax counter without deleting it.
+
+**Server:**
+- `Server_Player::cmdSetCounterActive`
+- Rejects if game not started (`RespGameNotStarted`)
+- Rejects if player has conceded (`RespContextError`)
+- Rejects for non-tax counters (`RespFunctionNotAllowed`)
+- Rejects if command zone is disabled (`RespContextError`)
+- Rejects if counter doesn't exist (`RespNameNotFound`)
+- Rejects deactivation when counter has non-zero value (`RespContextError`)
+- Emits `Event_SetCounterActive` only if the active state changed
+
+**Client:**
+- `PlayerEventHandler::eventSetCounterActive`
+- Updates the counter's active state in the UI
+
+---
+
 ## Notes
 
 - Game commands are handled by `Server_Player`, `Server_AbstractParticipant`, or `Server_Game`, depending on the command.

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

@@ -36,6 +36,9 @@ public:
      *
      * Reserved tax counters are server-managed and may never be deleted by a client.
      *
+     * @param gameStarted Whether the game has started
+     * @param playerConceded Whether the player has conceded
+     * @param counterId ID of the counter to delete
      * @param counter Counter with id counterId, or nullptr if the player has no such counter.
      * @return Response::RespOk if permitted, otherwise the error response for the client.
      */
@@ -48,6 +51,10 @@ public:
      * Only reserved tax counters can be toggled, and one holding a non-zero value must be reset
      * to zero before it can be deactivated.
      *
+     * @param gameStarted Whether the game has started
+     * @param playerConceded Whether the player has conceded
+     * @param commandZoneEnabled Whether command zone is enabled for this game
+     * @param counterId ID of the counter to toggle
      * @param counter Counter with id counterId, or nullptr if the player has no such counter.
      * @param requestedActive Active state the client asked for.
      * @return Response::RespOk if permitted, otherwise the error response for the client.
@@ -66,6 +73,10 @@ public:
      * inside a Commander game and only while active, so an inactive (hidden) tax counter can
      * never accumulate a value behind the scenes.
      *
+     * @param gameStarted Whether the game has started
+     * @param playerConceded Whether the player has conceded
+     * @param commandZoneEnabled Whether command zone is enabled for this game
+     * @param counterId ID of the counter to modify
      * @param counter Counter with id counterId, or nullptr if the player has no such counter.
      * @return Response::RespOk if permitted, otherwise the error response for the client.
      */

libcockatrice_protocol/libcockatrice/protocol/pb/game_commands.proto

@@ -175,6 +175,10 @@ message GameCommand {
         /// Server: Server_Player::cmdReverseTurn
         /// Client: reflected via subsequent turn events
         REVERSE_TURN = 1034;
+
+        /// Show or hide a reserved tax counter without deleting it.
+        /// Server: Server_Player::cmdSetCounterActive
+        /// Client: PlayerEventHandler::eventSetCounterActive
         SET_COUNTER_ACTIVE = 1035;
     }
 

libcockatrice_utility/libcockatrice/utility/counter_ids.h

@@ -1,6 +1,6 @@
 /**
  * @file counter_ids.h
- * @ingroup GameLogic
+ * @ingroup Core
  * @brief Shared counter IDs and names for system counters (e.g. commander tax).
  */
 
@@ -10,7 +10,9 @@
 #include <QString>
 
 /**
- * Shared counter IDs used by both client and server.
+ * @namespace CounterIds
+ * @brief Shared counter IDs used by both client and server.
+ *
  * Single source of truth: included directly by both sides, so they cannot drift.
  *
  * Reserved counter IDs for system counters:
@@ -28,17 +30,25 @@ constexpr int CommanderTax = 8; ///< Primary commander tax counter
 constexpr int PartnerTax = 9;   ///< Partner commander tax counter
 constexpr int FirstUserId = 10; ///< First ID available for user-created counters
 
+/** @brief Returns true if the given ID is a reserved tax counter. */
 inline bool isTaxCounter(int id)
 {
     return id == CommanderTax || id == PartnerTax;
 }
 } // namespace CounterIds
 
+/**
+ * @namespace CounterNames
+ * @brief Reserved counter names for server-managed tax counters.
+ *
+ * Used to reject user-created counters that would spoof system counters.
+ */
 namespace CounterNames
 {
 constexpr const char *CommanderTax = "commander_tax_counter";
 constexpr const char *PartnerTax = "partner_tax_counter";
 
+/** @brief Returns true if the given name is a reserved tax counter name. */
 inline bool isTaxCounter(const QString &name)
 {
     return name == CommanderTax || name == PartnerTax;