From b0ed79d9c3e805643e848026ff88e6c2d0b64565 Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Tue, 9 Jun 2026 14:29:20 -0400 Subject: [PATCH] Fix Command Zone graphics/logic layers separation 1. CommandZoneMenu: Changed to take PlayerGraphicsItem* instead of PlayerLogic*, accessing logic via player->getLogic() 2. Removed getCounterWidget() from PlayerLogic; method already exists correctly in PlayerGraphicsItem 3. PlayerMenu: CommandZoneMenu, fixed signal connection to use player->getLogic() for commandZoneSupportChanged 4. AbstractCounter: Connects to CounterState::activeChanged signal, removing direct graphics calls from PlayerEventHandler 5. CommandZone: Explicit tax counter registration via registerTaxCounter() with auto-cleanup, replacing childItems()/dynamic_cast iteration Also fixed PlayerActions to query CounterState instead of AbstractCounter for proper layer separation. --- cockatrice/src/game/player/player_actions.cpp | 19 +++++++-------- .../src/game/player/player_event_handler.cpp | 7 ------ cockatrice/src/game/player/player_logic.cpp | 7 ------ cockatrice/src/game/player/player_logic.h | 3 --- .../game_graphics/board/abstract_counter.cpp | 5 ++++ .../player/menu/command_zone_menu.cpp | 22 ++++++++++-------- .../player/menu/command_zone_menu.h | 6 ++--- .../game_graphics/player/menu/player_menu.cpp | 7 +++--- .../player/player_graphics_item.cpp | 1 + .../src/game_graphics/zones/command_zone.cpp | 23 +++++++++++-------- .../src/game_graphics/zones/command_zone.h | 11 ++++++--- 11 files changed, 55 insertions(+), 56 deletions(-) diff --git a/cockatrice/src/game/player/player_actions.cpp b/cockatrice/src/game/player/player_actions.cpp index 1021629cc..681641093 100644 --- a/cockatrice/src/game/player/player_actions.cpp +++ b/cockatrice/src/game/player/player_actions.cpp @@ -7,6 +7,7 @@ #include "../../game_graphics/zones/table_zone.h" #include "../../interface/widgets/tabs/tab_game.h" #include "../../interface/widgets/utility/get_text_with_max.h" +#include "../board/counter_state.h" #include "../zones/view_zone_logic.h" #include @@ -1654,8 +1655,8 @@ void PlayerActions::actPlayAndIncreaseTax(QList selectedCards) { playSelectedCardsImpl(selectedCards, false, [this](CardItem * /*card*/, const QString &originalZone) { if (originalZone == ZoneNames::COMMAND) { - AbstractCounter *ctr = player->getCounterWidget(CounterIds::CommanderTax); - if (ctr && ctr->isActive()) { + CounterState *state = player->getCounters().value(CounterIds::CommanderTax, nullptr); + if (state && state->isActive()) { sendIncCounter(CounterIds::CommanderTax, 2); } } @@ -1666,8 +1667,8 @@ void PlayerActions::actPlayAndIncreasePartnerTax(QList selectedCards { playSelectedCardsImpl(selectedCards, false, [this](CardItem * /*card*/, const QString &originalZone) { if (originalZone == ZoneNames::COMMAND) { - AbstractCounter *ctr = player->getCounterWidget(CounterIds::PartnerTax); - if (ctr && ctr->isActive()) { + CounterState *state = player->getCounters().value(CounterIds::PartnerTax, nullptr); + if (state && state->isActive()) { sendIncCounter(CounterIds::PartnerTax, 2); } } @@ -1684,8 +1685,8 @@ void PlayerActions::sendIncCounter(int counterId, int delta) void PlayerActions::actModifyTaxCounter(int counterId, int delta) { - AbstractCounter *ctr = player->getCounterWidget(counterId); - if (!ctr || !ctr->isActive()) { + CounterState *state = player->getCounters().value(counterId, nullptr); + if (!state || !state->isActive()) { return; } sendIncCounter(counterId, delta); @@ -1693,13 +1694,13 @@ void PlayerActions::actModifyTaxCounter(int counterId, int delta) void PlayerActions::actToggleTaxCounter(int counterId) { - AbstractCounter *ctr = player->getCounterWidget(counterId); - if (!ctr || (ctr->isActive() && ctr->getValue() != 0)) { + CounterState *state = player->getCounters().value(counterId, nullptr); + if (!state || (state->isActive() && state->getValue() != 0)) { return; } Command_SetCounterActive cmd; cmd.set_counter_id(counterId); - cmd.set_active(!ctr->isActive()); + cmd.set_active(!state->isActive()); sendGameCommand(cmd); } diff --git a/cockatrice/src/game/player/player_event_handler.cpp b/cockatrice/src/game/player/player_event_handler.cpp index 9f9aca671..0eb471bd9 100644 --- a/cockatrice/src/game/player/player_event_handler.cpp +++ b/cockatrice/src/game/player/player_event_handler.cpp @@ -285,13 +285,6 @@ void PlayerEventHandler::eventSetCounterActive(const Event_SetCounterActive &eve return; } state->setActive(event.active()); - - // TODO: The counters data should emit this and the widget hook up to it. Don't reach into graphics like this. - /*AbstractCounter *widget = player->getGraphicsItem()->getCounterWidget(event.counter_id()); - if (widget) { - widget->setActive(event.active()); - emit player->rearrangeCounters(); - }*/ } void PlayerEventHandler::eventDelCounter(const Event_DelCounter &event) diff --git a/cockatrice/src/game/player/player_logic.cpp b/cockatrice/src/game/player/player_logic.cpp index 84180a2f5..aabc014b3 100644 --- a/cockatrice/src/game/player/player_logic.cpp +++ b/cockatrice/src/game/player/player_logic.cpp @@ -316,13 +316,6 @@ CounterState *PlayerLogic::getLifeCounter() const return nullptr; } -AbstractCounter *PlayerLogic::getCounterWidget(int counterId) const -{ - Q_UNUSED(counterId); - return nullptr; - // TODO: Do not reach into graphics like this return graphicsItem->getCounterWidget(counterId); -} - bool PlayerLogic::clearCardsToDelete() { if (cardsToDelete.isEmpty()) { diff --git a/cockatrice/src/game/player/player_logic.h b/cockatrice/src/game/player/player_logic.h index b530d96bd..0394bbb74 100644 --- a/cockatrice/src/game/player/player_logic.h +++ b/cockatrice/src/game/player/player_logic.h @@ -223,9 +223,6 @@ public: */ CounterState *getLifeCounter() const; - /** @brief Returns the counter widget for the given ID, or nullptr if not found. */ - AbstractCounter *getCounterWidget(int counterId) const; - void setConceded(bool _conceded); bool getConceded() const { diff --git a/cockatrice/src/game_graphics/board/abstract_counter.cpp b/cockatrice/src/game_graphics/board/abstract_counter.cpp index ac092803d..46eb7cba3 100644 --- a/cockatrice/src/game_graphics/board/abstract_counter.cpp +++ b/cockatrice/src/game_graphics/board/abstract_counter.cpp @@ -33,6 +33,11 @@ AbstractCounter::AbstractCounter(CounterState *state, update(); }); + connect(state, &CounterState::activeChanged, this, [this](bool newActive) { + setActive(newActive); + emit player->rearrangeCounters(); + }); + if (player->getPlayerInfo()->getLocalOrJudge()) { menu = new TearOffMenu(TranslateCounterName::getDisplayName(state->getName())); aSet = new QAction(this); 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 2a4935bc6..dd2aa621f 100644 --- a/cockatrice/src/game_graphics/player/menu/command_zone_menu.cpp +++ b/cockatrice/src/game_graphics/player/menu/command_zone_menu.cpp @@ -11,7 +11,7 @@ #include #include -CommandZoneMenu::CommandZoneMenu(PlayerLogic *_player, QMenu *playerMenu) : QMenu(playerMenu), player(_player) +CommandZoneMenu::CommandZoneMenu(PlayerGraphicsItem *_player, QMenu *playerMenu) : QMenu(playerMenu), player(_player) { viewZoneShortcutKey = QStringLiteral("Player/aViewCommandZone"); incTaxShortcutKey = QStringLiteral("Player/aAddCommanderTax"); @@ -20,14 +20,17 @@ CommandZoneMenu::CommandZoneMenu(PlayerLogic *_player, QMenu *playerMenu) : QMen decPartnerTaxShortcutKey = QStringLiteral("Player/aRemovePartnerTax"); aViewZone = new QAction(this); - connect(aViewZone, &QAction::triggered, this, - [this]() { emit player->requestZoneViewToggle(player, ZoneNames::COMMAND, -1, false); }); + connect(aViewZone, &QAction::triggered, this, [this]() { + if (PlayerLogic *logic = player->getLogic()) { + emit logic->requestZoneViewToggle(logic, ZoneNames::COMMAND, -1, false); + } + }); - if (player->getPlayerInfo()->getLocalOrJudge()) { + if (player->getLogic()->getPlayerInfo()->getLocalOrJudge()) { addAction(aViewZone); addSeparator(); - PlayerActions *playerActions = player->getPlayerActions(); + PlayerActions *playerActions = player->getLogic()->getPlayerActions(); aIncreaseCommanderTax = new QAction(this); connect(aIncreaseCommanderTax, &QAction::triggered, this, @@ -106,20 +109,19 @@ void CommandZoneMenu::retranslateUi() void CommandZoneMenu::actToggleMinimized() { - // TODO - /*CommandZone *zone = player->getGraphicsItem()->getCommandZoneGraphicsItem(); + CommandZone *zone = player->getCommandZoneGraphicsItem(); if (zone) { zone->toggleMinimized(); - }*/ + } } void CommandZoneMenu::updateTaxCounterActionStates() { AbstractCounter *cmdTax = player->getCounterWidget(CounterIds::CommanderTax); - bool cmdActive = cmdTax && cmdTax->isActive(); + bool cmdActive = cmdTax != nullptr && cmdTax->isActive(); AbstractCounter *partnerTax = player->getCounterWidget(CounterIds::PartnerTax); - bool partnerActive = partnerTax && partnerTax->isActive(); + bool partnerActive = partnerTax != nullptr && partnerTax->isActive(); if (aIncreaseCommanderTax) { aIncreaseCommanderTax->setVisible(cmdActive); 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 2f36c6d7a..cdd2056cc 100644 --- a/cockatrice/src/game_graphics/player/menu/command_zone_menu.h +++ b/cockatrice/src/game_graphics/player/menu/command_zone_menu.h @@ -11,7 +11,7 @@ #include -class PlayerLogic; +class PlayerGraphicsItem; /** * @class CommandZoneMenu @@ -29,7 +29,7 @@ class CommandZoneMenu : public QMenu, public AbstractPlayerComponent Q_OBJECT public: - explicit CommandZoneMenu(PlayerLogic *player, QMenu *playerMenu); + explicit CommandZoneMenu(PlayerGraphicsItem *player, QMenu *playerMenu); void retranslateUi() override; void setShortcutsActive() override; void setShortcutsInactive() override; @@ -50,7 +50,7 @@ private slots: private: void updateTaxCounterActionStates(); - PlayerLogic *player; + PlayerGraphicsItem *player; QString viewZoneShortcutKey; QString incTaxShortcutKey; diff --git a/cockatrice/src/game_graphics/player/menu/player_menu.cpp b/cockatrice/src/game_graphics/player/menu/player_menu.cpp index dba319b21..450099709 100644 --- a/cockatrice/src/game_graphics/player/menu/player_menu.cpp +++ b/cockatrice/src/game_graphics/player/menu/player_menu.cpp @@ -33,15 +33,14 @@ PlayerMenu::PlayerMenu(PlayerGraphicsItem *_player) : QObject(_player), player(_ if (player->getLogic()->getPlayerInfo()->getLocalOrJudge()) { sideboardMenu = addManagedMenu(player, playerMenu); - // TODO - /*commandZoneMenu = addManagedMenu(player, playerMenu); + commandZoneMenu = addManagedMenu(player, playerMenu); auto updateCommandZoneMenuVisibility = [this](bool has) { if (commandZoneMenu) { commandZoneMenu->menuAction()->setVisible(has); } }; - connect(player, &PlayerLogic::commandZoneSupportChanged, this, updateCommandZoneMenuVisibility); - updateCommandZoneMenuVisibility(player->hasServerCommandZone());*/ + connect(player->getLogic(), &PlayerLogic::commandZoneSupportChanged, this, updateCommandZoneMenuVisibility); + updateCommandZoneMenuVisibility(player->getLogic()->hasServerCommandZone()); customZonesMenu = addManagedMenu(player); playerMenu->addSeparator(); diff --git a/cockatrice/src/game_graphics/player/player_graphics_item.cpp b/cockatrice/src/game_graphics/player/player_graphics_item.cpp index ca1f5572f..2f9ac91a6 100644 --- a/cockatrice/src/game_graphics/player/player_graphics_item.cpp +++ b/cockatrice/src/game_graphics/player/player_graphics_item.cpp @@ -205,6 +205,7 @@ void PlayerGraphicsItem::onCounterAdded(CounterState *state) } widget = new CommanderTaxCounter(state, player, commandZoneGraphicsItem); widget->setActive(state->isActive()); + commandZoneGraphicsItem->registerTaxCounter(widget); } else { widget = new GeneralCounter(state, player, true, this); } diff --git a/cockatrice/src/game_graphics/zones/command_zone.cpp b/cockatrice/src/game_graphics/zones/command_zone.cpp index ed3e52b9d..69824499a 100644 --- a/cockatrice/src/game_graphics/zones/command_zone.cpp +++ b/cockatrice/src/game_graphics/zones/command_zone.cpp @@ -4,11 +4,11 @@ #include "../../game/player/player_actions.h" #include "../../game/player/player_logic.h" #include "../../interface/theme_manager.h" +#include "../board/abstract_counter.h" #include "../board/card_drag_item.h" #include "../board/card_item.h" #include "../board/commander_tax_counter.h" #include "../z_values.h" -#include "select_zone.h" #include #include @@ -138,18 +138,21 @@ void CommandZone::reorganizeCards() update(); } -void CommandZone::rearrangeTaxCounters() +void CommandZone::registerTaxCounter(AbstractCounter *counter) { - // TODO - /*bool commandZoneVisible = isVisible(); - int activeTaxCounterCount = 0; - - auto *graphicsItem = getLogic()->getPlayer()->getGraphicsItem(); - if (!graphicsItem) { + if (!counter || taxCounters.contains(counter)) { return; } + taxCounters.append(counter); + connect(counter, &QObject::destroyed, this, [this, counter]() { taxCounters.removeOne(counter); }); +} - for (AbstractCounter *ctr : graphicsItem->getTaxCounterWidgets()) { +void CommandZone::rearrangeTaxCounters() +{ + bool commandZoneVisible = isVisible(); + int activeTaxCounterCount = 0; + + for (AbstractCounter *ctr : taxCounters) { qreal y = TaxCounterSizes::TAX_COUNTER_MARGIN + activeTaxCounterCount * (TaxCounterSizes::TAX_COUNTER_SIZE + TaxCounterSizes::TAX_COUNTER_MARGIN); ctr->setPos(TaxCounterSizes::TAX_COUNTER_MARGIN, y); @@ -163,7 +166,7 @@ void CommandZone::rearrangeTaxCounters() int minHeight = activeTaxCounterCount * (TaxCounterSizes::TAX_COUNTER_SIZE + TaxCounterSizes::TAX_COUNTER_MARGIN) + TaxCounterSizes::TAX_COUNTER_MARGIN; - setMinimumHeight(minHeight);*/ + setMinimumHeight(minHeight); } void CommandZone::mouseDoubleClickEvent(QGraphicsSceneMouseEvent *event) diff --git a/cockatrice/src/game_graphics/zones/command_zone.h b/cockatrice/src/game_graphics/zones/command_zone.h index 9a23b03f5..52d4f79a6 100644 --- a/cockatrice/src/game_graphics/zones/command_zone.h +++ b/cockatrice/src/game_graphics/zones/command_zone.h @@ -13,6 +13,8 @@ #include +class AbstractCounter; + inline Q_LOGGING_CATEGORY(CommandZoneLog, "command_zone"); /** @@ -50,9 +52,10 @@ public: 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 - int minimumHeight = 0; ///< Floor for minimized height (e.g. to fit tax counters) + int zoneHeight; ///< Full height in pixels when expanded + bool minimized = false; ///< Whether zone is at 25% height + int minimumHeight = 0; ///< Floor for minimized height (e.g. to fit tax counters) + QList taxCounters; ///< Registered tax counter widgets public: /** @@ -86,6 +89,8 @@ public: [[nodiscard]] qreal currentHeight() const; /** @brief Sets the minimum height floor, e.g. to ensure tax counters remain visible. */ void setMinimumHeight(int height); + /** @brief Registers a tax counter widget for layout management. */ + void registerTaxCounter(AbstractCounter *counter); /** @brief Lays out visible tax counters vertically in the top-left corner of the command zone. */ void rearrangeTaxCounters();