[ezequielzacca]

This step's own changes: compare with #816. The diff of this PR also holds every earlier step of #815, since a PR from a fork can only target this repository's branches.

Summary

Step 3 of 6 of #815 (data-driven item tooltips). Stacked on #816, which is stacked on #813: only the last commit, feat(items): named placeholders in item descriptions, is this step, and this PR will be rebased as the ones below it merge.

A description text can show values of the item: {name} is replaced when the tooltip draws it.

"description": [
  {"text": {"en": "At +{level}, a weapon deals {damageIncreaseWeapons} more damage",
            "de": "Bei +{level} macht eine Waffe {damageIncreaseWeapons} mehr Schaden"}}
]

• A name is one of two things:
  • a built-in value: level, the item's + level;
  • a level bonus table of Stats/ItemStats.json (refactor(items): item stats and requirements by level from data, stat profiles (phase 3c) #801), by its name: its value at the item's + level. damageIncreaseWeapons is 3 at +1 and 66 at +15.
  • So a new table is a new placeholder, without code. Step 4 uses that for the wings, whose formulas become tables.
• Item texts have no braces of their own, so every { and } belongs to a placeholder and there is no escape. I checked this against every tooltip text of the original client and every Game text in the 11 languages: none has a brace.
• The tooltip fills in the values after it picks the UI language's text, before the line wraps (feat(items): item descriptions in the item data #813).

Checks

They run when the item data loads, in the save checks and in Import from bmd, so a wrong placeholder in the shipped data fails the tests. All are errors:

Problem	Message (field description[0].text, or .text.<lang> for a translation)
{level	has a "{" without its "}"
{}	has an empty placeholder "{}"
level}	has a "}" without its "{"
{wing damage}	has "{wing damage}": a placeholder is a name of letters and digits
A translation with other placeholders than the English text (another order is fine)	has the placeholders {level}, {wingDamge}, the English text {level}, {wingDamage}
{damgeIncreaseWeapons}	has the unknown placeholder {damgeIncreaseWeapons}; did you mean {damageIncreaseWeapons}? (the nearest name at most two letters away)
{durability}	has the unknown placeholder {durability}; a placeholder is a built-in value ({level}) or a table of ItemStats.json

The stat file may not have a table named like a built-in value (tables[0].name: "level" is a built-in placeholder of the item descriptions; name the table otherwise).

Code

• Data/GameData/ItemData/ItemDescriptionPlaceholders.{h,cpp}: ScanPlaceholders, GetPlaceholderValue, FillPlaceholders, SuggestPlaceholder, and the built-in names.
• ItemStatProfiles::FindTable / GetTableNames.
• The checks:
  • the braces and the translations in ValidateItems;
  • the names in ValidateDescriptionPlaceholders, which needs the stat file, so it runs where ValidateStatProfiles runs (load, save, bmd import).
• The tooltip (RenderItemInfo) fills the values in.
• Docs: "Description placeholders" in docs/item-data.md.

No shipped text uses a placeholder yet. The first ones come with step 4: the wing lines, over new ItemStats.json tables. The values of the stat lines themselves ({damageMin}, {defense}, {requiredStrength}, …) come with step 5, which moves those lines into data. #815 lists the planned names.

Screenshots

The equipped Divine Sword of Archangel +15 with the example description above. The description is only in the screenshot's local item data, not in this PR. {level} gives 15 and {damageIncreaseWeapons} gives 66.

Legacy theme	Modern theme

Related issues

Step 3 of #815. Builds on #816 (step 2) and #813 (step 1).

Checklist

• I have read and followed docs/CODING_RULES.md.
• I have built the project locally (see docs/build/README.md).
  • Windows x64 Release: all 985 tests pass.
  • The new ones are in test_item_description_placeholders.cpp (reading, values, suggestions, every error, translations), "The shipped descriptions' placeholders are built-in values or stat file tables", and the stat file's built-in name error.
  • Changed lines pass clang-format 21.1.8 as CI checks them.
  • In game (Release x64 against a local OpenMU), the example description above shows its values in both themes.
• Changes are focused on one concern; the diff is as small as it reasonably can be.

[Mosch0512]

Verdict: changes requested for two items in the editor; the rest is non-blocking. The placeholder mechanism itself is solid. Braces, translations, typo suggestions and the reserved level name are checked where the data loads, saves and imports, and the tests cover each error. Thanks for merging main yourself; the docs conflict is resolved the way I would have.

The two items come from the stat file's new role. Since this PR, descriptions name level bonus tables, but Item Formulas doesn't know that yet. #818 makes it real right away: it adds 11 tables (wingDamageIncrease1st … capeOfLordDamageAbsorb) that only descriptions use, and no stat profile.

Required

• Item Formulas counts a table's users only from the stat sections (CountTableUsers, MuItemFormulasUI.cpp:74). A table that only descriptions use shows "(0)", is listed among the tables nothing uses, and its Remove button is enabled. Removing it goes into the database at once. Its tooltips then draw the raw {wingDamageIncrease2nd}, and Save Items is blocked by "unknown placeholder" errors that no tool can fix before step 6. D26: a definition that is still used can't be deleted, and the editor shows where it's used. That file isn't in this PR's diff, so there's no inline comment.
• Renaming a table updates the stat sections that name it, but not the descriptions (RenameTable, MuItemFormulasUI.cpp:92). After a rename, the same raw text and the same Save error follow. D26: renaming updates all references.

Non-blocking

• Built-ins and table names share one set of names. Worth deciding before step 4 writes table placeholders into the shipped data (inline, ItemStatsJson.cpp:237).
• Table names are looked up while drawing, every frame (inline, ItemDescriptionPlaceholders.cpp:107).
• No test checks that the tooltip fills in the values (inline, ZzzInventory.cpp:1640).
• Item Formulas' name dialog (IsFreeName, MuItemFormulasUI.cpp:970) accepts a table named level. Only Save Items' read-back check of the stat file refuses it. Refusing it in the dialog, with the "free name" note the other taken names get, catches it where it's typed.
• The docs example uses a flat bonus as a percentage (inline, item-data.md:143).
• "Description placeholders" sits between the fields table and Tags, away from the Descriptions section it extends (inline, item-data.md:139).
• Two file-private helpers have external linkage (inline, ItemDataValidation.cpp:237).
• The field name description[i].text is still built by hand at lines 94 and 444 of ItemDataValidation.cpp, next to the new DescriptionTextField. Calling it there keeps one spelling of the field name.
• Nothing tests that Load and Save Items actually run the name check (inline, ItemDataHandler.cpp:123).
• Two separate passes check the same description texts (inline, ItemDataValidation.cpp:320).

What I checked: I read this step's diff against #816, and again after your merge of main (this step's code is unchanged). I also went through:

