Refactor: Represent cardName + providerId with CardRef struct (#6039)

* card_ref.h

* update CardDatabase signatures

* make everything compile

* rename methods

* add docs

* mark stuff const

* set cardRef in CardItem

* cleanup

* fix build failure

* Fix builds on mac

---------

Co-authored-by: ZeldaZach <zahalpern+github@gmail.com>

common/card_ref.h

@@ -0,0 +1,24 @@
+#ifndef CARD_REF_H
+#define CARD_REF_H
+
+#include <QString>
+
+/**
+ * The information passed over the server that is required to identify the exact card to display.
+ *
+ * @param name The name of the card. Should not be empty, unless to indicate the lack of a card.
+ * @param providerId Determines which printing of the card to use. Can be empty, in which case Cockatrice should default
+ * to using the preferred set.
+ */
+struct CardRef
+{
+    QString name;
+    QString providerId = QString();
+
+    bool operator==(const CardRef &other) const
+    {
+        return name == other.name && providerId == other.providerId;
+    }
+};
+
+#endif // CARD_REF_H

common/decklist.cpp

@@ -424,7 +424,7 @@ bool DeckList::readElement(QXmlStreamReader *xml)
         } else if (childName == "bannerCard") {
             QString providerId = xml->attributes().value("providerId").toString();
             QString cardName = xml->readElementText();
-            bannerCard = QPair<QString, QString>(cardName, providerId);
+            bannerCard = {cardName, providerId};
         } else if (childName == "tags") {
             tags.clear(); // Clear existing tags
             while (xml->readNextStartElement()) {
@@ -456,8 +456,8 @@ void DeckList::write(QXmlStreamWriter *xml) const
     xml->writeTextElement("lastLoadedTimestamp", lastLoadedTimestamp);
     xml->writeTextElement("deckname", name);
     xml->writeStartElement("bannerCard");
-    xml->writeAttribute("providerId", bannerCard.second);
-    xml->writeCharacters(bannerCard.first);
+    xml->writeAttribute("providerId", bannerCard.providerId);
+    xml->writeCharacters(bannerCard.name);
     xml->writeEndElement();
     xml->writeTextElement("comments", comments);
 
@@ -805,7 +805,7 @@ void DeckList::cleanList(bool preserveMetadata)
     refreshDeckHash();
 }
 
-void DeckList::getCardListHelper(InnerDecklistNode *item, QSet<QString> &result) const
+void DeckList::getCardListHelper(InnerDecklistNode *item, QSet<QString> &result)
 {
     for (int i = 0; i < item->size(); ++i) {
         auto *node = dynamic_cast<DecklistCardNode *>(item->at(i));
@@ -818,15 +818,15 @@ void DeckList::getCardListHelper(InnerDecklistNode *item, QSet<QString> &result)
     }
 }
 
-void DeckList::getCardListWithProviderIdHelper(InnerDecklistNode *item, QMap<QString, QString> &result) const
+void DeckList::getCardRefListHelper(InnerDecklistNode *item, QList<CardRef> &result)
 {
     for (int i = 0; i < item->size(); ++i) {
         auto *node = dynamic_cast<DecklistCardNode *>(item->at(i));
 
         if (node) {
-            result.insert(node->getName(), node->getCardProviderId());
+            result.append(node->toCardRef());
         } else {
-            getCardListWithProviderIdHelper(dynamic_cast<InnerDecklistNode *>(item->at(i)), result);
+            getCardRefListHelper(dynamic_cast<InnerDecklistNode *>(item->at(i)), result);
         }
     }
 }
@@ -838,10 +838,10 @@ QStringList DeckList::getCardList() const
     return result.values();
 }
 
-QMap<QString, QString> DeckList::getCardListWithProviderId() const
+QList<CardRef> DeckList::getCardRefList() const
 {
-    QMap<QString, QString> result;
-    getCardListWithProviderIdHelper(root, result);
+    QList<CardRef> result;
+    getCardRefListHelper(root, result);
     return result;
 }
 

common/decklist.h

@@ -1,6 +1,8 @@
 #ifndef DECKLIST_H
 #define DECKLIST_H
 
+#include "card_ref.h"
+
 #include <QMap>
 #include <QVector>
 
@@ -246,6 +248,10 @@ public:
     {
         return false;
     }
+    CardRef toCardRef() const
+    {
+        return {name, cardProviderId};
+    }
 };
 
 class DeckList : public QObject
@@ -253,13 +259,13 @@ class DeckList : public QObject
     Q_OBJECT
 private:
     QString name, comments;
-    QPair<QString, QString> bannerCard;
+    CardRef bannerCard;
     QString lastLoadedTimestamp;
     QStringList tags;
     QMap<QString, SideboardPlan *> sideboardPlans;
     InnerDecklistNode *root;
