GCMemcard: Dismantle the global return value enum into a few function specific enum classes.

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

@@ -682,10 +682,10 @@ std::pair<u16, u16> BlockAlloc::CalculateChecksums() const
   return CalculateMemcardChecksums(&raw[checksum_area_start], checksum_area_size);
 }
 
-u32 GCMemcard::GetSaveData(u8 index, std::vector<GCMBlock>& Blocks) const
+GCMemcardGetSaveDataRetVal GCMemcard::GetSaveData(u8 index, std::vector<GCMBlock>& Blocks) const
 {
   if (!m_valid)
-    return NOMEMCARD;
+    return GCMemcardGetSaveDataRetVal::NOMEMCARD;
 
   u16 block = DEntry_FirstBlock(index);
   u16 BlockCount = DEntry_BlockCount(index);
@@ -693,44 +693,45 @@ u32 GCMemcard::GetSaveData(u8 index, std::vector<GCMBlock>& Blocks) const
 
   if ((block == 0xFFFF) || (BlockCount == 0xFFFF))
   {
-    return FAIL;
+    return GCMemcardGetSaveDataRetVal::FAIL;
   }
 
   u16 nextBlock = block;
   for (int i = 0; i < BlockCount; ++i)
   {
     if ((!nextBlock) || (nextBlock == 0xFFFF))
-      return FAIL;
+      return GCMemcardGetSaveDataRetVal::FAIL;
     Blocks.push_back(m_data_blocks[nextBlock - MC_FST_BLOCKS]);
     nextBlock = GetActiveBat().GetNextBlock(nextBlock);
   }
-  return SUCCESS;
+  return GCMemcardGetSaveDataRetVal::SUCCESS;
 }
 // End DEntry functions
 