• the brace parsing at its edges, and how GetBonus clamps the level;
• the item data handler's three calls, the stat file reader, and Save Items' read-back check;
• Item Formulas' table users, rename and name dialog;
• #818's ItemStats.json, for which tables only descriptions use.

I built this branch merged with main locally (Windows x64 Debug): test_item_database passes (165 test cases) and test_item_database_files passes (161). CI: Quality Gates pass; the Linux and macOS builds were still running.

[Mosch0512]

Non-blocking, but worth deciding before step 4. Built-in values and table names share one set of names, and a table named like a built-in is a load error. #815 plans about 40 more built-ins for step 5 (damageMin, defense, durability, requiredStrength, the resistances, …). Each one turns any stat file that already has a table of that name into one that no longer loads.

For example, a server owner adds a table named durability to their ItemStats.json. When step 5 makes durability a built-in, their client stops at start with "is a built-in placeholder of the item descriptions; name the table otherwise". Either of these avoids that whole class of problem, and is cheapest to decide before step 4 writes table placeholders into the shipped data:

• keeping the two kinds apart in the syntax, for example a prefix for tables;
• letting built-ins win, with a warning.

[Mosch0512]

Non-blocking. FillPlaceholders runs every frame while an item is hovered. It parses the braces again and finds each table by comparing its name with every table of the stat file. D26 says names are resolved when loading, so drawing never looks them up.

Resolving each placeholder to a built-in id or a table index when the data loads (and when the stat file changes in the editor) would leave drawing a lookup by index. It would also turn a removed or renamed table into an error at that point, instead of raw text in the tooltip.

[Mosch0512]

Non-blocking. The description says the tooltip fills in the values after it picks the UI language's text and before the line wraps. But the tooltip tests only call WriteItemDescriptionLines with level 0 and texts without placeholders. A later change that fills after wrapping, or passes the wrong level, would fail no test. A test_item_descriptions case would pin it, using the existing FiveUnitsPerLetter: {level} at +15, plus a table placeholder, plus a filled value that wraps.

[Mosch0512]

Non-blocking. damageIncreaseWeapons is the weapons' flat damage bonus per + level (0, 3, 6, … 57, 66), not a percentage. A data author copying this example would write "Increases damage by 66%" for a +15 weapon whose damage actually rises by 66. The PR description's own wording fits: "a weapon deals {damageIncreaseWeapons} more damage".

