Skip to content

Combat: no raw ids in action-bar targets; translate catalog item/attack names - #84

Merged
vladmesh merged 4 commits into
mainfrom
claude/combat-display-names-6w57du
Sep 29, 2026
Merged

vladmesh merged 4 commits into
mainfrom
claude/combat-display-names-6w57du

Conversation

@vladmesh

@vladmesh vladmesh commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Card W3-3: issues 076f74d5fcf57a1a5f10 (remainder), e904083d3ff4ec37c275, da580696f20123039c1b (perception-hardcoded-weapons). Base: main @ 2f79530.

I checked all four items against main first. None of them was already fixed.

1. Raw ids in combat action-bar targets

Root cause. TargetDropdown.buildTargets built its labels from e.id ("Атаковать goblin_chieftain_5", "Говорить с npc_…"). Three other places did the same:

  • DirectionalDropdown: move/dash toward and away from a target.
  • The smite panel opened from the action bar: targetName={smiteTargetId}.

Fix. All of these now use perceivedLabels from #74, the same rule and helper as the Nearby panel. The label is the perceived description (a stranger's race), then the known name, then unknown_creature. Duplicate labels get an ordinal suffix. The id is still sent as target_id, but it is never shown. I loosened the t parameter of perceivedLabels to a plain function type so the action-bar components can pass their t prop.

2–4. Item and attack names in RU

Root cause.

  • The combat log already calls _() on item and attack names. But the ru .po had no entries for most catalog items (Shield, Chain Mail, all armor, Rapier, Greatsword…), for any weapon attack_name, or for any monster attack name ("scimitar").
  • The only source of msgids was a hand-maintained, incomplete _() list in perception.py. It also listed names no content uses (Handaxe, Javelin, Light Crossbow, Spear).
  • Several player-facing surfaces never called gettext: the combat panel's self_weapon ("Оружие: longsword slash"), the inventory payload, ItemInfo.name (loot/trade/consumables) and the equipped-items names.

