Skip to content

Commit

Permalink
[UX Improvement] Catch save loading errors and notify user (#3979)
Browse files Browse the repository at this point in the history
* Add `SohModalWindow` and `SohModal`. Runs as window, always "visible", but not drawing if no popups are registered.

Adds error catching for save file corruption (malformed json) that renames the file in question to prevent future loading issues and uses `SohModalWindow` to inform the user of the error.

* Apply suggestions from code review

---------

Co-authored-by: briaguya <70942617+briaguya-ai@users.noreply.github.com>
  • Loading branch information
Malkierian and briaguya-ai committed Feb 29, 2024
1 parent 358dd47 commit b26f2b2
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 41 deletions.
100 changes: 59 additions & 41 deletions soh/soh/SaveManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <variables.h>
#include "soh/Enhancements/boss-rush/BossRush.h"
#include <libultraship/libultraship.h>
#include "SohGui.hpp"

#define NOGDI // avoid various windows defines that conflict with things in z64.h
#include <spdlog/spdlog.h>
Expand Down Expand Up @@ -1023,53 +1024,70 @@ void SaveManager::SaveGlobal() {
output << std::setw(4) << globalBlock << std::endl;
}


void SaveManager::LoadFile(int fileNum) {
SPDLOG_INFO("Load File - fileNum: {}", fileNum);
assert(std::filesystem::exists(GetFileName(fileNum)));
std::filesystem::path fileName = GetFileName(fileNum);
assert(std::filesystem::exists(fileName));
InitFile(false);

std::ifstream input(GetFileName(fileNum));

saveBlock = nlohmann::json::object();
input >> saveBlock;
if (!saveBlock.contains("version")) {
SPDLOG_ERROR("Save at " + GetFileName(fileNum).string() + " contains no version");
assert(false);
}
switch (saveBlock["version"].get<int>()) {
case 1:
for (auto& block : saveBlock["sections"].items()) {
int sectionVersion = block.value()["version"];
std::string sectionName = block.key();
if (!sectionLoadHandlers.contains(sectionName)) {
// Unloadable sections aren't necessarily errors, they are probably mods that were unloaded
// TODO report in a more noticeable manner
SPDLOG_WARN("Save " + GetFileName(fileNum).string() + " contains unloadable section " + sectionName);
continue;
}
SectionLoadHandler& handler = sectionLoadHandlers[sectionName];
if (!handler.contains(sectionVersion)) {
// A section that has a loader without a handler for the specific version means that the user has a mod
// at an earlier version than the save has. In this case, the user probably wants to load the save.
// Report the error so that the user can rectify the error.
// TODO report in a more noticeable manner
SPDLOG_ERROR("Save " + GetFileName(fileNum).string() + " contains section " + sectionName +
" with an unloadable version " + std::to_string(sectionVersion));
assert(false);
continue;
}
currentJsonContext = &block.value()["data"];
handler[sectionVersion]();
}
break;
default:
SPDLOG_ERROR("Unrecognized save version " + std::to_string(saveBlock["version"].get<int>()) + " in " +
GetFileName(fileNum).string());
std::ifstream input(fileName);

try {
saveBlock = nlohmann::json::object();
input >> saveBlock;
if (!saveBlock.contains("version")) {
SPDLOG_ERROR("Save at " + fileName.string() + " contains no version");
assert(false);
break;
}
switch (saveBlock["version"].get<int>()) {
case 1:
for (auto& block : saveBlock["sections"].items()) {
int sectionVersion = block.value()["version"];
std::string sectionName = block.key();
if (!sectionLoadHandlers.contains(sectionName)) {
// Unloadable sections aren't necessarily errors, they are probably mods that were unloaded
// TODO report in a more noticeable manner
SPDLOG_WARN("Save " + GetFileName(fileNum).string() + " contains unloadable section " +
sectionName);
continue;
}
SectionLoadHandler& handler = sectionLoadHandlers[sectionName];
if (!handler.contains(sectionVersion)) {
// A section that has a loader without a handler for the specific version means that the user
// has a mod at an earlier version than the save has. In this case, the user probably wants to
// load the save. Report the error so that the user can rectify the error.
// TODO report in a more noticeable manner
SPDLOG_ERROR("Save " + GetFileName(fileNum).string() + " contains section " + sectionName +
" with an unloadable version " + std::to_string(sectionVersion));
assert(false);
continue;
}
currentJsonContext = &block.value()["data"];
handler[sectionVersion]();
}
break;
default:
SPDLOG_ERROR("Unrecognized save version " + std::to_string(saveBlock["version"].get<int>()) + " in " +
GetFileName(fileNum).string());
assert(false);
break;
}
InitMeta(fileNum);
GameInteractor::Instance->ExecuteHooks<GameInteractor::OnLoadFile>(fileNum);
} catch (const std::exception& e) {
input.close();
std::filesystem::path newFile(LUS::Context::GetPathRelativeToAppDirectory("Save") + ("/file" + std::to_string(fileNum + 1) + ".bak"));
#if defined(__SWITCH__) || defined(__WIIU__)
copy_file(fileName.c_str(), newFile.c_str());
#else
std::filesystem::copy_file(fileName, newFile);
#endif

std::filesystem::remove(fileName);
SohGui::RegisterPopup("Error loading save file", "A problem occurred loading the save in slot " + std::to_string(fileNum + 1) + ".\nSave file corruption is suspected.\n" +
"The file has been renamed to prevent further issues.");
}
InitMeta(fileNum);
GameInteractor::Instance->ExecuteHooks<GameInteractor::OnLoadFile>(fileNum);
}

void SaveManager::ThreadPoolWait() {
Expand Down
9 changes: 9 additions & 0 deletions soh/soh/SohGui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ namespace SohGui {
std::shared_ptr<ItemTrackerSettingsWindow> mItemTrackerSettingsWindow;
std::shared_ptr<ItemTrackerWindow> mItemTrackerWindow;
std::shared_ptr<RandomizerSettingsWindow> mRandomizerSettingsWindow;
std::shared_ptr<SohModalWindow> mModalWindow;

void SetupGuiElements() {
auto gui = LUS::Context::GetInstance()->GetWindow()->GetGui();
Expand Down Expand Up @@ -183,9 +184,13 @@ namespace SohGui {
gui->AddGuiWindow(mItemTrackerSettingsWindow);
mRandomizerSettingsWindow = std::make_shared<RandomizerSettingsWindow>("gRandomizerSettingsEnabled", "Randomizer Settings");
gui->AddGuiWindow(mRandomizerSettingsWindow);
mModalWindow = std::make_shared<SohModalWindow>("gOpenWindows.modalWindowEnabled", "Modal Window");
gui->AddGuiWindow(mModalWindow);
mModalWindow->Show();
}

void Destroy() {
mModalWindow = nullptr;
mRandomizerSettingsWindow = nullptr;
mItemTrackerWindow = nullptr;
mItemTrackerSettingsWindow = nullptr;
Expand All @@ -205,4 +210,8 @@ namespace SohGui {
mConsoleWindow = nullptr;
mSohMenuBar = nullptr;
}

void RegisterPopup(std::string title, std::string message, std::string button1, std::string button2, std::function<void()> button1callback, std::function<void()> button2callback) {
mModalWindow->RegisterPopup(title, message, button1, button2, button1callback, button2callback);
}
}
2 changes: 2 additions & 0 deletions soh/soh/SohGui.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "Enhancements/randomizer/randomizer_entrance_tracker.h"
#include "Enhancements/randomizer/randomizer_item_tracker.h"
#include "Enhancements/randomizer/randomizer_settings_window.h"
#include "SohModals.h"

#ifdef __cplusplus
extern "C" {
Expand All @@ -37,6 +38,7 @@ namespace SohGui {
void SetupGuiElements();
void Draw();
void Destroy();
void RegisterPopup(std::string title, std::string message, std::string button1 = "OK", std::string button2 = "", std::function<void()> button1callback = nullptr, std::function<void()> button2callback = nullptr);
}

#endif /* SohGui_hpp */
54 changes: 54 additions & 0 deletions soh/soh/SohModals.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
#include "SohModals.h"
#include "ImGui/imgui.h"
#include <vector>
#include <string>
#include <libultraship/bridge.h>
#include <libultraship/libultraship.h>
#include "UIWidgets.hpp"
#include "OTRGlobals.h"
#include "z64.h"

extern "C" PlayState* gPlayState;
struct SohModal {
std::string title_;
std::string message_;
std::string button1_;
std::string button2_;
std::function<void()> button1callback_;
std::function<void()> button2callback_;
};
std::vector<SohModal> modals;

void SohModalWindow::DrawElement() {
if (modals.size() > 0) {
SohModal curModal = modals.at(0);
if (!ImGui::IsPopupOpen(curModal.title_.c_str())) {
ImGui::OpenPopup(curModal.title_.c_str());
}
if (ImGui::BeginPopupModal(curModal.title_.c_str(), NULL, ImGuiWindowFlags_AlwaysAutoResize | ImGuiWindowFlags_NoResize | ImGuiWindowFlags_NoMove | ImGuiWindowFlags_NoScrollbar | ImGuiWindowFlags_NoSavedSettings)) {
ImGui::Text(curModal.message_.c_str());
if (ImGui::Button(curModal.button1_.c_str())) {
if (curModal.button1callback_ != nullptr) {
curModal.button1callback_();
}
ImGui::CloseCurrentPopup();
modals.erase(modals.begin());
}
ImGui::SameLine();
if (curModal.button2_ != "") {
if (ImGui::Button(curModal.button2_.c_str())) {
if (curModal.button2callback_ != nullptr) {
curModal.button2callback_();
}
ImGui::CloseCurrentPopup();
modals.erase(modals.begin());
}
}
}
ImGui::EndPopup();
}
}

void SohModalWindow::RegisterPopup(std::string title, std::string message, std::string button1, std::string button2, std::function<void()> button1callback, std::function<void()> button2callback) {
modals.push_back({ title, message, button1, button2, button1callback, button2callback });
}
15 changes: 15 additions & 0 deletions soh/soh/SohModals.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#pragma once

#include <libultraship/libultraship.h>
#include "window/gui/GuiMenuBar.h"
#include "window/gui/GuiElement.h"

class SohModalWindow : public LUS::GuiWindow {
public:
using LUS::GuiWindow::GuiWindow;

void InitElement() override {};
void DrawElement() override;
void UpdateElement() override {};
void RegisterPopup(std::string title, std::string message, std::string button1 = "OK", std::string button2 = "", std::function<void()> button1callback = nullptr, std::function<void()> button2callback = nullptr);
};

0 comments on commit b26f2b2

Please sign in to comment.