From d0df70e68f539b65e7d69da25cc9a4f228534493 Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Fri, 7 Aug 2026 22:45:56 -0400 Subject: [PATCH] Fix stale documentation and align doc comments with CONTRIBUTING guidelines --- cockatrice/src/game/board/counter_state.h | 5 ++- cockatrice/src/game/player/player_actions.h | 3 ++ .../src/game/player/player_event_handler.h | 2 +- cockatrice/src/game/player/player_logic.h | 1 + .../game_graphics/board/abstract_counter.h | 7 ++-- .../board/commander_tax_counter.h | 9 ++--- .../board/translate_counter_name.cpp | 6 ++-- .../src/game_graphics/player/menu/card_menu.h | 5 ++- .../player/player_graphics_item.h | 2 +- .../src/game_graphics/zones/command_zone.h | 20 ++++++----- .../network/protocol/protocol_game_command.md | 35 +++++++++++++++++-- .../server/remote/game/server_player.h | 11 ++++++ .../protocol/pb/game_commands.proto | 4 +++ .../libcockatrice/utility/counter_ids.h | 14 ++++++-- 14 files changed, 96 insertions(+), 28 deletions(-) diff --git a/cockatrice/src/game/board/counter_state.h b/cockatrice/src/game/board/counter_state.h index c171dbb5a..003a7f8e9 100644 --- a/cockatrice/src/game/board/counter_state.h +++ b/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 diff --git a/cockatrice/src/game/player/player_actions.h b/cockatrice/src/game/player/player_actions.h index 0f295a396..19b163d16 100644 --- a/cockatrice/src/game/player/player_actions.h +++ b/cockatrice/src/game/player/player_actions.h @@ -228,6 +228,7 @@ public slots: void cardMenuAction(QList 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); diff --git a/cockatrice/src/game/player/player_event_handler.h b/cockatrice/src/game/player/player_event_handler.h index 199b01503..b7ecf8996 100644 --- a/cockatrice/src/game/player/player_event_handler.h +++ b/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. diff --git a/cockatrice/src/game/player/player_logic.h b/cockatrice/src/game/player/player_logic.h index 345ecdd7e..3832e55c3 100644 --- a/cockatrice/src/game/player/player_logic.h +++ b/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: diff --git a/cockatrice/src/game_graphics/board/abstract_counter.h b/cockatrice/src/game_graphics/board/abstract_counter.h index 66e384ec7..f030a27f1 100644 --- a/cockatrice/src/game_graphics/board/abstract_counter.h +++ b/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 diff --git a/cockatrice/src/game_graphics/board/commander_tax_counter.h b/cockatrice/src/game_graphics/board/commander_tax_counter.h index 220113303..5ddfb4674 100644 --- a/cockatrice/src/game_graphics/board/commander_tax_counter.h +++ b/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 - * is manually adjusted by the player to track their commander tax. Values - * are clamped to >= 0. + * Displays the number of times the commander has been cast from the command + * zone. Can be adjusted manually via +1/-1 menu actions, or automatically + * 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: /** diff --git a/cockatrice/src/game_graphics/board/translate_counter_name.cpp b/cockatrice/src/game_graphics/board/translate_counter_name.cpp index 892eea426..05f017f95 100644 --- a/cockatrice/src/game_graphics/board/translate_counter_name.cpp +++ b/cockatrice/src/game_graphics/board/translate_counter_name.cpp @@ -1,5 +1,7 @@ #include "translate_counter_name.h" +#include + const QMap TranslateCounterName::translated = { {"life", QT_TRANSLATE_NOOP("TranslateCounterName", "Life")}, {"w", QT_TRANSLATE_NOOP("TranslateCounterName", "White")}, @@ -9,5 +11,5 @@ const QMap 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")}, - {"partner_tax_counter", QT_TRANSLATE_NOOP("TranslateCounterName", "Partner Tax")}}; + {CounterNames::CommanderTax, QT_TRANSLATE_NOOP("TranslateCounterName", "Commander Tax")}, + {CounterNames::PartnerTax, QT_TRANSLATE_NOOP("TranslateCounterName", "Partner Tax")}}; diff --git a/cockatrice/src/game_graphics/player/menu/card_menu.h b/cockatrice/src/game_graphics/player/menu/card_menu.h index c4eba1c46..47104a4a2 100644 --- a/cockatrice/src/game_graphics/player/menu/card_menu.h +++ b/cockatrice/src/game_graphics/player/menu/card_menu.h @@ -32,9 +32,8 @@ public: QMenu *mCardCounters; QAction *aPlay, *aPlayFacedown; - QAction * - aPlayAndIncreaseTax; ///< Plays card and increments the primary commander tax counter (CounterIds::CommanderTax) - QAction *aPlayAndIncreasePartnerTax; + /** @brief Play actions that also increment the corresponding tax counter. */ + QAction *aPlayAndIncreaseTax, *aPlayAndIncreasePartnerTax; QAction *aRevealToAll; QAction *aHide; QAction *aClone; diff --git a/cockatrice/src/game_graphics/player/player_graphics_item.h b/cockatrice/src/game_graphics/player/player_graphics_item.h index 702bb9534..3338f19b0 100644 --- a/cockatrice/src/game_graphics/player/player_graphics_item.h +++ b/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(); diff --git a/cockatrice/src/game_graphics/zones/command_zone.h b/cockatrice/src/game_graphics/zones/command_zone.h index 6706da466..4fe57ba56 100644 --- a/cockatrice/src/game_graphics/zones/command_zone.h +++ b/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 - * horizontal stacking pattern: single cards display centered, multiple - * cards alternate left-right with vertical overlap compression. - * Can be minimized to 25% height via double-click. + * Always visible when enabled. Uses the generic vertical stacking layout + * with bottom overflow enabled. Can be minimized via double-click (25% height, + * or the tax-counter floor if higher). * + * @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 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 - // while minimized); lets neighbouring zones reposition. + /** @brief Emitted when display height changes without a minimized-state change (e.g. tax counter toggled while + * minimized). */ void effectiveHeightChanged(); protected: diff --git a/doc/doxygen/extra-pages/developer_documentation/network/protocol/protocol_game_command.md b/doc/doxygen/extra-pages/developer_documentation/network/protocol/protocol_game_command.md index 18e31064a..699977fdb 100644 --- a/doc/doxygen/extra-pages/developer_documentation/network/protocol/protocol_game_command.md +++ b/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. diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.h b/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.h index 4c7320da8..21f288b1d 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.h +++ b/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. */ diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/game_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/game_commands.proto index c7588e774..9292230a3 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/game_commands.proto +++ b/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; } diff --git a/libcockatrice_utility/libcockatrice/utility/counter_ids.h b/libcockatrice_utility/libcockatrice/utility/counter_ids.h index 5261d8d12..874e69695 100644 --- a/libcockatrice_utility/libcockatrice/utility/counter_ids.h +++ b/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 /** - * 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;