-u32 GCMemcard::ImportFile(const DEntry& direntry, std::vector<GCMBlock>& saveBlocks)
+GCMemcardImportFileRetVal GCMemcard::ImportFile(const DEntry& direntry,
+                                                std::vector<GCMBlock>& saveBlocks)
 {
   if (!m_valid)
-    return NOMEMCARD;
+    return GCMemcardImportFileRetVal::NOMEMCARD;
 
   if (GetNumFiles() >= DIRLEN)
   {
-    return OUTOFDIRENTRIES;
+    return GCMemcardImportFileRetVal::OUTOFDIRENTRIES;
   }
   if (GetActiveBat().m_free_blocks < direntry.m_block_count)
   {
-    return OUTOFBLOCKS;
+    return GCMemcardImportFileRetVal::OUTOFBLOCKS;
   }
   if (TitlePresent(direntry) != DIRLEN)
   {
-    return TITLEPRESENT;
+    return GCMemcardImportFileRetVal::TITLEPRESENT;
   }
 
   // find first free data block
   u16 firstBlock =
       GetActiveBat().NextFreeBlock(m_size_blocks, GetActiveBat().m_last_allocated_block);
   if (firstBlock == 0xFFFF)
-    return OUTOFBLOCKS;
+    return GCMemcardImportFileRetVal::OUTOFBLOCKS;
   Directory UpdatedDir = GetActiveDirectory();
 
   // find first free dir entry
@@ -775,22 +776,22 @@ u32 GCMemcard::ImportFile(const DEntry& direntry, std::vector<GCMBlock>& saveBlo
 
   FixChecksums();
 
-  return SUCCESS;
+  return GCMemcardImportFileRetVal::SUCCESS;
 }
 
-u32 GCMemcard::RemoveFile(u8 index)  // index in the directory array
+GCMemcardRemoveFileRetVal GCMemcard::RemoveFile(u8 index)  // index in the directory array
 {
   if (!m_valid)
-    return NOMEMCARD;
+    return GCMemcardRemoveFileRetVal::NOMEMCARD;
   if (index >= DIRLEN)
-    return DELETE_FAIL;
+    return GCMemcardRemoveFileRetVal::DELETE_FAIL;
 
   u16 startingblock = GetActiveDirectory().m_dir_entries[index].m_first_block;
   u16 numberofblocks = GetActiveDirectory().m_dir_entries[index].m_block_count;
 
   BlockAlloc UpdatedBat = GetActiveBat();
   if (!UpdatedBat.ClearBlocks(startingblock, numberofblocks))
-    return DELETE_FAIL;
+    return GCMemcardRemoveFileRetVal::DELETE_FAIL;
   UpdatedBat.m_update_counter = UpdatedBat.m_update_counter + 1;
   UpdateBat(UpdatedBat);
 
@@ -806,50 +807,52 @@ u32 GCMemcard::RemoveFile(u8 index)  // index in the directory array
 
   FixChecksums();
 
-  return SUCCESS;
+  return GCMemcardRemoveFileRetVal::SUCCESS;
 }
 
-u32 GCMemcard::CopyFrom(const GCMemcard& source, u8 index)
+GCMemcardImportFileRetVal GCMemcard::CopyFrom(const GCMemcard& source, u8 index)
 {
   if (!m_valid || !source.m_valid)
-    return NOMEMCARD;
+    return GCMemcardImportFileRetVal::NOMEMCARD;
 
   std::optional<DEntry> tempDEntry = source.GetDEntry(index);
   if (!tempDEntry)
-    return NOMEMCARD;
+    return GCMemcardImportFileRetVal::NOMEMCARD;
 
   u32 size = source.DEntry_BlockCount(index);
   if (size == 0xFFFF)
-    return INVALIDFILESIZE;
+    return GCMemcardImportFileRetVal::INVALIDFILESIZE;
 
   std::vector<GCMBlock> saveData;
   saveData.reserve(size);
   switch (source.GetSaveData(index, saveData))
   {
-  case FAIL:
+  case GCMemcardGetSaveDataRetVal::FAIL:
-    return FAIL;
+    return GCMemcardImportFileRetVal::FAIL;
-  case NOMEMCARD:
+  case GCMemcardGetSaveDataRetVal::NOMEMCARD:
-    return NOMEMCARD;
+    return GCMemcardImportFileRetVal::NOMEMCARD;
   default:
     FixChecksums();
     return ImportFile(*tempDEntry, saveData);
   }
 }
 
-u32 GCMemcard::ImportGci(const std::string& inputFile, const std::string& outputFile)
+GCMemcardImportFileRetVal GCMemcard::ImportGci(const std::string& inputFile,
+                                               const std::string& outputFile)
 {
   if (outputFile.empty() && !m_valid)
-    return OPENFAIL;
+    return GCMemcardImportFileRetVal::OPENFAIL;
 
   File::IOFile gci(inputFile, "rb");
   if (!gci)
-    return OPENFAIL;
+    return GCMemcardImportFileRetVal::OPENFAIL;
 
   return ImportGciInternal(std::move(gci), inputFile, outputFile);
 }
 
-u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFile,
+GCMemcardImportFileRetVal GCMemcard::ImportGciInternal(File::IOFile&& gci,
-                                 const std::string& outputFile)
+                                                       const std::string& inputFile,
+                                                       const std::string& outputFile)
 {
   unsigned int offset;
   std::string fileType;
@@ -866,17 +869,17 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil
       if (!memcmp(tmp, "GCSAVE", 6))  // Header must be uppercase
         offset = GCS;
       else
-        return GCSFAIL;
+        return GCMemcardImportFileRetVal::GCSFAIL;
     }
     else if (!strcasecmp(fileType.c_str(), ".sav"))
     {
       if (!memcmp(tmp, "DATELGC_SAVE", 0xC))  // Header must be uppercase
         offset = SAV;
       else
-        return SAVFAIL;
+        return GCMemcardImportFileRetVal::SAVFAIL;
     }
     else
-      return OPENFAIL;
+      return GCMemcardImportFileRetVal::OPENFAIL;
   }
   gci.Seek(offset, SEEK_SET);
 
@@ -890,9 +893,9 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil
   Gcs_SavConvert(tempDEntry, offset, length);
 
   if (length != tempDEntry.m_block_count * BLOCK_SIZE)
-    return LENGTHFAIL;
+    return GCMemcardImportFileRetVal::LENGTHFAIL;
   if (gci.Tell() != offset + DENTRY_SIZE)  // Verify correct file position
-    return OPENFAIL;
+    return GCMemcardImportFileRetVal::OPENFAIL;
 
   u32 size = tempDEntry.m_block_count;
   std::vector<GCMBlock> saveData;
@@ -904,14 +907,14 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil
     gci.ReadBytes(b.m_block.data(), b.m_block.size());
     saveData.push_back(b);
   }
-  u32 ret;
+  GCMemcardImportFileRetVal ret;
   if (!outputFile.empty())
   {
     File::IOFile gci2(outputFile, "wb");
     bool completeWrite = true;
     if (!gci2)
     {
-      return OPENFAIL;
+      return GCMemcardImportFileRetVal::OPENFAIL;
     }
     gci2.Seek(0, SEEK_SET);
 
@@ -926,10 +929,12 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil
         completeWrite = false;
     }
 
