Allow revealing specific cards from hand and library (#4743)

Currently Cockatrice allows revealing the whole hand, or one card at
random from the hand. Sometimes, a player needs to reveal a specific
card from their hand instead, which is not supported. To achieve a
similar effect, players usually move the corresponding card (or cards)
to a public zone, then back to their hand. While this works, it is
unsatisfactory (compared to a regular reveal, you can't keep the
"revealed" window around, for one) and somewhat unintuitive.

This patch adds a "Reveal to..." menu to cards and card selections in
the player's hand or in custom zones (this includes looking at the
player's library). This menu allows revealing a card or set of cards to
any given player, or to all players.

To implement this functionality at the protocol level, the existing
RevealCards command is extended to support revealing multiple specific
cards. This is done by making `card_id` a non-packed repeated field in
the `Command_RevealCards` and `Event_RevealCards` protobufs.  Using a
non-packed repeated fields allows maintaining backwards compatibility:
an empty optional field is encoded the same way as an empty non-packed
list, an optional field with a value is encoded the same way as a
one-element non-packed list, and when decoding a multi-elements
non-packed list as an optional, only the last item in the list is read.

Since the RevealCards command already exists, and due to the compatible
encodings, a new client connecting to an old server can reveal a single
specific card from their hand. When trying to reveal multiple cards at
once, the old server will only see the request for one of the cards to
be revealed, and the player will have to reveal each card separately.

On the other hand, `Event_RevealedCards` already has an explicit list of
cards revealed by the server, and the `card_id` field is only used when
exactly one card has been revealed: thus, old and new clients will
behave identically when receiving a new `Event_RevealedCards`. In
particular, if a player using a new client reveals multiple cards from
their hand on a new server, another player using an old client will
correctly see all the revealed cards.

The approach used to build the "Reveal to..." menu is slightly different
from the approach used to build other player selection menus. Because
the "Reveal to..." menu is specific to each card, but must also be
updated whenever a player is added to or removed from the game, I chose
to re-create it on the fly whenever a card is clicked, as that seemed
the safest way to avoid both memory leaks and inconsistent state given
my understanding of the code.

cockatrice/src/player.cpp

@@ -573,6 +573,8 @@ void Player::addPlayer(Player *player)
         newAction->setData(player->getId());
         connect(newAction, SIGNAL(triggered()), this, SLOT(playerListActionTriggered()));
     }
+
+    playersInfo.append(qMakePair(player->getName(), player->getId()));
 }
 
 void Player::removePlayer(Player *player)
@@ -589,6 +591,14 @@ void Player::removePlayer(Player *player)
                 j->deleteLater();
             }
     }
+
+    for (auto it = playersInfo.begin(); it != playersInfo.end();) {
+        if (it->second == player->getId()) {
+            it = playersInfo.erase(it);
+        } else {
+            ++it;
+        }
+    }
 }
 
 void Player::playerListActionTriggered()
@@ -614,14 +624,14 @@ void Player::playerListActionTriggered()
             cmd.set_zone_name("deck");
             cmd.set_top_cards(number);
             // backward compatibility: servers before #1051 only permits to reveal the first card
-            cmd.set_card_id(0);
+            cmd.add_card_id(0);
         }
 
     } else if (menu == mRevealHand) {
         cmd.set_zone_name("hand");
     } else if (menu == mRevealRandomHandCard) {
         cmd.set_zone_name("hand");
-        cmd.set_card_id(RANDOM_CARD_FROM_ZONE);
+        cmd.add_card_id(RANDOM_CARD_FROM_ZONE);
     } else {
         return;
     }
@@ -1000,6 +1010,16 @@ void Player::initSayMenu()
     }
 }
 
+void Player::initContextualPlayersMenu(QMenu *menu)
+{
+    menu->addAction(tr("&All players"))->setData(-1);
+    menu->addSeparator();
+
+    for (const auto &playerInfo : playersInfo) {
+        menu->addAction(playerInfo.first)->setData(playerInfo.second);
+    }
+}
+
 void Player::setDeck(const DeckLoader &_deck)
 {
     deck = new DeckLoader(_deck);
@@ -1086,7 +1106,7 @@ void Player::actRevealRandomGraveyardCard()
         cmd.set_player_id(otherPlayerId);
     }
     cmd.set_zone_name("grave");
