From c9491183f703e304e5e6911c38301f55424875a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= <159546+serprex@users.noreply.github.com> Date: Mon, 13 Jul 2026 01:12:41 +0000 Subject: [PATCH] Optimize CustomMessageManager.cpp (#6915) Reduce copies. Fix Capitalize & GetAllMessages only modifying unused copies --- .../custom-message/CustomMessageManager.cpp | 55 ++++++++++--------- .../custom-message/CustomMessageManager.h | 12 ++-- .../game-interactor/GameInteractionEffect.h | 1 + .../Enhancements/randomizer/SeedContext.cpp | 2 +- .../Enhancements/randomizer/static_data.cpp | 8 +-- soh/soh/Enhancements/randomizer/static_data.h | 4 +- soh/soh/Enhancements/randomizer/trial.cpp | 2 +- 7 files changed, 43 insertions(+), 41 deletions(-) diff --git a/soh/soh/Enhancements/custom-message/CustomMessageManager.cpp b/soh/soh/Enhancements/custom-message/CustomMessageManager.cpp index b52ee24cc2..5f5f806dec 100644 --- a/soh/soh/Enhancements/custom-message/CustomMessageManager.cpp +++ b/soh/soh/Enhancements/custom-message/CustomMessageManager.cpp @@ -80,8 +80,8 @@ CustomMessage::CustomMessage(std::string english_, std::string german_, std::str messages[LANGUAGE_ENG] = std::move(english_); messages[LANGUAGE_GER] = std::move(german_); messages[LANGUAGE_FRA] = std::move(french_); - colors = colors_; - capital = capital_; + colors = std::move(colors_); + capital = std::move(capital_); type = type_; position = position_; } @@ -94,8 +94,8 @@ CustomMessage::CustomMessage(std::string english_, TextBoxType type_, TextBoxPos CustomMessage::CustomMessage(std::string english_, std::vector colors_, std::vector capital_, TextBoxType type_, TextBoxPosition position_) { messages[LANGUAGE_ENG] = std::move(english_); - colors = colors_; - capital = capital_; + colors = std::move(colors_); + capital = std::move(capital_); type = type_; position = position_; } @@ -167,7 +167,7 @@ const std::string CustomMessage::GetForLanguage(uint8_t language, MessageFormat const std::vector CustomMessage::GetAllMessages(MessageFormat format) const { std::vector output = messages; - for (auto str : output) { + for (auto& str : output) { ProcessMessageFormat(str, format); } return output; @@ -190,14 +190,14 @@ const std::vector& CustomMessage::GetCapital() const { } void CustomMessage::SetCapital(std::vector capital_) { - capital = capital_; + capital = std::move(capital_); } const std::vector& CustomMessage::GetColors() const { return colors; } void CustomMessage::SetColors(std::vector colors_) { - colors = colors_; + colors = std::move(colors_); } const TextBoxType& CustomMessage::GetTextBoxType() const { @@ -218,8 +218,8 @@ void CustomMessage::SetTextBoxPosition(TextBoxPosition boxPos) { CustomMessage CustomMessage::operator+(const CustomMessage& right) const { std::vector newColors = colors; - std::vector rColors = right.GetColors(); - for (auto color : rColors) { + const std::vector& rColors = right.GetColors(); + for (const auto& color : rColors) { newColors.push_back(color); } std::vector newCapital = capital; @@ -253,7 +253,7 @@ bool CustomMessage::operator==(const CustomMessage& operand) const { } bool CustomMessage::operator==(const std::string& operand) const { - for (auto str : messages) { + for (const auto& str : messages) { if (str == operand) { return true; } @@ -686,14 +686,16 @@ void CustomMessage::SetSingularPlural() { } void CustomMessage::Capitalize() { - for (std::string str : messages) { - (str)[0] = std::toupper((str)[0]); + for (std::string& str : messages) { + if (!str.empty()) { + str[0] = std::toupper(str[0]); + } } } void CustomMessage::ReplaceSpecialCharacters(std::string& str) const { // add special characters - for (auto specialCharacterPair : textBoxSpecialCharacters) { + for (const auto& specialCharacterPair : textBoxSpecialCharacters) { size_t start_pos = 0; std::string textBoxSpecialCharacterString = ""s; textBoxSpecialCharacterString += specialCharacterPair.second; @@ -707,7 +709,7 @@ void CustomMessage::ReplaceSpecialCharacters(std::string& str) const { const char* Interface_ReplaceSpecialCharacters(char text[]) { std::string textString(text); - for (auto specialCharacterPair : textBoxSpecialCharacters) { + for (const auto& specialCharacterPair : textBoxSpecialCharacters) { size_t start_pos = 0; std::string textBoxSpecialCharacterString = ""s; textBoxSpecialCharacterString += specialCharacterPair.second; @@ -723,7 +725,7 @@ const char* Interface_ReplaceSpecialCharacters(char text[]) { } void CustomMessage::EncodeColors(std::string& str) const { - for (std::string color : colors) { + for (const std::string& color : colors) { if (const size_t firstHashtag = str.find('#'); firstHashtag != std::string::npos) { str.replace(firstHashtag, 1, colorToPercent.at(color)); if (const size_t secondHashtag = str.find('#', firstHashtag + 1); secondHashtag != std::string::npos) { @@ -804,33 +806,33 @@ std::string CustomMessage::TWO_WAY_CHOICE() { return "\x1B"s; } -bool CustomMessageManager::InsertCustomMessage(std::string tableID, uint16_t textID, CustomMessage messages) { +bool CustomMessageManager::InsertCustomMessage(const std::string& tableID, uint16_t textID, CustomMessage messages) { auto foundMessageTable = messageTables.find(tableID); if (foundMessageTable == messageTables.end()) { return false; } auto& messageTable = foundMessageTable->second; - auto messageInsertResult = messageTable.emplace(textID, messages); + auto messageInsertResult = messageTable.emplace(textID, std::move(messages)); return messageInsertResult.second; } -bool CustomMessageManager::CreateGetItemMessage(std::string tableID, uint16_t giid, ItemID iid, +bool CustomMessageManager::CreateGetItemMessage(const std::string& tableID, uint16_t giid, ItemID iid, CustomMessage messageEntry) { messageEntry.Format(iid); const uint16_t textID = giid; - return InsertCustomMessage(tableID, textID, messageEntry); + return InsertCustomMessage(tableID, textID, std::move(messageEntry)); } -bool CustomMessageManager::CreateMessage(std::string tableID, uint16_t textID, CustomMessage messageEntry) { - return InsertCustomMessage(tableID, textID, messageEntry); +bool CustomMessageManager::CreateMessage(const std::string& tableID, uint16_t textID, CustomMessage messageEntry) { + return InsertCustomMessage(tableID, textID, std::move(messageEntry)); } -CustomMessage CustomMessageManager::RetrieveMessage(std::string tableID, uint16_t textID, MessageFormat format) { +CustomMessage CustomMessageManager::RetrieveMessage(const std::string& tableID, uint16_t textID, MessageFormat format) { std::unordered_map::const_iterator foundMessageTable = messageTables.find(tableID); if (foundMessageTable == messageTables.end()) { throw(MessageNotFoundException(tableID, textID)); } - CustomMessageTable messageTable = foundMessageTable->second; + const CustomMessageTable& messageTable = foundMessageTable->second; std::unordered_map::const_iterator foundMessage = messageTable.find(textID); if (foundMessage == messageTable.end()) { throw(MessageNotFoundException(tableID, textID)); @@ -850,7 +852,7 @@ CustomMessage CustomMessageManager::RetrieveMessage(std::string tableID, uint16_ return message; } -bool CustomMessageManager::ClearMessageTable(std::string tableID) { +bool CustomMessageManager::ClearMessageTable(const std::string& tableID) { auto foundMessageTable = messageTables.find(tableID); if (foundMessageTable == messageTables.end()) { return false; @@ -860,7 +862,6 @@ bool CustomMessageManager::ClearMessageTable(std::string tableID) { return true; } -bool CustomMessageManager::AddCustomMessageTable(std::string tableID) { - CustomMessageTable newMessageTable; - return messageTables.emplace(tableID, newMessageTable).second; +bool CustomMessageManager::AddCustomMessageTable(const std::string& tableID) { + return messageTables.try_emplace(tableID).second; } diff --git a/soh/soh/Enhancements/custom-message/CustomMessageManager.h b/soh/soh/Enhancements/custom-message/CustomMessageManager.h index 792bfad3fb..37395e08fa 100644 --- a/soh/soh/Enhancements/custom-message/CustomMessageManager.h +++ b/soh/soh/Enhancements/custom-message/CustomMessageManager.h @@ -247,7 +247,7 @@ class CustomMessageManager { private: std::unordered_map messageTables; - bool InsertCustomMessage(std::string tableID, uint16_t textID, CustomMessage message); + bool InsertCustomMessage(const std::string& tableID, uint16_t textID, CustomMessage message); public: static CustomMessageManager* Instance; @@ -266,7 +266,7 @@ class CustomMessageManager { * @return true if adding the custom message succeeds, or * @return false if it does not. */ - bool CreateGetItemMessage(std::string tableID, uint16_t giid, ItemID iid, CustomMessage message); + bool CreateGetItemMessage(const std::string& tableID, uint16_t giid, ItemID iid, CustomMessage message); /** * @brief Formats the provided Custom Message Entry and inserts it into the table with the provided tableID, @@ -278,7 +278,7 @@ class CustomMessageManager { * @return true if adding the custom message succeeds, or * @return false if it does not. */ - bool CreateMessage(std::string tableID, uint16_t textID, CustomMessage message); + bool CreateMessage(const std::string& tableID, uint16_t textID, CustomMessage message); /** * @brief Retrieves a message from the table with id tableID with the provided textID. @@ -292,7 +292,7 @@ class CustomMessageManager { * @param format the type of formatting to apply to the retrieved message * @return CustomMessage */ - CustomMessage RetrieveMessage(std::string tableID, uint16_t textID, MessageFormat format = MF_RAW); + CustomMessage RetrieveMessage(const std::string& tableID, uint16_t textID, MessageFormat format = MF_RAW); /** * @brief Empties out the message table identified by tableID. @@ -301,7 +301,7 @@ class CustomMessageManager { * @return true if it was cleared successfully, or * @return false if the table did not exist */ - bool ClearMessageTable(std::string tableID); + bool ClearMessageTable(const std::string& tableID); /** * @brief Creates an empty CustomMessageTable accessible at the provided tableID @@ -311,7 +311,7 @@ class CustomMessageManager { * @return false if not (i.e. because a table with that ID * already exists.) */ - bool AddCustomMessageTable(std::string tableID); + bool AddCustomMessageTable(const std::string& tableID); }; class MessageNotFoundException : public std::exception { diff --git a/soh/soh/Enhancements/game-interactor/GameInteractionEffect.h b/soh/soh/Enhancements/game-interactor/GameInteractionEffect.h index 9243621d1f..2db0fd30d3 100644 --- a/soh/soh/Enhancements/game-interactor/GameInteractionEffect.h +++ b/soh/soh/Enhancements/game-interactor/GameInteractionEffect.h @@ -10,6 +10,7 @@ enum GameInteractionEffectQueryResult { Possible = 0x00, TemporarilyNotPossible class GameInteractionEffectBase { public: + virtual ~GameInteractionEffectBase() = default; virtual GameInteractionEffectQueryResult CanBeApplied() = 0; GameInteractionEffectQueryResult Apply(); diff --git a/soh/soh/Enhancements/randomizer/SeedContext.cpp b/soh/soh/Enhancements/randomizer/SeedContext.cpp index b150f804fa..472782e347 100644 --- a/soh/soh/Enhancements/randomizer/SeedContext.cpp +++ b/soh/soh/Enhancements/randomizer/SeedContext.cpp @@ -456,7 +456,7 @@ void Context::WriteHintJson(nlohmann::ordered_json& spoilerFileJson) { } nlohmann::json getValueForMessage(std::unordered_map map, CustomMessage message) { - std::vector strings = message.GetAllMessages(); + std::vector strings = message.GetAllMessages(MF_CLEAN); for (uint8_t language = 0; language < LANGUAGE_MAX; language++) { if (map.contains(strings[language])) { return strings[language]; diff --git a/soh/soh/Enhancements/randomizer/static_data.cpp b/soh/soh/Enhancements/randomizer/static_data.cpp index 390febf5e8..2315e8caf3 100644 --- a/soh/soh/Enhancements/randomizer/static_data.cpp +++ b/soh/soh/Enhancements/randomizer/static_data.cpp @@ -216,10 +216,10 @@ std::unordered_map StaticData::staticHintInfoMap }; std::unordered_map -StaticData::PopulateTranslationMap(std::unordered_map input) { +StaticData::PopulateTranslationMap(const std::unordered_map& input) { std::unordered_map output = {}; for (const auto& [key, message] : input) { - std::vector strings = message.GetAllMessages(); + std::vector strings = message.GetAllMessages(MF_CLEAN); for (std::string string : strings) { if (output.contains(string)) { if (output[string] != key) { @@ -235,10 +235,10 @@ StaticData::PopulateTranslationMap(std::unordered_map i } std::unordered_map -StaticData::PopulateTranslationMap(std::unordered_map input) { +StaticData::PopulateTranslationMap(const std::unordered_map& input) { std::unordered_map output = {}; for (const auto& [key, text] : input) { - std::vector strings = hintTextTable[text].GetClear().GetAllMessages(); + std::vector strings = hintTextTable[text].GetClear().GetAllMessages(MF_CLEAN); for (std::string string : strings) { if (output.contains(string)) { if (output[string] != key) { diff --git a/soh/soh/Enhancements/randomizer/static_data.h b/soh/soh/Enhancements/randomizer/static_data.h index 94aead4a91..3773ebacce 100644 --- a/soh/soh/Enhancements/randomizer/static_data.h +++ b/soh/soh/Enhancements/randomizer/static_data.h @@ -37,9 +37,9 @@ class StaticData { static Location* GetLocation(RandomizerCheck locKey); static std::array& GetLocationTable(); static std::unordered_map - PopulateTranslationMap(std::unordered_map input); + PopulateTranslationMap(const std::unordered_map& input); static std::unordered_map - PopulateTranslationMap(std::unordered_map input); + PopulateTranslationMap(const std::unordered_map& input); static std::multimap, RandomizerCheck> CheckFromActorMultimap; static std::vector GetAllDungeonLocations(); static std::vector dungeonRewardLocations; diff --git a/soh/soh/Enhancements/randomizer/trial.cpp b/soh/soh/Enhancements/randomizer/trial.cpp index 81ef97a978..04f38b89eb 100644 --- a/soh/soh/Enhancements/randomizer/trial.cpp +++ b/soh/soh/Enhancements/randomizer/trial.cpp @@ -77,7 +77,7 @@ void Trials::ParseJson(nlohmann::json spoilerFileJson) { for (auto& trial : mTrials) { trial.SetAsSkipped(); - for (auto nameInLang : trial.GetName().GetAllMessages()) { + for (auto nameInLang : trial.GetName().GetAllMessages(MF_CLEAN)) { if (std::find(trialsJson.begin(), trialsJson.end(), nameInLang) != trialsJson.end()) { trial.SetAsRequired(); }