Merge pull request #4655 from lioncash/internal2

patch_manager: Minor cleanup

2 files changed, 95 insertions, 87 deletions

diff --git a/src/core/file_sys/patch_manager.cpp b/src/core/file_sys/patch_manager.cpp
index c228d253e..87c354a43 100644
--- a/src/core/file_sys/patch_manager.cpp
+++ b/src/core/file_sys/patch_manager.cpp
@@ -27,6 +27,7 @@
 #include "core/settings.h"
 
 namespace FileSys {
+namespace {
 
 constexpr u64 SINGLE_BYTE_MODULUS = 0x100;
 constexpr u64 DLC_BASE_TITLE_ID_MASK = 0xFFFFFFFFFFFFE000;
@@ -36,19 +37,28 @@ constexpr std::array<const char*, 14> EXEFS_FILE_NAMES{
     "subsdk3", "subsdk4",   "subsdk5", "subsdk6", "subsdk7", "subsdk8", "subsdk9",
 };
 
-std::string FormatTitleVersion(u32 version, TitleVersionFormat format) {
+enum class TitleVersionFormat : u8 {
+    ThreeElements, ///< vX.Y.Z
+    FourElements,  ///< vX.Y.Z.W
+};
+
+std::string FormatTitleVersion(u32 version,
+                               TitleVersionFormat format = TitleVersionFormat::ThreeElements) {
     std::array<u8, sizeof(u32)> bytes{};
-    bytes[0] = version % SINGLE_BYTE_MODULUS;
+    bytes[0] = static_cast<u8>(version % SINGLE_BYTE_MODULUS);
     for (std::size_t i = 1; i < bytes.size(); ++i) {
         version /= SINGLE_BYTE_MODULUS;
-        bytes[i] = version % SINGLE_BYTE_MODULUS;
+        bytes[i] = static_cast<u8>(version % SINGLE_BYTE_MODULUS);
     }
 
-    if (format == TitleVersionFormat::FourElements)
+    if (format == TitleVersionFormat::FourElements) {
         return fmt::format("v{}.{}.{}.{}", bytes[3], bytes[2], bytes[1], bytes[0]);
+    }
     return fmt::format("v{}.{}.{}", bytes[3], bytes[2], bytes[1]);
 }
 
+// Returns a directory with name matching name case-insensitive. Returns nullptr if directory
+// doesn't have a directory with name.
 VirtualDir FindSubdirectoryCaseless(const VirtualDir dir, std::string_view name) {
 #ifdef _WIN32
     return dir->GetSubdirectory(name);
@@ -65,6 +75,45 @@ VirtualDir FindSubdirectoryCaseless(const VirtualDir dir, std::string_view name)
 #endif
 }
 
+std::optional<std::vector<Core::Memory::CheatEntry>> ReadCheatFileFromFolder(
+    const Core::System& system, u64 title_id, const PatchManager::BuildID& build_id_,
+    const VirtualDir& base_path, bool upper) {
+    const auto build_id_raw = Common::HexToString(build_id_, upper);
+    const auto build_id = build_id_raw.substr(0, sizeof(u64) * 2);
+    const auto file = base_path->GetFile(fmt::format("{}.txt", build_id));
+
+    if (file == nullptr) {
+        LOG_INFO(Common_Filesystem, "No cheats file found for title_id={:016X}, build_id={}",
+                 title_id, build_id);
+        return std::nullopt;
+    }
+
+    std::vector<u8> data(file->GetSize());
+    if (file->Read(data.data(), data.size()) != data.size()) {
+        LOG_INFO(Common_Filesystem, "Failed to read cheats file for title_id={:016X}, build_id={}",
+                 title_id, build_id);
+        return std::nullopt;
+    }
+
+    Core::Memory::TextCheatParser parser;
+    return parser.Parse(system,
+                        std::string_view(reinterpret_cast<const char*>(data.data()), data.size()));
+}
+
+void AppendCommaIfNotEmpty(std::string& to, std::string_view with) {
+    if (to.empty()) {
+        to += with;
+    } else {
+        to += ", ";
+        to += with;
+    }
+}
+
+bool IsDirValidAndNonEmpty(const VirtualDir& dir) {
+    return dir != nullptr && (!dir->GetFiles().empty() || !dir->GetSubdirectories().empty());
+}
+} // Anonymous namespace
+
 PatchManager::PatchManager(u64 title_id) : title_id(title_id) {}
 
 PatchManager::~PatchManager() = default;
@@ -245,7 +294,7 @@ std::vector<u8> PatchManager::PatchNSO(const std::vector<u8>& nso, const std::st
     return out;
 }
 
-bool PatchManager::HasNSOPatch(const std::array<u8, 32>& build_id_) const {
+bool PatchManager::HasNSOPatch(const BuildID& build_id_) const {
     const auto build_id_raw = Common::HexToString(build_id_);
     const auto build_id = build_id_raw.substr(0, build_id_raw.find_last_not_of('0') + 1);
 
@@ -265,36 +314,8 @@ bool PatchManager::HasNSOPatch(const std::array<u8, 32>& build_id_) const {
     return !CollectPatches(patch_dirs, build_id).empty();
 }
 
-namespace {
-std::optional<std::vector<Core::Memory::CheatEntry>> ReadCheatFileFromFolder(
-    const Core::System& system, u64 title_id, const std::array<u8, 0x20>& build_id_,
-    const VirtualDir& base_path, bool upper) {
-    const auto build_id_raw = Common::HexToString(build_id_, upper);
-    const auto build_id = build_id_raw.substr(0, sizeof(u64) * 2);
-    const auto file = base_path->GetFile(fmt::format("{}.txt", build_id));
-
-    if (file == nullptr) {
-        LOG_INFO(Common_Filesystem, "No cheats file found for title_id={:016X}, build_id={}",
-                 title_id, build_id);
-        return std::nullopt;
-    }
-
-    std::vector<u8> data(file->GetSize());
-    if (file->Read(data.data(), data.size()) != data.size()) {
-        LOG_INFO(Common_Filesystem, "Failed to read cheats file for title_id={:016X}, build_id={}",
-                 title_id, build_id);
-        return std::nullopt;
-    }
-
-    Core::Memory::TextCheatParser parser;
-    return parser.Parse(system,
-                        std::string_view(reinterpret_cast<const char*>(data.data()), data.size()));
-}
-
-} // Anonymous namespace
-
 std::vector<Core::Memory::CheatEntry> PatchManager::CreateCheatList(
-    const Core::System& system, const std::array<u8, 32>& build_id_) const {
+    const Core::System& system, const BuildID& build_id_) const {
     const auto load_dir = system.GetFileSystemController().GetModificationLoadRoot(title_id);
     if (load_dir == nullptr) {
         LOG_ERROR(Loader, "Cannot load mods for invalid title_id={:016X}", title_id);
@@ -435,21 +456,11 @@ VirtualFile PatchManager::PatchRomFS(VirtualFile romfs, u64 ivfc_offset, Content
     return romfs;
 }
 
-static void AppendCommaIfNotEmpty(std::string& to, const std::string& with) {
-    if (to.empty())
-        to += with;
-    else
-        to += ", " + with;
-}
-
-static bool IsDirValidAndNonEmpty(const VirtualDir& dir) {
-    return dir != nullptr && (!dir->GetFiles().empty() || !dir->GetSubdirectories().empty());
-}
-
-std::map<std::string, std::string, std::less<>> PatchManager::GetPatchVersionNames(
-    VirtualFile update_raw) const {
-    if (title_id == 0)
+PatchManager::PatchVersionNames PatchManager::GetPatchVersionNames(VirtualFile update_raw) const {
+    if (title_id == 0) {
         return {};
+    }
+
     std::map<std::string, std::string, std::less<>> out;
     const auto& installed = Core::System::GetInstance().GetContentProvider();
     const auto& disabled = Settings::values.disabled_addons[title_id];
@@ -472,8 +483,7 @@ std::map<std::string, std::string, std::less<>> PatchManager::GetPatchVersionNam
             if (meta_ver.value_or(0) == 0) {
                 out.insert_or_assign(update_label, "");
             } else {
-                out.insert_or_assign(
-                    update_label, FormatTitleVersion(*meta_ver, TitleVersionFormat::ThreeElements));
+                out.insert_or_assign(update_label, FormatTitleVersion(*meta_ver));
             }
         } else if (update_raw != nullptr) {
             out.insert_or_assign(update_label, "PACKED");
@@ -562,40 +572,46 @@ std::optional<u32> PatchManager::GetGameVersion() const {
     return installed.GetEntryVersion(title_id);
 }
 
-std::pair<std::unique_ptr<NACP>, VirtualFile> PatchManager::GetControlMetadata() const {
+PatchManager::Metadata PatchManager::GetControlMetadata() const {
     const auto& installed = Core::System::GetInstance().GetContentProvider();
 
     const auto base_control_nca = installed.GetEntry(title_id, ContentRecordType::Control);
-    if (base_control_nca == nullptr)
+    if (base_control_nca == nullptr) {
         return {};
+    }
 
     return ParseControlNCA(*base_control_nca);
 }
 
-std::pair<std::unique_ptr<NACP>, VirtualFile> PatchManager::ParseControlNCA(const NCA& nca) const {
+PatchManager::Metadata PatchManager::ParseControlNCA(const NCA& nca) const {
     const auto base_romfs = nca.GetRomFS();
-    if (base_romfs == nullptr)
+    if (base_romfs == nullptr) {
         return {};
+    }
 
     const auto romfs = PatchRomFS(base_romfs, nca.GetBaseIVFCOffset(), ContentRecordType::Control);
-    if (romfs == nullptr)
+    if (romfs == nullptr) {
         return {};
+    }
 
     const auto extracted = ExtractRomFS(romfs);
-    if (extracted == nullptr)
+    if (extracted == nullptr) {
         return {};
+    }
 
     auto nacp_file = extracted->GetFile("control.nacp");
-    if (nacp_file == nullptr)
+    if (nacp_file == nullptr) {
         nacp_file = extracted->GetFile("Control.nacp");
+    }
 
     auto nacp = nacp_file == nullptr ? nullptr : std::make_unique<NACP>(nacp_file);
 
     VirtualFile icon_file;
     for (const auto& language : FileSys::LANGUAGE_NAMES) {
-        icon_file = extracted->GetFile("icon_" + std::string(language) + ".dat");
-        if (icon_file != nullptr)
+        icon_file = extracted->GetFile(std::string("icon_").append(language).append(".dat"));
+        if (icon_file != nullptr) {
             break;
+        }
     }
 
     return {std::move(nacp), icon_file};
diff --git a/src/core/file_sys/patch_manager.h b/src/core/file_sys/patch_manager.h
index 532f4995f..1f28c6241 100644
--- a/src/core/file_sys/patch_manager.h
+++ b/src/core/file_sys/patch_manager.h
@@ -22,70 +22,62 @@ namespace FileSys {
 class NCA;
 class NACP;
 
-enum class TitleVersionFormat : u8 {
-    ThreeElements, ///< vX.Y.Z
-    FourElements,  ///< vX.Y.Z.W
-};
-
-std::string FormatTitleVersion(u32 version,
-                               TitleVersionFormat format = TitleVersionFormat::ThreeElements);
-
-// Returns a directory with name matching name case-insensitive. Returns nullptr if directory
-// doesn't have a directory with name.
-VirtualDir FindSubdirectoryCaseless(VirtualDir dir, std::string_view name);
-
 // A centralized class to manage patches to games.
 class PatchManager {
 public:
+    using BuildID = std::array<u8, 0x20>;
+    using Metadata = std::pair<std::unique_ptr<NACP>, VirtualFile>;
+    using PatchVersionNames = std::map<std::string, std::string, std::less<>>;
+
     explicit PatchManager(u64 title_id);
     ~PatchManager();
 
-    u64 GetTitleID() const;
+    [[nodiscard]] u64 GetTitleID() const;
 
     // Currently tracked ExeFS patches:
     // - Game Updates
-    VirtualDir PatchExeFS(VirtualDir exefs) const;
+    [[nodiscard]] VirtualDir PatchExeFS(VirtualDir exefs) const;
 
     // Currently tracked NSO patches:
     // - IPS
     // - IPSwitch
-    std::vector<u8> PatchNSO(const std::vector<u8>& nso, const std::string& name) const;
+    [[nodiscard]] std::vector<u8> PatchNSO(const std::vector<u8>& nso,
+                                           const std::string& name) const;
 
     // Checks to see if PatchNSO() will have any effect given the NSO's build ID.
     // Used to prevent expensive copies in NSO loader.
-    bool HasNSOPatch(const std::array<u8, 0x20>& build_id) const;
+    [[nodiscard]] bool HasNSOPatch(const BuildID& build_id) const;
 
     // Creates a CheatList object with all
-    std::vector<Core::Memory::CheatEntry> CreateCheatList(
-        const Core::System& system, const std::array<u8, 0x20>& build_id) const;
+    [[nodiscard]] std::vector<Core::Memory::CheatEntry> CreateCheatList(
+        const Core::System& system, const BuildID& build_id) const;
 
     // Currently tracked RomFS patches:
     // - Game Updates
     // - LayeredFS
-    VirtualFile PatchRomFS(VirtualFile base, u64 ivfc_offset,
-                           ContentRecordType type = ContentRecordType::Program,
-                           VirtualFile update_raw = nullptr) const;
+    [[nodiscard]] VirtualFile PatchRomFS(VirtualFile base, u64 ivfc_offset,
+                                         ContentRecordType type = ContentRecordType::Program,
+                                         VirtualFile update_raw = nullptr) const;
 
     // Returns a vector of pairs between patch names and patch versions.
     // i.e. Update 3.2.2 will return {"Update", "3.2.2"}
-    std::map<std::string, std::string, std::less<>> GetPatchVersionNames(
-        VirtualFile update_raw = nullptr) const;
+    [[nodiscard]] PatchVersionNames GetPatchVersionNames(VirtualFile update_raw = nullptr) const;
 
     // If the game update exists, returns the u32 version field in its Meta-type NCA. If that fails,
     // it will fallback to the Meta-type NCA of the base game. If that fails, the result will be
     // std::nullopt
-    std::optional<u32> GetGameVersion() const;
+    [[nodiscard]] std::optional<u32> GetGameVersion() const;
 
     // Given title_id of the program, attempts to get the control data of the update and parse
     // it, falling back to the base control data.
-    std::pair<std::unique_ptr<NACP>, VirtualFile> GetControlMetadata() const;
+    [[nodiscard]] Metadata GetControlMetadata() const;
 
     // Version of GetControlMetadata that takes an arbitrary NCA
-    std::pair<std::unique_ptr<NACP>, VirtualFile> ParseControlNCA(const NCA& nca) const;
+    [[nodiscard]] Metadata ParseControlNCA(const NCA& nca) const;
 
 private:
-    std::vector<VirtualFile> CollectPatches(const std::vector<VirtualDir>& patch_dirs,
-                                            const std::string& build_id) const;
+    [[nodiscard]] std::vector<VirtualFile> CollectPatches(const std::vector<VirtualDir>& patch_dirs,
+                                                          const std::string& build_id) const;
 
     u64 title_id;
 };