+    // TODO: This is interpreted as failure by the calling code if it only checks for SUCCESS.
+    // What is the logic here?
     if (completeWrite)
-      ret = GCS;
+      ret = GCMemcardImportFileRetVal::GCS;
     else
-      ret = WRITEFAIL;
+      ret = GCMemcardImportFileRetVal::WRITEFAIL;
   }
   else
     ret = ImportFile(tempDEntry, saveData);
@@ -937,7 +942,8 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil
   return ret;
 }
 
-u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::string& directory) const
+GCMemcardExportFileRetVal GCMemcard::ExportGci(u8 index, const std::string& fileName,
+                                               const std::string& directory) const
 {
   File::IOFile gci;
   int offset = GCI;
@@ -947,7 +953,7 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin
     std::string gciFilename;
     // GCI_FileName should only fail if the gamecode is 0xFFFFFFFF
     if (!GCI_FileName(index, gciFilename))
-      return SUCCESS;
+      return GCMemcardExportFileRetVal::SUCCESS;
     gci.Open(directory + DIR_SEP + gciFilename, "wb");
   }
   else
@@ -966,7 +972,7 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin
   }
 
   if (!gci)
-    return OPENFAIL;
+    return GCMemcardExportFileRetVal::OPENFAIL;
 
   gci.Seek(0, SEEK_SET);
 
@@ -989,7 +995,7 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin
 
   std::optional<DEntry> tempDEntry = GetDEntry(index);
   if (!tempDEntry)
-    return NOMEMCARD;
+    return GCMemcardExportFileRetVal::NOMEMCARD;
 
   Gcs_SavConvert(*tempDEntry, offset);
   gci.WriteBytes(&tempDEntry.value(), DENTRY_SIZE);
@@ -997,7 +1003,7 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin
   u32 size = DEntry_BlockCount(index);
   if (size == 0xFFFF)
   {
-    return FAIL;
+    return GCMemcardExportFileRetVal::FAIL;
   }
 
   std::vector<GCMBlock> saveData;
@@ -1005,10 +1011,10 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin
 
   switch (GetSaveData(index, saveData))
   {
-  case FAIL:
+  case GCMemcardGetSaveDataRetVal::FAIL:
-    return FAIL;
+    return GCMemcardExportFileRetVal::FAIL;
-  case NOMEMCARD:
+  case GCMemcardGetSaveDataRetVal::NOMEMCARD:
-    return NOMEMCARD;
+    return GCMemcardExportFileRetVal::NOMEMCARD;
   }
   gci.Seek(DENTRY_SIZE + offset, SEEK_SET);
   for (unsigned int i = 0; i < size; ++i)
@@ -1017,9 +1023,9 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin
   }
 
   if (gci.IsGood())
-    return SUCCESS;
+    return GCMemcardExportFileRetVal::SUCCESS;
   else
-    return WRITEFAIL;
+    return GCMemcardExportFileRetVal::WRITEFAIL;
 }
 
 void GCMemcard::Gcs_SavConvert(DEntry& tempDEntry, int saveType, u64 length)

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

@@ -32,27 +32,55 @@ enum
   SLOT_A = 0,
   SLOT_B = 1,
   GCI = 0,
-  SUCCESS,
-  NOMEMCARD,
-  OPENFAIL,
-  OUTOFBLOCKS,
-  OUTOFDIRENTRIES,
-  LENGTHFAIL,
-  INVALIDFILESIZE,
-  TITLEPRESENT,
   SAV = 0x80,
-  SAVFAIL,
   GCS = 0x110,
-  GCSFAIL,
-  FAIL,
-  WRITEFAIL,
-  DELETE_FAIL,
 
   CI8SHARED = 1,
   RGB5A3,
   CI8,
 };
 
+enum class GCMemcardGetSaveDataRetVal
+{
+  SUCCESS,
+  FAIL,
+  NOMEMCARD,
+};
+
+enum class GCMemcardImportFileRetVal
+{
+  SUCCESS,
+  FAIL,
+  NOMEMCARD,
+  OUTOFDIRENTRIES,
+  OUTOFBLOCKS,
+  TITLEPRESENT,
+  INVALIDFILESIZE,
+  GCSFAIL,
+  SAVFAIL,
+  OPENFAIL,
+  LENGTHFAIL,
+  WRITEFAIL,
+  GCS,
+};
+
+enum class GCMemcardExportFileRetVal
+{
+  SUCCESS,
+  FAIL,
+  NOMEMCARD,
+  OPENFAIL,
+  WRITEFAIL,
+  UNUSED,
+};
+
+enum class GCMemcardRemoveFileRetVal
+{
+  SUCCESS,
+  NOMEMCARD,
+  DELETE_FAIL,
+};
+
 // size of a single memory card block in bytes
 constexpr u32 BLOCK_SIZE = 0x2000;
 
