Use WriteFileSafely more (#7317)

Include config version check on speedrun settings

soh/soh/Enhancements/Presets/Presets.cpp

@@ -8,6 +8,7 @@
 #include <ship/resource/type/Json.h>
 #include <ship/Context.h>
 #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();
 }
 

soh/soh/Enhancements/controls/Mapper.cpp

@@ -20,6 +20,7 @@
 #include <ship/window/gui/Gui.h>
 
 #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<std::string> ReadUserMappingLines() {
 
 static bool WriteUserMappingLines(const std::vector<std::string>& 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) {

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) {

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();
 

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 <fstream>
 #include <filesystem>
@@ -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.

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<RandomizerHint> 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<std::mutex> 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) {

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();