From ef38a8bb2bce0844bee2468a72ca5547b764a2bd Mon Sep 17 00:00:00 2001
From: Basile Clement <Elarnon@users.noreply.github.com>
Date: Wed, 8 Feb 2023 18:59:14 +0000
Subject: [PATCH] Re-add missing '/' separator in <pt> after b282df2e27 (#4747)

In b282df2e275ecf0934981a23d3f585f97fc42e0b (#4728) the logic for
creating <pt> values was updated to avoid adding a final slash after an
existing power value and missing toughness value. This works by setting
the ptSeparator to an empty string when either the power or the
toughness is undefined. However, due to the ptSeparator variable being
scoped out of the `for` loop, this causes all remaining cards to have an
empty string as a separator, ending up with <pt> values of e.g. 21
instead of 2/1.

Moreover, the implementation from #4728 is ambiguous in the case of a
card having a toughness value but no power value: in that situation, it
creates a <pt> entry with the toughness value and no separator, which is
not a good idea since it is not possible to know if <pt>2</pt> means
power 2 and no toughness, or no power and toughness 2 (Cockatrice takes
the first interpretation).

To avoid ambiguities, the <pt> value is now one of:

 1. A regular P/T value when the card has power and toughness
 2. A simplified P value when the card has power but no toughness
 3. A simplified /T value when the card has toughness but no power
 4. Absent when the card has neither power nor toughness

Note that, as far as I can tell, Cockatrice seems to (incorrectly, IMO)
ignore the initial slash if present in Player::parsePT, and treat /T as
a power value. However that is a separate issue: this patch is concerned
with Oracle and ensuring proper values in cards.xml, not with how
Cockatrice interprets those values.
---
 oracle/src/oracleimporter.cpp | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp
index 2ccdbcef..bb3b22e3 100644
--- a/oracle/src/oracleimporter.cpp
+++ b/oracle/src/oracleimporter.cpp
@@ -203,7 +203,7 @@ int OracleImporter::importCardsFromSet(const CardSetPtr &currentSet,
     QMap<QString, QList<SplitCardPart>> splitCards;
     QString ptSeparator("/");
     QVariantMap card;
-    QString layout, name, text, colors, colorIdentity, maintype, power, toughness, faceName;
+    QString layout, name, text, colors, colorIdentity, maintype, faceName;
     static const bool isToken = false;
     QVariantHash properties;
     CardInfoPerSet setInfo;
@@ -332,12 +332,13 @@ int OracleImporter::importCardsFromSet(const CardSetPtr &currentSet,
             properties.insert("maintype", mainCardType);
         }
 
-        power = getStringPropertyFromMap(card, "power");
-        toughness = getStringPropertyFromMap(card, "toughness");
-        if (power.isEmpty() || toughness.isEmpty()) {
-            ptSeparator = "";
-        }
-        if (!(power.isEmpty() && toughness.isEmpty())) {
+        // Depending on whether power and/or toughness are present, the format
+        // is either P/T (most common), P (no toughness), or /T (no power).
+        QString power = getStringPropertyFromMap(card, "power");
+        QString toughness = getStringPropertyFromMap(card, "toughness");
+        if (toughness.isEmpty() && !power.isEmpty()) {
+            properties.insert("pt", power);
+        } else if (!toughness.isEmpty()) {
             properties.insert("pt", power + ptSeparator + toughness);
         }