Repository navigation
refactor(i18n): texts by resx key, no legacy ids; ancient set option fixes - #146
Merged
Merged
Conversation
Mosch0512
commented
Oct 30, 2026
Comment on lines
-2088
to
-2101
| Damage = 0; | ||
| g_csItemOption.PlusSpecialLevel(&Damage, Dexterity, AT_SET_OPTION_IMPROVE_ATTACK_1); | ||
| Character.AttackDamageMinRight += Damage; | ||
| Character.AttackDamageMaxRight += Damage; | ||
| Character.AttackDamageMinLeft += Damage; | ||
| Character.AttackDamageMaxLeft += Damage; | ||
|
|
||
| Damage = 0; | ||
| g_csItemOption.PlusSpecialLevel(&Damage, Strength, AT_SET_OPTION_IMPROVE_ATTACK_2); | ||
| Character.AttackDamageMinRight += Damage; | ||
| Character.AttackDamageMaxRight += Damage; | ||
| Character.AttackDamageMinLeft += Damage; | ||
| Character.AttackDamageMaxLeft += Damage; | ||
|
|
Mosch0512
commented
Oct 30, 2026
Mosch0512
left a comment
Owner
Author
There was a problem hiding this comment.
Moste of the code looks good but i have some questions pls answer them thx
Comment on lines
-2331
to
-2337
| WORD MagicDamage = 0; | ||
|
|
||
| MagicDamage = 0; | ||
| g_csItemOption.PlusSpecialLevel(&MagicDamage, Energy, AT_SET_OPTION_IMPROVE_MAGIC); | ||
| Character.MagicDamageMin += MagicDamage; | ||
| Character.MagicDamageMax += MagicDamage; | ||
|
|
| WORD Dexterity; | ||
|
|
||
| Dexterity = Character.Dexterity + Character.AddDexterity; | ||
| Vitality = Character.Vitality + Character.AddVitality; |
Comment on lines
-2691
to
-2692
| g_csItemOption.PlusSpecialLevel(&Character.Defense, Dexterity, AT_SET_OPTION_IMPROVE_DEFENCE_3); | ||
| g_csItemOption.PlusSpecialLevel(&Character.Defense, Vitality, AT_SET_OPTION_IMPROVE_DEFENCE_4); |
Comment on lines
-105
to
-134
| const int textNum = 824 + m_iMatchCountDownType - TYPE_MATCH_CASTLE_ENTER_CLOSE; | ||
| WriteWide(lpszStr, I18N::Game::Lookup(textNum), I18N::Game::BloodCastle, remainingSeconds); | ||
| WriteWide(lpszStr, format, I18N::Game::BloodCastle, remainingSeconds); | ||
| } | ||
| else if (m_iMatchCountDownType >= TYPE_MATCH_CHAOS_ENTER_START && m_iMatchCountDownType <= TYPE_MATCH_CHAOS_END) | ||
| { | ||
| int textNum = 824 + m_iMatchCountDownType - TYPE_MATCH_CHAOS_ENTER_START; | ||
| if (textNum == 825) | ||
| { | ||
| textNum = 828; | ||
| } | ||
| WriteWide(lpszStr, I18N::Game::Lookup(textNum), I18N::Game::ChaosCastle, remainingSeconds); | ||
| WriteWide(lpszStr, format, I18N::Game::ChaosCastle, remainingSeconds); | ||
| } | ||
| else if (m_iMatchCountDownType == TYPE_MATCH_CURSEDTEMPLE_ENTER_CLOSE | ||
| || m_iMatchCountDownType == TYPE_MATCH_CURSEDTEMPLE_GAME_START) | ||
| else | ||
| { | ||
| int textNum = (m_iMatchCountDownType == TYPE_MATCH_CURSEDTEMPLE_GAME_START) ? 2386 : 2384; | ||
| WriteWide(lpszStr, I18N::Game::Lookup(textNum), remainingSeconds); | ||
| WriteWide(lpszStr, format, remainingSeconds); | ||
| } | ||
| else if (m_iMatchCountDownType >= TYPE_MATCH_DOPPELGANGER_ENTER_CLOSE && m_iMatchCountDownType <= TYPE_MATCH_DOPPELGANGER_CLOSE) | ||
|
|
||
| return lpszStr; | ||
| } | ||
|
|
||
| const wchar_t* GetMatchCountdownFormat(int matchType) | ||
| { | ||
| if (matchType >= TYPE_MATCH_DEVIL_ENTER_START && matchType <= TYPE_MATCH_DEVIL_CLOSE) | ||
| { | ||
| const int textNum = 2860 + m_iMatchCountDownType - TYPE_MATCH_DOPPELGANGER_ENTER_CLOSE; | ||
| WriteWide(lpszStr, I18N::Game::Lookup(textNum), remainingSeconds); | ||
| return *kDevilSquareCountdownFormats[matchType - TYPE_MATCH_DEVIL_ENTER_START]; | ||
| } | ||
| else | ||
| if (matchType >= TYPE_MATCH_CASTLE_ENTER_CLOSE && matchType <= TYPE_MATCH_CASTLE_END) | ||
| { | ||
| const int textNum = 640 + m_iMatchCountDownType - TYPE_MATCH_DEVIL_ENTER_START; | ||
| WriteWide(lpszStr, I18N::Game::Lookup(textNum), remainingSeconds); | ||
| return *kBloodCastleCountdownFormats[matchType - TYPE_MATCH_CASTLE_ENTER_CLOSE]; | ||
| } | ||
|
|
||
| return lpszStr; |
Owner
Author
There was a problem hiding this comment.
explain me this code difference
Comment on lines
-120
to
-128
| constexpr int ResolveDescriptionTextId(CLASS_TYPE selectedClass) | ||
| { | ||
| if (selectedClass == CLASS_SUMMONER) | ||
| return kSummonerDescriptionTextId; | ||
| if (selectedClass == CLASS_RAGEFIGHTER) | ||
| return kRageFighterDescriptionTextId; | ||
| return kDefaultDescriptionBase + selectedClass; | ||
| } | ||
|
|
Owner
Author
There was a problem hiding this comment.
whree did it go? explain to me
Comment on lines
-495
to
-523
| void CSItemOption::PlusSpecialLevel(std::uint16_t* Value, std::uint16_t SrcValue, int Special) const | ||
| { | ||
| Special -= AT_SET_OPTION_IMPROVE_STRENGTH; | ||
| int optionValue = 0; | ||
| int count = 0; | ||
|
|
||
| for (int i = 0; i < m_SetSearchResultCount; i++) | ||
| { | ||
| const auto& set = m_SetSearchResult[i]; | ||
| for (int j = 0; j < set.SetOptionCount; j++) | ||
| { | ||
| const auto& option = set.SetOption[j]; | ||
| if (option.IsActive | ||
| && option.OptionNumber == Special | ||
| && option.Value != 0) | ||
| { | ||
| optionValue += option.Value; | ||
| count++; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (optionValue) | ||
| { | ||
| optionValue = SrcValue * optionValue / 100; | ||
| *Value += (optionValue * count); | ||
| } | ||
| } | ||
|
|
Comment on lines
-1061
to
1066
| else if (option.OptionNumber >= AT_SET_OPTION_IMPROVE_ATTACK_1 && !option.FulfillsClassRequirement) | ||
| { | ||
| // Mastery | ||
| TextListColor[textIndex] = TEXT_COLOR_RED; | ||
| } | ||
| else |
Owner
Author
There was a problem hiding this comment.
why gone dead branch or did it move?
Comment on lines
+22
to
+102
| namespace | ||
| { | ||
| struct MixRecipeText | ||
| { | ||
| int textId; | ||
| const wchar_t* const* text; | ||
| }; | ||
|
|
||
| // The text numbers mix.bmd stores in its recipes' names, descriptions and advice, sorted by number. | ||
| const MixRecipeText kMixRecipeTexts[] = { | ||
| {588, &I18N::Game::InCaseYouFailOn}, | ||
| {589, &I18N::Game::PleaseNoteThat}, | ||
| {590, &I18N::Game::TheLevelOfItemsDecreases}, | ||
| {600, &I18N::Game::ItemLevelsMustBeIdenticalToCombineItemsAreOfTheSameLevel}, | ||
| {603, &I18N::Game::CreateATicketOfDevilSquare}, | ||
| {604, &I18N::Game::Create10Item}, | ||
| {605, &I18N::Game::Create11Item}, | ||
| {606, &I18N::Game::DuringCreationOf1015Items}, | ||
| {607, &I18N::Game::NoticeThatThereIsAPossibility}, | ||
| {608, &I18N::Game::ThatYouMayLoseTheItems}, | ||
| {610, &I18N::Game::NoticeThatWhenYouFailToCombineTheItems}, | ||
| {611, &I18N::Game::YouCanLoseTheItems}, | ||
| {612, &I18N::Game::CreateDinorant}, | ||
| {613, &I18N::Game::CreateFruit}, | ||
| {614, &I18N::Game::CreateWings}, | ||
| {615, &I18N::Game::CreateCloakOfInvisibility}, | ||
| {665, &I18N::Game::_12ItemCreation}, | ||
| {666, &I18N::Game::_13ItemCreation}, | ||
| {736, &I18N::Game::ChaosWeaponCombination}, | ||
| {1172, &I18N::Game::CreateCloakItem}, | ||
| {1187, &I18N::Game::DarkHorse}, | ||
| {1212, &I18N::Game::Resurrection}, | ||
| {1214, &I18N::Game::DarkRaven}, | ||
| {1412, &I18N::Game::Create}, | ||
| {1413, &I18N::Game::PotionOfBless}, | ||
| {1414, &I18N::Game::PotionOfSoul}, | ||
| {1415, &I18N::Game::LifeStone}, | ||
| {1550, &I18N::Game::Improve}, | ||
| {1640, &I18N::Game::Store1640}, | ||
| {1666, &I18N::Game::CondorSFeather}, | ||
| {1667, &I18N::Game::_3rdWing}, | ||
| {1863, &I18N::Game::Destroy}, | ||
| {1864, &I18N::Game::Protect}, | ||
| {1921, &I18N::Game::FragmentOfHorn}, | ||
| {1922, &I18N::Game::BrokenHorn}, | ||
| {1923, &I18N::Game::FenrirSHorn}, | ||
| {2038, &I18N::Game::SDPotion}, | ||
| {2061, &I18N::Game::Refine}, | ||
| {2062, &I18N::Game::Restore}, | ||
| {2194, &I18N::Game::Add380ItemOption}, | ||
| {2203, &I18N::Game::RefineGemstone}, | ||
| {2222, &I18N::Game::ItemWillDisappearWhenFailed}, | ||
| {2261, &I18N::Game::YouCanAchieveSpecialItemsWithCombinations}, | ||
| {2263, &I18N::Game::ItemsExceptChaosCard}, | ||
| {2265, &I18N::Game::ChaosCardCombination}, | ||
| {2267, &I18N::Game::CanTExecuteCombination}, | ||
| {2336, &I18N::Game::EquipmentItem}, | ||
| {2396, &I18N::Game::ScrollOfBlood}, | ||
| {2560, &I18N::Game::CherryBlossomsBranchesAssembly}, | ||
| {2664, &I18N::Game::SeedExtraction}, | ||
| {2665, &I18N::Game::SeedSphereAssembly}, | ||
| {2669, &I18N::Game::SeedSphereApplication}, | ||
| {2670, &I18N::Game::SeedSphereDestruction}, | ||
| {2791, &I18N::Game::CompleteSecromicon}, | ||
| {2881, &I18N::Game::GoldenBox}, | ||
| {2882, &I18N::Game::SilverBox}, | ||
| {3086, &I18N::Game::_14ItemCreation}, | ||
| {3087, &I18N::Game::_15ItemCreation}, | ||
| }; | ||
| } // namespace | ||
|
|
||
| const wchar_t* SEASON3A::GetMixRecipeText(int textId) | ||
| { | ||
| const auto* const end = std::end(kMixRecipeTexts); | ||
| const auto* const found = std::lower_bound(std::begin(kMixRecipeTexts), end, textId, | ||
| [](const MixRecipeText& entry, int id) { return entry.textId < id; }); | ||
| if (found == end || found->textId != textId) | ||
| return L""; | ||
| return *found->text; | ||
| } | ||
|
|
Owner
Author
There was a problem hiding this comment.
how was this done bevore?
Comment on lines
-13
to
-22
| // I18N::Dialog legacy-id encoding (mirrored the removed DialogImporter tool): | ||
| // text: legacy_id = dialogIndex (0..199) | ||
| // answer (i, n): legacy_id = 1000 + dialogIndex*10 + slot (1000..2999) | ||
| inline constexpr int AnswerLegacyIdBase = 1000; | ||
| inline constexpr int AnswerLegacyIdStride = 10; | ||
| inline constexpr int DialogAnswerLegacyId(int dialogIndex, int answerSlot) noexcept | ||
| { | ||
| return AnswerLegacyIdBase + dialogIndex * AnswerLegacyIdStride + answerSlot; | ||
| } | ||
|
|
Owner
Author
There was a problem hiding this comment.
my guess is this was not requried anymore?
| /** | ||
| * @brief Handles item use confirmation dialogs (fruits, consumables). | ||
| */ | ||
| static void SetupItemUseStateMessage(int num, int index, int message) |
Owner
Author
There was a problem hiding this comment.
the howl file basically removed? i just want to understand
Mosch0512
force-pushed
the
fix/ancient-set-option-lines
branch
2 times, most recently
from
October 30, 2026 22:05
5d65dd1 to
5112263
Compare
Ancient set options 24 to 28 were class-only bonuses that grew with a
stat ("%s Increase damage strength/%d" and four more). No set in the
client data, OpenMU or the server files we checked uses them, and they
could not work as written:
- the texts were printed with one number, which went to the %s meant
for the class name, so a set using them would show garbage or crash;
- the number they printed is a percent (stat x value / 100) but reads
as a divisor, and two active copies of an option added four times
one copy's bonus.
They are now unsupported: their texts (11 languages), the five
PlusSpecialLevel calls and the enum entries are gone, and an option
without a description adds nothing to the set's option list. The
element mastery options 29 to 35 keep their numbers, which the set
data uses, and work as before (MASTERY_OPTION is now their first
number, so PlusMastery drops its "- 5").
Also removed: the red "class doesn't match" colour of the set option
list and the class flag it read. It compared a set option number
(0-35) with the enum value 191, so it never showed. The class lock that
leaves out mastery options for other classes is unchanged.
The option table shown for /<set name> in the chat (e.g. /Warrior) printed the ext and full set options with their option number as the value: Warrior's full set read "Increase critical damage rate 15%" for its 5% and "Increase strength +0" for its +25. They now print their values, as the per-item options already did. The table is built by BuildSetOptionTextList(set) now, so a test can check it with a set of its own.
calcSetOptionList built the option list of each equipped set inline and needed the hero for it. That part is CSItemOption::AddSetOptions now, and the mastery bonus of one set is GetMasteryBonus, which PlusMastery adds up. Both take the set and the wearer's class as arguments, so tests can build a set's options without a hero. No change in what the options do.
A set's mastery options (29 to 35, damage of one element's skills) are bound to classes by the set's byRequireClass: 1 for a class as it is, 2 only once it has its second class. The original client added them to the set's options only when the wearer's class was named. The client had this the wrong way round: it skipped them for the named class and gave them to every other class, so with the shipped data (no class named anywhere) every class would have got them. No set in the shipped data or in OpenMU uses options 29 to 35 yet, so nothing changes in the game today; this matters once the set editor writes them. A test covers no class, a named class, another class and the second-class requirement.
Dropping options 24 to 28 moved MASTERY_OPTION from 24 to 29 and took the '- 5' out of the mastery bonus; so far only the option texts were tested. Two cases build a set's option list: options 24 to 28 add nothing, and each mastery option (29 ice to 35 water) raises only its own element's skills, and only with the complete set. Both fail when the old '- 5' comes back or when options 24 to 28 are added again.
Whether a set option counts (TryAddSetOption) was decided by whether its description slot was empty, so the text table carried a game rule. IsSupportedSetOption is that rule now: 0 to 23 and the mastery options, not 24 to 28. TryAddSetOption and GetSetOptionFormat both ask it, and a static_assert checks that the text table describes exactly the supported options.
The test switched the game texts to English and left them so, which a later case in the same binary would have inherited. A scope now switches to English and back to the language before the test.
No doc said which set option numbers the client supports. item-data.md now has a section on the ancient set options: the files they come from, when standard, ext and full options count, the numbers 0 to 35 (24 to 28 not supported, 29 to 35 one element's skills), the class requirement of the element options as in the original client, and the /<set name> table in the chat.
The chat's /<set name> opened a set's option table only for its exact
name, capitals and "'s" included: /Warrior did nothing, only /Warrior's
worked, and the two sets whose names use the typographic apostrophe
(Chrono's, Semeden's) could not be typed at all. FindSetByName now
ignores capitals and the apostrophe (' or U+2019) and makes the
trailing s optional, so /Warrior, /warrior, /Warriors and /Warrior's
all open Warrior's.
When a text fits two sets, the set whose name it is (apart from
capitals and the apostrophe's form) wins, then the lower set number.
An item named like the text still opens first, as before. No two
shipped sets (Eng, Por, Spn) and no set and item name of any language
fit the same text. The docs say how the lookup works.
Mosch0512
force-pushed
the
fix/ancient-set-option-lines
branch
from
October 30, 2026 22:13
5112263 to
ed63709
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every text the client looked up by its old number (
I18N::Game::Lookup(971),I18N::Dialog::Lookup(...)) is named by its resx key now, and the legacy ids are gone from ResxGen and the resx files. On top of that, fixes in the ancient set options that the conversion turned up, and the fixes from the review of this PR. Client only: OpenMU sends the same numbers as before.This PR is for the fork's CI on both branches. Upstream they are two PRs, the text keys first: sven-n#905 and sven-n#906 (on top).
refactor/resx-keys-no-legacy-idsfix/ancient-set-option-lines/<set name>table values and lookup, mastery class requirement, review fixesTexts by resx key
&I18N::Game::Xslots and read them when the text is shown, so a language switch still reaches them.mix.bmdorSkill.bmdpicks a text by number, the number maps to slots where it arrives (Network/Server/ServerMessageTexts.cpp,SEASON3A::GetMixRecipeTextwith the 58 numbers the shippedmix.bmduses, small tables next to the event code). The file formats are unchanged.I18N::Dialogslots next to its reply links.static_assertchecks it against the range, so an entry too few fails the build instead of leaving a null slot. The two tables searched withstd::lower_bound(mix recipe texts, guild ranks) have astatic_assertthat they stay sorted, as the generated legacy table had.ZzzCharacter.cpptoGameLogic/NPCs/PlayerNpcChat.cpp.ShowCheckBox,SaveTextFile,M34CryWolf1st::Set_Message_Box(no callers),COMGEM::GetJewelIndex(replaced byGetJewelName),eGEMINDEXTYPE; ResxGen'sLookup/LookupSlotand the 33,751legacy_idcomments.docs/translation-system.mddescribes the slot tables instead.A number outside its message's table shows no text. Before, it showed whatever text came next in the old numbering:
%s);SetMasterType(it showed the font name);mix.bmdnumbers other than the 58.Every value OpenMU sends is inside its table.
Jewel combine question: Lahap's confirmation before combining jewels used the jewel's name as the format, so it read only "Jewel of Bless". It now asks "Are you sure to combine %s x %d?" with the jewel and the count (the original client had the same slip).
Ancient set options
%s Increase damage strength/%dand four more). No set in the client data, OpenMU'sAncientSets.csor five serverItemSetOption.txtfiles (Season 3, 4, 6, 9 and Webzen's 1.00.93) uses them. They printed one number into a format with%sfirst, showed a percent as a divisor, and two copies of one gave four times one copy's bonus. Their texts (11 languages), the fivePlusSpecialLevelcalls and enum entries are gone;IsSupportedSetOptionsays which options count (0 to 23 and the mastery options), and astatic_assertchecks that the text table describes exactly those.byRequireClass(1: the class, 2: once it has its second class), as in the original client. The client had this reversed: it left them out for the named class and gave them to every other class. No set uses them yet, so nothing changes in the game today./<set name>table fixed: typing a set's name after/in the chat showed the ext and full options with their option number as the value: Warrior's full set read "Increase critical damage rate 15%" for its 5% and "Increase strength +0" for its +25./<set name>lookup: the original needed the exact name (/Warrior's), and the two names with a typographic apostrophe (Chrono’s, Semeden’s) could not be typed at all. Capitals and'or’no longer matter and the trailingsis optional:/Warrior,/warrior,/Warriorsand/Warrior'sall open Warrior's. When a text fits two sets, the set whose name it is wins, then the lower set number; an item named like the text still opens first. No two shipped sets (Eng, Por, Spn), and no set and item name of any language, fit the same text.docs/item-data.mdhas a section on the ancient set options (files, when options count, the numbers 0-35, the class requirement, the/<set name>table).Checks
tests/i18nchecked every converted text againstLookupof the id it replaced, in all 12 locales (33 cases, 1.69 million checks, no mismatch). It ran on the branch before it was rebased onto87a404102. Since then upstream removed only the ids of the unused texts Heart, Jewel, Firecracker and Stone (101, 102, 106, 810, 2064): no id was remapped, and no conversion uses those five. The comparison is deleted together with the ids.test_ancient_set_options(7 cases) checks which option numbers have a description, that the/<set name>table prints each option's own value, which typed names find which set, that a mastery option counts only for the named class, that options 24-28 add nothing and that each mastery option raises only its own element's skills, with the complete set. Each case fails when its fix is undone; the newstatic_asserts fail the build on a table mismatch.5d65dd122: every target builds with no new warnings; withctest -j 3 -E rml-theme1,786 of 1,787 tests pass, andbundled_font_assetsfailed only in the parallel run (another test held its log file open) and passes alone. The rml-theme tests are left out: 4theme_file_interfacetests stop at a hidden RmlUi assertion box (URL.cpp:20,SetURL) on this machine, in the Debug builds of other branches too. The two later commits (a comment, the/<set name>lookup) rantest_ancient_set_optionson Windows and Linux.6492c3242): every commit builds and passeshelp_pages_testsandserver_list_data_tests(text-key branch) ortest_ancient_set_optionsandhelp_pages_tests(set option branch). The set option commits before the last one ran before their rebase onto the comment-only commit0bf1bb0a5.ci.yml's flags): pass on both branch tips against87a404102.