Skip to content

fix: show translated chat messages in world nametag bubbles (#8552)#8993

Open
biotech77 wants to merge 5 commits into
devfrom
fix/8552-world-chat-bubble-translations
Open

fix: show translated chat messages in world nametag bubbles (#8552)#8993
biotech77 wants to merge 5 commits into
devfrom
fix/8552-world-chat-bubble-translations

Conversation

@biotech77

Copy link
Copy Markdown
Contributor

Pull Request Description

What does this PR change?

Closes #8552.

When auto-translation is enabled for a conversation, the in-world nametag chat bubble now mirrors the chat panel and shows the translated text, instead of always showing the original message.

Behavior

  • The bubble appears immediately with the original text on arrival (unchanged).
  • If the auto-translation lands while the bubble is still visible (within its ~5s window), the text is swapped in place — without re-arming the auto-hide timer.
  • Late translations (arriving after the bubble has hidden) are ignored — no "resurrection" of a dead bubble.
  • Manual chat-panel translations are not mirrored; only automatic translations for conversations with auto-translate enabled.
  • Honors the existing on/off controls: the global translation feature flag and the per-conversation auto-translate toggle are respected (the bubble reads the same setting the chat panel uses).

How it works

  • ChatWorldBubbleService subscribes to TranslationEvents.MessageTranslated, resolves the sender's avatar entity via the existing entityParticipantTable (sender wallet derived from the message id), gates on the per-conversation auto-translate setting (read from the bubble's own ChannelId), and updates the entity's ChatBubbleComponent.
  • NametagPlacementSystem distinguishes a translation swap (in-place text, visible-only) from a fresh message (full DisplayMessage) via a flag on the component.
  • Added ChatUtils.TryGetSenderWalletAddress (the inverse of GetId).

Glyph coverage (so translated / foreign text renders instead of tofu boxes)

  • Nametag UI Toolkit text previously had no fallback for non-Latin scripts. Added CJK fallback fonts (Noto Sans KR/SC, TextCore) to the nametag panel's text settings, and switched the message label to the dynamic Inter .ttf so Latin-Extended/Cyrillic/Greek render.
  • Enabled emoji support on the nametag panel text settings using the existing emojis32_uitk sprite asset.

Test Instructions

Steps (standard run):

metaforge explorer run XXXX  # ← replace with this PR number

Expected result:

  • With another account, enable auto-translate for the Nearby conversation and send a message in a non-English language (e.g. Korean, Spanish, Russian). The sender's in-world nametag bubble should display the translated text (after a brief moment), with correct glyphs (no □ boxes) and emojis rendering as sprites.
  • Disable auto-translate for the conversation → new messages' bubbles stay in the original language.
  • Manually translating a single message from the chat panel context menu does not change the bubble.

Test Steps

  1. Run two clients (or use a second account) in the same Nearby scene so each sees the other's nametag.
  2. From client A, enable auto-translate for Nearby.
  3. From client B, send a message in Korean/Japanese/Chinese, then one in Spanish/Russian, then one with emoji.
  4. On client A, the bubble above B's avatar should swap to translated text with proper glyphs/emoji while visible.
  5. Toggle auto-translate off on A → B's next message bubble stays original.

Additional Testing Notes

  • Edge case: a translation that arrives after the bubble already auto-hid is intentionally not shown (no resurrection); the bubble's 5s lifetime is unchanged.

Quality Checklist

  • Changes have been tested locally
  • Documentation has been updated (if required)
  • Performance impact has been considered
  • For SDK features: Test scene is included

  When auto-translation is enabled for a conversation, the in-world nametag
  chat bubble now mirrors the chat panel and displays the translated text,
  instead of always showing the original message.
@biotech77
biotech77 requested review from a team as code owners June 15, 2026 11:01
@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

@biotech77

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

This comment has been minimized.


// Mirror automatic translations only. A manual translate from the chat panel targets a conversation
// where auto-translate is off (or an older message whose bubble is already gone), so it is ignored here.
if (!translationSettings.GetAutoTranslateForConversation(bubble.ChannelId))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: global feature flag not checked

The per-conversation check is here but the global IsTranslationFeatureActive() guard is missing. If a translation is still in-flight when the user disables the global flag, OnMessageTranslated will still fire (the async work was already kicked off) and the bubble text will be swapped — contradicting the documented behaviour ("global translation feature flag … respected").

ChatMessageFeedPresenter correctly gates on both:

// from ChatMessageFeedPresenter, line ~119 / 354
if (!translationSettings.IsTranslationFeatureActive())if (!translationSettings.GetAutoTranslateForConversation())

Fix:

Suggested change
if (!translationSettings.GetAutoTranslateForConversation(bubble.ChannelId))
// Guard against in-flight translations arriving after the global feature is disabled.
if (!translationSettings.IsTranslationFeatureActive())
return;
// Mirror automatic translations only. A manual translate from the chat panel targets a conversation
// where auto-translate is off (or an older message whose bubble is already gone), so it is ignored here.
if (!translationSettings.GetAutoTranslateForConversation(bubble.ChannelId))
return;

/// </summary>
public static bool TryGetSenderWalletAddress(string messageId, out string walletAddress)
{
int separator = messageId.IndexOf(':');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Potential NullReferenceException

messageId.IndexOf(':') throws if messageId is null. TranslationEvents.MessageTranslated.MessageId is a struct field — its default is null — so any caller that forgets to set it will crash here.

All current callers always set it, so this is low risk today, but since the method is public static a null guard is cheap insurance:

Suggested change
int separator = messageId.IndexOf(':');
public static bool TryGetSenderWalletAddress(string messageId, out string walletAddress)
{
if (string.IsNullOrEmpty(messageId))
{
walletAddress = string.Empty;
return false;
}
int separator = messageId.IndexOf(':');

@biotech77

Copy link
Copy Markdown
Contributor Author

@claude re-review

@claude

This comment has been minimized.

@biotech77 biotech77 changed the title feat: show translated chat messages in world nametag bubbles (#8552) fix: show translated chat messages in world nametag bubbles (#8552) Jun 15, 2026

@Ludmilafantaniella Ludmilafantaniella left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested on Windows and Mac. ✅

  • Auto-translate enabled: nametag bubble swaps from original to translated text correctly for Korean, Japanese, Chinese, Russian, Spanish ✅
  • Glyphs render correctly, no □ boxes ✅
  • Emoji renders as sprites in the bubble ✅
  • Auto-translate disabled: bubble stays in original language ✅
  • Manual translation from chat panel context menu does not affect the bubble ✅
  • Late translation (arriving after bubble auto-hides) does not resurrect the bubble ✅
  • Chat panel translation regression: working correctly ✅

LGTM 🚀

8993-evi.mp4

✅Smoke test performed:

  • ✔️ Backpack and wearables in world
  • ✔️ Emotes in world and in backpack
  • ✔️ Teleport with map/coordinates/Jump In
  • ✔️ Chat and multiplayer

@pravusjif
pravusjif removed request for dalkia and pravusjif June 23, 2026 10:13
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @pravusjif's task in 2m 26s —— View job


PR Review: fix/8552-world-chat-bubble-translations

  • Load tools
  • Read CLAUDE.md and docs/README.md
  • Read changed source files
  • Check prior review issues are resolved
  • Post findings

Root-cause check

Issue #8552: world nametag bubbles always showed the original message text, even when auto-translation was enabled in the chat panel, because ChatWorldBubbleService never subscribed to translation-completion events. This PR wires up that subscription and propagates the translated text to the ECS component. The diff fixes the root cause, not a symptom.


Prior blocking issues — resolution status

Issue 1 — Global feature flag not checked in OnMessageTranslated ✅ FIXED

ChatWorldBubbleService.cs:156–157 correctly gates on translationSettings.IsTranslationFeatureActive() before the per-conversation check.

Issue 2 — TryGetSenderWalletAddress throws on null input ✅ FIXED

ChatMessage.cs:90–96 guards with string.IsNullOrEmpty(messageId) and returns false / string.Empty before any .IndexOf call.


New issues

None. The implementation is clean. Key correctness points verified:

  • ref ChatBubbleComponent bubble is obtained via world.TryGetRef and mutated with no structural changes (Add/Remove) in between — safe per CLAUDE.md rule Implement CRDT Protocol #5.
  • ChatBubbleComponent.ChatMessage was correctly changed from readonly to mutable to allow in-place update via the ref.
  • bubble.MessageId != evt.MessageId guards against a stale translation arriving after the sender has already posted a newer message.
  • ShowMessage guard in ProcessChatBubbleComponents (NametagPlacementSystem.cs:152) correctly prevents reviving an already-hidden bubble.
  • translationSubscription.Dispose() is called in ChatWorldBubbleService.Dispose() — no event handler leak.
  • GUIDs used for system-message MessageId values don't contain ':', so TryGetSenderWalletAddress correctly returns false for them.

Non-blocking warning (carried from prior review, still unaddressed)

ProcessChatBubbleComponents query missing [None(typeof(DeleteEntityIntention))] (NametagPlacementSystem.cs:138). CLAUDE.md rule #3 requires all queries to filter DeleteEntityIntention. The load-order dependency on UpdateElementTagQuery removing NametagHolder first is fragile. Not introduced by this PR, but worth a follow-up ticket.


REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Touches an ECS component (ChatBubbleComponent), an ECS system (NametagPlacementSystem), and a service that mutates ECS state from a translation event subscription, spanning the chat, nametag, and translation pipeline subsystems.
QA_REQUIRED: YES

@pravusjif
pravusjif removed request for a team and anicalbano June 23, 2026 10:14
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Claude reviewed this PR and found no blocking issues, but assessed it as complex — human DEV review is still required before merging.

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.

[QA] Chat | Bubbles not displaying translated text and special characters

4 participants