Merge pull request #8300 from AdmiralCurtiss/gcmemcard-construction

GCMemcard: Rework construction logic.

Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp

@@ -141,7 +141,7 @@ CEXIMemoryCard::CEXIMemoryCard(const int index, bool gciFolder) : card_index(ind
   bool useMC251;
   IniFile gameIni = SConfig::GetInstance().LoadGameIni();
   gameIni.GetOrCreateSection("Core")->Get("MemoryCard251", &useMC251, false);
-  u16 sizeMb = useMC251 ? MemCard251Mb : MemCard2043Mb;
+  u16 sizeMb = useMC251 ? MBIT_SIZE_MEMORY_CARD_251 : MBIT_SIZE_MEMORY_CARD_2043;
 
   if (gciFolder)
   {
@@ -241,7 +241,7 @@ void CEXIMemoryCard::SetupRawMemcard(u16 sizeMb)
       SConfig::GetDirectoryForRegion(SConfig::ToGameCubeRegion(SConfig::GetInstance().m_region));
   MemoryCard::CheckPath(filename, region_dir, is_slot_a);
 
-  if (sizeMb == MemCard251Mb)
+  if (sizeMb == MBIT_SIZE_MEMORY_CARD_251)
     filename.insert(filename.find_last_of("."), ".251");
 
   memorycard = std::make_unique<MemoryCard>(filename, card_index, sizeMb);

Source/Core/Core/HW/GCMemcard/GCMemcard.h

@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <array>
+#include <bitset>
 #include <string>
 #include <vector>
 
@@ -79,6 +80,31 @@ enum class GCMemcardRemoveFileRetVal
   DELETE_FAIL,
 };
 
+enum class GCMemcardValidityIssues
+{
+  FAILED_TO_OPEN,
+  IO_ERROR,
+  INVALID_CARD_SIZE,
+  INVALID_CHECKSUM,
+  MISMATCHED_CARD_SIZE,
+  FREE_BLOCK_MISMATCH,
+  DIR_BAT_INCONSISTENT,
+  DATA_IN_UNUSED_AREA,
+  COUNT
+};
+
+class GCMemcardErrorCode
+{
+public:
+  bool HasCriticalErrors() const;
+  bool Test(GCMemcardValidityIssues code) const;
+  void Set(GCMemcardValidityIssues code);
+  GCMemcardErrorCode& operator|=(const GCMemcardErrorCode& other);
+
+private:
+  std::bitset<static_cast<size_t>(GCMemcardValidityIssues::COUNT)> m_errors;
+};
+
 // size of a single memory card block in bytes
 constexpr u32 BLOCK_SIZE = 0x2000;
 
@@ -103,17 +129,17 @@ constexpr u16 BAT_SIZE = 0xFFB;
 // possible sizes of memory cards in megabits
 // TODO: Do memory card sizes have to be power of two?
 // TODO: Are these all of them? A 4091 block card should work in theory at least.
-constexpr u16 MemCard59Mb = 0x04;
-constexpr u16 MemCard123Mb = 0x08;
-constexpr u16 MemCard251Mb = 0x10;
-constexpr u16 Memcard507Mb = 0x20;
-constexpr u16 MemCard1019Mb = 0x40;
-constexpr u16 MemCard2043Mb = 0x80;
+constexpr u16 MBIT_SIZE_MEMORY_CARD_59 = 0x04;
+constexpr u16 MBIT_SIZE_MEMORY_CARD_123 = 0x08;
+constexpr u16 MBIT_SIZE_MEMORY_CARD_251 = 0x10;
+constexpr u16 MBIT_SIZE_MEMORY_CARD_507 = 0x20;
+constexpr u16 MBIT_SIZE_MEMORY_CARD_1019 = 0x40;
+constexpr u16 MBIT_SIZE_MEMORY_CARD_2043 = 0x80;
 
 class MemoryCardBase
 {
 public:
-  explicit MemoryCardBase(int card_index = 0, int size_mbits = MemCard2043Mb)
+  explicit MemoryCardBase(int card_index = 0, int size_mbits = MBIT_SIZE_MEMORY_CARD_2043)
       : m_card_index(card_index), m_nintendo_card_id(size_mbits)
   {
   }
@@ -185,13 +211,16 @@ struct Header
   // 0x1e00 bytes at 0x0200: Unused (0xff)
   std::array<u8, 7680> m_unused_2;
 
-  explicit Header(int slot = 0, u16 size_mbits = MemCard2043Mb, bool shift_jis = false);
+  explicit Header(int slot = 0, u16 size_mbits = MBIT_SIZE_MEMORY_CARD_2043,
+                  bool shift_jis = false);
 
   // Calculates the card serial numbers used for encrypting some save files.
   std::pair<u32, u32> CalculateSerial() const;
 
   void FixChecksums();
   std::pair<u16, u16> CalculateChecksums() const;
+
+  GCMemcardErrorCode CheckForErrors(u16 card_size_mbits) const;
 };
 static_assert(sizeof(Header) == BLOCK_SIZE);
 
@@ -274,6 +303,8 @@ struct DEntry
 };
 static_assert(sizeof(DEntry) == DENTRY_SIZE);
 
