From de841d30e4f32ad2e91f1f529345d2f228373f0c Mon Sep 17 00:00:00 2001 From: phosit Date: Fri, 17 Oct 2025 16:19:46 +0200 Subject: [PATCH] Return description and progress from loader There was a comment implying that `wchar_t*` is more iteroperable then `std::wstring`. When there is no allocator `std::wstring` won't work, sure. But when there is no allocator other things in Pyrogenisis will also not work. And for the progress there was just no reason to use a return parameter. --- source/main.cpp | 17 ++++++------ source/ps/Loader.cpp | 61 ++++++++++++++------------------------------ source/ps/Loader.h | 47 +++++++++++++++++++--------------- 3 files changed, 55 insertions(+), 70 deletions(-) diff --git a/source/main.cpp b/source/main.cpp index 1fa5308112..32fe39a6de 100644 --- a/source/main.cpp +++ b/source/main.cpp @@ -35,7 +35,6 @@ that of Atlas depending on commandline parameters. #include "graphics/GameView.h" #include "graphics/TextureManager.h" #include "gui/GUIManager.h" -#include "lib/code_annotation.h" #include "lib/config2.h" #include "lib/debug.h" #include "lib/external_libraries/libsdl.h" @@ -326,12 +325,14 @@ static int ProgressiveLoad() { PROFILE3("progressive load"); - wchar_t description[100]; - int progress_percent; + std::wstring description; + int progressPercent{0}; try { - Status ret = LDR_ProgressiveLoad(10e-3, description, ARRAY_SIZE(description), &progress_percent); - switch(ret) + const LDR_ProgressiveLoadResult result{LDR_ProgressiveLoad(10e-3)}; + description = result.nextDescription; + progressPercent = result.progressPercent; + switch(result.status) { // no load active => no-op (skip code below) case INFO::OK: @@ -343,13 +344,13 @@ static int ProgressiveLoad() // just finished loading case INFO::ALL_COMPLETE: g_Game->ReallyStartGame(); - wcscpy_s(description, ARRAY_SIZE(description), L"Game is starting.."); + description = L"Game is starting.."; // LDR_ProgressiveLoad returns L""; set to valid text to // avoid problems in converting to JSString break; // error! default: - WARN_RETURN_STATUS_IF_ERR(ret); + WARN_RETURN_STATUS_IF_ERR(result.status); // can't do this above due to legit ERR::TIMED_OUT break; } @@ -363,7 +364,7 @@ static int ProgressiveLoad() CancelLoad(CStr(e.what()).FromUTF8()); } - g_GUI->DisplayLoadProgress(progress_percent, description); + g_GUI->DisplayLoadProgress(progressPercent, description.c_str()); return 0; } diff --git a/source/ps/Loader.cpp b/source/ps/Loader.cpp index d8773bbc68..56277a5497 100644 --- a/source/ps/Loader.cpp +++ b/source/ps/Loader.cpp @@ -26,7 +26,6 @@ #include "lib/secure_crt.h" #include "lib/timer.h" #include "lib/utf8.h" -#include "ps/CStr.h" #include #include @@ -68,13 +67,13 @@ struct LoadRequest LoadFunc func; // Translatable string shown to the player. - CStrW description; + std::wstring description; int estimated_duration_ms; // LDR_Register gets these as parameters; pack everything together. - LoadRequest(LoadFunc func_, const wchar_t* desc_, int ms_) - : func(func_), description(desc_), estimated_duration_ms(ms_) + LoadRequest(LoadFunc func_, std::wstring desc_, int ms_) + : func(std::move(func_)), description(std::move(desc_)), estimated_duration_ms(ms_) { } }; @@ -110,11 +109,11 @@ void LDR_BeginRegistering() // : used to calculate progress, and when checking // whether there is enough of the time budget left to process this task // (reduces timeslice overruns, making the main loop more responsive). -void LDR_Register(LoadFunc func, const wchar_t* description, int estimatedDurationMs) +void LDR_Register(LoadFunc func, std::wstring description, int estimatedDurationMs) { ENSURE(state == REGISTERING); // must be called between LDR_(Begin|End)Register - load_requests.emplace_back(std::move(func), description, estimatedDurationMs); + load_requests.emplace_back(std::move(func), std::move(description), estimatedDurationMs); } @@ -166,27 +165,9 @@ static bool HaveTimeForNextTask(double time_left, double time_budget, int estima return true; } - -// process as many of the queued tasks as possible within [s]. -// if a task is lengthy, the budget may be exceeded. call from the main loop. -// -// passes back a description of the next task that will be undertaken -// ("" if finished) and the current progress value. -// -// return semantics: -// - if the final load task just completed, return INFO::ALL_COMPLETE. -// - if loading is in progress but didn't finish, return ERR::TIMED_OUT. -// - if not currently loading (no-op), return 0. -// - any other value indicates a failure; the request has been de-queued. -// -// string interface rationale: for better interoperability, we avoid C++ -// std::wstring and PS CStr. since the registered description may not be -// persistent, we can't just store a pointer. returning a pointer to -// our copy of the description doesn't work either, since it's freed when -// the request is de-queued. that leaves writing into caller's buffer. -Status LDR_ProgressiveLoad(double time_budget, wchar_t* description, size_t max_chars, int* progress_percent) +LDR_ProgressiveLoadResult LDR_ProgressiveLoad(double time_budget) { - Status ret; // single exit; this is returned + LDR_ProgressiveLoadResult ret; double progress = 0.0; // used to set progress_percent double time_left = time_budget; @@ -196,7 +177,7 @@ Status LDR_ProgressiveLoad(double time_budget, wchar_t* description, size_t max_ { state = LOADING; - ret = ERR::TIMED_OUT; // make caller think we did something + ret.status = ERR::TIMED_OUT; // make caller think we did something // progress already set to 0.0; that'll be passed back. goto done; } @@ -204,7 +185,7 @@ Status LDR_ProgressiveLoad(double time_budget, wchar_t* description, size_t max_ // we're called unconditionally from the main loop, so this isn't // an error; there is just nothing to do. if(state != LOADING) - return INFO::OK; + return {}; while(!load_requests.empty()) { @@ -213,7 +194,7 @@ Status LDR_ProgressiveLoad(double time_budget, wchar_t* description, size_t max_ const double estimated_duration = lr.estimated_duration_ms*1e-3; if(!HaveTimeForNextTask(time_left, time_budget, lr.estimated_duration_ms)) { - ret = ERR::TIMED_OUT; + ret.status = ERR::TIMED_OUT; goto done; } @@ -252,7 +233,7 @@ Status LDR_ProgressiveLoad(double time_budget, wchar_t* description, size_t max_ // .. function interrupted itself, i.e. timed out; abort. if(timed_out) { - ret = ERR::TIMED_OUT; + ret.status = ERR::TIMED_OUT; goto done; } // .. failed; abort. loading will continue when we're called in @@ -261,14 +242,14 @@ Status LDR_ProgressiveLoad(double time_budget, wchar_t* description, size_t max_ // error that came up so we can report all errors that happen. else if(status < 0) { - ret = (Status)status; + ret.status = static_cast(status); goto done; } // .. function called LDR_Cancel; abort. return OK since this is an // intentional cancellation, not an error. else if(state != LOADING) { - ret = INFO::OK; + ret.status = INFO::OK; goto done; } // .. succeeded; continue and process next queued task. @@ -276,22 +257,20 @@ Status LDR_ProgressiveLoad(double time_budget, wchar_t* description, size_t max_ // queue is empty, we just finished. state = IDLE; - ret = INFO::ALL_COMPLETE; + ret.status = INFO::ALL_COMPLETE; // set output params (there are several return points above) done: - *progress_percent = (int)(progress * 100.0); - ENSURE(0 <= *progress_percent && *progress_percent <= 100); + ret.progressPercent = static_cast(progress * 100.0); + ENSURE(0 <= ret.progressPercent && ret.progressPercent <= 100); // we want the next task, instead of what just completed: // it will be displayed during the next load phase. - const wchar_t* new_description = L""; // assume finished if(!load_requests.empty()) - new_description = load_requests.front().description.c_str(); - wcscpy_s(description, max_chars, new_description); + ret.nextDescription = load_requests.front().description; - debug_printf("LOADER| returning; desc=%s progress=%d\n", utf8_from_wstring(description).c_str(), *progress_percent); + debug_printf("LOADER| returning; desc=%s progress=%d\n", utf8_from_wstring(ret.nextDescription).c_str(), ret.progressPercent); return ret; } @@ -304,12 +283,10 @@ Status LDR_NonprogressiveLoad() const double time_budget = 100.0; // large enough so that individual functions won't time out // (that'd waste time). - wchar_t description[100]; - int progress_percent; for(;;) { - Status ret = LDR_ProgressiveLoad(time_budget, description, ARRAY_SIZE(description), &progress_percent); + const auto [ret, description, progress_percent] = LDR_ProgressiveLoad(time_budget); switch(ret) { case INFO::OK: diff --git a/source/ps/Loader.h b/source/ps/Loader.h index d8a8d821d3..2a841393f5 100644 --- a/source/ps/Loader.h +++ b/source/ps/Loader.h @@ -24,8 +24,8 @@ #include "lib/debug.h" #include "lib/status.h" -#include #include +#include /* @@ -126,7 +126,7 @@ using LoadFunc = std::function; // : used to calculate progress, and when checking // whether there is enough of the time budget left to process this task // (reduces timeslice overruns, making the main loop more responsive). -void LDR_Register(LoadFunc func, const wchar_t* description, int estimated_duration_ms); +void LDR_Register(LoadFunc func, std::wstring description, int estimated_duration_ms); // call when finished registering tasks; subsequent calls to @@ -139,25 +139,32 @@ extern void LDR_EndRegistering(); // note: no special notification will be returned by LDR_ProgressiveLoad. extern void LDR_Cancel(); +struct LDR_ProgressiveLoadResult +{ + /** + * @c INFO::All_COMPLETE if the final load task just completed. + * @c ERR::TIMED_OUT if loading is in progress but didn't finish. + * @c 0 if not currently loading (no-op). + * Otherwise an error code. the request has been de-queued. + */ + Status status{0}; -// process as many of the queued tasks as possible within [s]. -// if a task is lengthy, the budget may be exceeded. call from the main loop. -// -// passes back a description of the next task that will be undertaken -// ("" if finished) and the current progress value. -// -// return semantics: -// - if the final load task just completed, return INFO::ALL_COMPLETE. -// - if loading is in progress but didn't finish, return ERR::TIMED_OUT. -// - if not currently loading (no-op), return 0. -// - any other value indicates a failure; the request has been de-queued. -// -// string interface rationale: for better interoperability, we avoid C++ -// std::wstring and PS CStr. since the registered description may not be -// persistent, we can't just store a pointer. returning a pointer to -// our copy of the description doesn't work either, since it's freed when -// the request is de-queued. that leaves writing into caller's buffer. -extern Status LDR_ProgressiveLoad(double time_budget, wchar_t* next_description, size_t max_chars, int* progress_percent); + /** + * An empty string when finished. + * Otherwise the description of the next task that will be undertaken. + */ + std::wstring nextDescription; + + /** + * The current progress value. + */ + int progressPercent; +}; +/** + * Process as many of the queued tasks as possible within @c timeBudget [s]. + * if a task is lengthy, the budget may be exceeded. call from the main loop. + */ +LDR_ProgressiveLoadResult LDR_ProgressiveLoad(double time_budget); // immediately process all queued load requests. // returns 0 on success or a negative error code.