Fix. Instead of another hand-kept list, the catalogs themselves are the source:

  • New module content_loader/catalog_messages.py. catalog_msgids() lists every translatable catalog string, with source references:

    • item name
    • weapon attack_name
    • monster attacks[].name

    Monster names are already {en, ru} maps in the YAML, so they are not included.

  • make messages now appends any missing catalog msgids to messages.pot (with #: content/catalogs/... references). I regenerated the .pot with the tool. It had been stale since 2026-09-13, so its diff is large but fully generated.

  • ru .po/.mo: translations for all 50 catalog names, compiled with make compile-messages.

  • Hardcoded list: the item/weapon names are removed from perception._TRANSLATABLE_STRINGS. Damage types, "AC" and the fallback labels stay there, because they are not catalog content.

  • gettext applied to self_weapon (awareness_builder), item_info().name, build_equipped names and build_inventory_payload names.

English output is unchanged: _() returns the msgid in EN, and the tests assert this.

Tests

The frontend tests are in ActionButton.test.tsx, "target labels never show internal ids". I verified that all 4 fail on main and pass with the fix:

I updated one existing assertion in ActionBar.test.tsx. It expected the raw id in the smite menu (/Attack practice_thug$/), which is the bug itself, and now expects Attack Thug.

The backend tests are the new file tests/unit/test_catalog_translations.py. I verified that 6 of them fail on main's code:

  • every catalog name has a ru msgstr in the .po (this is the guard that fails when a new catalog item or attack ships untranslated);
  • the compiled .mo translates every catalog name;
  • perception._TRANSLATABLE_STRINGS no longer hand-lists catalog names;
  • combat panel weapon: EN "longsword slash", RU "удар длинным мечом";
  • inventory payload, item_info and equipped names: EN unchanged, RU "Щит"/"Кольчуга";
  • "You put away Shield" log line has "Щит" in RU.

The other two test the new extractor, so they cannot run on main: it covers item, attack and monster-attack names, and it appends only missing msgids and is idempotent.

Checks

  • make check is green: ruff, mypy, 2947 pytest, 357 vitest.
  • Integration: Docker has no daemon in this environment, so make test-integration could not run. I ran tests/integration/ against a local backend with the docker-compose.test.yml settings (DND_DICE_SEED=42, DND_CONTENT_DIR=tests/integration/content, BACKEND_URL=http://localhost:8001): 184 passed.

No new test revealed a separate bug.

Found but not fixed (out of scope)

  • Inline attack names in world content are not covered. The extractor only reads content/catalogs. Inline names in world YAML, e.g. flail/club in worlds/level_up_test/entities/npcs.yaml or longsword in test_vale, still show in English. The same mechanism could extend to world/library YAML.
  • describe_item() is English, machine-style text. Example: "Longsword (weapon: 1d8 slashing, reach 5ft)". It is shown as the item description in the inventory, loot and trade panels. RuleBrain and the frontend InventoryDrawer match on the substrings "weapon"/"heal" in it, so localizing it needs those matches moved to item_type/props first.
  • make messages does not pass --keyword=N_, so strings marked with N_() (about 87 call sites) are never extracted into the .pot.
  • Dead .po entries: Handaxe, Javelin, Light Crossbow and Spear (from the removed list) are not used by any content. They are harmless and left in place.
  • Monster names in combat are not localized when the name falls back to target.name. I did not check whether any monster name reaches the player without its localized value.

secretary owner added 2 commits September 29, 2026 08:40
TargetDropdown (attack/talk), DirectionalDropdown (move/dash toward/away)
and the smite choice panel labelled targets with the internal entity id
("Атаковать goblin_chieftain_5"). They now use perceivedLabels, the same
rule as the Nearby panel: perceived description (a stranger's race), else
known name, with an ordinal suffix for duplicates.
Attack names ("longsword slash") reached the combat panel without gettext
and neither they nor most item names (Shield, Chain Mail, ...) had a ru
translation; perception.py kept a hand-maintained, incomplete _() list.

- content_loader/catalog_messages.py lists every catalog item name,
  weapon attack_name and monster attack name; make messages appends them
  to messages.pot (regenerated here).
- ru .po/.mo: translations for all catalog names; the hardcoded item list
  in perception.py is removed.
- Combat panel weapon, inventory payload, ItemInfo and equipped names go
  through gettext (English output unchanged).
- test_catalog_translations fails when a catalog name has no ru msgstr.
@vladmesh

Copy link
Copy Markdown
Owner Author

Independent review: PASS

  • The diff matches card W3-3.
    • Action-bar labels: the target menus (attack, talk, move and dash toward or away) and the smite panel now build labels with the same perceivedLabels rule as Nearby. The id is only sent as target_id and is never shown.
    • Catalog names: item names, weapon attack_name and monster attack names are now extracted from the YAML catalogs by catalog_messages.py, hooked into make messages. Russian translations are added for all 50 of them.
    • gettext coverage: it is now applied to self_weapon, item_info().name, equipped names and inventory payload names. Server-side actions address items by id, so localising the display name does not change any lookup.
  • Checks: local make check is green and required CI is green.
  • Regression check:
    • On main's code, 5 frontend tests fail in ActionButton.test.tsx and ActionBar.test.tsx.
    • test_catalog_translations.py cannot run on main, because it imports the new module. It asserts that every catalog msgid has a Russian translation, which is not true on main.
  • Non-blocking: the .pot was regenerated in full because it was stale, so the diff is large. This PR is likely to conflict with Bound client input at the API boundary #82 and W3-4: extract messages with pybabel, add i18n catalog staleness test #80 in the .pot/.po files, so the PRs should be merged in order and rebased in between.

secretary owner and others added 2 commits September 29, 2026 09:15
…-names-6w57du

# Conflicts:
#	src/dnd_simulator/locale/messages.pot
#	src/dnd_simulator/locale/ru/LC_MESSAGES/dnd_simulator.mo
@vladmesh
vladmesh merged commit 587f191 into main Sep 29, 2026
4 checks passed
@vladmesh
vladmesh deleted the claude/combat-display-names-6w57du branch September 29, 2026 09:27
vladmesh pushed a commit that referenced this pull request Sep 29, 2026
make messages keeps both steps: pybabel extract for code msgids, then
#84's catalog_messages appends the YAML catalog names. The catalog test
expects the babel code msgids plus catalog_msgids(content) in the .pot.
README keeps the gettext workflow and the containerized-stack section.
Regenerated messages.pot, kept every ru translation from both sides,
recompiled the .mo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant