Fix ZeldaZach review for PR 7492: preserve printing in DeckDiff::merge, atomic sidecar save with QSaveFile

cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp

@@ -5,6 +5,7 @@
 #include <QFileInfo>
 #include <QJsonDocument>
 #include <QJsonObject>
+#include <QSaveFile>
 
 namespace
 {
@@ -63,12 +64,12 @@ bool DeckSyncSidecar::save() const
     root.insert("records", records);
 
     QDir().mkpath(QFileInfo(filePath).absolutePath());
-    QFile file(filePath);
-    if (!file.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
+    QSaveFile file(filePath);
+    if (!file.open(QIODevice::WriteOnly)) {
         return false;
     }
     file.write(QJsonDocument(root).toJson(QJsonDocument::Compact));
-    return true;
+    return file.commit();
 }
 
 bool DeckSyncSidecar::contains(const QString &relativePath) const

libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp

@@ -13,12 +13,36 @@ namespace
 {
 using ZoneCardKey = QPair<QString, QString>;
 
+struct ZoneCardInfo
+{
+    int count = 0;
+    QString cardSetShortName;
+    QString cardSetNumber;
+    QString cardProviderId;
+};
+
+QMap<ZoneCardKey, ZoneCardInfo> zoneCardInfos(const DeckList &deck)
+{
+    QMap<ZoneCardKey, ZoneCardInfo> infos;
+    deck.forEachCard([&infos](InnerDecklistNode *zone, DecklistCardNode *card) {
+        ZoneCardInfo &info = infos[{zone->getName(), card->getName()}];
+        if (info.count == 0) {
+            info.cardSetShortName = card->getCardSetShortName();
+            info.cardSetNumber = card->getCardCollectorNumber();
+            info.cardProviderId = card->getCardProviderId();
+        }
+        info.count += card->getNumber();
+    });
+    return infos;
+}
+
 QMap<ZoneCardKey, int> zoneCardCounts(const DeckList &deck)
 {
     QMap<ZoneCardKey, int> counts;
-    deck.forEachCard([&counts](InnerDecklistNode *zone, DecklistCardNode *card) {
-        counts[{zone->getName(), card->getName()}] += card->getNumber();
-    });
+    const QMap<ZoneCardKey, ZoneCardInfo> infos = zoneCardInfos(deck);
+    for (auto it = infos.begin(); it != infos.end(); ++it) {
+        counts.insert(it.key(), it.value().count);
+    }
     return counts;
 }
 
@@ -34,17 +58,24 @@ bool changeLessThan(const DeckCardChange &a, const DeckCardChange &b)
 DeckDiffResult DeckDiff::diff(const DeckList &oldDeck, const DeckList &newDeck)
 {
     const QMap<ZoneCardKey, int> oldCounts = zoneCardCounts(oldDeck);
-    const QMap<ZoneCardKey, int> newCounts = zoneCardCounts(newDeck);
+    const QMap<ZoneCardKey, ZoneCardInfo> newInfos = zoneCardInfos(newDeck);
 
     QSet<ZoneCardKey> keys(oldCounts.keyBegin(), oldCounts.keyEnd());
-    keys.unite(QSet<ZoneCardKey>(newCounts.keyBegin(), newCounts.keyEnd()));
+    keys.unite(QSet<ZoneCardKey>(newInfos.keyBegin(), newInfos.keyEnd()));
 
     DeckDiffResult result;
     for (const ZoneCardKey &key : keys) {
         const int oldCount = oldCounts.value(key, 0);
-        const int newCount = newCounts.value(key, 0);
-        if (oldCount != newCount) {
-            result.changes.append({key.first, key.second, oldCount, newCount});
+        const ZoneCardInfo newInfo = newInfos.value(key, ZoneCardInfo());
+        if (oldCount != newInfo.count) {
+            const DeckCardChange change{.zone = key.first,
+                                        .cardName = key.second,
+                                        .oldCount = oldCount,
+                                        .newCount = newInfo.count,
+                                        .cardSetShortName = newInfo.cardSetShortName,
+                                        .cardSetNumber = newInfo.cardSetNumber,
+                                        .cardProviderId = newInfo.cardProviderId};
+            result.changes.append(change);
         }
     }
     std::sort(result.changes.begin(), result.changes.end(), changeLessThan);
@@ -69,21 +100,41 @@ DeckList
 DeckDiff::merge(const DeckList &baseDeck, const DeckDiffResult &localChanges, const DeckDiffResult &remoteChanges)
 {
     QMap<ZoneCardKey, int> counts = zoneCardCounts(baseDeck);
+    QMap<ZoneCardKey, DeckCardChange> printings;
     for (const DeckCardChange &change : localChanges.changes) {
         counts[{change.zone, change.cardName}] = change.newCount;
+        printings[{change.zone, change.cardName}] = change;
     }
     for (const DeckCardChange &change : remoteChanges.changes) {
         counts[{change.zone, change.cardName}] = change.newCount;
+        printings[{change.zone, change.cardName}] = change;
     }
 
-    DeckList merged(baseDeck.getMetadata(), DecklistNodeTree(), baseDeck.getSideboardPlans());
-    QList<ZoneCardKey> keys = counts.keys();
-    std::sort(keys.begin(), keys.end());
-    for (const ZoneCardKey &key : keys) {
-        const int count = qMax(0, counts.value(key));
-        for (int i = 0; i < count; ++i) {
-            merged.addCard(key.second, key.first);
+    DeckList merged = baseDeck;
+    QMap<ZoneCardKey, QList<DecklistCardNode *>> nodesByKey;
+    merged.forEachCard([&nodesByKey](InnerDecklistNode *zone, DecklistCardNode *card) {
+        nodesByKey[{zone->getName(), card->getName()}].append(card);
+    });
+
+    for (auto it = counts.begin(); it != counts.end(); ++it) {
+        const ZoneCardKey &key = it.key();
+        const int target = qMax(0, it.value());
+        QList<DecklistCardNode *> nodes = nodesByKey.value(key);
+        if (target == 0) {
+            for (DecklistCardNode *node : nodes) {
+                merged.getTree()->deleteNode(node);
+            }
+        } else if (nodes.isEmpty()) {
+            const DeckCardChange printing = printings.value(key);
+            merged.getTree()->addCard(printing.cardName, target, printing.zone, -1, printing.cardSetShortName,
+                                      printing.cardSetNumber, printing.cardProviderId, true);
+        } else {
+            nodes.first()->setNumber(target);
+            for (int i = 1; i < nodes.size(); ++i) {
+                merged.getTree()->deleteNode(nodes.at(i));
+            }
         }
     }
+    merged.refreshDeckHash();
     return merged;
 }

libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h

@@ -18,6 +18,9 @@ struct DeckCardChange
     QString cardName; ///< Card name.
     int oldCount = 0; ///< Copies in the old deck.
     int newCount = 0; ///< Copies in the new deck.
+    QString cardSetShortName; ///< Printing set carried from the new side, empty when unchanged.
+    QString cardSetNumber;    ///< Printing collector number carried from the new side.
+    QString cardProviderId;   ///< Printing provider id carried from the new side.
     /** @brief Copies added (positive) or removed (negative). */
     [[nodiscard]] int delta() const
     {
@@ -40,9 +43,9 @@ struct DeckDiffResult
  * @class DeckDiff
  * @brief Pure card-count diff and three-way merge for deck sync.
  *
- * Compares card names and copy counts per zone only; printing information
- * (set, collector number, provider) is not tracked, so a merged deck keeps
- * base printings with merged counts.
+ * Compares card names and copy counts per zone only; a merged deck keeps
+ * base printings for existing cards and carries the changed side's printing
+ * for newly added cards.
  */
 class DeckDiff
 {