@@ -352,8 +380,8 @@ private:
   int m_active_directory;
   int m_active_bat;
 
-  u32 ImportGciInternal(File::IOFile&& gci, const std::string& inputFile,
+  GCMemcardImportFileRetVal ImportGciInternal(File::IOFile&& gci, const std::string& inputFile,
-                        const std::string& outputFile);
+                                              const std::string& outputFile);
   void InitActiveDirBat();
 
   const Directory& GetActiveDirectory() const;
@@ -417,22 +445,23 @@ public:
   // Fetches a DEntry from the given file index.
   std::optional<DEntry> GetDEntry(u8 index) const;
 
-  u32 GetSaveData(u8 index, std::vector<GCMBlock>& saveBlocks) const;
+  GCMemcardGetSaveDataRetVal GetSaveData(u8 index, std::vector<GCMBlock>& saveBlocks) const;
 
   // adds the file to the directory and copies its contents
-  u32 ImportFile(const DEntry& direntry, std::vector<GCMBlock>& saveBlocks);
+  GCMemcardImportFileRetVal ImportFile(const DEntry& direntry, std::vector<GCMBlock>& saveBlocks);
 
   // delete a file from the directory
-  u32 RemoveFile(u8 index);
+  GCMemcardRemoveFileRetVal RemoveFile(u8 index);
 
   // reads a save from another memcard, and imports the data into this memcard
-  u32 CopyFrom(const GCMemcard& source, u8 index);
+  GCMemcardImportFileRetVal CopyFrom(const GCMemcard& source, u8 index);
 
   // reads a .gci/.gcs/.sav file and calls ImportFile or saves out a gci file
-  u32 ImportGci(const std::string& inputFile, const std::string& outputFile);
+  GCMemcardImportFileRetVal ImportGci(const std::string& inputFile, const std::string& outputFile);
 
   // writes a .gci file to disk containing index
-  u32 ExportGci(u8 index, const std::string& fileName, const std::string& directory) const;
+  GCMemcardExportFileRetVal ExportGci(u8 index, const std::string& fileName,
+                                      const std::string& directory) const;
 
   // GCI files are untouched, SAV files are byteswapped
   // GCS files have the block count set, default is 1 (For export as GCS)

Source/Core/DolphinQt/GCMemcardManager.cpp

@@ -300,7 +300,9 @@ void GCMemcardManager::ExportFiles(bool prompt)
              QStringLiteral("/%1").arg(QString::fromStdString(gci_filename));
     }
 
-    if (!memcard->ExportGci(file_index, path.toStdString(), ""))
+    // TODO: This is obviously intended to check for success instead.
+    const auto exportRetval = memcard->ExportGci(file_index, path.toStdString(), "");
+    if (exportRetval == GCMemcardExportFileRetVal::UNUSED)
     {
       File::Delete(path.toStdString());
     }
@@ -330,7 +332,7 @@ void GCMemcardManager::ImportFile()
 
   const auto result = m_slot_memcard[m_active_slot]->ImportGci(path.toStdString(), "");
 
-  if (result != SUCCESS)
+  if (result != GCMemcardImportFileRetVal::SUCCESS)
   {
     ModalMessageBox::critical(this, tr("Import failed"), tr("Failed to import \"%1\".").arg(path));
     return;
@@ -356,7 +358,7 @@ void GCMemcardManager::CopyFiles()
 
     const auto result = m_slot_memcard[!m_active_slot]->CopyFrom(*memcard, file_index);
 
-    if (result != SUCCESS)
+    if (result != GCMemcardImportFileRetVal::SUCCESS)
     {
       ModalMessageBox::warning(this, tr("Copy failed"), tr("Failed to copy file"));
     }
@@ -400,7 +402,7 @@ void GCMemcardManager::DeleteFiles()
 
   for (int file_index : file_indices)
   {
-    if (memcard->RemoveFile(file_index) != SUCCESS)
+    if (memcard->RemoveFile(file_index) != GCMemcardRemoveFileRetVal::SUCCESS)
     {
       ModalMessageBox::warning(this, tr("Remove failed"), tr("Failed to remove file"));
     }