From 4354191918e8b45fdb191e847915a041be1b055d Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Mon, 10 Aug 2026 20:04:51 -0400 Subject: [PATCH] Generalize tax counters to 5 sequential slots with server-enforced ordering --- .../src/client/settings/shortcuts_settings.h | 8 +- cockatrice/src/game/player/player_actions.cpp | 8 +- cockatrice/src/game/player/player_actions.h | 10 +- .../board/translate_counter_name.cpp | 7 +- .../game_graphics/player/menu/card_menu.cpp | 16 +- .../src/game_graphics/player/menu/card_menu.h | 2 +- .../player/menu/command_zone_menu.cpp | 193 +++++++++--------- .../player/menu/command_zone_menu.h | 26 +-- .../server/remote/game/server_player.cpp | 37 +++- .../server/remote/game/server_player.h | 6 +- .../libcockatrice/utility/counter_ids.h | 71 +++++-- .../counter_command_auth_test.cpp | 157 ++++++++------ .../new_counter_id_test.cpp | 16 +- .../setup_zones_command_zone_test.cpp | 46 +++-- 14 files changed, 352 insertions(+), 251 deletions(-) diff --git a/cockatrice/src/client/settings/shortcuts_settings.h b/cockatrice/src/client/settings/shortcuts_settings.h index 987e373c1..34a85078b 100644 --- a/cockatrice/src/client/settings/shortcuts_settings.h +++ b/cockatrice/src/client/settings/shortcuts_settings.h @@ -597,16 +597,16 @@ private: {"Player/aViewBottomCards", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Bottom Cards of Library"), parseSequenceString("Ctrl+Shift+W"), ShortcutGroup::View)}, - {"Player/aAddCommanderTax", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Increase Commander Tax"), + {"Player/aAddCommanderTax", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Increase 1st Tax"), parseSequenceString(""), ShortcutGroup::Player_Counters)}, - {"Player/aRemoveCommanderTax", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Decrease Commander Tax"), + {"Player/aRemoveCommanderTax", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Decrease 1st Tax"), parseSequenceString(""), ShortcutGroup::Player_Counters)}, - {"Player/aAddPartnerTax", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Increase Partner Tax"), + {"Player/aAddPartnerTax", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Increase 2nd Tax"), parseSequenceString(""), ShortcutGroup::Player_Counters)}, - {"Player/aRemovePartnerTax", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Decrease Partner Tax"), + {"Player/aRemovePartnerTax", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Decrease 2nd Tax"), parseSequenceString(""), ShortcutGroup::Player_Counters)}, {"Player/aCloseMostRecentZoneView", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Close Recent View"), diff --git a/cockatrice/src/game/player/player_actions.cpp b/cockatrice/src/game/player/player_actions.cpp index 07d6a1e5e..1954426ab 100644 --- a/cockatrice/src/game/player/player_actions.cpp +++ b/cockatrice/src/game/player/player_actions.cpp @@ -1674,14 +1674,14 @@ void PlayerActions::playSelectedCardsImpl( } } -void PlayerActions::actPlayAndIncreaseTax(QList selectedCards) +void PlayerActions::actPlayAndIncrease1stTax(QList selectedCards) { - playAndIncreaseTax(selectedCards, CounterIds::CommanderTax); + playAndIncreaseTax(selectedCards, CounterIds::TaxCounter1); } -void PlayerActions::actPlayAndIncreasePartnerTax(QList selectedCards) +void PlayerActions::actPlayAndIncrease2ndTax(QList selectedCards) { - playAndIncreaseTax(selectedCards, CounterIds::PartnerTax); + playAndIncreaseTax(selectedCards, CounterIds::TaxCounter2); } void PlayerActions::playAndIncreaseTax(QList selectedCards, int counterId) diff --git a/cockatrice/src/game/player/player_actions.h b/cockatrice/src/game/player/player_actions.h index 19b163d16..a722e42d3 100644 --- a/cockatrice/src/game/player/player_actions.h +++ b/cockatrice/src/game/player/player_actions.h @@ -127,10 +127,10 @@ public slots: void actPlay(QList selectedCards); void actPlayFacedown(QList selectedCards); - /** @brief Plays the selected card and increments the primary commander tax counter. */ - void actPlayAndIncreaseTax(QList selectedCards); - /** @brief Plays the selected card and increments the partner commander tax counter. */ - void actPlayAndIncreasePartnerTax(QList selectedCards); + /** @brief Plays the selected card and increments the 1st tax counter. */ + void actPlayAndIncrease1stTax(QList selectedCards); + /** @brief Plays the selected card and increments the 2nd tax counter. */ + void actPlayAndIncrease2ndTax(QList selectedCards); /** @brief Modifies a tax counter by delta if it is active. */ void actModifyTaxCounter(int counterId, int delta); /** @brief Toggles a tax counter's active state (only if inactive or value is 0). */ @@ -282,7 +282,7 @@ private: * @brief Plays the selected cards and, for each that came from the command zone and whose move * the server accepts, increments the given (active) tax counter by one. * @param selectedCards Cards to play - * @param counterId The tax counter to increment (CounterIds::CommanderTax or PartnerTax) + * @param counterId The tax counter to increment (CounterIds::TaxCounter1 through TaxCounter5) */ void playAndIncreaseTax(QList selectedCards, int counterId); diff --git a/cockatrice/src/game_graphics/board/translate_counter_name.cpp b/cockatrice/src/game_graphics/board/translate_counter_name.cpp index 05f017f95..b0fa84e72 100644 --- a/cockatrice/src/game_graphics/board/translate_counter_name.cpp +++ b/cockatrice/src/game_graphics/board/translate_counter_name.cpp @@ -11,5 +11,8 @@ const QMap TranslateCounterName::translated = { {"g", QT_TRANSLATE_NOOP("TranslateCounterName", "Green")}, {"x", QT_TRANSLATE_NOOP("TranslateCounterName", "Colorless")}, {"storm", QT_TRANSLATE_NOOP("TranslateCounterName", "Other")}, - {CounterNames::CommanderTax, QT_TRANSLATE_NOOP("TranslateCounterName", "Commander Tax")}, - {CounterNames::PartnerTax, QT_TRANSLATE_NOOP("TranslateCounterName", "Partner Tax")}}; + {CounterNames::TaxCounter1, QT_TRANSLATE_NOOP("TranslateCounterName", "1st Tax")}, + {CounterNames::TaxCounter2, QT_TRANSLATE_NOOP("TranslateCounterName", "2nd Tax")}, + {CounterNames::TaxCounter3, QT_TRANSLATE_NOOP("TranslateCounterName", "3rd Tax")}, + {CounterNames::TaxCounter4, QT_TRANSLATE_NOOP("TranslateCounterName", "4th Tax")}, + {CounterNames::TaxCounter5, QT_TRANSLATE_NOOP("TranslateCounterName", "5th Tax")}}; diff --git a/cockatrice/src/game_graphics/player/menu/card_menu.cpp b/cockatrice/src/game_graphics/player/menu/card_menu.cpp index 7622518e4..3cf026ab2 100644 --- a/cockatrice/src/game_graphics/player/menu/card_menu.cpp +++ b/cockatrice/src/game_graphics/player/menu/card_menu.cpp @@ -84,8 +84,8 @@ CardMenu::CardMenu(PlayerGraphicsItem *_player, const CardItem *_card, bool _sho aUnattach = makeAction(this, [actions, sel]() { actions->actUnattach(sel()); }); aSetAnnotation = makeAction(this, [actions, sel]() { actions->actRequestSetAnnotationDialog(sel()); }); aPlay = makeAction(this, [actions, sel]() { actions->actPlay(sel()); }); - aPlayAndIncreaseTax = makeAction(this, [actions, sel]() { actions->actPlayAndIncreaseTax(sel()); }); - aPlayAndIncreasePartnerTax = makeAction(this, [actions, sel]() { actions->actPlayAndIncreasePartnerTax(sel()); }); + aPlayAndIncrease1stTax = makeAction(this, [actions, sel]() { actions->actPlayAndIncrease1stTax(sel()); }); + aPlayAndIncrease2ndTax = makeAction(this, [actions, sel]() { actions->actPlayAndIncrease2ndTax(sel()); }); aPlayFacedown = makeAction(this, [actions, sel]() { actions->actPlayFacedown(sel()); }); aHide = makeAction(this, [actions, sel]() { actions->actHide(sel()); }); aReduceLifeByPower = makeAction(this, [actions, sel]() { actions->actReduceLifeByPower(sel()); }); @@ -170,12 +170,12 @@ CardMenu::CardMenu(PlayerGraphicsItem *_player, const CardItem *_card, bool _sho // bumping one commander's tax counter once per command-zone card. const bool singleSelection = gameScene->selectedCards().size() <= 1; - if (singleSelection && player->getTaxCounterIfActive(CounterIds::CommanderTax)) { - addAction(aPlayAndIncreaseTax); + if (singleSelection && player->getTaxCounterIfActive(CounterIds::TaxCounter1)) { + addAction(aPlayAndIncrease1stTax); } - if (singleSelection && player->getTaxCounterIfActive(CounterIds::PartnerTax)) { - addAction(aPlayAndIncreasePartnerTax); + if (singleSelection && player->getTaxCounterIfActive(CounterIds::TaxCounter2)) { + addAction(aPlayAndIncrease2ndTax); } // No reveal submenu - command zone is public @@ -524,8 +524,8 @@ void CardMenu::retranslateUi() aPlay->setText(tr("&Play")); aHide->setText(tr("&Hide")); aPlayFacedown->setText(tr("Play &Face Down")); - aPlayAndIncreaseTax->setText(tr("Play and &Increase Commander Tax")); - aPlayAndIncreasePartnerTax->setText(tr("Play and Increase &Partner Tax")); + aPlayAndIncrease1stTax->setText(tr("Play and &Increase 1st Tax")); + aPlayAndIncrease2ndTax->setText(tr("Play and Increase &2nd Tax")); aRevealToAll->setText(tr("&All players")); //: Turn sideways or back again aTap->setText(tr("&Tap / Untap")); diff --git a/cockatrice/src/game_graphics/player/menu/card_menu.h b/cockatrice/src/game_graphics/player/menu/card_menu.h index 47104a4a2..a645cb03c 100644 --- a/cockatrice/src/game_graphics/player/menu/card_menu.h +++ b/cockatrice/src/game_graphics/player/menu/card_menu.h @@ -33,7 +33,7 @@ public: QAction *aPlay, *aPlayFacedown; /** @brief Play actions that also increment the corresponding tax counter. */ - QAction *aPlayAndIncreaseTax, *aPlayAndIncreasePartnerTax; + QAction *aPlayAndIncrease1stTax, *aPlayAndIncrease2ndTax; QAction *aRevealToAll; QAction *aHide; QAction *aClone; diff --git a/cockatrice/src/game_graphics/player/menu/command_zone_menu.cpp b/cockatrice/src/game_graphics/player/menu/command_zone_menu.cpp index c1f537b7d..d6e73b26f 100644 --- a/cockatrice/src/game_graphics/player/menu/command_zone_menu.cpp +++ b/cockatrice/src/game_graphics/player/menu/command_zone_menu.cpp @@ -14,66 +14,45 @@ CommandZoneMenu::CommandZoneMenu(PlayerGraphicsItem *_player, QMenu *playerMenu) : QMenu(playerMenu), player(_player) { - incTaxShortcutKey = QStringLiteral("Player/aAddCommanderTax"); - decTaxShortcutKey = QStringLiteral("Player/aRemoveCommanderTax"); - incPartnerTaxShortcutKey = QStringLiteral("Player/aAddPartnerTax"); - decPartnerTaxShortcutKey = QStringLiteral("Player/aRemovePartnerTax"); + // Shortcuts only for first two tax counters (matching legacy behavior) + incTax1ShortcutKey = QStringLiteral("Player/aAddCommanderTax"); + decTax1ShortcutKey = QStringLiteral("Player/aRemoveCommanderTax"); + incTax2ShortcutKey = QStringLiteral("Player/aAddPartnerTax"); + decTax2ShortcutKey = QStringLiteral("Player/aRemovePartnerTax"); PlayerLogic *logic = player->getLogic(); if (logic && logic->getPlayerInfo()->getLocalOrJudge()) { - aIncreaseCommanderTax = new QAction(this); - connect(aIncreaseCommanderTax, &QAction::triggered, this, [this]() { - if (auto *logic = player->getLogic()) { - logic->getPlayerActions()->actModifyTaxCounter(CounterIds::CommanderTax, 1); - } - }); - addAction(aIncreaseCommanderTax); + for (int i = 0; i < TaxCounterCount; ++i) { + int counterId = CounterIds::taxCounterIdFromIndex(i); - aDecreaseCommanderTax = new QAction(this); - connect(aDecreaseCommanderTax, &QAction::triggered, this, [this]() { - if (auto *logic = player->getLogic()) { - logic->getPlayerActions()->actModifyTaxCounter(CounterIds::CommanderTax, -1); - } - }); - addAction(aDecreaseCommanderTax); + aIncreaseTax[i] = new QAction(this); + connect(aIncreaseTax[i], &QAction::triggered, this, [this, counterId]() { + if (auto *l = player->getLogic()) { + l->getPlayerActions()->actModifyTaxCounter(counterId, 1); + } + }); + addAction(aIncreaseTax[i]); - addSeparator(); + aDecreaseTax[i] = new QAction(this); + connect(aDecreaseTax[i], &QAction::triggered, this, [this, counterId]() { + if (auto *l = player->getLogic()) { + l->getPlayerActions()->actModifyTaxCounter(counterId, -1); + } + }); + addAction(aDecreaseTax[i]); - aIncreasePartnerTax = new QAction(this); - connect(aIncreasePartnerTax, &QAction::triggered, this, [this]() { - if (auto *logic = player->getLogic()) { - logic->getPlayerActions()->actModifyTaxCounter(CounterIds::PartnerTax, 1); - } - }); - addAction(aIncreasePartnerTax); + addSeparator(); - aDecreasePartnerTax = new QAction(this); - connect(aDecreasePartnerTax, &QAction::triggered, this, [this]() { - if (auto *logic = player->getLogic()) { - logic->getPlayerActions()->actModifyTaxCounter(CounterIds::PartnerTax, -1); - } - }); - addAction(aDecreasePartnerTax); + aToggleTax[i] = new QAction(this); + connect(aToggleTax[i], &QAction::triggered, this, [this, counterId]() { + if (auto *l = player->getLogic()) { + l->getPlayerActions()->actToggleTaxCounter(counterId); + } + }); + addAction(aToggleTax[i]); - addSeparator(); - - aToggleCommanderTaxCounter = new QAction(this); - connect(aToggleCommanderTaxCounter, &QAction::triggered, this, [this]() { - if (auto *logic = player->getLogic()) { - logic->getPlayerActions()->actToggleTaxCounter(CounterIds::CommanderTax); - } - }); - addAction(aToggleCommanderTaxCounter); - - aTogglePartnerTaxCounter = new QAction(this); - connect(aTogglePartnerTaxCounter, &QAction::triggered, this, [this]() { - if (auto *logic = player->getLogic()) { - logic->getPlayerActions()->actToggleTaxCounter(CounterIds::PartnerTax); - } - }); - addAction(aTogglePartnerTaxCounter); - - addSeparator(); + addSeparator(); + } aToggleMinimized = new QAction(this); connect(aToggleMinimized, &QAction::triggered, this, &CommandZoneMenu::actToggleMinimized); @@ -88,19 +67,19 @@ CommandZoneMenu::CommandZoneMenu(PlayerGraphicsItem *_player, QMenu *playerMenu) void CommandZoneMenu::retranslateUi() { setTitle(tr("Co&mmander")); - if (aIncreaseCommanderTax) { - aIncreaseCommanderTax->setText(tr("&Increase Commander Tax (+1)")); + + static const char *ordinals[] = {"1st", "2nd", "3rd", "4th", "5th"}; + + for (int i = 0; i < TaxCounterCount; ++i) { + if (aIncreaseTax[i]) { + aIncreaseTax[i]->setText(tr("&Increase %1 Tax (+1)").arg(ordinals[i])); + } + if (aDecreaseTax[i]) { + aDecreaseTax[i]->setText(tr("&Decrease %1 Tax (-1)").arg(ordinals[i])); + } + // Toggle action labels are derived dynamically in updateTaxCounterActionStates() } - if (aDecreaseCommanderTax) { - aDecreaseCommanderTax->setText(tr("&Decrease Commander Tax (-1)")); - } - if (aIncreasePartnerTax) { - aIncreasePartnerTax->setText(tr("Increase &Partner Tax (+1)")); - } - if (aDecreasePartnerTax) { - aDecreasePartnerTax->setText(tr("Decrease P&artner Tax (-1)")); - } - // Toggle action labels are derived dynamically in updateTaxCounterActionStates() + if (aToggleMinimized) { aToggleMinimized->setText(tr("&Minimize")); } @@ -116,29 +95,43 @@ void CommandZoneMenu::actToggleMinimized() void CommandZoneMenu::updateTaxCounterActionStates() { - AbstractCounter *cmdTax = player->getTaxCounterIfActive(CounterIds::CommanderTax); - AbstractCounter *partnerTax = player->getTaxCounterIfActive(CounterIds::PartnerTax); + static const char *ordinals[] = {"1st", "2nd", "3rd", "4th", "5th"}; - if (aIncreaseCommanderTax) { - aIncreaseCommanderTax->setVisible(cmdTax && cmdTax->getValue() < MAX_COUNTER_VALUE); - } - if (aDecreaseCommanderTax) { - aDecreaseCommanderTax->setVisible(cmdTax && cmdTax->getValue() > 0); - } - if (aToggleCommanderTaxCounter) { - aToggleCommanderTaxCounter->setText(cmdTax ? tr("&Remove Commander Tax") : tr("&Add Commander Tax")); - aToggleCommanderTaxCounter->setVisible(!cmdTax || (cmdTax->getValue() == 0 && !partnerTax)); + // Collect all tax counter states + std::array taxCounters{}; + for (int i = 0; i < TaxCounterCount; ++i) { + taxCounters[i] = player->getTaxCounterIfActive(CounterIds::taxCounterIdFromIndex(i)); } - if (aIncreasePartnerTax) { - aIncreasePartnerTax->setVisible(partnerTax && partnerTax->getValue() < MAX_COUNTER_VALUE); + // Find highest active tax counter index + int highestActive = -1; + for (int i = TaxCounterCount - 1; i >= 0; --i) { + if (taxCounters[i]) { + highestActive = i; + break; + } } - if (aDecreasePartnerTax) { - aDecreasePartnerTax->setVisible(partnerTax && partnerTax->getValue() > 0); - } - if (aTogglePartnerTaxCounter) { - aTogglePartnerTaxCounter->setText(partnerTax ? tr("R&emove Partner Tax") : tr("&Add Partner Tax")); - aTogglePartnerTaxCounter->setVisible(!partnerTax || partnerTax->getValue() == 0); + + for (int i = 0; i < TaxCounterCount; ++i) { + AbstractCounter *counter = taxCounters[i]; + + if (aIncreaseTax[i]) { + aIncreaseTax[i]->setVisible(counter && counter->getValue() < MAX_COUNTER_VALUE); + } + if (aDecreaseTax[i]) { + aDecreaseTax[i]->setVisible(counter && counter->getValue() > 0); + } + if (aToggleTax[i]) { + aToggleTax[i]->setText(counter ? tr("&Remove %1 Tax").arg(ordinals[i]) + : tr("&Add %1 Tax").arg(ordinals[i])); + + // Toggle visible if: + // - Counter doesn't exist and previous counter is active (can add next in sequence) + // - Counter exists with value 0 and is the highest active (can remove last in sequence) + bool canAdd = !counter && (i == 0 || taxCounters[i - 1]); + bool canRemove = counter && counter->getValue() == 0 && i == highestActive; + aToggleTax[i]->setVisible(canAdd || canRemove); + } } if (aToggleMinimized) { @@ -151,32 +144,34 @@ void CommandZoneMenu::setShortcutsActive() { ShortcutsSettings &shortcuts = SettingsCache::instance().shortcuts(); - if (aIncreaseCommanderTax) { - aIncreaseCommanderTax->setShortcuts(shortcuts.getShortcut(incTaxShortcutKey)); + // Only first two tax counters have shortcuts + if (aIncreaseTax[0]) { + aIncreaseTax[0]->setShortcuts(shortcuts.getShortcut(incTax1ShortcutKey)); } - if (aDecreaseCommanderTax) { - aDecreaseCommanderTax->setShortcuts(shortcuts.getShortcut(decTaxShortcutKey)); + if (aDecreaseTax[0]) { + aDecreaseTax[0]->setShortcuts(shortcuts.getShortcut(decTax1ShortcutKey)); } - if (aIncreasePartnerTax) { - aIncreasePartnerTax->setShortcuts(shortcuts.getShortcut(incPartnerTaxShortcutKey)); + if (aIncreaseTax[1]) { + aIncreaseTax[1]->setShortcuts(shortcuts.getShortcut(incTax2ShortcutKey)); } - if (aDecreasePartnerTax) { - aDecreasePartnerTax->setShortcuts(shortcuts.getShortcut(decPartnerTaxShortcutKey)); + if (aDecreaseTax[1]) { + aDecreaseTax[1]->setShortcuts(shortcuts.getShortcut(decTax2ShortcutKey)); } } void CommandZoneMenu::setShortcutsInactive() { - if (aIncreaseCommanderTax) { - aIncreaseCommanderTax->setShortcut(QKeySequence()); + // Only first two tax counters have shortcuts + if (aIncreaseTax[0]) { + aIncreaseTax[0]->setShortcut(QKeySequence()); } - if (aDecreaseCommanderTax) { - aDecreaseCommanderTax->setShortcut(QKeySequence()); + if (aDecreaseTax[0]) { + aDecreaseTax[0]->setShortcut(QKeySequence()); } - if (aIncreasePartnerTax) { - aIncreasePartnerTax->setShortcut(QKeySequence()); + if (aIncreaseTax[1]) { + aIncreaseTax[1]->setShortcut(QKeySequence()); } - if (aDecreasePartnerTax) { - aDecreasePartnerTax->setShortcut(QKeySequence()); + if (aDecreaseTax[1]) { + aDecreaseTax[1]->setShortcut(QKeySequence()); } } diff --git a/cockatrice/src/game_graphics/player/menu/command_zone_menu.h b/cockatrice/src/game_graphics/player/menu/command_zone_menu.h index 17f4fd275..de2fabdb6 100644 --- a/cockatrice/src/game_graphics/player/menu/command_zone_menu.h +++ b/cockatrice/src/game_graphics/player/menu/command_zone_menu.h @@ -10,6 +10,8 @@ #include "abstract_player_component.h" #include +#include +#include class PlayerGraphicsItem; @@ -18,7 +20,7 @@ class PlayerGraphicsItem; * @brief Context menu for the command zone. * * Appears when right-clicking on the command zone. Provides actions for - * adjusting the commander tax counter and toggling minimized state. + * adjusting tax counters (up to 5) and toggling minimized state. * * @see PlayerMenu * @see CommandZone @@ -34,13 +36,12 @@ public: void setShortcutsInactive() override; private: - QAction *aIncreaseCommanderTax = nullptr; ///< Increments the primary commander tax counter - QAction *aDecreaseCommanderTax = nullptr; ///< Decrements the primary commander tax counter - QAction *aToggleCommanderTaxCounter = nullptr; ///< Toggles primary commander tax counter visibility - QAction *aIncreasePartnerTax = nullptr; ///< Increments the partner commander tax counter - QAction *aDecreasePartnerTax = nullptr; ///< Decrements the partner commander tax counter - QAction *aTogglePartnerTaxCounter = nullptr; ///< Toggles partner commander tax counter visibility - QAction *aToggleMinimized = nullptr; ///< Toggles command zone minimized state + static constexpr int TaxCounterCount = CounterIds::TaxCounterCount; + + std::array aIncreaseTax{}; + std::array aDecreaseTax{}; + std::array aToggleTax{}; + QAction *aToggleMinimized = nullptr; public slots: void updateTaxCounterActionStates(); @@ -51,10 +52,11 @@ private slots: private: PlayerGraphicsItem *player; - QString incTaxShortcutKey; - QString decTaxShortcutKey; - QString incPartnerTaxShortcutKey; - QString decPartnerTaxShortcutKey; + // Shortcuts only for first two tax counters + QString incTax1ShortcutKey; + QString decTax1ShortcutKey; + QString incTax2ShortcutKey; + QString decTax2ShortcutKey; }; #endif // COCKATRICE_COMMAND_ZONE_MENU_H diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.cpp index 42d382733..01434ee40 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.cpp @@ -109,12 +109,15 @@ void Server_Player::setupZones() // Command zone for Commander format if (game->getEnableCommandZone()) { addZone(new Server_CardZone(this, ZoneNames::COMMAND, false, ServerInfo_Zone::PublicZone)); - addCounter(new Server_Counter(CounterIds::CommanderTax, CounterNames::CommanderTax, makeColor(128, 128, 128), - 20, 0, 0, MAX_COUNTER_VALUE)); - auto *partnerTax = new Server_Counter(CounterIds::PartnerTax, CounterNames::PartnerTax, - makeColor(128, 128, 128), 20, 0, 0, MAX_COUNTER_VALUE); - (void)partnerTax->setActive(false); - addCounter(partnerTax); + for (int i = 0; i < CounterIds::TaxCounterCount; ++i) { + int id = CounterIds::taxCounterIdFromIndex(i); + const char *name = CounterNames::forId(id); + auto *counter = new Server_Counter(id, name, makeColor(128, 128, 128), 20, 0, 0, MAX_COUNTER_VALUE); + if (i > 0) { + (void)counter->setActive(false); + } + addCounter(counter); + } } // ------------------------------------------------------------------ @@ -607,7 +610,9 @@ Response::ResponseCode Server_Player::evaluateSetCounterActive(bool gameStarted, bool commandZoneEnabled, int counterId, const Server_Counter *counter, - bool requestedActive) + bool requestedActive, + const Server_Counter *predecessorCounter, + const Server_Counter *successorCounter) { if (!gameStarted) { return Response::RespGameNotStarted; @@ -628,6 +633,14 @@ Response::ResponseCode Server_Player::evaluateSetCounterActive(bool gameStarted, if (!requestedActive && counter->getCount() != 0) { return Response::RespContextError; } + // Enforce ordering: can only activate if predecessor is active + if (requestedActive && predecessorCounter && !predecessorCounter->isActive()) { + return Response::RespContextError; + } + // Enforce ordering: can only deactivate if successor is inactive + if (!requestedActive && successorCounter && successorCounter->isActive()) { + return Response::RespContextError; + } return Response::RespOk; } @@ -638,8 +651,14 @@ Response::ResponseCode Server_Player::cmdSetCounterActive(const Command_SetCount const int counterId = cmd.counter_id(); Server_Counter *c = counters.value(counterId, nullptr); - const Response::ResponseCode authResult = evaluateSetCounterActive( - game->getGameStarted(), conceded, game->getEnableCommandZone(), counterId, c, cmd.active()); + int predecessorId = CounterIds::taxCounterIdFromIndex(CounterIds::taxCounterIndex(counterId) - 1); + int successorId = CounterIds::taxCounterIdFromIndex(CounterIds::taxCounterIndex(counterId) + 1); + Server_Counter *predecessor = predecessorId >= 0 ? counters.value(predecessorId, nullptr) : nullptr; + Server_Counter *successor = successorId >= 0 ? counters.value(successorId, nullptr) : nullptr; + + const Response::ResponseCode authResult = + evaluateSetCounterActive(game->getGameStarted(), conceded, game->getEnableCommandZone(), counterId, c, + cmd.active(), predecessor, successor); if (authResult != Response::RespOk) { return authResult; } 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 2b2be7fe1..2513896fa 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.h @@ -57,6 +57,8 @@ public: * @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. + * @param predecessorCounter Tax counter that must be active before this one can be activated (nullptr if none). + * @param successorCounter Tax counter that must be inactive before this one can be deactivated (nullptr if none). * @return Response::RespOk if permitted, otherwise the error response for the client. */ static Response::ResponseCode evaluateSetCounterActive(bool gameStarted, @@ -64,7 +66,9 @@ public: bool commandZoneEnabled, int counterId, const Server_Counter *counter, - bool requestedActive); + bool requestedActive, + const Server_Counter *predecessorCounter, + const Server_Counter *successorCounter); /** * @brief Decide whether a client may change a counter's value. diff --git a/libcockatrice_utility/libcockatrice/utility/counter_ids.h b/libcockatrice_utility/libcockatrice/utility/counter_ids.h index 874e69695..a5c03f314 100644 --- a/libcockatrice_utility/libcockatrice/utility/counter_ids.h +++ b/libcockatrice_utility/libcockatrice/utility/counter_ids.h @@ -1,7 +1,7 @@ /** * @file counter_ids.h * @ingroup Core - * @brief Shared counter IDs and names for system counters (e.g. commander tax). + * @brief Shared counter IDs and names for system counters (e.g. tax counters). */ #ifndef COCKATRICE_COUNTER_IDS_H @@ -16,24 +16,48 @@ * Single source of truth: included directly by both sides, so they cannot drift. * * Reserved counter IDs for system counters: - * IDs 0-7: Standard player counters (life, mana colors, storm) - * IDs 8-9: Commander tax counters - * IDs 10+: Available for user-created counters (FirstUserId) + * IDs 0-7: Standard player counters (life, mana colors, storm) + * IDs 8-12: Tax counters (1st through 5th) + * IDs 13+: Available for user-created counters (FirstUserId) * * newCounterId() always returns >= FirstUserId to prevent user counters * from colliding with reserved IDs, even in non-Commander games. */ namespace CounterIds { -// Tax counters store a cast count (times cast from command zone). -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 +constexpr int TaxCounter1 = 8; ///< 1st tax counter +constexpr int TaxCounter2 = 9; ///< 2nd tax counter +constexpr int TaxCounter3 = 10; ///< 3rd tax counter +constexpr int TaxCounter4 = 11; ///< 4th tax counter +constexpr int TaxCounter5 = 12; ///< 5th tax counter +constexpr int FirstUserId = 13; ///< First ID available for user-created counters + +constexpr int FirstTaxCounterId = TaxCounter1; +constexpr int LastTaxCounterId = TaxCounter5; +constexpr int TaxCounterCount = LastTaxCounterId - FirstTaxCounterId + 1; /** @brief Returns true if the given ID is a reserved tax counter. */ inline bool isTaxCounter(int id) { - return id == CommanderTax || id == PartnerTax; + return id >= FirstTaxCounterId && id <= LastTaxCounterId; +} + +/** @brief Returns the tax counter index (0-based) for display, or -1 if not a tax counter. */ +inline int taxCounterIndex(int id) +{ + if (!isTaxCounter(id)) { + return -1; + } + return id - FirstTaxCounterId; +} + +/** @brief Returns the tax counter ID for the given 0-based index, or -1 if out of range. */ +inline int taxCounterIdFromIndex(int index) +{ + if (index < 0 || index >= TaxCounterCount) { + return -1; + } + return FirstTaxCounterId + index; } } // namespace CounterIds @@ -45,13 +69,36 @@ inline bool isTaxCounter(int id) */ namespace CounterNames { -constexpr const char *CommanderTax = "commander_tax_counter"; -constexpr const char *PartnerTax = "partner_tax_counter"; +constexpr const char *TaxCounter1 = "1st_tax_counter"; +constexpr const char *TaxCounter2 = "2nd_tax_counter"; +constexpr const char *TaxCounter3 = "3rd_tax_counter"; +constexpr const char *TaxCounter4 = "4th_tax_counter"; +constexpr const char *TaxCounter5 = "5th_tax_counter"; + +/** @brief Returns the name for the given tax counter ID, or nullptr if not a tax counter. */ +inline const char *forId(int id) +{ + switch (id) { + case CounterIds::TaxCounter1: + return TaxCounter1; + case CounterIds::TaxCounter2: + return TaxCounter2; + case CounterIds::TaxCounter3: + return TaxCounter3; + case CounterIds::TaxCounter4: + return TaxCounter4; + case CounterIds::TaxCounter5: + return TaxCounter5; + default: + return nullptr; + } +} /** @brief Returns true if the given name is a reserved tax counter name. */ inline bool isTaxCounter(const QString &name) { - return name == CommanderTax || name == PartnerTax; + return name == TaxCounter1 || name == TaxCounter2 || name == TaxCounter3 || name == TaxCounter4 || + name == TaxCounter5; } } // namespace CounterNames diff --git a/tests/command_zone_tests/counter_command_auth_test.cpp b/tests/command_zone_tests/counter_command_auth_test.cpp index 2b76509dd..92c0ee45f 100644 --- a/tests/command_zone_tests/counter_command_auth_test.cpp +++ b/tests/command_zone_tests/counter_command_auth_test.cpp @@ -18,9 +18,11 @@ namespace { constexpr int UserCounterId = CounterIds::FirstUserId; -Server_Counter makeCounter(int id, int count) +Server_Counter makeCounter(int id, int count, bool active = true) { - return Server_Counter(id, "c", color(), 20, count); + Server_Counter c(id, "c", color(), 20, count); + (void)c.setActive(active); + return c; } } // namespace @@ -41,12 +43,12 @@ TEST(EvaluateDelCounter, RejectsWhenPlayerConceded) TEST(EvaluateDelCounter, RejectsTaxCounters) { - Server_Counter commander = makeCounter(CounterIds::CommanderTax, 0); - EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, CounterIds::CommanderTax, &commander), + Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0); + EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, CounterIds::TaxCounter1, &tax1), Response::RespFunctionNotAllowed); - Server_Counter partner = makeCounter(CounterIds::PartnerTax, 0); - EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, CounterIds::PartnerTax, &partner), + Server_Counter tax2 = makeCounter(CounterIds::TaxCounter2, 0); + EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, CounterIds::TaxCounter2, &tax2), Response::RespFunctionNotAllowed); } @@ -63,125 +65,146 @@ TEST(EvaluateDelCounter, AllowsDeletingUserCounter) TEST(EvaluateDelCounter, GameNotStartedTakesPrecedenceOverTaxGuard) { - Server_Counter commander = makeCounter(CounterIds::CommanderTax, 0); - EXPECT_EQ(Server_Player::evaluateDelCounter(false, false, CounterIds::CommanderTax, &commander), + Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0); + EXPECT_EQ(Server_Player::evaluateDelCounter(false, false, CounterIds::TaxCounter1, &tax1), Response::RespGameNotStarted); } TEST(EvaluateSetCounterActive, RejectsWhenGameNotStarted) { - Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); + Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0); EXPECT_EQ(Server_Player::evaluateSetCounterActive(/*gameStarted=*/false, /*playerConceded=*/false, - /*commandZoneEnabled=*/true, CounterIds::CommanderTax, &counter, - /*requestedActive=*/true), + /*commandZoneEnabled=*/true, CounterIds::TaxCounter1, &counter, + /*requestedActive=*/true, nullptr, nullptr), Response::RespGameNotStarted); } TEST(EvaluateSetCounterActive, RejectsWhenPlayerConceded) { - Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); - EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, /*playerConceded=*/true, true, CounterIds::CommanderTax, - &counter, true), + Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0); + EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, /*playerConceded=*/true, true, CounterIds::TaxCounter1, + &counter, true, nullptr, nullptr), Response::RespContextError); } TEST(EvaluateSetCounterActive, RejectsNonTaxCounter) { Server_Counter counter = makeCounter(UserCounterId, 0); - EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, UserCounterId, &counter, true), - Response::RespFunctionNotAllowed); + EXPECT_EQ( + Server_Player::evaluateSetCounterActive(true, false, true, UserCounterId, &counter, true, nullptr, nullptr), + Response::RespFunctionNotAllowed); } TEST(EvaluateSetCounterActive, RejectsWhenCommandZoneDisabled) { - Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); + Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0); EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, /*commandZoneEnabled=*/false, - CounterIds::CommanderTax, &counter, true), + CounterIds::TaxCounter1, &counter, true, nullptr, nullptr), Response::RespContextError); } TEST(EvaluateSetCounterActive, RejectsMissingCounter) { - EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::CommanderTax, nullptr, true), + EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, nullptr, true, + nullptr, nullptr), Response::RespNameNotFound); } TEST(EvaluateSetCounterActive, RejectsDisablingWhenTaxAccumulated) { - Server_Counter counter = makeCounter(CounterIds::CommanderTax, 3); - EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::CommanderTax, &counter, - /*requestedActive=*/false), + Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 3); + EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, &counter, + /*requestedActive=*/false, nullptr, nullptr), Response::RespContextError); } TEST(EvaluateSetCounterActive, AllowsEnablingWithAccumulatedTax) { - Server_Counter counter = makeCounter(CounterIds::CommanderTax, 3); - EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::CommanderTax, &counter, - /*requestedActive=*/true), + Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 3); + EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, &counter, + /*requestedActive=*/true, nullptr, nullptr), Response::RespOk); } TEST(EvaluateSetCounterActive, AllowsDisablingWhenCounterIsZero) { - Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); - EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::CommanderTax, &counter, - /*requestedActive=*/false), + Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0); + EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, &counter, + /*requestedActive=*/false, nullptr, nullptr), Response::RespOk); } -TEST(EvaluateSetCounterActive, AllowsEnablingPartnerTax) +TEST(EvaluateSetCounterActive, AllowsEnabling2ndTaxWhen1stIsActive) { - Server_Counter counter = makeCounter(CounterIds::PartnerTax, 0); - EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::PartnerTax, &counter, - /*requestedActive=*/true), + Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0, true); + Server_Counter tax2 = makeCounter(CounterIds::TaxCounter2, 0, false); + EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter2, &tax2, + /*requestedActive=*/true, &tax1, nullptr), Response::RespOk); } -TEST(EvaluateSetCounterActive, RejectsDisablingPartnerTaxWhenAccumulated) +TEST(EvaluateSetCounterActive, RejectsEnabling2ndTaxWhen1stIsInactive) { - Server_Counter counter = makeCounter(CounterIds::PartnerTax, 2); - EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::PartnerTax, &counter, - /*requestedActive=*/false), + Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0, false); + Server_Counter tax2 = makeCounter(CounterIds::TaxCounter2, 0, false); + EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter2, &tax2, + /*requestedActive=*/true, &tax1, nullptr), + Response::RespContextError); +} + +TEST(EvaluateSetCounterActive, AllowsDisabling1stTaxWhen2ndIsInactive) +{ + Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0, true); + Server_Counter tax2 = makeCounter(CounterIds::TaxCounter2, 0, false); + EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, &tax1, + /*requestedActive=*/false, nullptr, &tax2), + Response::RespOk); +} + +TEST(EvaluateSetCounterActive, RejectsDisabling1stTaxWhen2ndIsActive) +{ + Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0, true); + Server_Counter tax2 = makeCounter(CounterIds::TaxCounter2, 0, true); + EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, &tax1, + /*requestedActive=*/false, nullptr, &tax2), Response::RespContextError); } TEST(EvaluateCreateCounter, RejectsWhenGameNotStarted) { - EXPECT_EQ(Server_Player::evaluateCreateCounter(/*gameStarted=*/false, /*playerConceded=*/false, "mycounter"), + EXPECT_EQ(Server_Player::evaluateCreateCounter(/*gameStarted=*/false, /*playerConceded=*/false, "test"), Response::RespGameNotStarted); } TEST(EvaluateCreateCounter, RejectsWhenPlayerConceded) { - EXPECT_EQ(Server_Player::evaluateCreateCounter(/*gameStarted=*/true, /*playerConceded=*/true, "mycounter"), - Response::RespContextError); + EXPECT_EQ(Server_Player::evaluateCreateCounter(true, /*playerConceded=*/true, "test"), Response::RespContextError); } -TEST(EvaluateCreateCounter, RejectsCommanderTaxName) +TEST(EvaluateCreateCounter, RejectsTaxCounterNames) { - EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::CommanderTax), + EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::TaxCounter1), + Response::RespFunctionNotAllowed); + EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::TaxCounter2), + Response::RespFunctionNotAllowed); + EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::TaxCounter3), + Response::RespFunctionNotAllowed); + EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::TaxCounter4), + Response::RespFunctionNotAllowed); + EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::TaxCounter5), Response::RespFunctionNotAllowed); } -TEST(EvaluateCreateCounter, RejectsPartnerTaxName) +TEST(EvaluateCreateCounter, AllowsUserCounterName) { - EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::PartnerTax), - Response::RespFunctionNotAllowed); -} - -TEST(EvaluateCreateCounter, AllowsOrdinaryNames) -{ - EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, "life"), Response::RespOk); EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, "poison"), Response::RespOk); - EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, ""), Response::RespOk); } TEST(EvaluateModifyCounter, RejectsWhenGameNotStarted) { Server_Counter counter = makeCounter(UserCounterId, 0); - EXPECT_EQ(Server_Player::evaluateModifyCounter(/*gameStarted=*/false, false, /*commandZoneEnabled=*/true, - UserCounterId, &counter), + EXPECT_EQ(Server_Player::evaluateModifyCounter(/*gameStarted=*/false, /*playerConceded=*/false, true, UserCounterId, + &counter), Response::RespGameNotStarted); } @@ -192,48 +215,46 @@ TEST(EvaluateModifyCounter, RejectsWhenPlayerConceded) Response::RespContextError); } -TEST(EvaluateModifyCounter, AllowsUserCounter) -{ - Server_Counter counter = makeCounter(UserCounterId, 0); - EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/false, UserCounterId, &counter), - Response::RespOk); -} - TEST(EvaluateModifyCounter, RejectsMissingCounter) { EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, true, UserCounterId, nullptr), Response::RespNameNotFound); } +TEST(EvaluateModifyCounter, AllowsUserCounter) +{ + Server_Counter counter = makeCounter(UserCounterId, 5); + EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, true, UserCounterId, &counter), Response::RespOk); +} + TEST(EvaluateModifyCounter, RejectsTaxCounterWhenCommandZoneDisabled) { - Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); - EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/false, CounterIds::CommanderTax, + Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0); + EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/false, CounterIds::TaxCounter1, &counter), Response::RespContextError); } TEST(EvaluateModifyCounter, RejectsInactiveTaxCounter) { - Server_Counter counter = makeCounter(CounterIds::PartnerTax, 0); - (void)counter.setActive(false); - EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/true, CounterIds::PartnerTax, + Server_Counter counter = makeCounter(CounterIds::TaxCounter2, 0, false); + EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/true, CounterIds::TaxCounter2, &counter), Response::RespContextError); } TEST(EvaluateModifyCounter, AllowsActiveTaxCounter) { - Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); - EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/true, CounterIds::CommanderTax, + Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0, true); + EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/true, CounterIds::TaxCounter1, &counter), Response::RespOk); } TEST(EvaluateModifyCounter, RejectsMissingTaxCounter) { - EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/true, CounterIds::CommanderTax, - /*counter=*/nullptr), + EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/true, CounterIds::TaxCounter1, + nullptr), Response::RespNameNotFound); } diff --git a/tests/command_zone_tests/new_counter_id_test.cpp b/tests/command_zone_tests/new_counter_id_test.cpp index f5a56a8b4..dfd2239b7 100644 --- a/tests/command_zone_tests/new_counter_id_test.cpp +++ b/tests/command_zone_tests/new_counter_id_test.cpp @@ -52,25 +52,25 @@ TEST(NewCounterId, SkipsReservedRangeWhenOnlyReservedCountersExist) TEST(NewCounterId, SkipsTaxCounterIds) { PlayerFixture f; - f.player.addCounter(new Server_Counter(CounterIds::CommanderTax, "tax1", color(), 0, 0)); - f.player.addCounter(new Server_Counter(CounterIds::PartnerTax, "tax2", color(), 0, 0)); + f.player.addCounter(new Server_Counter(CounterIds::TaxCounter1, "tax1", color(), 0, 0)); + f.player.addCounter(new Server_Counter(CounterIds::TaxCounter2, "tax2", color(), 0, 0)); EXPECT_EQ(f.player.newCounterId(), CounterIds::FirstUserId); } TEST(NewCounterId, ReturnsNextIdAboveHighestUserCounter) { PlayerFixture f; - f.player.addCounter(new Server_Counter(CounterIds::FirstUserId, "a", color(), 20, 0)); // 10 - f.player.addCounter(new Server_Counter(15, "b", color(), 20, 0)); - EXPECT_EQ(f.player.newCounterId(), 16); + f.player.addCounter(new Server_Counter(CounterIds::FirstUserId, "a", color(), 20, 0)); + f.player.addCounter(new Server_Counter(CounterIds::FirstUserId + 2, "b", color(), 20, 0)); + EXPECT_EQ(f.player.newCounterId(), CounterIds::FirstUserId + 3); } TEST(NewCounterId, IgnoresReservedCountersWhenUserCountersPresent) { PlayerFixture f; - f.player.addCounter(new Server_Counter(5, "r", color(), 20, 0)); // reserved range - f.player.addCounter(new Server_Counter(11, "user", color(), 20, 0)); // user range - EXPECT_EQ(f.player.newCounterId(), 12); + f.player.addCounter(new Server_Counter(5, "r", color(), 20, 0)); // reserved range + f.player.addCounter(new Server_Counter(CounterIds::FirstUserId, "user", color(), 20, 0)); // user range + EXPECT_EQ(f.player.newCounterId(), CounterIds::FirstUserId + 1); } int main(int argc, char **argv) diff --git a/tests/command_zone_tests/setup_zones_command_zone_test.cpp b/tests/command_zone_tests/setup_zones_command_zone_test.cpp index 6b6b95615..16f9fb9ef 100644 --- a/tests/command_zone_tests/setup_zones_command_zone_test.cpp +++ b/tests/command_zone_tests/setup_zones_command_zone_test.cpp @@ -86,34 +86,40 @@ struct SetupFixture }; } // namespace -TEST(SetupZonesCommandZone, CreatesTaxCountersWhenEnabled) +TEST(SetupZonesCommandZone, CreatesAllTaxCountersWhenEnabled) { SetupFixture f(true); const QMap &counters = f.player.getCounters(); EXPECT_TRUE(f.player.getZones().contains(ZoneNames::COMMAND)); - ASSERT_TRUE(counters.contains(CounterIds::CommanderTax)); - ASSERT_TRUE(counters.contains(CounterIds::PartnerTax)); - const Server_Counter *commander = counters.value(CounterIds::CommanderTax); - const Server_Counter *partner = counters.value(CounterIds::PartnerTax); + for (int i = 0; i < CounterIds::TaxCounterCount; ++i) { + int id = CounterIds::taxCounterIdFromIndex(i); + ASSERT_TRUE(counters.contains(id)); + } - EXPECT_TRUE(commander->isActive()); - EXPECT_FALSE(partner->isActive()); - EXPECT_EQ(commander->getCount(), 0); - EXPECT_EQ(partner->getCount(), 0); + const Server_Counter *tax1 = counters.value(CounterIds::TaxCounter1); + EXPECT_TRUE(tax1->isActive()); + EXPECT_EQ(tax1->getCount(), 0); + + for (int i = 1; i < CounterIds::TaxCounterCount; ++i) { + int id = CounterIds::taxCounterIdFromIndex(i); + const Server_Counter *counter = counters.value(id); + EXPECT_FALSE(counter->isActive()); + EXPECT_EQ(counter->getCount(), 0); + } } -TEST(SetupZonesCommandZone, TaxCountersUseCommanderBounds) +TEST(SetupZonesCommandZone, TaxCountersUseBounds) { SetupFixture f(true); - Server_Counter *commander = f.player.getCounters().value(CounterIds::CommanderTax); - ASSERT_NE(commander, nullptr); + Server_Counter *tax1 = f.player.getCounters().value(CounterIds::TaxCounter1); + ASSERT_NE(tax1, nullptr); - EXPECT_TRUE(commander->setCount(MAX_COUNTER_VALUE + 1000)); - EXPECT_EQ(commander->getCount(), MAX_COUNTER_VALUE); - EXPECT_TRUE(commander->setCount(-1)); - EXPECT_EQ(commander->getCount(), 0); + EXPECT_TRUE(tax1->setCount(MAX_COUNTER_VALUE + 1000)); + EXPECT_EQ(tax1->getCount(), MAX_COUNTER_VALUE); + EXPECT_TRUE(tax1->setCount(-1)); + EXPECT_EQ(tax1->getCount(), 0); } TEST(SetupZonesCommandZone, NoTaxCountersWhenDisabled) @@ -122,8 +128,12 @@ TEST(SetupZonesCommandZone, NoTaxCountersWhenDisabled) const QMap &counters = f.player.getCounters(); EXPECT_FALSE(f.player.getZones().contains(ZoneNames::COMMAND)); - EXPECT_FALSE(counters.contains(CounterIds::CommanderTax)); - EXPECT_FALSE(counters.contains(CounterIds::PartnerTax)); + + for (int i = 0; i < CounterIds::TaxCounterCount; ++i) { + int id = CounterIds::taxCounterIdFromIndex(i); + EXPECT_FALSE(counters.contains(id)); + } + EXPECT_TRUE(counters.contains(0)); }