From a7ef0314304e3a7413f3879fb99ec82d15c41a7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 16 Feb 2023 19:32:32 +0200 Subject: [PATCH 1/7] Remove String::Duplicate in TextComposition.cpp --- src/openrct2-ui/TextComposition.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/openrct2-ui/TextComposition.cpp b/src/openrct2-ui/TextComposition.cpp index 76ff77fcd6..e0c12ca7c9 100644 --- a/src/openrct2-ui/TextComposition.cpp +++ b/src/openrct2-ui/TextComposition.cpp @@ -88,9 +88,7 @@ void TextComposition::HandleMessage(const SDL_Event* e) break; } - utf8* newText = String::Duplicate(e->text.text); - Insert(newText); - Memory::Free(newText); + Insert(e->text.text); console.RefreshCaret(_session.SelectionStart); WindowUpdateTextbox(); From a0bc5d9f3fa23e3c037a39e1f20b7295896ff60d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 16 Feb 2023 19:36:55 +0200 Subject: [PATCH 2/7] Remove unused functions --- src/openrct2/core/String.cpp | 12 ------------ src/openrct2/core/String.hpp | 10 ---------- 2 files changed, 22 deletions(-) diff --git a/src/openrct2/core/String.cpp b/src/openrct2/core/String.cpp index 8bb306a563..907623b806 100644 --- a/src/openrct2/core/String.cpp +++ b/src/openrct2/core/String.cpp @@ -420,18 +420,6 @@ namespace String return result; } - utf8* DiscardUse(utf8** ptr, utf8* replacement) - { - Memory::Free(*ptr); - *ptr = replacement; - return replacement; - } - - utf8* DiscardDuplicate(utf8** ptr, const utf8* replacement) - { - return DiscardUse(ptr, String::Duplicate(replacement)); - } - std::vector Split(std::string_view s, std::string_view delimiter) { if (delimiter.empty()) diff --git a/src/openrct2/core/String.hpp b/src/openrct2/core/String.hpp index e4c34ab8d4..14b2e20815 100644 --- a/src/openrct2/core/String.hpp +++ b/src/openrct2/core/String.hpp @@ -85,16 +85,6 @@ namespace String utf8* Duplicate(const std::string& src); utf8* Duplicate(const utf8* src); - /** - * Helper method to free the string a string pointer points to and set it to a replacement string. - */ - utf8* DiscardUse(utf8** ptr, utf8* replacement); - - /** - * Helper method to free the string a string pointer points to and set it to a copy of a replacement string. - */ - utf8* DiscardDuplicate(utf8** ptr, const utf8* replacement); - /** * Splits the given string by a delimiter and returns the values as a new string array. * @returns the number of values. From 1b216c4d013796c584ce4b930988031bdec34cd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 16 Feb 2023 19:38:00 +0200 Subject: [PATCH 3/7] Use string to store ScenarioIndexEntry filename --- src/openrct2/scenario/ScenarioRepository.cpp | 25 +++++++++++++------- src/openrct2/scenario/ScenarioRepository.h | 3 ++- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/openrct2/scenario/ScenarioRepository.cpp b/src/openrct2/scenario/ScenarioRepository.cpp index c1fa517d8f..d403f9b97a 100644 --- a/src/openrct2/scenario/ScenarioRepository.cpp +++ b/src/openrct2/scenario/ScenarioRepository.cpp @@ -117,7 +117,6 @@ static int32_t ScenarioIndexEntryCompareByIndex(const ScenarioIndexEntry& entryA static void ScenarioHighscoreFree(ScenarioHighscoreEntry* highscore) { - SafeFree(highscore->fileName); SafeFree(highscore->name); SafeDelete(highscore); } @@ -479,10 +478,9 @@ public: { highscore->timestamp = Platform::GetDatetimeNowUTC(); } - SafeFree(highscore->fileName); SafeFree(highscore->name); } - highscore->fileName = String::Duplicate(Path::GetFileName(scenario->Path)); + highscore->fileName = Path::GetFileName(scenario->Path); highscore->name = String::Duplicate(name); highscore->company_value = companyValue; SaveHighscores(); @@ -493,12 +491,21 @@ public: } private: - ScenarioIndexEntry* GetByFilename(const utf8* filename) + ScenarioIndexEntry* GetByFilename(u8string_view filename) { - const ScenarioRepository* repo = this; - return const_cast(repo->GetByFilename(filename)); - } + for (auto& scenario : _scenarios) + { + const auto scenarioFilename = Path::GetFileName(scenario.Path); + // Note: this is always case insensitive search for cross platform consistency + if (String::Equals(filename, scenarioFilename, true)) + { + return &scenario; + } + } + return nullptr; + } + ScenarioIndexEntry* GetByPath(const utf8* path) { const ScenarioRepository* repo = this; @@ -623,7 +630,7 @@ private: for (uint32_t i = 0; i < numHighscores; i++) { ScenarioHighscoreEntry* highscore = InsertHighscore(); - highscore->fileName = fs.ReadString(); + highscore->fileName = fs.ReadStdString(); highscore->name = fs.ReadString(); highscore->company_value = fileVersion == 1 ? fs.ReadValue() : fs.ReadValue(); highscore->timestamp = fs.ReadValue(); @@ -696,7 +703,7 @@ private: if (notFound) { ScenarioHighscoreEntry* highscore = InsertHighscore(); - highscore->fileName = String::Duplicate(scBasic.Path); + highscore->fileName = scBasic.Path; std::string name = RCT2StringToUTF8(scBasic.CompletedBy, RCT2LanguageId::EnglishUK); highscore->name = String::Duplicate(name.c_str()); highscore->company_value = scBasic.CompanyValue; diff --git a/src/openrct2/scenario/ScenarioRepository.h b/src/openrct2/scenario/ScenarioRepository.h index d55c49d643..6854556ede 100644 --- a/src/openrct2/scenario/ScenarioRepository.h +++ b/src/openrct2/scenario/ScenarioRepository.h @@ -11,6 +11,7 @@ #pragma once #include "../common.h" +#include "../core/String.hpp" #include "../scenario/Scenario.h" #include @@ -19,7 +20,7 @@ struct RCTObjectEntry; struct ScenarioHighscoreEntry { - utf8* fileName; + u8string fileName; utf8* name; money64 company_value; datetime64 timestamp; From b465b9d6e58e575e9473d500a8d5f8552422ea5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 16 Feb 2023 19:52:13 +0200 Subject: [PATCH 4/7] Use string to store ScenarioIndexEntry name --- src/openrct2-ui/windows/ScenarioSelect.cpp | 13 ++++++------- src/openrct2/scenario/ScenarioRepository.cpp | 17 +++++++---------- src/openrct2/scenario/ScenarioRepository.h | 2 +- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/openrct2-ui/windows/ScenarioSelect.cpp b/src/openrct2-ui/windows/ScenarioSelect.cpp index 2f4108a09b..4766919cab 100644 --- a/src/openrct2-ui/windows/ScenarioSelect.cpp +++ b/src/openrct2-ui/windows/ScenarioSelect.cpp @@ -539,14 +539,14 @@ static void WindowScenarioselectPaint(WindowBase* w, DrawPixelInfo* dpi) if (scenario->Highscore != nullptr) { // TODO: Should probably be translatable - const utf8* completedByName = "???"; - if (!String::IsNullOrEmpty(scenario->Highscore->name)) + u8string completedByName = "???"; + if (!scenario->Highscore->name.empty()) { completedByName = scenario->Highscore->name; } ft = Formatter(); ft.Add(STR_STRING); - ft.Add(completedByName); + ft.Add(completedByName.c_str()); ft.Add(scenario->Highscore->company_value); screenPos.y += DrawTextWrapped(dpi, screenPos, 170, STR_COMPLETED_BY_WITH_COMPANY_VALUE, ft); } @@ -623,16 +623,15 @@ static void WindowScenarioselectScrollpaint(WindowBase* w, DrawPixelInfo* dpi, i { window_scenarioselect_widgets[WIDX_SCENARIOLIST].width() - 45, y + 1 }); // Draw completion score - const utf8* completedByName = "???"; - if (!String::IsNullOrEmpty(scenario->Highscore->name)) + u8string completedByName = "???"; + if (!scenario->Highscore->name.empty()) { completedByName = scenario->Highscore->name; } - SafeStrCpy(buffer, completedByName, 64); ft = Formatter(); ft.Add(STR_COMPLETED_BY); ft.Add(STR_STRING); - ft.Add(buffer); + ft.Add(completedByName.c_str()); DrawTextBasic( dpi, { scrollCentre, y + scenarioTitleHeight + 1 }, format, ft, { FontStyle::Small, TextAlignment::CENTRE }); diff --git a/src/openrct2/scenario/ScenarioRepository.cpp b/src/openrct2/scenario/ScenarioRepository.cpp index d403f9b97a..e7de5281ea 100644 --- a/src/openrct2/scenario/ScenarioRepository.cpp +++ b/src/openrct2/scenario/ScenarioRepository.cpp @@ -117,7 +117,6 @@ static int32_t ScenarioIndexEntryCompareByIndex(const ScenarioIndexEntry& entryA static void ScenarioHighscoreFree(ScenarioHighscoreEntry* highscore) { - SafeFree(highscore->name); SafeDelete(highscore); } @@ -464,7 +463,7 @@ public: // Check if record company value has been broken or the highscore is the same but no name is registered ScenarioHighscoreEntry* highscore = scenario->Highscore; if (highscore == nullptr || companyValue > highscore->company_value - || (String::IsNullOrEmpty(highscore->name) && companyValue == highscore->company_value)) + || (highscore->name.empty() && companyValue == highscore->company_value)) { if (highscore == nullptr) { @@ -474,14 +473,13 @@ public: } else { - if (!String::IsNullOrEmpty(highscore->name)) + if (!highscore->name.empty()) { highscore->timestamp = Platform::GetDatetimeNowUTC(); } - SafeFree(highscore->name); } highscore->fileName = Path::GetFileName(scenario->Path); - highscore->name = String::Duplicate(name); + highscore->name = name; highscore->company_value = companyValue; SaveHighscores(); return true; @@ -505,7 +503,7 @@ private: } return nullptr; } - + ScenarioIndexEntry* GetByPath(const utf8* path) { const ScenarioRepository* repo = this; @@ -631,7 +629,7 @@ private: { ScenarioHighscoreEntry* highscore = InsertHighscore(); highscore->fileName = fs.ReadStdString(); - highscore->name = fs.ReadString(); + highscore->name = fs.ReadStdString(); highscore->company_value = fileVersion == 1 ? fs.ReadValue() : fs.ReadValue(); highscore->timestamp = fs.ReadValue(); } @@ -691,9 +689,8 @@ private: // Check if legacy highscore is better if (scBasic.CompanyValue > highscore->company_value) { - SafeFree(highscore->name); std::string name = RCT2StringToUTF8(scBasic.CompletedBy, RCT2LanguageId::EnglishUK); - highscore->name = String::Duplicate(name.c_str()); + highscore->name = name; highscore->company_value = scBasic.CompanyValue; highscore->timestamp = DATETIME64_MIN; break; @@ -705,7 +702,7 @@ private: ScenarioHighscoreEntry* highscore = InsertHighscore(); highscore->fileName = scBasic.Path; std::string name = RCT2StringToUTF8(scBasic.CompletedBy, RCT2LanguageId::EnglishUK); - highscore->name = String::Duplicate(name.c_str()); + highscore->name = name; highscore->company_value = scBasic.CompanyValue; highscore->timestamp = DATETIME64_MIN; } diff --git a/src/openrct2/scenario/ScenarioRepository.h b/src/openrct2/scenario/ScenarioRepository.h index 6854556ede..f07b3499ea 100644 --- a/src/openrct2/scenario/ScenarioRepository.h +++ b/src/openrct2/scenario/ScenarioRepository.h @@ -21,7 +21,7 @@ struct RCTObjectEntry; struct ScenarioHighscoreEntry { u8string fileName; - utf8* name; + u8string name; money64 company_value; datetime64 timestamp; }; From 1f03bc89d77ba6e74a768be6ae68188ca569792e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 16 Feb 2023 21:59:08 +0200 Subject: [PATCH 5/7] Remove unused String::Duplicate --- src/openrct2/core/String.cpp | 17 ----------------- src/openrct2/core/String.hpp | 2 -- 2 files changed, 19 deletions(-) diff --git a/src/openrct2/core/String.cpp b/src/openrct2/core/String.cpp index 907623b806..ccdd7d0fbf 100644 --- a/src/openrct2/core/String.cpp +++ b/src/openrct2/core/String.cpp @@ -403,23 +403,6 @@ namespace String return buffer; } - utf8* Duplicate(const std::string& src) - { - return String::Duplicate(src.c_str()); - } - - utf8* Duplicate(const utf8* src) - { - utf8* result = nullptr; - if (src != nullptr) - { - size_t srcSize = SizeOf(src) + 1; - result = Memory::Allocate(srcSize); - std::memcpy(result, src, srcSize); - } - return result; - } - std::vector Split(std::string_view s, std::string_view delimiter) { if (delimiter.empty()) diff --git a/src/openrct2/core/String.hpp b/src/openrct2/core/String.hpp index 14b2e20815..49389b0804 100644 --- a/src/openrct2/core/String.hpp +++ b/src/openrct2/core/String.hpp @@ -82,8 +82,6 @@ namespace String u8string StdFormat(const utf8* format, ...); u8string Format_VA(const utf8* format, va_list args); utf8* AppendFormat(utf8* buffer, size_t bufferSize, const utf8* format, ...); - utf8* Duplicate(const std::string& src); - utf8* Duplicate(const utf8* src); /** * Splits the given string by a delimiter and returns the values as a new string array. From b057f0fe82793778d531bed3ae651ebeea4b9370 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 16 Feb 2023 22:48:21 +0200 Subject: [PATCH 6/7] Remove memset --- src/openrct2/scenario/ScenarioRepository.cpp | 1 - src/openrct2/scenario/ScenarioRepository.h | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/openrct2/scenario/ScenarioRepository.cpp b/src/openrct2/scenario/ScenarioRepository.cpp index e7de5281ea..8b735709b9 100644 --- a/src/openrct2/scenario/ScenarioRepository.cpp +++ b/src/openrct2/scenario/ScenarioRepository.cpp @@ -732,7 +732,6 @@ private: ScenarioHighscoreEntry* InsertHighscore() { auto highscore = new ScenarioHighscoreEntry(); - std::memset(highscore, 0, sizeof(ScenarioHighscoreEntry)); _highscores.push_back(highscore); return highscore; } diff --git a/src/openrct2/scenario/ScenarioRepository.h b/src/openrct2/scenario/ScenarioRepository.h index f07b3499ea..f887147ee0 100644 --- a/src/openrct2/scenario/ScenarioRepository.h +++ b/src/openrct2/scenario/ScenarioRepository.h @@ -22,8 +22,8 @@ struct ScenarioHighscoreEntry { u8string fileName; u8string name; - money64 company_value; - datetime64 timestamp; + money64 company_value{}; + datetime64 timestamp{}; }; enum class ScenarioSource : uint8_t From 60bd8a4f3a9f4681c64a66819f322a6677309202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Sat, 18 Feb 2023 17:03:50 +0200 Subject: [PATCH 7/7] Fix crash at scenario completion --- src/openrct2/scenario/ScenarioRepository.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2/scenario/ScenarioRepository.cpp b/src/openrct2/scenario/ScenarioRepository.cpp index 8b735709b9..6b7a4c78aa 100644 --- a/src/openrct2/scenario/ScenarioRepository.cpp +++ b/src/openrct2/scenario/ScenarioRepository.cpp @@ -479,7 +479,7 @@ public: } } highscore->fileName = Path::GetFileName(scenario->Path); - highscore->name = name; + highscore->name = name != nullptr ? name : ""; highscore->company_value = companyValue; SaveHighscores(); return true;