From 0bcb779acbc4366c0533027d59a9cd5da9d89991 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Mon, 1 Aug 2022 00:05:03 +0200 Subject: [PATCH 1/9] Present more info to user in case of save failure --- src/openrct2/park/ParkFile.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/openrct2/park/ParkFile.cpp b/src/openrct2/park/ParkFile.cpp index 2273bb1145..592d1b4a73 100644 --- a/src/openrct2/park/ParkFile.cpp +++ b/src/openrct2/park/ParkFile.cpp @@ -2312,6 +2312,10 @@ int32_t scenario_save(u8string_view path, int32_t flags) catch (const std::exception& e) { log_error(e.what()); + + Formatter ft; + ft.Add(e.what()); + context_show_error(STR_FILE_DIALOG_TITLE_SAVE_SCENARIO, STR_STRING, ft); } gfx_invalidate_screen(); From a6e84fab5c4c2c94ca8bcf6510f056ad8fe8dba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Tue, 2 Aug 2022 08:28:20 +0200 Subject: [PATCH 2/9] Add a message box with buttons to UiContext --- src/openrct2-ui/UiContext.cpp | 23 +++++++++++++++++++++++ src/openrct2/ui/DummyUiContext.cpp | 4 ++++ src/openrct2/ui/UiContext.h | 2 ++ 3 files changed, 29 insertions(+) diff --git a/src/openrct2-ui/UiContext.cpp b/src/openrct2-ui/UiContext.cpp index 4d02484f1e..17e84abfc8 100644 --- a/src/openrct2-ui/UiContext.cpp +++ b/src/openrct2-ui/UiContext.cpp @@ -640,6 +640,29 @@ public: _platformUiContext->ShowMessageBox(_window, message); } + int ShowMessageBox(const std::string& title, const std::string& message, const std::vector& options) override + { + auto message_box_button_data = std::make_unique(options.size()); + for (size_t i = 0; i < options.size(); i++) + { + message_box_button_data[i].buttonid = static_cast(i); + message_box_button_data[i].text = options[i].c_str(); + } + + SDL_MessageBoxData message_box_data{}; + message_box_data.window = _window; + message_box_data.title = title.c_str(); + message_box_data.message = message.c_str(); + message_box_data.numbuttons = static_cast(options.size()); + message_box_data.buttons = message_box_button_data.get(); + + int buttonid{}; + + SDL_ShowMessageBox(&message_box_data, &buttonid); + + return buttonid; + } + bool HasMenuSupport() override { return _platformUiContext->HasMenuSupport(); diff --git a/src/openrct2/ui/DummyUiContext.cpp b/src/openrct2/ui/DummyUiContext.cpp index aa573ae300..77aa929b4e 100644 --- a/src/openrct2/ui/DummyUiContext.cpp +++ b/src/openrct2/ui/DummyUiContext.cpp @@ -90,6 +90,10 @@ namespace OpenRCT2::Ui void ShowMessageBox(const std::string& /*message*/) override { } + int ShowMessageBox(const std::string&, const std::string&, const std::vector&) override + { + return 0; + } bool HasMenuSupport() override { return false; diff --git a/src/openrct2/ui/UiContext.h b/src/openrct2/ui/UiContext.h index 3660fc42f4..cac084e8da 100644 --- a/src/openrct2/ui/UiContext.h +++ b/src/openrct2/ui/UiContext.h @@ -120,6 +120,8 @@ namespace OpenRCT2 virtual void TriggerResize() abstract; virtual void ShowMessageBox(const std::string& message) abstract; + virtual int ShowMessageBox( + const std::string& title, const std::string& message, const std::vector& options) abstract; virtual bool HasMenuSupport() abstract; // Creates a menu with a series of options, returns the index of the selected option From 15fad6b979f541606cb11159d3018f9e356bcb10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Tue, 2 Aug 2022 08:29:12 +0200 Subject: [PATCH 3/9] Prompt user to submit a bug report on park save failure --- src/openrct2/park/ParkFile.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/openrct2/park/ParkFile.cpp b/src/openrct2/park/ParkFile.cpp index 592d1b4a73..17a6b511d8 100644 --- a/src/openrct2/park/ParkFile.cpp +++ b/src/openrct2/park/ParkFile.cpp @@ -49,6 +49,7 @@ #include "../scenario/Scenario.h" #include "../scenario/ScenarioRepository.h" #include "../scripting/ScriptEngine.h" +#include "../ui/UiContext.h" #include "../world/Climate.h" #include "../world/Entrance.h" #include "../world/Map.h" @@ -2316,6 +2317,26 @@ int32_t scenario_save(u8string_view path, int32_t flags) Formatter ft; ft.Add(e.what()); context_show_error(STR_FILE_DIALOG_TITLE_SAVE_SCENARIO, STR_STRING, ft); + + auto ctx = OpenRCT2::GetContext(); + auto uictx = ctx->GetUiContext(); + + std::string title = "Error while saving"; + std::string message + = "There was an error while saving scenario.\nhttps://github.com/OpenRCT2/OpenRCT2/issues/17664\nWe would like to " + "collect more information about this issue, if this did not happen due to missing permissions, lack of space, " + "etc. please consider submitting a bug report. To collect information we would like to trigger an assert."; + + std::string report_bug_button = "Report bug, trigger an assert"; + std::string skip_button = "Skip reporting, let me continue"; + + std::vector buttons{ report_bug_button, skip_button }; + int choice = uictx->ShowMessageBox(title, message, buttons); + + if (choice == 0) + { + Guard::Assert(false, "Error while saving: %s", e.what()); + } } gfx_invalidate_screen(); From c25ebdd1c893e2677f5c6b7517f80d7802d1b089 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Tue, 2 Aug 2022 08:29:40 +0200 Subject: [PATCH 4/9] Provide more info in case of write failure --- src/openrct2/core/FileStream.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/openrct2/core/FileStream.cpp b/src/openrct2/core/FileStream.cpp index b95b220fe5..bfe8ee70bc 100644 --- a/src/openrct2/core/FileStream.cpp +++ b/src/openrct2/core/FileStream.cpp @@ -184,9 +184,11 @@ namespace OpenRCT2 { return; } - if (fwrite(buffer, static_cast(length), 1, _file) != 1) + if (auto count = fwrite(buffer, static_cast(length), 1, _file); count != 1) { - throw IOException("Unable to write to file."); + std::string error = "Unable to write " + std::to_string(length) + " bytes to file. Count = " + std::to_string(count) + + ", errno = " + std::to_string(errno); + throw IOException(error); } uint64_t position = GetPosition(); From e146ca0853ad74c27ff8400e47e6beae54a6ec6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Tue, 2 Aug 2022 08:37:11 +0200 Subject: [PATCH 5/9] Make sure to update the screen after displaying an error --- src/openrct2/park/ParkFile.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/openrct2/park/ParkFile.cpp b/src/openrct2/park/ParkFile.cpp index 17a6b511d8..f2506b32d9 100644 --- a/src/openrct2/park/ParkFile.cpp +++ b/src/openrct2/park/ParkFile.cpp @@ -2317,6 +2317,7 @@ int32_t scenario_save(u8string_view path, int32_t flags) Formatter ft; ft.Add(e.what()); context_show_error(STR_FILE_DIALOG_TITLE_SAVE_SCENARIO, STR_STRING, ft); + gfx_invalidate_screen(); auto ctx = OpenRCT2::GetContext(); auto uictx = ctx->GetUiContext(); From a9fe4da2792a2b6e913f209f1a7e64634d457986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Thu, 4 Aug 2022 18:38:51 +0200 Subject: [PATCH 6/9] Use SDL_ShowMessageBox's 'close' value for dummy UI context --- src/openrct2-ui/UiContext.cpp | 3 ++- src/openrct2/park/ParkFile.cpp | 2 +- src/openrct2/ui/DummyUiContext.cpp | 4 ++-- src/openrct2/ui/UiContext.h | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/openrct2-ui/UiContext.cpp b/src/openrct2-ui/UiContext.cpp index 17e84abfc8..b061cbb17b 100644 --- a/src/openrct2-ui/UiContext.cpp +++ b/src/openrct2-ui/UiContext.cpp @@ -640,7 +640,8 @@ public: _platformUiContext->ShowMessageBox(_window, message); } - int ShowMessageBox(const std::string& title, const std::string& message, const std::vector& options) override + int32_t ShowMessageBox( + const std::string& title, const std::string& message, const std::vector& options) override { auto message_box_button_data = std::make_unique(options.size()); for (size_t i = 0; i < options.size(); i++) diff --git a/src/openrct2/park/ParkFile.cpp b/src/openrct2/park/ParkFile.cpp index f2506b32d9..85769403da 100644 --- a/src/openrct2/park/ParkFile.cpp +++ b/src/openrct2/park/ParkFile.cpp @@ -2331,7 +2331,7 @@ int32_t scenario_save(u8string_view path, int32_t flags) std::string report_bug_button = "Report bug, trigger an assert"; std::string skip_button = "Skip reporting, let me continue"; - std::vector buttons{ report_bug_button, skip_button }; + std::vector buttons{ std::move(report_bug_button), std::move(skip_button) }; int choice = uictx->ShowMessageBox(title, message, buttons); if (choice == 0) diff --git a/src/openrct2/ui/DummyUiContext.cpp b/src/openrct2/ui/DummyUiContext.cpp index 77aa929b4e..d4040832e6 100644 --- a/src/openrct2/ui/DummyUiContext.cpp +++ b/src/openrct2/ui/DummyUiContext.cpp @@ -90,9 +90,9 @@ namespace OpenRCT2::Ui void ShowMessageBox(const std::string& /*message*/) override { } - int ShowMessageBox(const std::string&, const std::string&, const std::vector&) override + int32_t ShowMessageBox(const std::string&, const std::string&, const std::vector&) override { - return 0; + return -1; } bool HasMenuSupport() override { diff --git a/src/openrct2/ui/UiContext.h b/src/openrct2/ui/UiContext.h index cac084e8da..142b79d874 100644 --- a/src/openrct2/ui/UiContext.h +++ b/src/openrct2/ui/UiContext.h @@ -120,7 +120,7 @@ namespace OpenRCT2 virtual void TriggerResize() abstract; virtual void ShowMessageBox(const std::string& message) abstract; - virtual int ShowMessageBox( + virtual int32_t ShowMessageBox( const std::string& title, const std::string& message, const std::vector& options) abstract; virtual bool HasMenuSupport() abstract; From b42ff262efe19c9eb2194b39d5e8e68acad5bf7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Thu, 4 Aug 2022 18:40:25 +0200 Subject: [PATCH 7/9] Add more info to 'trigger assert' button --- src/openrct2/park/ParkFile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2/park/ParkFile.cpp b/src/openrct2/park/ParkFile.cpp index 85769403da..871c5406af 100644 --- a/src/openrct2/park/ParkFile.cpp +++ b/src/openrct2/park/ParkFile.cpp @@ -2328,7 +2328,7 @@ int32_t scenario_save(u8string_view path, int32_t flags) "collect more information about this issue, if this did not happen due to missing permissions, lack of space, " "etc. please consider submitting a bug report. To collect information we would like to trigger an assert."; - std::string report_bug_button = "Report bug, trigger an assert"; + std::string report_bug_button = "Report bug, trigger an assert, potentially terminating the game"; std::string skip_button = "Skip reporting, let me continue"; std::vector buttons{ std::move(report_bug_button), std::move(skip_button) }; From 4243bf0d8ef7f12747683cde30083e97c0644583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Thu, 4 Aug 2022 19:45:19 +0200 Subject: [PATCH 8/9] Allow to register additional files for upload in case of crash --- src/openrct2/platform/Crash.cpp | 43 +++++++++++++++++++++++---------- src/openrct2/platform/Crash.h | 4 +++ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/openrct2/platform/Crash.cpp b/src/openrct2/platform/Crash.cpp index 130a6e2349..b88157d805 100644 --- a/src/openrct2/platform/Crash.cpp +++ b/src/openrct2/platform/Crash.cpp @@ -47,13 +47,14 @@ # define WSZ(x) L"" x # ifdef OPENRCT2_COMMIT_SHA1_SHORT -const wchar_t* _wszCommitSha1Short = WSZ(OPENRCT2_COMMIT_SHA1_SHORT); +static const wchar_t* _wszCommitSha1Short = WSZ(OPENRCT2_COMMIT_SHA1_SHORT); # else -const wchar_t* _wszCommitSha1Short = WSZ(""); +static const wchar_t* _wszCommitSha1Short = WSZ(""); # endif // OPENRCT2_ARCHITECTURE is required to be defined in version.h -const wchar_t* _wszArchitecture = WSZ(OPENRCT2_ARCHITECTURE); +static const wchar_t* _wszArchitecture = WSZ(OPENRCT2_ARCHITECTURE); +static std::map _uploadFiles; # define BACKTRACE_TOKEN L"0ca992e20aca116b5e090fd2eaff6e7b5c8225f778fd8f4e77cb077d70329324" @@ -114,8 +115,6 @@ static bool OnCrash( return succeeded; } - std::map uploadFiles; - // Get filenames wchar_t dumpFilePath[MAX_PATH]; wchar_t saveFilePath[MAX_PATH]; @@ -147,7 +146,7 @@ static bool OnCrash( // advertise it officially. /* - uploadFiles[L"upload_file_minidump"] = dumpFilePathGZIP; + _uploadFiles[L"upload_file_minidump"] = dumpFilePathGZIP; */ } fclose(input); @@ -161,7 +160,7 @@ static bool OnCrash( { std::wcscpy(dumpFilePath, dumpFilePathNew); } - uploadFiles[L"upload_file_minidump"] = dumpFilePath; + _uploadFiles[L"upload_file_minidump"] = dumpFilePath; // Compress to gzip-compatible stream @@ -195,13 +194,13 @@ static bool OnCrash( // Compress the save if (savedGameDumped) { - uploadFiles[L"attachment_park.park"] = saveFilePath; + _uploadFiles[L"attachment_park.park"] = saveFilePath; } auto configFilePathUTF8 = String::ToUtf8(configFilePath); if (config_save(configFilePathUTF8)) { - uploadFiles[L"attachment_config.ini"] = configFilePath; + _uploadFiles[L"attachment_config.ini"] = configFilePath; } // janisozaur: https://github.com/OpenRCT2/OpenRCT2/pull/17634 @@ -219,7 +218,7 @@ static bool OnCrash( if (!screenshotPath.empty()) { auto screenshotPathW = String::ToWideChar(screenshotPath.c_str()); - uploadFiles[L"attachment_screenshot.png"] = screenshotPathW; + _uploadFiles[L"attachment_screenshot.png"] = screenshotPathW; } } @@ -229,7 +228,7 @@ static bool OnCrash( bool record_copied = CopyFileW(parkReplayPathW.c_str(), recordFilePathNew, true); if (record_copied) { - uploadFiles[L"attachment_replay.parkrep"] = recordFilePathNew; + _uploadFiles[L"attachment_replay.parkrep"] = recordFilePathNew; } else { @@ -242,7 +241,7 @@ static bool OnCrash( printf("Uploading minidump in silent mode...\n"); int error; std::wstring response; - UploadMinidump(uploadFiles, error, response); + UploadMinidump(_uploadFiles, error, response); return succeeded; } @@ -260,7 +259,7 @@ static bool OnCrash( { int error; std::wstring response; - bool ok = UploadMinidump(uploadFiles, error, response); + bool ok = UploadMinidump(_uploadFiles, error, response); if (!ok) { const wchar_t* MessageFormat2 = L"There was a problem while uploading the dump. Please upload it manually to " @@ -340,3 +339,21 @@ CExceptionHandler crash_init() return nullptr; #endif // USE_BREAKPAD } + +void crash_register_additional_file(const std::string& key, const std::string& path) +{ +#ifdef USE_BREAKPAD + _uploadFiles[String::ToWideChar(key.c_str())] = String::ToWideChar(path.c_str()); +#endif // USE_BREAKPAD +} + +void crash_unregister_additional_file(const std::string& key) +{ +#ifdef USE_BREAKPAD + auto it = _uploadFiles.find(String::ToWideChar(key.c_str())); + if (it != _uploadFiles.end()) + { + _uploadFiles.erase(it); + } +#endif // USE_BREAKPAD +} diff --git a/src/openrct2/platform/Crash.h b/src/openrct2/platform/Crash.h index 46c4a1c966..51caa4237f 100644 --- a/src/openrct2/platform/Crash.h +++ b/src/openrct2/platform/Crash.h @@ -9,7 +9,11 @@ #pragma once +#include + using CExceptionHandler = void*; extern bool gOpenRCT2SilentBreakpad; CExceptionHandler crash_init(); +void crash_register_additional_file(const std::string& key, const std::string& path); +void crash_unregister_additional_file(const std::string& key); From 0be5c9d720adefd6fe8b88237219602984bf237e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Thu, 4 Aug 2022 19:45:48 +0200 Subject: [PATCH 9/9] Register park being loaded for crash upload --- src/openrct2/Context.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/openrct2/Context.cpp b/src/openrct2/Context.cpp index 4bc6e7eef9..e9cb9ae8f3 100644 --- a/src/openrct2/Context.cpp +++ b/src/openrct2/Context.cpp @@ -541,6 +541,18 @@ namespace OpenRCT2 const std::string& path, bool loadTitleScreenOnFail = false, bool asScenario = false) final override { log_verbose("Context::LoadParkFromFile(%s)", path.c_str()); + + // Register the file for crash upload if it asserts while loading. + crash_register_additional_file("load_park", path); + // Deregister park file in case it was processed without hitting an assert. + struct foo + { + ~foo() + { + crash_unregister_additional_file("load_park"); + } + } f; + try { if (String::Equals(Path::GetExtension(path), ".sea", true))