Skip to content

fix: translate unit names in event panel messages#3242

Open
slegarraga wants to merge 2 commits intoopenfrontio:mainfrom
slegarraga:fix/translate-unit-names-event-panel
Open

fix: translate unit names in event panel messages#3242
slegarraga wants to merge 2 commits intoopenfrontio:mainfrom
slegarraga:fix/translate-unit-names-event-panel

Conversation

@slegarraga
Copy link

Fixes #3071

Problem

Unit names in event panel messages show raw English type strings (e.g., Port) instead of translated names (e.g., Dutch Haven).

Solution

In EventsDisplay.ts, before interpolating event message params, the unit param is now translated via translateText("unit_type.<key>") — converting the raw unit type string (e.g., Port) to its translation key (e.g., unit_type.port).

This affects all three event messages that include unit names:

  • unit_captured_by_enemy
  • captured_enemy_unit
  • unit_destroyed

Changes

  • src/client/graphics/layers/EventsDisplay.ts: Translate the unit param before passing to translateText()

Happy to iterate on feedback!

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 19, 2026

Walkthrough

The change fixes unit name translation in event panel messages by mapping unit string values to localized unit_type keys before translation. When processing events_display messages, the code now copies and modifies params, converting unit names to their translated equivalents so messages display in the user's language.

Changes

Cohort / File(s) Summary
Unit Translation in Events Display
src/client/graphics/layers/EventsDisplay.ts
Modified event message handling to translate unit names. When unit field is a string, it now maps to a unit_type key (normalized to lowercase with spaces as underscores), translates that key, and uses the result in message translation instead of the raw unit string.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

⚔️ A Port becomes a Haven in the Dutch tongue,
Units now speak the language of the lands they're from,
Messages bloom in colors both near and far,
Where every warrior's name shines like a localized star ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: translating unit names in event panel messages, which directly matches the changeset.
Description check ✅ Passed The description is well-related to the changeset, explaining the problem, solution, and specific files modified with clear context.
Linked Issues check ✅ Passed The pull request fully addresses issue #3071 by implementing unit name translation in EventsDisplay.ts for all three affected event messages as required.
Out of Scope Changes check ✅ Passed All changes in EventsDisplay.ts are directly related to translating unit parameters in event messages, with no out-of-scope modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/client/graphics/layers/EventsDisplay.ts`:
- Around line 387-391: In EventsDisplay.ts, avoid assigning the raw i18n key
string back into params.unit when translateText fails: capture the original unit
string before transforming (e.g., const raw = params.unit), call
translateText(unitKey), then if the returned value is the same as the key or
otherwise falsy, keep params.unit = raw (or a capitalized/raw fallback) instead
of the translation; update the block that computes unitKey/params.unit to
perform this guarded assignment so missing translations show the original unit
instead of "unit_type.xxx".

Comment on lines 387 to 391
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing fallback: raw i18n key shown in UI when translation key is absent

translateText(unitKey) returns the key string verbatim (e.g., "unit_type.port") when the key is missing from the translation map — this is the documented behaviour in Utils.ts (if (!message) return key). Assigning that value back to params.unit without a guard means a missing or not-yet-loaded key produces output like "Your unit_type.port was captured", which is a worse regression than the original English "Port".

Add a guard that falls back to the original unit value when the key is not resolved:

🛡️ Proposed fix
     if (typeof params.unit === "string") {
       const unitKey =
         "unit_type." + params.unit.toLowerCase().replace(/ /g, "_");
-      params.unit = translateText(unitKey);
+      const translated = translateText(unitKey);
+      params.unit = translated !== unitKey ? translated : params.unit;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/client/graphics/layers/EventsDisplay.ts` around lines 387 - 391, In
EventsDisplay.ts, avoid assigning the raw i18n key string back into params.unit
when translateText fails: capture the original unit string before transforming
(e.g., const raw = params.unit), call translateText(unitKey), then if the
returned value is the same as the key or otherwise falsy, keep params.unit = raw
(or a capitalized/raw fallback) instead of the translation; update the block
that computes unitKey/params.unit to perform this guarded assignment so missing
translations show the original unit instead of "unit_type.xxx".

@github-project-automation github-project-automation bot moved this from Triage to Development in OpenFront Release Management Feb 19, 2026
@VariableVince
Copy link
Contributor

VariableVince commented Feb 19, 2026

@slegarraga What if a developer (not knowing about the need for translation) just puts "building: unit.type()"? They'll test using English and it will work. We can't be sure they'll use "unit" as a key name. And also, how can we be sure they'll send a unit translation key string, and not just untranslatable unit.type()?

I think what we can do is:

  • prevent them from using UnitType as a value for the params, so that unit.type() will lead to a TS error.
  • but we cannot really confine the naming of the keys for the params. We could only allow a number of key names, but because there are many possible options for what will be in the params, you still don't know which of the allowed keys would hold a unit translation key as value once you recieve them in EventsDisplay. In other words, a developer may use "conquered" if that is an allowed key to send a player name, but he uses it to send a unit translation key because it is about a conquered unit. If you wouldn't have foreseen that, it can still bypass the key filter in EventsDisplay.
  • So, imo, because of the above, better just and leave EventsDisplay agnostic to how the key is named. If a param value is a string, check if the key exists in en.json first or directly pass it to translateText to see if there's a translation for it. The number of params per in-game displayMessage won't be more than 3-4 probably so it shouldn't hurt performance too much.

Address review feedback:
- Check all string params against unit_type translations, not just 'unit'
- Keep original value if no translation exists (avoids showing raw keys)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Development

Development

Successfully merging this pull request may close these issues.

Bug: unit name not translated in some event panel messages

3 participants

Comments