[Mosch0512]

Non-blocking. This section extends Descriptions, which now has its own section further down (after Flight equipment), but it sits between the fields table and Tags. Moving it right after Descriptions keeps the description docs in one place. The link from the text bullet already points here.

[Mosch0512]

Non-blocking. DescriptionTextField and ListPlaceholders are defined after the file's anonymous namespace closes. That makes them external functions of Data::Items that no header declares, while every other helper in this file is file-private. Another translation unit with its own ListPlaceholders(const std::set<std::string>&), a likely name once the layout steps add their own checks, would then hit a duplicate-symbol link error. Moving both into the anonymous namespace avoids it.

[Mosch0512]

Non-blocking. ValidateDescriptionPlaceholders is wired into Load, Save Items (Validate) and Import from bmd. The tests call it directly, but none checks that an unknown placeholder actually stops Load or blocks Save. If one of the three calls gets lost in a later refactor, a typo in custom data loads silently and draws as raw text. A case like the existing "Item data errors stop loading", with one description naming an unknown table, would cover Load.

[Mosch0512]

Non-blocking. ValidateItems now runs ValidateDescription (#813: the English text, language codes) and ValidateDescriptionPlaceholderSyntax (braces, the translations' placeholders). Each walks every line and translation on its own. One ValidateDescription that checks a line's texts once keeps the per-line checks together as the later steps add more (shared texts, conditions, colors).

[ezequielzacca]

Thanks for the thorough review — all addressed, carried through the stack (#818–#836):

Required

• Item Formulas counts the item descriptions (and name lines) that name a table, including a line's values; the table lists them, and Remove stays off while one names it.
• Renaming a table renames it in those descriptions too, in every language and in their values, as part of the same undo step.

Built-ins vs tables: tables are now written {table:<name>} in texts and "table:<name>" in a line's values, so a later built-in can never clash with a stat file's table. The "named like a built-in" load error is gone (and with it the level case in the name dialog), and a bare table name gets the table: form as its suggestion. A line's values names and an item's levelValues are the item's own and win over a built-in of the same name. The shipped data is converted in #818, #827 and #835.

Also done: one pass for the description text checks, the two helpers are file-private, one spelling of the description field, tests that an unknown placeholder stops Load and blocks Save Items, a test that the tooltip fills values for the item's level before wrapping, the example uses the flat bonus correctly, and the placeholders section follows Descriptions.

Not in this PR: resolving placeholder names at load instead of each frame. It's a bigger change, and I'd like to do it as its own PR after the stack.

[ezequielzacca]

@Mosch0512 ready for another look when you have a moment.

[ezequielzacca]

On resolving the placeholder names at load instead of each frame: before starting it, I measured what it could save, at the end of the stack (#836).

Every item at every level 0–15, three rounds, for a level 400 Blade Master (43,728 tooltips, Windows x64 Release):

	per tooltip
the whole tooltip (BuildItemTooltipLines)	4.2 µs
the item's values (ComputeItemTooltipValues)	0.36 µs
the description and name lines (placeholders filled in, conditions, wrapping; GetItemDisplayName too)	0.57 µs

So even if resolving at load removed all of the description and name lines' work, a hovered tooltip would gain about half a microsecond of a 16.6 ms frame, and the ground labels and windows that draw item names stay in the low microseconds with dozens of items. I'd rather not add the cache and its invalidation (items, stat file, tooltip file, language) for that.

The other half of the suggestion, a removed or renamed table turning into raw text, is covered now: an unknown placeholder stops Load and blocks Save Items, Item Formulas keeps Remove off while a description names the table, and a rename carries into the descriptions. Only a stat file edited by hand while the game runs could still show {table:…}.

If profiling ever points at the tooltips, the plan is written down as step 5e in #815.

[Mosch0512]

Thanks, the fixes look good. I checked them and ran the tests locally. I pushed three small commits on top, so please pull before your next push here:

• 0a8ac43: a bare name that is exactly a table's name now gets {table:<name>} as its hint before the typo search. Before, a table close to a built-in value, written without table:, was told it meant the built-in: {levels} for the table "levels" got "did you mean {level}?". Following that hint would show the item's + level.
• 4022b75: the test for a table named level reads the file without errors now. The changed file had still failed because the profile named the old table. A new test also shows {level} and {table:level} side by side.
• 1b899b1: the header comments describe the table: form and the description checks.

#818 has the same hint lines unchanged, so they should apply cleanly as you carry them up the stack.