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.
This commit is contained in:
DawnFire42 2026-06-09 14:29:20 -04:00
parent 8ca693ef70
commit b0ed79d9c3
No known key found for this signature in database
GPG key ID: 24BB855EE2911B33
11 changed files with 55 additions and 56 deletions

View file

@ -7,6 +7,7 @@
#include "../../game_graphics/zones/table_zone.h" #include "../../game_graphics/zones/table_zone.h"
#include "../../interface/widgets/tabs/tab_game.h" #include "../../interface/widgets/tabs/tab_game.h"
#include "../../interface/widgets/utility/get_text_with_max.h" #include "../../interface/widgets/utility/get_text_with_max.h"
#include "../board/counter_state.h"
#include "../zones/view_zone_logic.h" #include "../zones/view_zone_logic.h"
#include <libcockatrice/card/database/card_database_manager.h> #include <libcockatrice/card/database/card_database_manager.h>
@ -1654,8 +1655,8 @@ void PlayerActions::actPlayAndIncreaseTax(QList<CardItem *> selectedCards)
{ {
playSelectedCardsImpl(selectedCards, false, [this](CardItem * /*card*/, const QString &originalZone) { playSelectedCardsImpl(selectedCards, false, [this](CardItem * /*card*/, const QString &originalZone) {
if (originalZone == ZoneNames::COMMAND) { if (originalZone == ZoneNames::COMMAND) {
AbstractCounter *ctr = player->getCounterWidget(CounterIds::CommanderTax); CounterState *state = player->getCounters().value(CounterIds::CommanderTax, nullptr);
if (ctr && ctr->isActive()) { if (state && state->isActive()) {
sendIncCounter(CounterIds::CommanderTax, 2); sendIncCounter(CounterIds::CommanderTax, 2);
} }
} }
@ -1666,8 +1667,8 @@ void PlayerActions::actPlayAndIncreasePartnerTax(QList<CardItem *> selectedCards
{ {
playSelectedCardsImpl(selectedCards, false, [this](CardItem * /*card*/, const QString &originalZone) { playSelectedCardsImpl(selectedCards, false, [this](CardItem * /*card*/, const QString &originalZone) {
if (originalZone == ZoneNames::COMMAND) { if (originalZone == ZoneNames::COMMAND) {
AbstractCounter *ctr = player->getCounterWidget(CounterIds::PartnerTax); CounterState *state = player->getCounters().value(CounterIds::PartnerTax, nullptr);
if (ctr && ctr->isActive()) { if (state && state->isActive()) {
sendIncCounter(CounterIds::PartnerTax, 2); sendIncCounter(CounterIds::PartnerTax, 2);
} }
} }
@ -1684,8 +1685,8 @@ void PlayerActions::sendIncCounter(int counterId, int delta)
void PlayerActions::actModifyTaxCounter(int counterId, int delta) void PlayerActions::actModifyTaxCounter(int counterId, int delta)
{ {
AbstractCounter *ctr = player->getCounterWidget(counterId); CounterState *state = player->getCounters().value(counterId, nullptr);
if (!ctr || !ctr->isActive()) { if (!state || !state->isActive()) {
return; return;
} }
sendIncCounter(counterId, delta); sendIncCounter(counterId, delta);
@ -1693,13 +1694,13 @@ void PlayerActions::actModifyTaxCounter(int counterId, int delta)
void PlayerActions::actToggleTaxCounter(int counterId) void PlayerActions::actToggleTaxCounter(int counterId)
{ {
AbstractCounter *ctr = player->getCounterWidget(counterId); CounterState *state = player->getCounters().value(counterId, nullptr);
if (!ctr || (ctr->isActive() && ctr->getValue() != 0)) { if (!state || (state->isActive() && state->getValue() != 0)) {
return; return;
} }
Command_SetCounterActive cmd; Command_SetCounterActive cmd;
cmd.set_counter_id(counterId); cmd.set_counter_id(counterId);
cmd.set_active(!ctr->isActive()); cmd.set_active(!state->isActive());
sendGameCommand(cmd); sendGameCommand(cmd);
} }

View file

@ -285,13 +285,6 @@ void PlayerEventHandler::eventSetCounterActive(const Event_SetCounterActive &eve
return; return;
} }
state->setActive(event.active()); 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) void PlayerEventHandler::eventDelCounter(const Event_DelCounter &event)

View file

@ -316,13 +316,6 @@ CounterState *PlayerLogic::getLifeCounter() const
return nullptr; 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() bool PlayerLogic::clearCardsToDelete()
{ {
if (cardsToDelete.isEmpty()) { if (cardsToDelete.isEmpty()) {

View file

@ -223,9 +223,6 @@ public:
*/ */
CounterState *getLifeCounter() const; 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); void setConceded(bool _conceded);
bool getConceded() const bool getConceded() const
{ {

View file

@ -33,6 +33,11 @@ AbstractCounter::AbstractCounter(CounterState *state,
update(); update();
}); });
connect(state, &CounterState::activeChanged, this, [this](bool newActive) {
setActive(newActive);
emit player->rearrangeCounters();
});
if (player->getPlayerInfo()->getLocalOrJudge()) { if (player->getPlayerInfo()->getLocalOrJudge()) {
menu = new TearOffMenu(TranslateCounterName::getDisplayName(state->getName())); menu = new TearOffMenu(TranslateCounterName::getDisplayName(state->getName()));
aSet = new QAction(this); aSet = new QAction(this);

View file

@ -11,7 +11,7 @@
#include <libcockatrice/utility/counter_ids.h> #include <libcockatrice/utility/counter_ids.h>
#include <libcockatrice/utility/zone_names.h> #include <libcockatrice/utility/zone_names.h>
CommandZoneMenu::CommandZoneMenu(PlayerLogic *_player, QMenu *playerMenu) : QMenu(playerMenu), player(_player) CommandZoneMenu::CommandZoneMenu(PlayerGraphicsItem *_player, QMenu *playerMenu) : QMenu(playerMenu), player(_player)
{ {
viewZoneShortcutKey = QStringLiteral("Player/aViewCommandZone"); viewZoneShortcutKey = QStringLiteral("Player/aViewCommandZone");
incTaxShortcutKey = QStringLiteral("Player/aAddCommanderTax"); incTaxShortcutKey = QStringLiteral("Player/aAddCommanderTax");
@ -20,14 +20,17 @@ CommandZoneMenu::CommandZoneMenu(PlayerLogic *_player, QMenu *playerMenu) : QMen
decPartnerTaxShortcutKey = QStringLiteral("Player/aRemovePartnerTax"); decPartnerTaxShortcutKey = QStringLiteral("Player/aRemovePartnerTax");
aViewZone = new QAction(this); aViewZone = new QAction(this);
connect(aViewZone, &QAction::triggered, this, connect(aViewZone, &QAction::triggered, this, [this]() {
[this]() { emit player->requestZoneViewToggle(player, ZoneNames::COMMAND, -1, false); }); if (PlayerLogic *logic = player->getLogic()) {
emit logic->requestZoneViewToggle(logic, ZoneNames::COMMAND, -1, false);
}
});
if (player->getPlayerInfo()->getLocalOrJudge()) { if (player->getLogic()->getPlayerInfo()->getLocalOrJudge()) {
addAction(aViewZone); addAction(aViewZone);
addSeparator(); addSeparator();
PlayerActions *playerActions = player->getPlayerActions(); PlayerActions *playerActions = player->getLogic()->getPlayerActions();
aIncreaseCommanderTax = new QAction(this); aIncreaseCommanderTax = new QAction(this);
connect(aIncreaseCommanderTax, &QAction::triggered, this, connect(aIncreaseCommanderTax, &QAction::triggered, this,
@ -106,20 +109,19 @@ void CommandZoneMenu::retranslateUi()
void CommandZoneMenu::actToggleMinimized() void CommandZoneMenu::actToggleMinimized()
{ {
// TODO CommandZone *zone = player->getCommandZoneGraphicsItem();
/*CommandZone *zone = player->getGraphicsItem()->getCommandZoneGraphicsItem();
if (zone) { if (zone) {
zone->toggleMinimized(); zone->toggleMinimized();
}*/ }
} }
void CommandZoneMenu::updateTaxCounterActionStates() void CommandZoneMenu::updateTaxCounterActionStates()
{ {
AbstractCounter *cmdTax = player->getCounterWidget(CounterIds::CommanderTax); AbstractCounter *cmdTax = player->getCounterWidget(CounterIds::CommanderTax);
bool cmdActive = cmdTax && cmdTax->isActive(); bool cmdActive = cmdTax != nullptr && cmdTax->isActive();
AbstractCounter *partnerTax = player->getCounterWidget(CounterIds::PartnerTax); AbstractCounter *partnerTax = player->getCounterWidget(CounterIds::PartnerTax);
bool partnerActive = partnerTax && partnerTax->isActive(); bool partnerActive = partnerTax != nullptr && partnerTax->isActive();
if (aIncreaseCommanderTax) { if (aIncreaseCommanderTax) {
aIncreaseCommanderTax->setVisible(cmdActive); aIncreaseCommanderTax->setVisible(cmdActive);

View file

@ -11,7 +11,7 @@
#include <QMenu> #include <QMenu>
class PlayerLogic; class PlayerGraphicsItem;
/** /**
* @class CommandZoneMenu * @class CommandZoneMenu
@ -29,7 +29,7 @@ class CommandZoneMenu : public QMenu, public AbstractPlayerComponent
Q_OBJECT Q_OBJECT
public: public:
explicit CommandZoneMenu(PlayerLogic *player, QMenu *playerMenu); explicit CommandZoneMenu(PlayerGraphicsItem *player, QMenu *playerMenu);
void retranslateUi() override; void retranslateUi() override;
void setShortcutsActive() override; void setShortcutsActive() override;
void setShortcutsInactive() override; void setShortcutsInactive() override;
@ -50,7 +50,7 @@ private slots:
private: private:
void updateTaxCounterActionStates(); void updateTaxCounterActionStates();
PlayerLogic *player; PlayerGraphicsItem *player;
QString viewZoneShortcutKey; QString viewZoneShortcutKey;
QString incTaxShortcutKey; QString incTaxShortcutKey;

View file

@ -33,15 +33,14 @@ PlayerMenu::PlayerMenu(PlayerGraphicsItem *_player) : QObject(_player), player(_
if (player->getLogic()->getPlayerInfo()->getLocalOrJudge()) { if (player->getLogic()->getPlayerInfo()->getLocalOrJudge()) {
sideboardMenu = addManagedMenu<SideboardMenu>(player, playerMenu); sideboardMenu = addManagedMenu<SideboardMenu>(player, playerMenu);
// TODO commandZoneMenu = addManagedMenu<CommandZoneMenu>(player, playerMenu);
/*commandZoneMenu = addManagedMenu<CommandZoneMenu>(player, playerMenu);
auto updateCommandZoneMenuVisibility = [this](bool has) { auto updateCommandZoneMenuVisibility = [this](bool has) {
if (commandZoneMenu) { if (commandZoneMenu) {
commandZoneMenu->menuAction()->setVisible(has); commandZoneMenu->menuAction()->setVisible(has);
} }
}; };
connect(player, &PlayerLogic::commandZoneSupportChanged, this, updateCommandZoneMenuVisibility); connect(player->getLogic(), &PlayerLogic::commandZoneSupportChanged, this, updateCommandZoneMenuVisibility);
updateCommandZoneMenuVisibility(player->hasServerCommandZone());*/ updateCommandZoneMenuVisibility(player->getLogic()->hasServerCommandZone());
customZonesMenu = addManagedMenu<CustomZoneMenu>(player); customZonesMenu = addManagedMenu<CustomZoneMenu>(player);
playerMenu->addSeparator(); playerMenu->addSeparator();

View file

@ -205,6 +205,7 @@ void PlayerGraphicsItem::onCounterAdded(CounterState *state)
} }
widget = new CommanderTaxCounter(state, player, commandZoneGraphicsItem); widget = new CommanderTaxCounter(state, player, commandZoneGraphicsItem);
widget->setActive(state->isActive()); widget->setActive(state->isActive());
commandZoneGraphicsItem->registerTaxCounter(widget);
} else { } else {
widget = new GeneralCounter(state, player, true, this); widget = new GeneralCounter(state, player, true, this);
} }

View file

@ -4,11 +4,11 @@
#include "../../game/player/player_actions.h" #include "../../game/player/player_actions.h"
#include "../../game/player/player_logic.h" #include "../../game/player/player_logic.h"
#include "../../interface/theme_manager.h" #include "../../interface/theme_manager.h"
#include "../board/abstract_counter.h"
#include "../board/card_drag_item.h" #include "../board/card_drag_item.h"
#include "../board/card_item.h" #include "../board/card_item.h"
#include "../board/commander_tax_counter.h" #include "../board/commander_tax_counter.h"
#include "../z_values.h" #include "../z_values.h"
#include "select_zone.h"
#include <QGraphicsSceneMouseEvent> #include <QGraphicsSceneMouseEvent>
#include <QPainter> #include <QPainter>
@ -138,18 +138,21 @@ void CommandZone::reorganizeCards()
update(); update();
} }
void CommandZone::rearrangeTaxCounters() void CommandZone::registerTaxCounter(AbstractCounter *counter)
{ {
// TODO if (!counter || taxCounters.contains(counter)) {
/*bool commandZoneVisible = isVisible();
int activeTaxCounterCount = 0;
auto *graphicsItem = getLogic()->getPlayer()->getGraphicsItem();
if (!graphicsItem) {
return; 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 + qreal y = TaxCounterSizes::TAX_COUNTER_MARGIN +
activeTaxCounterCount * (TaxCounterSizes::TAX_COUNTER_SIZE + TaxCounterSizes::TAX_COUNTER_MARGIN); activeTaxCounterCount * (TaxCounterSizes::TAX_COUNTER_SIZE + TaxCounterSizes::TAX_COUNTER_MARGIN);
ctr->setPos(TaxCounterSizes::TAX_COUNTER_MARGIN, y); 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) + int minHeight = activeTaxCounterCount * (TaxCounterSizes::TAX_COUNTER_SIZE + TaxCounterSizes::TAX_COUNTER_MARGIN) +
TaxCounterSizes::TAX_COUNTER_MARGIN; TaxCounterSizes::TAX_COUNTER_MARGIN;
setMinimumHeight(minHeight);*/ setMinimumHeight(minHeight);
} }
void CommandZone::mouseDoubleClickEvent(QGraphicsSceneMouseEvent *event) void CommandZone::mouseDoubleClickEvent(QGraphicsSceneMouseEvent *event)

View file

@ -13,6 +13,8 @@
#include <QLoggingCategory> #include <QLoggingCategory>
class AbstractCounter;
inline Q_LOGGING_CATEGORY(CommandZoneLog, "command_zone"); inline Q_LOGGING_CATEGORY(CommandZoneLog, "command_zone");
/** /**
@ -50,9 +52,10 @@ public:
private: private:
static constexpr double MINIMIZED_HEIGHT_RATIO = 0.25; static constexpr double MINIMIZED_HEIGHT_RATIO = 0.25;
int zoneHeight; ///< Full height in pixels when expanded int zoneHeight; ///< Full height in pixels when expanded
bool minimized = false; ///< Whether zone is at 25% height bool minimized = false; ///< Whether zone is at 25% height
int minimumHeight = 0; ///< Floor for minimized height (e.g. to fit tax counters) int minimumHeight = 0; ///< Floor for minimized height (e.g. to fit tax counters)
QList<AbstractCounter *> taxCounters; ///< Registered tax counter widgets
public: public:
/** /**
@ -86,6 +89,8 @@ public:
[[nodiscard]] qreal currentHeight() const; [[nodiscard]] qreal currentHeight() const;
/** @brief Sets the minimum height floor, e.g. to ensure tax counters remain visible. */ /** @brief Sets the minimum height floor, e.g. to ensure tax counters remain visible. */
void setMinimumHeight(int height); 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. */ /** @brief Lays out visible tax counters vertically in the top-left corner of the command zone. */
void rearrangeTaxCounters(); void rearrangeTaxCounters();