Skip to content

fix: translate unit names in event panel messages #3242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
slegarraga wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: translate unit names in event panel messages #3242

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d424caf
fix: translate unit names in event panel messages
slegarraga Feb 19, 2026
b50d845
fix: translate all string params generically with fallback
slegarraga Feb 19, 2026
ce4b19f
style: fix prettier formatting
slegarraga Feb 19, 2026
70fa419
Merge branch 'main' into fix/translate-unit-names-event-panel
iiamlewis Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/client/graphics/layers/EventsDisplay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,13 @@ export class EventsDisplay extends LitElement implements Layer {

let description: string = event.message;
if (event.message.startsWith("events_display.")) {
description = translateText(event.message, event.params ?? {});
const params = { ...(event.params ?? {}) };
if (typeof params.unit === "string") {
const unitKey =
"unit_type." + params.unit.toLowerCase().replace(/ /g, "_");
params.unit = translateText(unitKey);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment on lines +393 to +403
Copy link
Copy Markdown
Collaborator

@evanpelle evanpelle Feb 21, 2026

Choose a reason for hiding this comment

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

maybe this should go in translateText() instead? that way we'll automatically translate units if we render unit names somewhere else in the UI.

Copy link
Copy Markdown
Contributor

@VariableVince VariableVince Mar 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@evanpelle would strongly advice against that, because what if a player is named "Factory"? There are keys with params carrying player names. Which would then suddenly be translated because the player name so happens to be the same as the unit name in English. Not entirely inconceivable.

I would also advice against the above code in this PR. Because he current solution in the code above has the same issue, it would translate a player name like "Factory" which it should not do.
(EventsDisplay just prepends all param strings with "unit_type." and then sees if there is a translation. So again, if a param string contains a player name "Factory", their name would be translated because when prepended it would be unit_type.factory and EventsDisplay will find a translatable key.)

That is why i proposed (in comment above) to send the key to displayMessage(). And prevent the unit.type() from being send. So we prevent the type UnitType to be put in params of displayMessage. Which forces the developer to use the only correct way of sending unit_type." + unit.type() as a param to displayMessage. Then, EventsDisplay can indeed test this key for an existing translation without a player name like "Factory" being translated. And as said we prevent a developer from just sending type UnitType unit.type() ("Factory") since, again, "Factory" may just be a player name and we should not just translate every param string for that reason. Only translate a param string if it contains a translatable key.

description = translateText(event.message, params);
}

this.addEvent({
Expand Down
Loading