+struct BlockAlloc;
+
 struct Directory
 {
   // 127 files of 0x40 bytes each
@@ -283,8 +314,7 @@ struct Directory
   std::array<u8, 0x3a> m_padding;
 
   // 2 bytes at 0x1ffa: Update Counter
-  // TODO: What happens if this overflows? Is there a special case for preferring 0 over max value?
-  Common::BigEndianValue<u16> m_update_counter;
+  Common::BigEndianValue<s16> m_update_counter;
 
   // 2 bytes at 0x1ffc: Additive Checksum
   u16 m_checksum;
@@ -301,6 +331,10 @@ struct Directory
 
   void FixChecksums();
   std::pair<u16, u16> CalculateChecksums() const;
+
+  GCMemcardErrorCode CheckForErrors() const;
+
+  GCMemcardErrorCode CheckForErrorsWithBat(const BlockAlloc& bat) const;
 };
 static_assert(sizeof(Directory) == BLOCK_SIZE);
 
@@ -313,7 +347,7 @@ struct BlockAlloc
   u16 m_checksum_inv;
 
   // 2 bytes at 0x0004: Update Counter
-  Common::BigEndianValue<u16> m_update_counter;
+  Common::BigEndianValue<s16> m_update_counter;
 
   // 2 bytes at 0x0006: Free Blocks
   Common::BigEndianValue<u16> m_free_blocks;
@@ -324,7 +358,7 @@ struct BlockAlloc
   // 0x1ff8 bytes at 0x000a: Map of allocated Blocks
   std::array<Common::BigEndianValue<u16>, BAT_SIZE> m_map;
 
-  explicit BlockAlloc(u16 size_mbits = MemCard2043Mb);
+  explicit BlockAlloc(u16 size_mbits = MBIT_SIZE_MEMORY_CARD_2043);
 
   u16 GetNextBlock(u16 block) const;
   u16 NextFreeBlock(u16 max_block, u16 starting_block = MC_FST_BLOCKS) const;
@@ -333,6 +367,8 @@ struct BlockAlloc
 
   void FixChecksums();
   std::pair<u16, u16> CalculateChecksums() const;
+
+  GCMemcardErrorCode CheckForErrors(u16 size_mbits) const;
 };
 static_assert(sizeof(BlockAlloc) == BLOCK_SIZE);
 #pragma pack(pop)
@@ -354,8 +390,9 @@ private:
   int m_active_directory;
   int m_active_bat;
 
+  GCMemcard();
+
   GCMemcardImportFileRetVal ImportGciInternal(File::IOFile&& gci, const std::string& inputFile);
-  void InitActiveDirBat();
 
   const Directory& GetActiveDirectory() const;
   const BlockAlloc& GetActiveBat() const;
@@ -364,8 +401,9 @@ private:
   void UpdateBat(const BlockAlloc& bat);
 
 public:
