From bb4937f827d2cfd60c42a905d5698edfd2c131e0 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Thu, 21 Aug 2025 23:06:24 +0200 Subject: [PATCH 1/5] Allow composition of GMST l10n values --- apps/components_tests/lua/testl10n.cpp | 77 ++++++++ apps/openmw/engine.cpp | 24 +-- components/l10n/manager.hpp | 4 +- components/l10n/messagebundles.cpp | 259 +++++++++++++++++++------ components/l10n/messagebundles.hpp | 16 +- 5 files changed, 298 insertions(+), 82 deletions(-) diff --git a/apps/components_tests/lua/testl10n.cpp b/apps/components_tests/lua/testl10n.cpp index 7446822f7f..9102f9165f 100644 --- a/apps/components_tests/lua/testl10n.cpp +++ b/apps/components_tests/lua/testl10n.cpp @@ -26,6 +26,8 @@ namespace constexpr VFS::Path::NormalizedView test3DePath("l10n/test3/de.yaml"); constexpr VFS::Path::NormalizedView test4RuPath("l10n/test4/ru.yaml"); constexpr VFS::Path::NormalizedView test4EnPath("l10n/test4/en.yaml"); + constexpr VFS::Path::NormalizedView test5GmstPath("l10n/test5/gmst.yaml"); + constexpr VFS::Path::NormalizedView test5EnPath("l10n/test5/en.yaml"); VFSTestFile invalidScript("not a script"); VFSTestFile incorrectScript( @@ -83,6 +85,31 @@ stat_increase: "Your {stat} has increased to {value}" speed: "Speed" )X"); + VFSTestFile test5(R"X( +string: "sSimpleString" +format_string: "sFormatString" +inject_string: "sStringInjection" +not_found: "sNoSuchString" +no_gmst: + pattern: "Hello world" +interpreted_string: + pattern: "{gmst:sFormatString} {sSimpleString} {gmst:sFormatString}" + variables: + - ["a", "b"] + - ["b", "a"] +plural_string: + pattern: |- + {count, plural, + one{{gmst:sSimpleString}} + other{{gmst:sFormatString}} + } + variables: + - [] + - ["count", "count"] +unnamed_string: + pattern: "{gmst:sFormatString}" +)X"); + struct LuaL10nTest : Test { std::unique_ptr mVFS = createTestVFS({ @@ -94,6 +121,8 @@ speed: "Speed" { test3DePath, &test1De }, { test4RuPath, &test4Ru }, { test4EnPath, &test4En }, + { test5GmstPath, &test5 }, + { test5EnPath, &test5 }, }); LuaUtil::ScriptsConfiguration mCfg; @@ -197,4 +226,52 @@ speed: "Speed" "Your Speed has increased to 100"); }); } + + TEST_F(LuaL10nTest, L10nGMST) + { + LuaUtil::LuaState lua{ mVFS.get(), &mCfg }; + lua.protectedCall([&](LuaUtil::LuaView& view) { + sol::state_view& l = view.sol(); + L10n::Manager l10nManager(mVFS.get()); + const std::map gmsts{ + { "sSimpleString", "Hello it's the world" }, + { "sFormatString", "You have %i %s" }, + { "sStringInjection", "{a}" }, + }; + l10nManager.setGmstLoader([&](std::string_view gmst) -> const std::string* { + auto it = gmsts.find(gmst); + if (it != gmsts.end()) + return &it->second; + return nullptr; + }); + + internal::CaptureStdout(); + l10nManager.setPreferredLocales({ "en" }); + EXPECT_THAT(internal::GetCapturedStdout(), "Preferred locales: gmst en\n"); + + l["l10n"] = LuaUtil::initL10nLoader(l, &l10nManager); + l.safe_script("t5 = l10n('Test5')"); + + EXPECT_EQ(get(l, "t5('string')"), "Hello it's the world"); + EXPECT_EQ(get(l, "t5('format_string')"), "You have %i %s"); + EXPECT_EQ(get(l, "t5('format_string', { i = 1, s = 'a' })"), "You have %i %s"); + EXPECT_EQ( + get(l, "t5('interpreted_string')"), "You have {a} {b} {sSimpleString} You have {b} {a}"); + EXPECT_EQ(get(l, "t5('interpreted_string', { a = 1, b = 2, sSimpleString = 3 })"), + "You have 1 2 3 You have 2 1"); + EXPECT_EQ(get(l, "t5('inject_string')"), "{a}"); + EXPECT_EQ(get(l, "t5('inject_string', { a = 1 })"), "{a}"); + EXPECT_EQ(get(l, "t5('plural_string', { count = 1 })"), "Hello it's the world"); + EXPECT_EQ(get(l, "t5('plural_string', { count = 2 })"), "You have 2 2"); + EXPECT_EQ(get(l, "t5('unnamed_string')"), "You have {0} {1}"); + EXPECT_EQ(get(l, "t5('unnamed_string', { ['0'] = 'a', ['1'] = 'b' })"), "You have a b"); + EXPECT_EQ(get(l, "t5('no_gmst')"), "Hello world"); + EXPECT_EQ(get(l, "t5('not_found')"), "GMST:sNoSuchString"); + + l10nManager.setPreferredLocales({ "en" }, false); + EXPECT_EQ(get(l, "t5('string')"), "sSimpleString"); + EXPECT_EQ(get(l, "t5('format_string')"), "sFormatString"); + EXPECT_EQ(get(l, "t5('not_found')"), "sNoSuchString"); + }); + } } diff --git a/apps/openmw/engine.cpp b/apps/openmw/engine.cpp index cabcf3e883..aa91368f58 100644 --- a/apps/openmw/engine.cpp +++ b/apps/openmw/engine.cpp @@ -864,21 +864,15 @@ void OMW::Engine::prepareEngine() mWorld->setRandomSeed(mRandomSeed); const MWWorld::Store* gmst = &mWorld->getStore().get(); - mL10nManager->setGmstLoader( - [gmst, misses = std::set>()](std::string_view gmstName) mutable { - const ESM::GameSetting* res = gmst->search(gmstName); - if (res && res->mValue.getType() == ESM::VT_String) - return res->mValue.getString(); - else - { - if (misses.count(gmstName) == 0) - { - misses.emplace(gmstName); - Log(Debug::Error) << "GMST " << gmstName << " not found"; - } - return std::string("GMST:") + std::string(gmstName); - } - }); + mL10nManager->setGmstLoader([gmst, misses = std::set()]( + std::string_view gmstName) mutable -> const std::string* { + const ESM::GameSetting* res = gmst->search(gmstName); + if (res && res->mValue.getType() == ESM::VT_String) + return &res->mValue.getString(); + if (misses.emplace(gmstName).second) + Log(Debug::Error) << "GMST " << gmstName << " not found"; + return nullptr; + }); mWindowManager->setStore(mWorld->getStore()); mWindowManager->initUI(); diff --git a/components/l10n/manager.hpp b/components/l10n/manager.hpp index 89a9bd4b05..36558db72c 100644 --- a/components/l10n/manager.hpp +++ b/components/l10n/manager.hpp @@ -24,7 +24,7 @@ namespace L10n void dropCache() { mCache.clear(); } void setPreferredLocales(const std::vector& locales, bool gmstHasPriority = true); const std::vector& getPreferredLocales() const { return mPreferredLocales; } - void setGmstLoader(std::function fn) { mGmstLoader = std::move(fn); } + void setGmstLoader(GmstLoader fn) { mGmstLoader = std::move(fn); } std::shared_ptr getContext( std::string_view contextName, const std::string& fallbackLocale = "en"); @@ -41,7 +41,7 @@ namespace L10n const VFS::Manager* mVFS; std::vector mPreferredLocales; std::map, std::shared_ptr, std::less<>> mCache; - std::function mGmstLoader; + GmstLoader mGmstLoader; }; } diff --git a/components/l10n/messagebundles.cpp b/components/l10n/messagebundles.cpp index 665704e30d..410dc2ec8c 100644 --- a/components/l10n/messagebundles.cpp +++ b/components/l10n/messagebundles.cpp @@ -1,11 +1,16 @@ #include "messagebundles.hpp" +#include #include +#include +#include + #include #include #include #include +#include namespace L10n { @@ -39,17 +44,174 @@ namespace L10n return status.isSuccess(); } - std::string loadGmst( - const std::function& gmstLoader, const icu::MessageFormat* message) + std::optional parseMessageFormat( + const icu::Locale& lang, std::string_view key, std::string_view value, std::string_view locale) { - icu::UnicodeString gmstNameUnicode; - std::string gmstName; + icu::UnicodeString pattern + = icu::UnicodeString::fromUTF8(icu::StringPiece(value.data(), static_cast(value.size()))); + icu::ErrorCode status; + UParseError parseError; + icu::MessageFormat message(pattern, lang, parseError, status); + if (checkSuccess(status, parseError, "Failed to create message ", key, " for locale ", locale)) + return message; + return {}; + } + + template + using StringMap = std::unordered_map>; + + void loadLocaleYaml(const YAML::Node& data, const icu::Locale& lang, StringMap& bundle) + { + const std::string_view localeName = lang.getName(); + for (const auto& it : data) + { + const auto key = it.first.as(); + const auto value = it.second.as(); + std::optional message = parseMessageFormat(lang, key, value, localeName); + if (message) + bundle.emplace(key, *message); + } + } + + constexpr std::string_view gmstTokenStart = "{gmst:"; + + void loadGmstYaml(const YAML::Node& data, StringMap& gmsts) + { + for (const auto& it : data) + { + const auto key = it.first.as(); + GmstMessageFormat message; + if (it.second.IsMap()) + { + message.mPattern = it.second["pattern"].as(); + if (YAML::Node variables = it.second["variables"]) + message.mVariableNames = variables.as>>(); + message.mReplaceFormat = true; + } + else + { + const auto value = it.second.as(); + message.mPattern.reserve(gmstTokenStart.size() + 1 + value.size()); + message.mPattern = gmstTokenStart; + message.mPattern += value; + message.mPattern += '}'; + } + gmsts.emplace(key, std::move(message)); + } + } + + class GmstFormatParser : public Misc::MessageFormatParser + { + std::array mBuffer; + std::string& mOut; + std::span mVariableNames; + std::size_t mVariableIndex; + + public: + GmstFormatParser(std::string& out, std::span variables) + : mOut(out) + , mVariableNames(variables) + , mVariableIndex(0) + { + } + + protected: + void visitedPlaceholder(Placeholder, char, int, int, Notation) override + { + mOut += '{'; + if (mVariableIndex < mVariableNames.size() && !mVariableNames[mVariableIndex].empty()) + mOut += mVariableNames[mVariableIndex]; + else + { + const auto [ptr, ec] + = std::to_chars(mBuffer.data(), mBuffer.data() + mBuffer.size(), mVariableIndex); + if (ec == std::errc()) + mOut += std::string_view(mBuffer.data(), ptr); + } + mOut += '}'; + mVariableIndex++; + } + + void visitedCharacter(char c) override + { + if (c == '\'' || c == '{' || c == '}') + mOut += '\''; + mOut += c; + } + }; + + std::optional convertToMessageFormat( + std::string_view key, const GmstMessageFormat& gmstFormat, const GmstLoader& gmstLoader) + { + std::string formatString; + std::size_t offset = 0; + std::size_t tokenIndex = 0; + const std::string_view pattern(gmstFormat.mPattern); + while (offset < pattern.size()) + { + const std::size_t start = pattern.find(gmstTokenStart, offset); + if (start == std::string_view::npos) + { + formatString += pattern.substr(offset); + break; + } + const std::size_t tokenStart = start + gmstTokenStart.size(); + const std::size_t end = pattern.find_first_of("{}", tokenStart); + if (end == std::string_view::npos || pattern[end] == '{') + { + // Not a GMST token + formatString += pattern.substr(offset, end - offset); + offset = end; + continue; + } + // Replace GMST token + formatString += pattern.substr(offset, start - offset); + offset = end + 1; + std::string_view gmst = pattern.substr(tokenStart, end - tokenStart); + const std::string* value = gmstLoader(gmst); + const auto appendEscaped = [&](std::string_view string) { + for (char c : string) + { + if (c == '\'' || c == '{' || c == '}') + formatString += '\''; + formatString += c; + } + }; + if (value == nullptr) + { + // Unknown GMST string + formatString += "GMST:"; + appendEscaped(gmst); + } + else if (gmstFormat.mReplaceFormat) + { + std::span variableNames; + if (tokenIndex < gmstFormat.mVariableNames.size()) + variableNames = gmstFormat.mVariableNames[tokenIndex]; + GmstFormatParser parser(formatString, variableNames); + parser.process(*value); + } + else + appendEscaped(*value); + tokenIndex++; + } + const icu::Locale& english = icu::Locale::getEnglish(); + return parseMessageFormat(english, key, formatString, "gmst"); + } + + std::string formatArgs(const icu::MessageFormat& message, std::string_view key, + const std::vector& argNames, const std::vector& args) + { + icu::UnicodeString result; + std::string resultString; icu::ErrorCode success; - message->format(nullptr, nullptr, 0, gmstNameUnicode, success); - gmstNameUnicode.toUTF8String(gmstName); - if (gmstLoader) - return gmstLoader(gmstName); - return "GMST:" + gmstName; + if (!args.empty() && !argNames.empty()) + message.format(argNames.data(), args.data(), static_cast(args.size()), result, success); + else + message.format(nullptr, nullptr, static_cast(args.size()), result, success); + checkSuccess(success, {}, "Failed to format message ", key); + result.toUTF8String(resultString); + return resultString; } } @@ -87,21 +249,10 @@ namespace L10n { YAML::Node data = YAML::Load(input); std::string localeName = lang.getName(); - const icu::Locale& langOrEn = localeName == "gmst" ? icu::Locale::getEnglish() : lang; - for (const auto& it : data) - { - const auto key = it.first.as(); - const auto value = it.second.as(); - icu::UnicodeString pattern - = icu::UnicodeString::fromUTF8(icu::StringPiece(value.data(), static_cast(value.size()))); - icu::ErrorCode status; - UParseError parseError; - icu::MessageFormat message(pattern, langOrEn, parseError, status); - if (checkSuccess(status, parseError, "Failed to create message ", key, " for locale ", lang.getName())) - { - mBundles[localeName].emplace(key, message); - } - } + if (localeName == "gmst") + loadGmstYaml(data, mGmsts); + else + loadLocaleYaml(data, lang, mBundles[localeName]); } const icu::MessageFormat* MessageBundles::findMessage(std::string_view key, std::string_view localeName) const @@ -115,6 +266,21 @@ namespace L10n return &(message->second); } } + if (localeName == "gmst" && mGmstLoader) + { + auto found = mGmsts.find(key); + if (found != mGmsts.end()) + { + auto message = convertToMessageFormat(key, found->second, mGmstLoader); + mGmsts.erase(found); + if (message) + { + if (iter == mBundles.end()) + iter = mBundles.emplace(localeName, StringMap()).first; + return &iter->second.emplace(key, *message).first->second; + } + } + } return nullptr; } @@ -135,55 +301,24 @@ namespace L10n std::string MessageBundles::formatMessage(std::string_view key, const std::vector& argNames, const std::vector& args) const { - icu::UnicodeString result; - std::string resultString; - icu::ErrorCode success; - - const icu::MessageFormat* message = nullptr; for (auto& loc : mPreferredLocaleStrings) { - message = findMessage(key, loc); - if (message) - { - if (loc == "gmst") - return loadGmst(mGmstLoader, message); - break; - } + if (const icu::MessageFormat* message = findMessage(key, loc)) + return formatArgs(*message, key, argNames, args); } // If no requested locales included the message, try the fallback locale - if (!message) - message = findMessage(key, mFallbackLocale.getName()); + if (const icu::MessageFormat* message = findMessage(key, mFallbackLocale.getName())) + return formatArgs(*message, key, argNames, args); - if (message) - { - if (!args.empty() && !argNames.empty()) - message->format(argNames.data(), args.data(), static_cast(args.size()), result, success); - else - message->format(nullptr, nullptr, static_cast(args.size()), result, success); - checkSuccess(success, {}, "Failed to format message ", key); - result.toUTF8String(resultString); - return resultString; - } icu::Locale defaultLocale(nullptr); if (!mPreferredLocales.empty()) { defaultLocale = mPreferredLocales[0]; } - UParseError parseError; - icu::MessageFormat defaultMessage( - icu::UnicodeString::fromUTF8(icu::StringPiece(key.data(), static_cast(key.size()))), - defaultLocale, parseError, success); - if (!checkSuccess(success, parseError, "Failed to create message ", key)) + std::optional defaultMessage = parseMessageFormat(defaultLocale, key, key, "default"); + if (!defaultMessage) // If we can't parse the key as a pattern, just return the key return std::string(key); - - if (!args.empty() && !argNames.empty()) - defaultMessage.format( - argNames.data(), args.data(), static_cast(args.size()), result, success); - else - defaultMessage.format(nullptr, nullptr, static_cast(args.size()), result, success); - checkSuccess(success, {}, "Failed to format message ", key); - result.toUTF8String(resultString); - return resultString; + return formatArgs(*defaultMessage, key, argNames, args); } } diff --git a/components/l10n/messagebundles.hpp b/components/l10n/messagebundles.hpp index 15ccbaa630..ef4ce49cee 100644 --- a/components/l10n/messagebundles.hpp +++ b/components/l10n/messagebundles.hpp @@ -14,6 +14,15 @@ namespace L10n { + struct GmstMessageFormat + { + std::string mPattern; + std::vector> mVariableNames; + bool mReplaceFormat = false; + }; + + using GmstLoader = std::function; + /** * @brief A collection of Message Bundles * @@ -48,17 +57,18 @@ namespace L10n return mBundles.find(std::string_view(loc.getName())) != mBundles.end(); } const icu::Locale& getFallbackLocale() const { return mFallbackLocale; } - void setGmstLoader(std::function fn) { mGmstLoader = std::move(fn); } + void setGmstLoader(GmstLoader fn) { mGmstLoader = std::move(fn); } private: template using StringMap = std::unordered_map>; // icu::Locale isn't hashable (or comparable), so we use the string form instead, which is canonicalized - StringMap> mBundles; + mutable StringMap> mBundles; + mutable StringMap mGmsts; const icu::Locale mFallbackLocale; std::vector mPreferredLocaleStrings; std::vector mPreferredLocales; - std::function mGmstLoader; + GmstLoader mGmstLoader; const icu::MessageFormat* findMessage(std::string_view key, std::string_view localeName) const; }; From 0a04bfa46d4233dce05f741d43f30e2fefdab136 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Mon, 24 Nov 2025 19:43:39 +0100 Subject: [PATCH 2/5] Fix rebase --- components/l10n/messagebundles.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/l10n/messagebundles.cpp b/components/l10n/messagebundles.cpp index 410dc2ec8c..7d37b4358f 100644 --- a/components/l10n/messagebundles.cpp +++ b/components/l10n/messagebundles.cpp @@ -116,7 +116,7 @@ namespace L10n } protected: - void visitedPlaceholder(Placeholder, char, int, int, Notation) override + void visitedPlaceholder(Placeholder, int, int, int, Notation) override { mOut += '{'; if (mVariableIndex < mVariableNames.size() && !mVariableNames[mVariableIndex].empty()) From f7ad7a8263002240bacedfba93d50ed15452059c Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Mon, 24 Nov 2025 20:11:07 +0100 Subject: [PATCH 3/5] Replace string.format with l10n in playerskillhandlers --- files/data-mw/CMakeLists.txt | 3 +++ files/data-mw/l10n/Mechanics/gmst.yaml | 17 +++++++++++++++++ .../data-mw/scripts/omw/playerskillhandlers.lua | 16 ++++++---------- 3 files changed, 26 insertions(+), 10 deletions(-) create mode 100644 files/data-mw/l10n/Mechanics/gmst.yaml diff --git a/files/data-mw/CMakeLists.txt b/files/data-mw/CMakeLists.txt index fd36d6d314..41924315c5 100644 --- a/files/data-mw/CMakeLists.txt +++ b/files/data-mw/CMakeLists.txt @@ -15,6 +15,9 @@ set(BUILTIN_DATA_MW_FILES # Generic UI messages that can be reused by mods l10n/Interface/gmst.yaml + # L10n for game-specific mechanics + l10n/Mechanics/gmst.yaml + # L10n for OpenMW menus and non-game-specific messages l10n/OMWEngine/gmst.yaml diff --git a/files/data-mw/l10n/Mechanics/gmst.yaml b/files/data-mw/l10n/Mechanics/gmst.yaml new file mode 100644 index 0000000000..d672e1d4b1 --- /dev/null +++ b/files/data-mw/l10n/Mechanics/gmst.yaml @@ -0,0 +1,17 @@ +ReleasedFromPrison: + pattern: |- + {days, plural, + one{{gmst:sNotifyMessage42}} + other{{gmst:sNotifyMessage43}} + } + variables: + - ["days"] + - ["days"] +SkillIncreasedTo: + pattern: "{gmst:sNotifyMessage39}" + variables: + - ["skill", "level"] +SkillDecreasedTo: + pattern: "{gmst:sNotifyMessage44}" + variables: + - ["skill", "level"] diff --git a/files/data-mw/scripts/omw/playerskillhandlers.lua b/files/data-mw/scripts/omw/playerskillhandlers.lua index d070dd0311..14fc1c6bbf 100644 --- a/files/data-mw/scripts/omw/playerskillhandlers.lua +++ b/files/data-mw/scripts/omw/playerskillhandlers.lua @@ -8,6 +8,7 @@ local NPC = types.NPC local Actor = types.Actor local ui = require('openmw.ui') local auxUtil = require('openmw_aux.util') +local mechanicsL10n = core.l10n('Mechanics') local function tableHasValue(table, value) for _, v in pairs(table) do @@ -99,7 +100,7 @@ local function skillLevelUpHandler(skillid, source, params) ambient.playSound("skillraise") - local message = string.format(core.getGMST('sNotifyMessage39'),skillRecord.name,skillStat.base) + local message = mechanicsL10n('SkillIncreasedTo', { skill = skillRecord.name, level = skillStat.base }) if source == I.SkillProgression.SKILL_INCREASE_SOURCES.Book then message = '#{sBookSkillMessage}\n'..message @@ -134,21 +135,16 @@ local function jailTimeServed(days) I.SkillProgression.skillLevelUp(skillid, I.SkillProgression.SKILL_INCREASE_SOURCES.Jail) end - local message = '' - if days == 1 then - message = string.format(core.getGMST('sNotifyMessage42'), days) - else - message = string.format(core.getGMST('sNotifyMessage43'), days) - end + local message = mechanicsL10n('ReleasedFromPrison', { days = days }); for skillid, skillStat in pairs(NPC.stats.skills) do local diff = skillStat(self).base - oldSkillLevels[skillid] if diff ~= 0 then - local skillMsg = core.getGMST('sNotifyMessage39') + local skillMsg = 'SkillIncreasedTo' if diff < 0 then - skillMsg = core.getGMST('sNotifyMessage44') + skillMsg = 'SkillDecreasedTo' end local skillRecord = Skill.record(skillid) - message = message..'\n'..string.format(skillMsg, skillRecord.name, skillStat(self).base) + message = message..'\n'..mechanicsL10n(skillMsg, { skill = skillRecord.name, level = skillStat(self).base }) end end From 0c95e5c7b8419c707455e63e34b53784866743f1 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Tue, 25 Nov 2025 22:20:27 +0100 Subject: [PATCH 4/5] Ensure thread safety --- components/l10n/messagebundles.cpp | 19 ++++++++++++++----- components/l10n/messagebundles.hpp | 2 ++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/components/l10n/messagebundles.cpp b/components/l10n/messagebundles.cpp index 7d37b4358f..7a0799f21d 100644 --- a/components/l10n/messagebundles.cpp +++ b/components/l10n/messagebundles.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -257,17 +258,24 @@ namespace L10n const icu::MessageFormat* MessageBundles::findMessage(std::string_view key, std::string_view localeName) const { - auto iter = mBundles.find(localeName); - if (iter != mBundles.end()) + std::shared_lock sharedLock(mMutex); { - auto message = iter->second.find(key); - if (message != iter->second.end()) + auto iter = mBundles.find(localeName); + if (iter != mBundles.end()) { - return &(message->second); + auto message = iter->second.find(key); + if (message != iter->second.end()) + { + return &(message->second); + } } } if (localeName == "gmst" && mGmstLoader) { + if (!mGmsts.contains(key)) + return nullptr; + sharedLock.unlock(); + std::unique_lock lock(mMutex); auto found = mGmsts.find(key); if (found != mGmsts.end()) { @@ -275,6 +283,7 @@ namespace L10n mGmsts.erase(found); if (message) { + auto iter = mBundles.find(localeName); if (iter == mBundles.end()) iter = mBundles.emplace(localeName, StringMap()).first; return &iter->second.emplace(key, *message).first->second; diff --git a/components/l10n/messagebundles.hpp b/components/l10n/messagebundles.hpp index ef4ce49cee..ac67c94ff0 100644 --- a/components/l10n/messagebundles.hpp +++ b/components/l10n/messagebundles.hpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -65,6 +66,7 @@ namespace L10n // icu::Locale isn't hashable (or comparable), so we use the string form instead, which is canonicalized mutable StringMap> mBundles; mutable StringMap mGmsts; + mutable std::shared_mutex mMutex; const icu::Locale mFallbackLocale; std::vector mPreferredLocaleStrings; std::vector mPreferredLocales; From b424929fce4c36662076826ca6013747ed2829d0 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Mon, 1 Dec 2025 17:02:28 +0100 Subject: [PATCH 5/5] Address feedback --- components/l10n/messagebundles.cpp | 46 +++++++++++-------- .../scripts/omw/playerskillhandlers.lua | 4 +- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/components/l10n/messagebundles.cpp b/components/l10n/messagebundles.cpp index 7a0799f21d..b1ccbd6b47 100644 --- a/components/l10n/messagebundles.cpp +++ b/components/l10n/messagebundles.cpp @@ -214,6 +214,19 @@ namespace L10n result.toUTF8String(resultString); return resultString; } + + const icu::MessageFormat* getMessage( + const StringMap>& bundles, std::string_view key, std::string_view localeName) + { + auto iter = bundles.find(localeName); + if (iter != bundles.end()) + { + auto message = iter->second.find(key); + if (message != iter->second.end()) + return &(message->second); + } + return nullptr; + } } MessageBundles::MessageBundles(const std::vector& preferredLocales, icu::Locale& fallbackLocale) @@ -260,15 +273,9 @@ namespace L10n { std::shared_lock sharedLock(mMutex); { - auto iter = mBundles.find(localeName); - if (iter != mBundles.end()) - { - auto message = iter->second.find(key); - if (message != iter->second.end()) - { - return &(message->second); - } - } + auto message = getMessage(mBundles, key, localeName); + if (message != nullptr) + return message; } if (localeName == "gmst" && mGmstLoader) { @@ -277,17 +284,18 @@ namespace L10n sharedLock.unlock(); std::unique_lock lock(mMutex); auto found = mGmsts.find(key); - if (found != mGmsts.end()) + // Another thread deleted the key, retry mBundles + if (found == mGmsts.end()) + return getMessage(mBundles, key, localeName); + // We're the first thread to resolve this key + auto message = convertToMessageFormat(key, found->second, mGmstLoader); + mGmsts.erase(found); + if (message) { - auto message = convertToMessageFormat(key, found->second, mGmstLoader); - mGmsts.erase(found); - if (message) - { - auto iter = mBundles.find(localeName); - if (iter == mBundles.end()) - iter = mBundles.emplace(localeName, StringMap()).first; - return &iter->second.emplace(key, *message).first->second; - } + auto iter = mBundles.find(localeName); + if (iter == mBundles.end()) + iter = mBundles.emplace(localeName, StringMap()).first; + return &iter->second.emplace(key, *message).first->second; } } return nullptr; diff --git a/files/data-mw/scripts/omw/playerskillhandlers.lua b/files/data-mw/scripts/omw/playerskillhandlers.lua index 14fc1c6bbf..856f83cf4b 100644 --- a/files/data-mw/scripts/omw/playerskillhandlers.lua +++ b/files/data-mw/scripts/omw/playerskillhandlers.lua @@ -90,7 +90,7 @@ local function skillLevelUpHandler(skillid, source, params) if params.levelUpSpecialization and params.levelUpSpecializationIncreaseValue then levelStat.skillIncreasesForSpecialization[params.levelUpSpecialization] - = levelStat.skillIncreasesForSpecialization[params.levelUpSpecialization] + params.levelUpSpecializationIncreaseValue; + = levelStat.skillIncreasesForSpecialization[params.levelUpSpecialization] + params.levelUpSpecializationIncreaseValue end if source ~= 'jail' then @@ -135,7 +135,7 @@ local function jailTimeServed(days) I.SkillProgression.skillLevelUp(skillid, I.SkillProgression.SKILL_INCREASE_SOURCES.Jail) end - local message = mechanicsL10n('ReleasedFromPrison', { days = days }); + local message = mechanicsL10n('ReleasedFromPrison', { days = days }) for skillid, skillStat in pairs(NPC.stats.skills) do local diff = skillStat(self).base - oldSkillLevels[skillid] if diff ~= 0 then