From 7cdf69b20bc7743e445eee77728afd8f755a583d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= <159546+serprex@users.noreply.github.com> Date: Mon, 5 Oct 2026 14:38:23 +0000 Subject: [PATCH] Use WriteFileSafely more (#7317) Include config version check on speedrun settings --- soh/soh/Enhancements/Presets/Presets.cpp | 14 +++--- soh/soh/Enhancements/controls/Mapper.cpp | 32 +++----------- .../Enhancements/randomizer/Plandomizer.cpp | 7 +-- soh/soh/Enhancements/speedrun/Speedrun.cpp | 43 +++++++++++++------ .../timesplits/TimeSplitsActions.cpp | 13 ++---- soh/soh/SaveManager.cpp | 17 +++----- soh/soh/SaveManager.h | 3 +- 7 files changed, 58 insertions(+), 71 deletions(-) diff --git a/soh/soh/Enhancements/Presets/Presets.cpp b/soh/soh/Enhancements/Presets/Presets.cpp index b9daf1c3b6..952a900429 100644 --- a/soh/soh/Enhancements/Presets/Presets.cpp +++ b/soh/soh/Enhancements/Presets/Presets.cpp @@ -8,6 +8,7 @@ #include #include #include "soh/OTRGlobals.h" +#include "soh/SaveManager.h" #include "soh/util.h" #include "soh/SohGui/MenuTypes.h" #include "soh/SohGui/SohMenu.h" @@ -332,6 +333,10 @@ void LoadPresets() { } if (fs::exists(presetFolder)) { for (auto const& preset : fs::directory_iterator(presetFolder)) { + // Skip leftover temp files from an interrupted save + if (preset.path().extension() != ".json") { + continue; + } try { std::ifstream ifs(preset.path()); if (auto json = nlohmann::json::parse(ifs); !json.contains("presetName")) { @@ -372,14 +377,11 @@ void SavePreset(std::string& presetName) { presets[presetName].presetValues["fileType"] = FILE_TYPE_PRESET; presets[presetName].presetValues["ConfigVersion"] = SOH::GetLatestConfigVersion(); - std::ofstream file(FormatPresetPath(presetName)); - if (!file.is_open()) { - spdlog::error("Failed to save preset '{}': Could not create file", presetName); + std::string presetPath = FormatPresetPath(presetName); + if (!SaveManager::WriteFileSafely(presetPath, presets[presetName].presetValues.dump(4))) { + spdlog::error("Failed to save preset '{}'", presetName); return; } - - file << presets[presetName].presetValues.dump(4); - file.close(); LoadPresets(); } diff --git a/soh/soh/Enhancements/controls/Mapper.cpp b/soh/soh/Enhancements/controls/Mapper.cpp index 0621833f2c..823270a376 100644 --- a/soh/soh/Enhancements/controls/Mapper.cpp +++ b/soh/soh/Enhancements/controls/Mapper.cpp @@ -20,6 +20,7 @@ #include #include "soh/OTRGlobals.h" +#include "soh/SaveManager.h" #include "soh/SohGui/SohGui.hpp" #include "soh/SohGui/SohMenu.h" #include "soh/SohGui/UIWidgets.hpp" @@ -758,33 +759,12 @@ static std::vector ReadUserMappingLines() { static bool WriteUserMappingLines(const std::vector& lines) { const std::string path = GetUserMappingsPath(); - const std::string tempPath = path + ".tmp"; - - { - std::ofstream file(tempPath, std::ios::binary | std::ios::trunc); - if (!file.is_open()) { - SPDLOG_ERROR("Failed to open \"{}\" for writing", tempPath); - return false; - } - file << "# SoH user gamepad mappings, written by the built-in gamepad mapper.\n"; - file << "# Loaded on top of gamecontrollerdb.txt, so entries here win.\n"; - for (const auto& line : lines) { - file << line << "\n"; - } - if (!file.good()) { - SPDLOG_ERROR("Failed while writing \"{}\"", tempPath); - return false; - } + std::string contents = "# SoH user gamepad mappings, written by the built-in gamepad mapper.\n" + "# Loaded on top of gamecontrollerdb.txt, so entries here win.\n"; + for (const auto& line : lines) { + contents += line + "\n"; } - - std::error_code error; - std::filesystem::rename(tempPath, path, error); - if (error) { - SPDLOG_ERROR("Failed to move \"{}\" into place: {}", tempPath, error.message()); - std::filesystem::remove(tempPath, error); - return false; - } - return true; + return SaveManager::WriteFileSafely(path, contents); } std::string FindUserMapping(const std::string& guid) { diff --git a/soh/soh/Enhancements/randomizer/Plandomizer.cpp b/soh/soh/Enhancements/randomizer/Plandomizer.cpp index ebc5a8638a..f67cb59934 100644 --- a/soh/soh/Enhancements/randomizer/Plandomizer.cpp +++ b/soh/soh/Enhancements/randomizer/Plandomizer.cpp @@ -11,6 +11,7 @@ #include "soh/Notification/Notification.h" #include "soh/Enhancements/randomizer/3drando/hints.hpp" #include "soh/OTRGlobals.h" +#include "soh/SaveManager.h" #include "soh/SohGui/ImGuiUtils.h" #include "soh/Enhancements/randomizer/logic.h" #include "soh/Enhancements/randomizer/rando_hash.h" @@ -553,11 +554,7 @@ void PlandomizerSaveSpoilerLog() { } } - std::ofstream outputFile(filename); - if (outputFile.is_open()) { - outputFile << spoilerSave.dump(4); - outputFile.close(); - } + SaveManager::WriteFileSafely(filename, spoilerSave.dump(4)); } void PlandomizerLoadSpoilerLog(std::string logFile) { diff --git a/soh/soh/Enhancements/speedrun/Speedrun.cpp b/soh/soh/Enhancements/speedrun/Speedrun.cpp index 68ae98be5e..6ee47ebb43 100644 --- a/soh/soh/Enhancements/speedrun/Speedrun.cpp +++ b/soh/soh/Enhancements/speedrun/Speedrun.cpp @@ -126,16 +126,10 @@ static void OverlayExemptSettings(nlohmann::json& blocks, const nlohmann::json& } // Saves the player's settings before a speedrun file replaces them. An existing backup is kept, since it means the -// last run never restored it and it still holds the player's real settings. -static void BackupSettings() { +// last run never restored it and it still holds the player's real settings. Returns false if nothing is backed up. +static bool BackupSettings() { if (fs::exists(GetBackupPath())) { - return; - } - - std::ofstream file(GetBackupPath()); - if (!file.is_open()) { - SPDLOG_ERROR("Speedrun: could not write settings backup"); - return; + return true; } // Cosmetics are backed up with the owned blocks, since a run clears them too. @@ -148,7 +142,11 @@ static void BackupSettings() { nlohmann::json backup; backup["blocks"] = blocks; - file << backup.dump(4); + if (!SaveManager::WriteFileSafely(GetBackupPath(), backup.dump(4))) { + SPDLOG_ERROR("Speedrun: could not write settings backup"); + return false; + } + return true; } static void LockMenu() { @@ -229,7 +227,11 @@ static void RestoreSettings() { nlohmann::json backup = nlohmann::json::parse(file); file.close(); SetOwnedBlocks(backup.value("blocks", nlohmann::json::object())); - } catch (const std::exception& e) { SPDLOG_ERROR("Speedrun: could not read settings backup: {}", e.what()); } + } catch (const std::exception& e) { + // Keep the file, it may be the only copy of the player's settings. + SPDLOG_ERROR("Speedrun: could not read settings backup: {}", e.what()); + return; + } fs::remove(GetBackupPath()); } @@ -390,7 +392,13 @@ static void LoadSaveSection() { return; } + // Don't touch the player's settings unless they can be put back. + if (!BackupSettings()) { + return; + } + // Blocks missing from the file are cleared rather than left as the player's. + nlohmann::json custom = GetOwnedBlocks(); nlohmann::json blocks = sSettings; for (const char* block : sOwnedBlocks) { if (!blocks.contains(block)) { @@ -398,12 +406,21 @@ static void LoadSaveSection() { } } - OverlayExemptSettings(blocks, GetOwnedBlocks()); + // Settings from an older build are migrated after they're live, since the updaters work on cvars. The file keeps + // its old settings and version, so the hash stays the same on every load. + if (sConfigVersion < SOH::GetLatestConfigVersion()) { + SetOwnedBlocks(blocks); + SOH::RunVersionUpdatesFrom(sConfigVersion); + // Updaters change cvars, not the config json GetOwnedBlocks reads. + Ship::Context::GetRawInstance()->GetConsoleVariables()->Save(); + blocks = GetOwnedBlocks(); + } + + OverlayExemptSettings(blocks, custom); // No cosmetics during a run. Some scale Link or his sword, which changes his hitbox and reach. blocks[CVAR_PREFIX_COSMETIC] = nlohmann::json::object(); - BackupSettings(); SetOwnedBlocks(blocks); sSettingsHash = HashSettings(); diff --git a/soh/soh/Enhancements/timesplits/TimeSplitsActions.cpp b/soh/soh/Enhancements/timesplits/TimeSplitsActions.cpp index e153b38e3b..d9735f7eeb 100644 --- a/soh/soh/Enhancements/timesplits/TimeSplitsActions.cpp +++ b/soh/soh/Enhancements/timesplits/TimeSplitsActions.cpp @@ -3,6 +3,7 @@ #include "soh/SohGui/UIWidgets.hpp" #include "fast/Fast3dGui.h" #include "soh/Enhancements/game-interactor/GameInteractor.h" +#include "soh/SaveManager.h" #include "soh/ShipInit.hpp" #include #include @@ -307,11 +308,7 @@ void SplitSaveFileAction(uint32_t action, std::string listName) { } saveFile[listName] = listArray; - std::ofstream outputFile(filename); - if (outputFile.is_open()) { - outputFile << saveFile.dump(4); - outputFile.close(); - } + SaveManager::WriteFileSafely(filename, saveFile.dump(4)); } if (action == SPLIT_LOAD) { @@ -379,10 +376,8 @@ void SplitSaveFileAction(uint32_t action, std::string listName) { void RegisterTimesplits() { if (!std::filesystem::exists(Ship::Context::GetPathRelativeToAppDirectory("SoHTimeSplitData.json"))) { - json initFile; - std::ofstream file(Ship::Context::GetPathRelativeToAppDirectory("SoHTimeSplitData.json")); - file << initFile.dump(4); - file.close(); + std::string initPath = Ship::Context::GetPathRelativeToAppDirectory("SoHTimeSplitData.json"); + SaveManager::WriteFileSafely(initPath, json().dump(4)); } // Handles Converting any old save data into the new format. diff --git a/soh/soh/SaveManager.cpp b/soh/soh/SaveManager.cpp index f9464b5b72..5b55b033b1 100644 --- a/soh/soh/SaveManager.cpp +++ b/soh/soh/SaveManager.cpp @@ -60,9 +60,9 @@ int copy_file(const char* src, const char* dst) { } #endif -// Write to temp file and only swap once fully on disk -static bool WriteFileSafely(const std::filesystem::path& fileName, const std::filesystem::path& tempFile, - const std::string& contents) { +bool SaveManager::WriteFileSafely(const std::filesystem::path& fileName, const std::string& contents) { + std::filesystem::path tempFile = fileName; + tempFile += ".temp"; #ifdef _WIN32 FILE* w = _wfopen(tempFile.c_str(), L"wb"); #else @@ -128,11 +128,6 @@ std::filesystem::path SaveManager::GetFileName(int fileNum) { return sSavePath / ("file" + std::to_string(fileNum + 1) + ".sav"); } -std::filesystem::path SaveManager::GetFileTempName(int fileNum) { - const std::filesystem::path sSavePath(Ship::Context::GetPathRelativeToAppDirectory("Save")); - return sSavePath / ("file" + std::to_string(fileNum + 1) + ".temp"); -} - std::vector Rando::StaticData::oldVerHintOrder{ RH_COLOSSUS_GOSSIP_STONE, RH_DMC_GOSSIP_STONE, @@ -664,7 +659,7 @@ void SaveManager::StartupCheckAndInitMeta(int fileNum) { sections.erase("randomizer"); metaSaveBlock["fileType"] = FILE_TYPE_SAVE_VANILLA; std::lock_guard guard(saveMtx); - WriteFileSafely(fileName, GetFileTempName(fileNum), metaSaveBlock.dump(1)); + WriteFileSafely(fileName, metaSaveBlock.dump(1)); } else { nlohmann::json& statsBlock = sections["sohStats"]["data"]; s16 major = statsBlock.value("buildVersionMajor", 0); @@ -1299,7 +1294,7 @@ void SaveManager::SaveFileThreaded(int fileNum, const SaveContext& saveContext, svi.func(saveContext, sectionID, false); } - if (!WriteFileSafely(GetFileName(fileNum), GetFileTempName(fileNum), saveBlock.dump(1))) { + if (!WriteFileSafely(GetFileName(fileNum), saveBlock.dump(1))) { return; } @@ -1344,7 +1339,7 @@ void SaveManager::SaveGlobal() { const std::filesystem::path sSavePath(Ship::Context::GetPathRelativeToAppDirectory("Save")); const std::filesystem::path sGlobalPath = sSavePath / std::string("global.sav"); - WriteFileSafely(sGlobalPath, sSavePath / std::string("global.temp"), globalBlock.dump(1)); + WriteFileSafely(sGlobalPath, globalBlock.dump(1)); } void SaveManager::LoadFile(int fileNum) { diff --git a/soh/soh/SaveManager.h b/soh/soh/SaveManager.h index 03e9f68310..aea8b5f774 100644 --- a/soh/soh/SaveManager.h +++ b/soh/soh/SaveManager.h @@ -63,6 +63,8 @@ class SaveManager { static void WriteSaveFile(const std::filesystem::path& savePath, uintptr_t addr, void* dramAddr, size_t size); static void ReadSaveFile(std::filesystem::path savePath, uintptr_t addr, void* dramAddr, size_t size); + // Write to fileName + ".temp" and only swap once fully on disk + static bool WriteFileSafely(const std::filesystem::path& fileName, const std::string& contents); using InitFunc = void (*)(bool isDebug); using LoadFunc = void (*)(); @@ -154,7 +156,6 @@ class SaveManager { private: std::filesystem::path GetFileName(int fileNum); - std::filesystem::path GetFileTempName(int fileNum); nlohmann::json saveBlock; void ConvertFromUnversioned();