-  explicit GCMemcard(const std::string& fileName, bool forceCreation = false,
-                     bool shift_jis = false);
+  static std::optional<GCMemcard> Create(std::string filename, u16 size_mbits, bool shift_jis);
+
+  static std::pair<GCMemcardErrorCode, std::optional<GCMemcard>> Open(std::string filename);
 
   GCMemcard(const GCMemcard&) = delete;
   GCMemcard& operator=(const GCMemcard&) = delete;
@@ -375,14 +413,14 @@ public:
   bool IsValid() const { return m_valid; }
   bool IsShiftJIS() const;
   bool Save();
-  bool Format(bool shift_jis = false, u16 SizeMb = MemCard2043Mb);
-  static bool Format(u8* card_data, bool shift_jis = false, u16 SizeMb = MemCard2043Mb);
+  bool Format(bool shift_jis = false, u16 SizeMb = MBIT_SIZE_MEMORY_CARD_2043);
+  static bool Format(u8* card_data, bool shift_jis = false,
+                     u16 SizeMb = MBIT_SIZE_MEMORY_CARD_2043);
   static s32 FZEROGX_MakeSaveGameValid(const Header& cardheader, const DEntry& direntry,
                                        std::vector<GCMBlock>& FileBuffer);
   static s32 PSO_MakeSaveGameValid(const Header& cardheader, const DEntry& direntry,
                                    std::vector<GCMBlock>& FileBuffer);
 
-  u32 TestChecksums() const;
   bool FixChecksums();
 
   // get number of file entries in the directory

Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.cpp

@@ -703,12 +703,12 @@ void MigrateFromMemcardFile(const std::string& directory_name, int card_index)
                                                 Config::Get(Config::MAIN_MEMCARD_B_PATH);
   if (File::Exists(ini_memcard))
   {
-    GCMemcard memcard(ini_memcard.c_str());
-    if (memcard.IsValid())
+    auto [error_code, memcard] = GCMemcard::Open(ini_memcard.c_str());
+    if (!error_code.HasCriticalErrors() && memcard && memcard->IsValid())
     {
       for (u8 i = 0; i < DIRLEN; i++)
       {
-        memcard.ExportGci(i, "", directory_name);
+        memcard->ExportGci(i, "", directory_name);
       }
     }
   }

Source/Core/Core/HW/GCMemcard/GCMemcardRaw.h