-    void getCardListHelper(InnerDecklistNode *node, QSet<QString> &result) const;
-    void getCardListWithProviderIdHelper(InnerDecklistNode *item, QMap<QString, QString> &result) const;
+    static void getCardListHelper(InnerDecklistNode *node, QSet<QString> &result);
+    static void getCardRefListHelper(InnerDecklistNode *item, QList<CardRef> &result);
     InnerDecklistNode *getZoneObjFromName(const QString &zoneName);
 
     /**
@@ -305,7 +311,7 @@ public slots:
         tags.clear();
         emit deckTagsChanged();
     }
-    void setBannerCard(const QPair<QString, QString> &_bannerCard = QPair<QString, QString>())
+    void setBannerCard(const CardRef &_bannerCard = {})
     {
         bannerCard = _bannerCard;
     }
@@ -331,7 +337,7 @@ public:
     {
         return tags;
     }
-    QPair<QString, QString> getBannerCard() const
+    CardRef getBannerCard() const
     {
         return bannerCard;
     }
@@ -365,7 +371,7 @@ public:
         return root->isEmpty() && name.isEmpty() && comments.isEmpty() && sideboardPlans.isEmpty();
     }
     QStringList getCardList() const;
-    QMap<QString, QString> getCardListWithProviderId() const;
+    QList<CardRef> getCardRefList() const;
 
     int getSideboardSize() const;
 

common/server_card.cpp

@@ -27,15 +27,9 @@
 
 #include <QVariant>
 
-Server_Card::Server_Card(QString _name,
-                         QString _provider_id,
-                         int _id,
-                         int _coord_x,
-                         int _coord_y,
-                         Server_CardZone *_zone)
-    : zone(_zone), id(_id), coord_x(_coord_x), coord_y(_coord_y), name(_name), provider_id(_provider_id), tapped(false),
-      attacking(false), facedown(false), color(), ptString(), annotation(), destroyOnZoneChange(false),
-      doesntUntap(false), parentCard(0), stashedCard(nullptr)
+Server_Card::Server_Card(const CardRef &cardRef, int _id, int _coord_x, int _coord_y, Server_CardZone *_zone)
+    : zone(_zone), id(_id), coord_x(_coord_x), coord_y(_coord_y), cardRef(cardRef), tapped(false), attacking(false),
+      facedown(false), destroyOnZoneChange(false), doesntUntap(false), parentCard(0), stashedCard(nullptr)
 {
 }
 
@@ -134,10 +128,10 @@ void Server_Card::setParentCard(Server_Card *_parentCard)
 
 void Server_Card::getInfo(ServerInfo_Card *info)
 {
-    QString displayedName = facedown ? QString() : name;
+    QString displayedName = facedown ? QString() : cardRef.name;
 
     info->set_id(id);
-    info->set_provider_id(provider_id.toStdString());
+    info->set_provider_id(cardRef.providerId.toStdString());
     info->set_name(displayedName.toStdString());
     info->set_x(coord_x);
     info->set_y(coord_y);

common/server_card.h

@@ -20,6 +20,7 @@
 #ifndef SERVER_CARD_H
 #define SERVER_CARD_H
 
+#include "card_ref.h"
 #include "pb/card_attributes.pb.h"
 #include "pb/serverinfo_card.pb.h"
 #include "server_arrowtarget.h"
@@ -38,8 +39,7 @@ private:
     Server_CardZone *zone;
     int id;
     int coord_x, coord_y;
-    QString name;
-    QString provider_id;
+    CardRef cardRef;
     QMap<int, int> counters;
     bool tapped;
     bool attacking;
@@ -55,12 +55,7 @@ private:
     Server_Card *stashedCard;
 
 public:
-    Server_Card(QString _name,
-                QString _provider_id,
-                int _id,
-                int _coord_x,
-                int _coord_y,
-                Server_CardZone *_zone = nullptr);
+    Server_Card(const CardRef &cardRef, int _id, int _coord_x, int _coord_y, Server_CardZone *_zone = nullptr);
     ~Server_Card() override;
 
     Server_CardZone *getZone() const
@@ -76,9 +71,13 @@ public:
     {
         return id;
     }
+    CardRef getCardRef() const
+    {
+        return cardRef;
+    }
     QString getProviderId() const
     {
-        return provider_id;
+        return cardRef.providerId;
     }
     int getX() const
     {
@@ -90,7 +89,7 @@ public:
     }
     QString getName() const
     {
-        return name;
+        return cardRef.name;
     }
     const QMap<int, int> &getCounters() const
     {
@@ -150,9 +149,9 @@ public:
         coord_x = x;
         coord_y = y;
     }
-    void setName(const QString &_name)
+    void setCardRef(const CardRef &_cardRef)
     {
-        name = _name;
+        cardRef = _cardRef;
     }
     void setCounter(int _id, int value, Event_SetCardCounter *event = nullptr);
     void setTapped(bool _tapped)

common/server_player.cpp

@@ -210,9 +210,7 @@ void Server_Player::setupZones()
                 continue;
             }
             for (int k = 0; k < currentCard->getNumber(); ++k) {
-                z->insertCard(
-                    new Server_Card(currentCard->getName(), currentCard->getCardProviderId(), nextCardId++, 0, 0, z),
-                    -1, 0);
+                z->insertCard(new Server_Card(currentCard->toCardRef(), nextCardId++, 0, 0, z), -1, 0);
             }
         }
     }
@@ -1517,7 +1515,7 @@ Server_Player::cmdCreateToken(const Command_CreateToken &cmd, ResponseContainer
         yCoord = 0;
     }
 
-    auto *card = new Server_Card(cardName, cardProviderId, newCardId(), xCoord, yCoord);
+    auto *card = new Server_Card({cardName, cardProviderId}, newCardId(), xCoord, yCoord);
     card->moveToThread(thread());
     // Client should already prevent face-down tokens from having attributes; this just an extra server-side check
     if (!cmd.face_down()) {