From 0c8f4d403c41e9363d672e56c9d5285788157476 Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Sat, 14 Mar 2026 18:29:08 -0400 Subject: [PATCH 1/4] refactor: extract tableRowToGridY helper on TableZone Consolidates 5 duplicated callsites into a single helper that encapsulates the table-row-to-grid-Y mapping and the clamping of tableRow > 2 (instants/sorceries) to the noncreatures row. Resolves the TODO at createCard() requesting this extraction. Also fixes a latent bug where tableRow 3 mapped to the land row (gridY 0) instead of the noncreature row (gridY 1) at 4 callsites that lacked the > 2 guard. --- cockatrice/src/game/player/player_actions.cpp | 17 +++++------------ cockatrice/src/game/zones/table_zone.cpp | 8 ++++++++ cockatrice/src/game/zones/table_zone.h | 7 +++++++ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/cockatrice/src/game/player/player_actions.cpp b/cockatrice/src/game/player/player_actions.cpp index 20e526727..ee8a7b1fc 100644 --- a/cockatrice/src/game/player/player_actions.cpp +++ b/cockatrice/src/game/player/player_actions.cpp @@ -75,7 +75,7 @@ void PlayerActions::playCard(CardItem *card, bool faceDown) cmd.set_y(0); } else { tableRow = faceDown ? 2 : info.getUiAttributes().tableRow; - QPoint gridPoint = QPoint(-1, TableZone::clampValidTableRow(2 - tableRow)); + QPoint gridPoint = QPoint(-1, TableZone::tableRowToGridY(tableRow)); cardToMove->set_face_down(faceDown); if (!faceDown) { cardToMove->set_pt(info.getPowTough().toStdString()); @@ -114,12 +114,7 @@ void PlayerActions::playCardToTable(const CardItem *card, bool faceDown) const CardInfo &info = exactCard.getInfo(); int tableRow = faceDown ? 2 : info.getUiAttributes().tableRow; - // default instant/sorcery cards to the noncreatures row - if (tableRow > 2) { - tableRow = 1; - } - - QPoint gridPoint = QPoint(-1, TableZone::clampValidTableRow(2 - tableRow)); + QPoint gridPoint = QPoint(-1, TableZone::tableRowToGridY(tableRow)); cardToMove->set_face_down(faceDown); if (!faceDown) { cardToMove->set_pt(info.getPowTough().toStdString()); @@ -869,7 +864,7 @@ void PlayerActions::actCreateToken() ExactCard correctedCard = CardDatabaseManager::query()->guessCard({lastTokenInfo.name, lastTokenInfo.providerId}); if (correctedCard) { lastTokenInfo.name = correctedCard.getName(); - lastTokenTableRow = TableZone::clampValidTableRow(2 - correctedCard.getInfo().getUiAttributes().tableRow); + lastTokenTableRow = TableZone::tableRowToGridY(correctedCard.getInfo().getUiAttributes().tableRow); if (lastTokenInfo.pt.isEmpty()) { lastTokenInfo.pt = correctedCard.getInfo().getPowTough(); } @@ -920,7 +915,7 @@ void PlayerActions::setLastToken(CardInfoPtr cardInfo) .providerId = SettingsCache::instance().cardOverrides().getCardPreferenceOverride(cardInfo->getName())}; - lastTokenTableRow = TableZone::clampValidTableRow(2 - cardInfo->getUiAttributes().tableRow); + lastTokenTableRow = TableZone::tableRowToGridY(cardInfo->getUiAttributes().tableRow); utilityMenu->setAndEnableCreateAnotherTokenAction(tr("C&reate another %1 token").arg(lastTokenInfo.name)); } @@ -1088,9 +1083,7 @@ void PlayerActions::createCard(const CardItem *sourceCard, return; } - // get the target token's location - // TODO: Define this QPoint into its own function along with the one below - QPoint gridPoint = QPoint(-1, TableZone::clampValidTableRow(2 - cardInfo->getUiAttributes().tableRow)); + QPoint gridPoint = QPoint(-1, TableZone::tableRowToGridY(cardInfo->getUiAttributes().tableRow)); // create the token for the related card Command_CreateToken cmd; diff --git a/cockatrice/src/game/zones/table_zone.cpp b/cockatrice/src/game/zones/table_zone.cpp index b6ac2150b..2a382fafe 100644 --- a/cockatrice/src/game/zones/table_zone.cpp +++ b/cockatrice/src/game/zones/table_zone.cpp @@ -382,3 +382,11 @@ int TableZone::clampValidTableRow(const int row) return TABLEROWS - 1; return row; } + +int TableZone::tableRowToGridY(int tableRow) +{ + if (tableRow > 2) { + tableRow = 1; + } + return clampValidTableRow(2 - tableRow); +} diff --git a/cockatrice/src/game/zones/table_zone.h b/cockatrice/src/game/zones/table_zone.h index 61eb48d7b..7a53a9eb4 100644 --- a/cockatrice/src/game/zones/table_zone.h +++ b/cockatrice/src/game/zones/table_zone.h @@ -151,6 +151,13 @@ public: static int clampValidTableRow(const int row); + /** + * Converts a card's logical table row (0=creatures, 1=noncreatures, 2=lands) + * to the corresponding grid Y coordinate. Cards with tableRow > 2 (e.g., + * instants/sorceries) default to the noncreatures row. + */ + static int tableRowToGridY(int tableRow); + /** Resizes the TableZone in case CardItems are within or outside of the TableZone constraints. From 651b898ee17bc2db7ec0d523f5b75530aaa3ab43 Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Sat, 14 Mar 2026 18:42:21 -0400 Subject: [PATCH 2/4] feat: add 'Move to Table' contextual menu action Adds a 'Table' option to the 'Move to' right-click menu that places each card in its correct table row based on card type, rather than requiring drag-and-drop. Uses per-card iteration so creatures, lands, and noncreatures each land in their proper row. Contrary to the Play option, instants and sorceries also go to the table, covering for what the play option doesn't do. --- .../src/client/settings/shortcuts_settings.h | 6 ++--- .../src/game/player/card_menu_action_type.h | 5 +++- cockatrice/src/game/player/menu/move_menu.cpp | 7 +++++ cockatrice/src/game/player/menu/move_menu.h | 1 + cockatrice/src/game/player/player_actions.cpp | 27 +++++++++++++++++++ 5 files changed, 42 insertions(+), 4 deletions(-) diff --git a/cockatrice/src/client/settings/shortcuts_settings.h b/cockatrice/src/client/settings/shortcuts_settings.h index 47332b8c4..76b9df727 100644 --- a/cockatrice/src/client/settings/shortcuts_settings.h +++ b/cockatrice/src/client/settings/shortcuts_settings.h @@ -563,9 +563,9 @@ private: {"Player/aPlayFacedown", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Battlefield, Face Down"), parseSequenceString(""), ShortcutGroup::Move_selected)}, - {"Player/aPlay", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Battlefield"), - parseSequenceString(""), - ShortcutGroup::Move_selected)}, + {"Player/aMoveToTable", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Battlefield"), + parseSequenceString(""), + ShortcutGroup::Move_selected)}, {"Player/aViewHand", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Hand"), parseSequenceString(""), ShortcutGroup::View)}, {"Player/aViewGraveyard", diff --git a/cockatrice/src/game/player/card_menu_action_type.h b/cockatrice/src/game/player/card_menu_action_type.h index aec6d6397..1b63674fa 100644 --- a/cockatrice/src/game/player/card_menu_action_type.h +++ b/cockatrice/src/game/player/card_menu_action_type.h @@ -9,17 +9,20 @@ enum CardMenuActionType { + // Per-card attribute actions (must be <= cmClone for cardMenuAction() dispatch) cmTap, cmUntap, cmDoesntUntap, cmFlip, cmPeek, cmClone, + // Move actions (must be > cmClone for cardMenuAction() dispatch) cmMoveToTopLibrary, cmMoveToBottomLibrary, cmMoveToHand, cmMoveToGraveyard, - cmMoveToExile + cmMoveToExile, + cmMoveToTable }; #endif // COCKATRICE_CARD_MENU_ACTION_TYPE_H diff --git a/cockatrice/src/game/player/menu/move_menu.cpp b/cockatrice/src/game/player/menu/move_menu.cpp index d27e16009..736d2fcc2 100644 --- a/cockatrice/src/game/player/menu/move_menu.cpp +++ b/cockatrice/src/game/player/menu/move_menu.cpp @@ -17,6 +17,8 @@ MoveMenu::MoveMenu(Player *player) : QMenu(tr("Move to")) aMoveToGraveyard->setData(cmMoveToGraveyard); aMoveToExile = new QAction(this); aMoveToExile->setData(cmMoveToExile); + aMoveToTable = new QAction(this); + aMoveToTable->setData(cmMoveToTable); connect(aMoveToTopLibrary, &QAction::triggered, player->getPlayerActions(), &PlayerActions::cardMenuAction); connect(aMoveToBottomLibrary, &QAction::triggered, player->getPlayerActions(), &PlayerActions::cardMenuAction); @@ -25,11 +27,14 @@ MoveMenu::MoveMenu(Player *player) : QMenu(tr("Move to")) connect(aMoveToHand, &QAction::triggered, player->getPlayerActions(), &PlayerActions::cardMenuAction); connect(aMoveToGraveyard, &QAction::triggered, player->getPlayerActions(), &PlayerActions::cardMenuAction); connect(aMoveToExile, &QAction::triggered, player->getPlayerActions(), &PlayerActions::cardMenuAction); + connect(aMoveToTable, &QAction::triggered, player->getPlayerActions(), &PlayerActions::cardMenuAction); addAction(aMoveToTopLibrary); addAction(aMoveToXfromTopOfLibrary); addAction(aMoveToBottomLibrary); addSeparator(); + addAction(aMoveToTable); + addSeparator(); addAction(aMoveToHand); addSeparator(); addAction(aMoveToGraveyard); @@ -50,6 +55,7 @@ void MoveMenu::setShortcutsActive() aMoveToHand->setShortcuts(shortcuts.getShortcut("Player/aMoveToHand")); aMoveToGraveyard->setShortcuts(shortcuts.getShortcut("Player/aMoveToGraveyard")); aMoveToExile->setShortcuts(shortcuts.getShortcut("Player/aMoveToExile")); + aMoveToTable->setShortcuts(shortcuts.getShortcut("Player/aMoveToTable")); } void MoveMenu::retranslateUi() @@ -59,5 +65,6 @@ void MoveMenu::retranslateUi() aMoveToBottomLibrary->setText(tr("&Bottom of library in random order")); aMoveToHand->setText(tr("&Hand")); aMoveToGraveyard->setText(tr("&Graveyard")); + aMoveToTable->setText(tr("T&able")); aMoveToExile->setText(tr("&Exile")); } \ No newline at end of file diff --git a/cockatrice/src/game/player/menu/move_menu.h b/cockatrice/src/game/player/menu/move_menu.h index 5bf657fa4..dc39cb6a5 100644 --- a/cockatrice/src/game/player/menu/move_menu.h +++ b/cockatrice/src/game/player/menu/move_menu.h @@ -23,6 +23,7 @@ public: QAction *aMoveToBottomLibrary = nullptr; QAction *aMoveToHand = nullptr; + QAction *aMoveToTable = nullptr; QAction *aMoveToGraveyard = nullptr; QAction *aMoveToExile = nullptr; }; diff --git a/cockatrice/src/game/player/player_actions.cpp b/cockatrice/src/game/player/player_actions.cpp index ee8a7b1fc..2a0061c77 100644 --- a/cockatrice/src/game/player/player_actions.cpp +++ b/cockatrice/src/game/player/player_actions.cpp @@ -1926,6 +1926,33 @@ void PlayerActions::cardMenuAction() commandList.append(cmd); break; } + case cmMoveToTable: { + for (const auto &card : cardList) { + auto *cmd = new Command_MoveCard; + cmd->set_start_player_id(startPlayerId); + cmd->set_start_zone(startZone.toStdString()); + cmd->set_target_player_id(player->getPlayerInfo()->getId()); + cmd->set_target_zone(ZoneNames::TABLE); + cmd->set_x(-1); + + CardToMove *ctm = cmd->mutable_cards_to_move()->add_card(); + ctm->set_card_id(card->getId()); + ctm->set_face_down(false); + + int tableRow = 0; + ExactCard exactCard = card->getCard(); + if (exactCard) { + const CardInfo &info = exactCard.getInfo(); + tableRow = info.getUiAttributes().tableRow; + ctm->set_pt(info.getPowTough().toStdString()); + ctm->set_tapped(info.getUiAttributes().cipt); + } + + cmd->set_y(TableZone::tableRowToGridY(tableRow)); + commandList.append(cmd); + } + break; + } default: break; } From 7409e1552052b2e8b39e6d592e600ccba111f792 Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Sat, 14 Mar 2026 18:46:57 -0400 Subject: [PATCH 3/4] feat: add Play action to stack context menu --- cockatrice/src/game/player/menu/card_menu.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cockatrice/src/game/player/menu/card_menu.cpp b/cockatrice/src/game/player/menu/card_menu.cpp index cd77c2968..b9accd43c 100644 --- a/cockatrice/src/game/player/menu/card_menu.cpp +++ b/cockatrice/src/game/player/menu/card_menu.cpp @@ -203,6 +203,9 @@ void CardMenu::createStackMenu(bool canModifyCard) { // Card is on the stack if (canModifyCard) { + addAction(aPlay); + addSeparator(); + addAction(aAttach); addAction(aDrawArrow); addSeparator(); From dc97fd2bbc9a74abfb876081f4af0c0f65f3ecbd Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Mon, 16 Mar 2026 22:53:15 -0400 Subject: [PATCH 4/4] reorganize card context menus for consistency and readability --- .../src/client/settings/shortcuts_settings.h | 2 +- cockatrice/src/game/player/menu/card_menu.cpp | 75 ++++++++++--------- 2 files changed, 39 insertions(+), 38 deletions(-) diff --git a/cockatrice/src/client/settings/shortcuts_settings.h b/cockatrice/src/client/settings/shortcuts_settings.h index 76b9df727..9c64b0d6c 100644 --- a/cockatrice/src/client/settings/shortcuts_settings.h +++ b/cockatrice/src/client/settings/shortcuts_settings.h @@ -562,7 +562,7 @@ private: ShortcutGroup::Move_selected)}, {"Player/aPlayFacedown", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Battlefield, Face Down"), parseSequenceString(""), - ShortcutGroup::Move_selected)}, + ShortcutGroup::Playing_Area)}, {"Player/aMoveToTable", ShortcutKey(QT_TRANSLATE_NOOP("shortcutsTab", "Battlefield"), parseSequenceString(""), ShortcutGroup::Move_selected)}, diff --git a/cockatrice/src/game/player/menu/card_menu.cpp b/cockatrice/src/game/player/menu/card_menu.cpp index b9accd43c..7da259a04 100644 --- a/cockatrice/src/game/player/menu/card_menu.cpp +++ b/cockatrice/src/game/player/menu/card_menu.cpp @@ -146,11 +146,10 @@ void CardMenu::createTableMenu(bool canModifyCard) { // Card is on the battlefield if (!canModifyCard) { + addAction(aDrawArrow); + addSeparator(); addRelatedCardView(); addRelatedCardActions(); - - addSeparator(); - addAction(aDrawArrow); addSeparator(); addAction(aClone); addSeparator(); @@ -165,10 +164,9 @@ void CardMenu::createTableMenu(bool canModifyCard) if (card->getFaceDown()) { addAction(aPeek); } - - addRelatedCardView(); - addRelatedCardActions(); - + addSeparator(); + addAction(aClone); + addMenu(new MoveMenu(player)); addSeparator(); addAction(aAttach); if (card->getAttachedTo()) { @@ -179,9 +177,6 @@ void CardMenu::createTableMenu(bool canModifyCard) addMenu(new PtMenu(player)); addAction(aSetAnnotation); addSeparator(); - addAction(aClone); - addMenu(new MoveMenu(player)); - addSeparator(); addAction(aSelectAll); addAction(aSelectRow); @@ -197,30 +192,35 @@ void CardMenu::createTableMenu(bool canModifyCard) } addSeparator(); addMenu(mCardCounters); + addRelatedCardView(); + addRelatedCardActions(); } void CardMenu::createStackMenu(bool canModifyCard) { // Card is on the stack - if (canModifyCard) { - addAction(aPlay); - addSeparator(); - - addAction(aAttach); + if (!canModifyCard) { addAction(aDrawArrow); addSeparator(); - addAction(aClone); - addMenu(new MoveMenu(player)); - addSeparator(); - addAction(aSelectAll); - } else { - addAction(aDrawArrow); + addRelatedCardView(); + addRelatedCardActions(); addSeparator(); addAction(aClone); addSeparator(); addAction(aSelectAll); + return; } + addAction(aPlay); + addAction(aPlayFacedown); + addSeparator(); + addAction(aClone); + addMenu(new MoveMenu(player)); + addSeparator(); + addAction(aAttach); + addAction(aDrawArrow); + addSeparator(); + addAction(aSelectAll); addRelatedCardView(); addRelatedCardActions(); } @@ -228,29 +228,30 @@ void CardMenu::createStackMenu(bool canModifyCard) void CardMenu::createGraveyardOrExileMenu(bool canModifyCard) { // Card is in the graveyard or exile - if (canModifyCard) { - addAction(aPlay); - addAction(aPlayFacedown); - - addSeparator(); - addAction(aClone); - addMenu(new MoveMenu(player)); - addSeparator(); - addAction(aSelectAll); - addAction(aSelectColumn); - - addSeparator(); - addAction(aAttach); + if (!canModifyCard) { addAction(aDrawArrow); - } else { + addSeparator(); + addRelatedCardView(); + addRelatedCardActions(); + addSeparator(); addAction(aClone); addSeparator(); addAction(aSelectAll); addAction(aSelectColumn); - addSeparator(); - addAction(aDrawArrow); + return; } + addAction(aPlay); + addAction(aPlayFacedown); + addSeparator(); + addAction(aClone); + addMenu(new MoveMenu(player)); + addSeparator(); + addAction(aAttach); + addAction(aDrawArrow); + addSeparator(); + addAction(aSelectAll); + addAction(aSelectColumn); addRelatedCardView(); addRelatedCardActions(); }