-    cmd.set_card_id(RANDOM_CARD_FROM_ZONE);
+    cmd.add_card_id(RANDOM_CARD_FROM_ZONE);
     sendGameCommand(cmd);
 }
 
@@ -2257,9 +2277,10 @@ void Player::eventRevealCards(const Event_RevealCards &event)
     } else {
         bool showZoneView = true;
         QString cardName;
+        auto cardId = event.card_id_size() == 0 ? -1 : event.card_id(0);
         if (cardList.size() == 1) {
             cardName = QString::fromStdString(cardList.first()->name());
-            if ((event.card_id() == 0) && dynamic_cast<PileZone *>(zone)) {
+            if ((cardId == 0) && dynamic_cast<PileZone *>(zone)) {
                 zone->getCards().first()->setName(cardName);
                 zone->update();
                 showZoneView = false;
@@ -2269,7 +2290,7 @@ void Player::eventRevealCards(const Event_RevealCards &event)
             static_cast<GameScene *>(scene())->addRevealedZoneView(this, zone, cardList, event.grant_write_access());
         }
 
-        emit logRevealCards(this, zone, event.card_id(), cardName, otherPlayer, false,
+        emit logRevealCards(this, zone, cardId, cardName, otherPlayer, false,
                             event.has_number_of_cards() ? event.number_of_cards() : cardList.size());
     }
 }
@@ -2826,7 +2847,7 @@ void Player::cardMenuAction()
                 case cmPeek: {
                     auto *cmd = new Command_RevealCards;
                     cmd->set_zone_name(card->getZone()->getName().toStdString());
-                    cmd->set_card_id(card->getId());
+                    cmd->add_card_id(card->getId());
                     cmd->set_player_id(id);
                     commandList.append(cmd);
                     break;
@@ -3304,6 +3325,27 @@ void Player::actPlayFacedown()
     playCard(game->getActiveCard(), true, false);
 }
 
+void Player::actReveal(QAction *action)
+{
+    const int otherPlayerId = action->data().toInt();
+
+    Command_RevealCards cmd;
+    if (otherPlayerId != -1) {
+        cmd.set_player_id(otherPlayerId);
+    }
+
+    QList<QGraphicsItem *> sel = scene()->selectedItems();
+    while (!sel.isEmpty()) {
+        const auto *card = qgraphicsitem_cast<CardItem *>(sel.takeFirst());
+        if (!cmd.has_zone_name()) {
+            cmd.set_zone_name(card->getZone()->getName().toStdString());
+        }
+        cmd.add_card_id(card->getId());
+    }
+
+    sendGameCommand(cmd);
+}
+
 void Player::refreshShortcuts()
 {
     if (shortcutsActive) {
@@ -3429,6 +3471,11 @@ void Player::updateCardMenu(const CardItem *card)
                 // Card is in hand or a custom zone specified by server
                 cardMenu->addAction(aPlay);
                 cardMenu->addAction(aPlayFacedown);
+
+                QMenu *revealMenu = cardMenu->addMenu(tr("Re&veal to..."));
+                initContextualPlayersMenu(revealMenu);
+                connect(revealMenu, &QMenu::triggered, this, &Player::actReveal);
+
                 cardMenu->addMenu(moveMenu);
                 addRelatedCardView(card, cardMenu);
             }

cockatrice/src/player.h

@@ -217,6 +217,7 @@ private slots:
     void actPlay();
     void actHide();
     void actPlayFacedown();
+    void actReveal(QAction *action);
     void refreshShortcuts();
 
 private:
@@ -227,6 +228,7 @@ private:
         *bottomLibraryMenu, *rfgMenu, *playerMenu;
     QList<QMenu *> playerLists;
     QList<QAction *> allPlayersActions;
+    QList<QPair<QString, int>> playersInfo;
     QAction *aMoveHandToTopLibrary, *aMoveHandToBottomLibrary, *aMoveHandToGrave, *aMoveHandToRfg,
         *aMoveGraveToTopLibrary, *aMoveGraveToBottomLibrary, *aMoveGraveToHand, *aMoveGraveToRfg, *aMoveRfgToTopLibrary,
         *aMoveRfgToBottomLibrary, *aMoveRfgToHand, *aMoveRfgToGrave, *aViewHand, *aViewLibrary, *aViewTopCards,
@@ -297,6 +299,7 @@ private:
     void rearrangeCounters();
 
     void initSayMenu();
+    void initContextualPlayersMenu(QMenu *menu);
 
     // void eventConnectionStateChanged(const Event_ConnectionStateChanged &event);
     void eventGameSay(const Event_GameSay &event);

common/pb/command_reveal_cards.proto

@@ -5,7 +5,7 @@ message Command_RevealCards {
         optional Command_RevealCards ext = 1026;
     }
     optional string zone_name = 1;
-    optional sint32 card_id = 2 [default = -1];
+    repeated sint32 card_id = 2 [packed = false];
     optional sint32 player_id = 3 [default = -1];
     optional bool grant_write_access = 4;
     optional sint32 top_cards = 5 [default = -1];

common/pb/event_reveal_cards.proto

@@ -7,7 +7,7 @@ message Event_RevealCards {
         optional Event_RevealCards ext = 2006;
     }
     optional string zone_name = 1;
-    optional sint32 card_id = 2 [default = -1];
+    repeated sint32 card_id = 2 [packed = false];
     optional sint32 other_player_id = 3 [default = -1];
     repeated ServerInfo_Card cards = 4;
     optional bool grant_write_access = 5;

common/server_player.cpp

@@ -354,7 +354,7 @@ void Server_Player::revealTopCardIfNeeded(Server_CardZone *zone, GameEventStorag
     if (zone->getAlwaysRevealTopCard()) {
         Event_RevealCards revealEvent;
         revealEvent.set_zone_name(zone->getName().toStdString());
-        revealEvent.set_card_id(0);
+        revealEvent.add_card_id(0);
         zone->getCards().first()->getInfo(revealEvent.add_cards());
 
         ges.enqueueGameEvent(revealEvent, playerId);
@@ -370,7 +370,7 @@ void Server_Player::revealTopCardIfNeeded(Server_CardZone *zone, GameEventStorag
         Event_RevealCards revealEvent;
         revealEvent.set_zone_name(zone->getName().toStdString());
         revealEvent.set_number_of_cards(1);
-        revealEvent.set_card_id(0);
+        revealEvent.add_card_id(0);
         zone->getCards().first()->getInfo(revealEvent.add_cards());
         ges.enqueueGameEvent(revealEvent, playerId, GameEventStorageItem::SendToPrivate, playerId);
     }
@@ -1843,27 +1843,35 @@ Server_Player::cmdRevealCards(const Command_RevealCards &cmd, ResponseContainer
             }
             cardsToReveal.append(card);
         }
-    } else if (!cmd.has_card_id()) {
+    } else if (cmd.card_id_size() == 0) {
         cardsToReveal = zone->getCards();
-    } else if (cmd.card_id() == -2) {
+    } else if (cmd.card_id_size() == 1 && cmd.card_id(0) == -2) {
+        // If there is a single card_id with value -2 (ie
+        // Player::RANDOM_CARD_FROM_ZONE), pick a random card.
+        //
+        // This is to be compatible with clients supporting a single card_id
+        // value, which send value -2 to request a random card.
         if (zone->getCards().isEmpty()) {
             return Response::RespContextError;
         }
+
         cardsToReveal.append(zone->getCards().at(rng->rand(0, zone->getCards().size() - 1)));
     } else {
-        Server_Card *card = zone->getCard(cmd.card_id());
+        for (auto cardId : cmd.card_id()) {
-        if (!card) {
+            Server_Card *card = zone->getCard(cardId);
-            return Response::RespNameNotFound;
+            if (!card) {
+                return Response::RespNameNotFound;
+            }
+            cardsToReveal.append(card);
         }
-        cardsToReveal.append(card);
     }
 
     Event_RevealCards eventOthers;
     eventOthers.set_grant_write_access(cmd.grant_write_access());
     eventOthers.set_zone_name(zone->getName().toStdString());
     eventOthers.set_number_of_cards(cardsToReveal.size());
-    if (cmd.has_card_id()) {
+    for (auto cardId : cmd.card_id()) {
-        eventOthers.set_card_id(cmd.card_id());
+        eventOthers.add_card_id(cardId);
     }
     if (cmd.has_player_id()) {
         eventOthers.set_other_player_id(cmd.player_id());