From 556e93f357a3e008481d4e8bb453e387e948df04 Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 28 Jun 2020 00:26:40 +0200 Subject: [PATCH] GCMemcard: Change behavior of TitlePresent() to more closely resemble how saves are actually identified. This modifies GCMemcard::TitlePresent() to match my findings of how the GC BIOS and various games behave when you alter the fields in the directory entry. It looks like for a save to be recognized by a game, the following have to be true: - Game code and maker code must exactly match what the game expects. - Filename is only checked up to the first null byte. All bytes afterwards can be whatever. The BIOS itself does a full compare of the filename when checking for whether it should allow copying a file from one card to another, but behaves oddly in some cases when there's non-null bytes after the first null. See the big comment in `HasSameIdentity()` for details. --- Source/Core/Core/CMakeLists.txt | 2 + Source/Core/Core/Core.vcxproj | 2 + Source/Core/Core/Core.vcxproj.filters | 6 ++ Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 23 ++++---- Source/Core/Core/HW/GCMemcard/GCMemcard.h | 5 +- .../Core/Core/HW/GCMemcard/GCMemcardUtils.cpp | 56 +++++++++++++++++++ .../Core/Core/HW/GCMemcard/GCMemcardUtils.h | 12 ++++ 7 files changed, 92 insertions(+), 14 deletions(-) create mode 100644 Source/Core/Core/HW/GCMemcard/GCMemcardUtils.cpp create mode 100644 Source/Core/Core/HW/GCMemcard/GCMemcardUtils.h diff --git a/Source/Core/Core/CMakeLists.txt b/Source/Core/Core/CMakeLists.txt index 0750be3eba..f14f6ae24d 100644 --- a/Source/Core/Core/CMakeLists.txt +++ b/Source/Core/Core/CMakeLists.txt @@ -226,6 +226,8 @@ add_library(core HW/GCMemcard/GCMemcardDirectory.h HW/GCMemcard/GCMemcardRaw.cpp HW/GCMemcard/GCMemcardRaw.h + HW/GCMemcard/GCMemcardUtils.cpp + HW/GCMemcard/GCMemcardUtils.h HW/GCPad.cpp HW/GCPad.h HW/GCPadEmu.cpp diff --git a/Source/Core/Core/Core.vcxproj b/Source/Core/Core/Core.vcxproj index fe33616c2f..9df28713a8 100644 --- a/Source/Core/Core/Core.vcxproj +++ b/Source/Core/Core/Core.vcxproj @@ -158,6 +158,7 @@ + @@ -517,6 +518,7 @@ + diff --git a/Source/Core/Core/Core.vcxproj.filters b/Source/Core/Core/Core.vcxproj.filters index b820c04802..8adb893174 100644 --- a/Source/Core/Core/Core.vcxproj.filters +++ b/Source/Core/Core/Core.vcxproj.filters @@ -481,6 +481,9 @@ HW %28Flipper/Hollywood%29\GCMemcard + + HW %28Flipper/Hollywood%29\GCMemcard + HW %28Flipper/Hollywood%29\GCPad @@ -1254,6 +1257,9 @@ HW %28Flipper/Hollywood%29\GCMemcard + + HW %28Flipper/Hollywood%29\GCMemcard + HW %28Flipper/Hollywood%29\GCPad diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index aec7f97105..27149e7895 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -21,6 +21,8 @@ #include "Common/StringUtil.h" #include "Common/Swap.h" +#include "Core/HW/GCMemcard/GCMemcardUtils.h" + static constexpr std::optional BytesToMegabits(u64 bytes) { const u64 factor = ((1024 * 1024) / 8); @@ -405,22 +407,19 @@ u16 GCMemcard::GetFreeBlocks() const return GetActiveBat().m_free_blocks; } -u8 GCMemcard::TitlePresent(const DEntry& d) const +std::optional GCMemcard::TitlePresent(const DEntry& d) const { if (!m_valid) - return DIRLEN; + return std::nullopt; - u8 i = 0; - while (i < DIRLEN) + const Directory& dir = GetActiveDirectory(); + for (u8 i = 0; i < DIRLEN; ++i) { - if (GetActiveDirectory().m_dir_entries[i].m_gamecode == d.m_gamecode && - GetActiveDirectory().m_dir_entries[i].m_filename == d.m_filename) - { - break; - } - i++; + if (HasSameIdentity(dir.m_dir_entries[i], d)) + return i; } - return i; + + return std::nullopt; } bool GCMemcard::GCI_FileName(u8 index, std::string& filename) const @@ -853,7 +852,7 @@ GCMemcardImportFileRetVal GCMemcard::ImportFile(const DEntry& direntry, { return GCMemcardImportFileRetVal::OUTOFBLOCKS; } - if (TitlePresent(direntry) != DIRLEN) + if (TitlePresent(direntry)) { return GCMemcardImportFileRetVal::TITLEPRESENT; } diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index 7ec6e912bd..427abf8470 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -460,8 +460,9 @@ public: // get the free blocks from bat u16 GetFreeBlocks() const; - // If title already on memcard returns index, otherwise returns -1 - u8 TitlePresent(const DEntry& d) const; + // Returns index of the save with the same identity as the given DEntry, or nullopt if no save + // with that identity exists in this card. + std::optional TitlePresent(const DEntry& d) const; bool GCI_FileName(u8 index, std::string& filename) const; // DEntry functions, all take u8 index < DIRLEN (127) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcardUtils.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcardUtils.cpp new file mode 100644 index 0000000000..3230e86339 --- /dev/null +++ b/Source/Core/Core/HW/GCMemcard/GCMemcardUtils.cpp @@ -0,0 +1,56 @@ +#include "Core/HW/GCMemcard/GCMemcardUtils.h" + +#include "Core/HW/GCMemcard/GCMemcard.h" + +namespace Memcard +{ +bool HasSameIdentity(const DEntry& lhs, const DEntry& rhs) +{ + // The Gamecube BIOS identifies two files as being 'the same' (that is, disallows copying from one + // card to another when both contain a file like it) when the full array of all of m_gamecode, + // m_makercode, and m_filename match between them. + + // However, despite that, it seems like the m_filename should be treated as a nullterminated + // string instead, because: + // - Games seem to identify their saves regardless of what bytes appear after the first null byte. + // - If you have two files that match except for bytes after the first null in m_filename, the + // BIOS behaves oddly if you attempt to copy the files, as it seems to clear out those extra + // non-null bytes. See below for details. + + // Specifically, the following chain of actions fails with a rather vague 'The data may not have + // been copied.' error message: + // - Have two memory cards with one save file each. + // - The two save files should have identical gamecode and makercode, as well as an equivalent + // filename up until and including the first null byte. + // - On Card A have all remaining bytes of the filename also be null. + // - On Card B have at least one of the remaining bytes be non-null. + // - Copy the file on Card B to Card A. + // The BIOS will abort halfway through the copy process and declare Card B as unusable until you + // eject and reinsert it, and leave a "Broken File000" file on Card A, though note that the file + // is not visible and will be cleaned up when reinserting the card while still within the BIOS. + // Additionally, either during or after the copy process, the bytes after the first null on Card B + // are changed to null, which is presumably why the copy process ends up failing as Card A would + // then have two identical files. For reference, the Wii System Menu behaves exactly the same. + + // With all that in mind, even if it mismatches the comparison behavior of the BIOS, we treat + // m_filename as a nullterminated string for determining if two files identify as the same, as not + // doing so would cause more harm and confusion that good in practice. + + if (lhs.m_gamecode != rhs.m_gamecode) + return false; + if (lhs.m_makercode != rhs.m_makercode) + return false; + + for (size_t i = 0; i < lhs.m_filename.size(); ++i) + { + const u8 a = lhs.m_filename[i]; + const u8 b = rhs.m_filename[i]; + if (a == 0) + return b == 0; + if (a != b) + return false; + } + + return true; +} +} // namespace Memcard diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcardUtils.h b/Source/Core/Core/HW/GCMemcard/GCMemcardUtils.h new file mode 100644 index 0000000000..ebe0daf4fa --- /dev/null +++ b/Source/Core/Core/HW/GCMemcard/GCMemcardUtils.h @@ -0,0 +1,12 @@ +// Copyright 2020 Dolphin Emulator Project +// Licensed under GPLv2+ +// Refer to the license.txt file included. + +#pragma once + +namespace Memcard +{ +struct DEntry; + +bool HasSameIdentity(const DEntry& lhs, const DEntry& rhs); +} // namespace Memcard