@@ -17,7 +17,8 @@ class PointerWrap;
 class MemoryCard : public MemoryCardBase
 {
 public:
-  MemoryCard(const std::string& filename, int card_index, u16 size_mbits = MemCard2043Mb);
+  MemoryCard(const std::string& filename, int card_index,
+             u16 size_mbits = MBIT_SIZE_MEMORY_CARD_2043);
   ~MemoryCard();
   static void CheckPath(std::string& memcardPath, const std::string& gameRegion, bool isSlotA);
   void FlushThread();

Source/Core/DolphinQt/GCMemcardManager.cpp

@@ -17,6 +17,8 @@
 #include <QLineEdit>
 #include <QPixmap>
 #include <QPushButton>
+#include <QString>
+#include <QStringList>
 #include <QTableWidget>
 #include <QTimer>
 
@@ -241,13 +243,20 @@ void GCMemcardManager::UpdateActions()
 
 void GCMemcardManager::SetSlotFile(int slot, QString path)
 {
-  auto memcard = std::make_unique<GCMemcard>(path.toStdString());
+  auto [error_code, memcard] = GCMemcard::Open(path.toStdString());
 
-  if (!memcard->IsValid())
-    return;
-
-  m_slot_file_edit[slot]->setText(path);
-  m_slot_memcard[slot] = std::move(memcard);
+  if (!error_code.HasCriticalErrors() && memcard && memcard->IsValid())
+  {
+    m_slot_file_edit[slot]->setText(path);
+    m_slot_memcard[slot] = std::make_unique<GCMemcard>(std::move(*memcard));
+  }
+  else
+  {
+    m_slot_memcard[slot] = nullptr;
+    ModalMessageBox::critical(
+        this, tr("Error"),
+        tr("Failed opening memory card:\n%1").arg(GetErrorMessagesForErrorCode(error_code)));
+  }
 
   UpdateSlotTable(slot);
   UpdateActions();
@@ -523,3 +532,37 @@ std::vector<QPixmap> GCMemcardManager::GetIconFromSaveFile(int file_index, int s
 
   return frame_pixmaps;
 }
+
+QString GCMemcardManager::GetErrorMessagesForErrorCode(const GCMemcardErrorCode& code)
+{
+  QStringList sl;
+
+  if (code.Test(GCMemcardValidityIssues::FAILED_TO_OPEN))
+    sl.push_back(tr("Couldn't open file."));
+
+  if (code.Test(GCMemcardValidityIssues::IO_ERROR))
+    sl.push_back(tr("Couldn't read file."));
+
+  if (code.Test(GCMemcardValidityIssues::INVALID_CARD_SIZE))
+    sl.push_back(tr("Filesize does not match any known GameCube Memory Card size."));
+
+  if (code.Test(GCMemcardValidityIssues::MISMATCHED_CARD_SIZE))
+    sl.push_back(tr("Filesize in header mismatches actual card size."));
+
+  if (code.Test(GCMemcardValidityIssues::INVALID_CHECKSUM))
+    sl.push_back(tr("Invalid checksums."));
+
+  if (code.Test(GCMemcardValidityIssues::FREE_BLOCK_MISMATCH))
+    sl.push_back(tr("Mismatch between free block count in header and actually unused blocks."));
+
+  if (code.Test(GCMemcardValidityIssues::DIR_BAT_INCONSISTENT))
+    sl.push_back(tr("Mismatch between internal data structures."));
+
+  if (code.Test(GCMemcardValidityIssues::DATA_IN_UNUSED_AREA))
+    sl.push_back(tr("Data in area of file that should be unused."));
+
+  if (sl.empty())
+    return QStringLiteral("No errors.");
+
+  return sl.join(QStringLiteral("\n"));
+}

Source/Core/DolphinQt/GCMemcardManager.h

@@ -12,6 +12,7 @@
 #include <QDialog>
 
 class GCMemcard;
+class GCMemcardErrorCode;
 
 class QDialogButtonBox;
 class QGroupBox;
@@ -19,6 +20,7 @@ class QLabel;
 class QLineEdit;
 class QPixmap;
 class QPushButton;
+class QString;
 class QTableWidget;
 class QTimer;
 
@@ -29,6 +31,8 @@ public:
   explicit GCMemcardManager(QWidget* parent = nullptr);
   ~GCMemcardManager();
 
+  static QString GetErrorMessagesForErrorCode(const GCMemcardErrorCode& code);
+
 private:
   void CreateWidgets();
   void ConnectWidgets();

Source/Core/DolphinQt/Settings/GameCubePane.cpp

@@ -28,6 +28,7 @@
 #include "Core/HW/GCMemcard/GCMemcard.h"
 
 #include "DolphinQt/Config/Mapping/MappingWindow.h"
+#include "DolphinQt/GCMemcardManager.h"
 #include "DolphinQt/QtUtils/ModalMessageBox.h"
 
 enum
@@ -212,15 +213,15 @@ void GameCubePane::OnConfigPressed(int slot)
   {
     if (File::Exists(filename.toStdString()))
     {
-      GCMemcard mc(filename.toStdString());
+      auto [error_code, mc] = GCMemcard::Open(filename.toStdString());
 
-      if (!mc.IsValid())
+      if (error_code.HasCriticalErrors() || !mc || !mc->IsValid())
       {
-        ModalMessageBox::critical(this, tr("Error"),
-                                  tr("Cannot use that file as a memory card.\n%1\n"
-                                     "is not a valid GameCube memory card file")
-                                      .arg(filename));
-
+        ModalMessageBox::critical(
+            this, tr("Error"),
+            tr("The file\n%1\nis either corrupted or not a GameCube memory card file.\n%2")
+                .arg(filename)
+                .arg(GCMemcardManager::GetErrorMessagesForErrorCode(error_code)));
         return;
       }
     }