-
Notifications
You must be signed in to change notification settings - Fork 16
fix: show translated chat messages in world nametag bubbles (#8552) #8993
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.
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
base: dev
Are you sure you want to change the base?
Changes from all commits
a4226f6
ae80f63
6e2a6fc
d70b2c9
bae3387
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,10 +5,12 @@ | |||||||||||||||||||
| using DCL.Nametags; | ||||||||||||||||||||
| using DCL.Profiles; | ||||||||||||||||||||
| using DCL.Settings.Settings; | ||||||||||||||||||||
| using DCL.Translation; | ||||||||||||||||||||
| using DCL.Utilities; | ||||||||||||||||||||
| using System; | ||||||||||||||||||||
| using UnityEngine; | ||||||||||||||||||||
| using UnityEngine.InputSystem; | ||||||||||||||||||||
| using Utility; | ||||||||||||||||||||
| using Utility.Arch; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| namespace DCL.Chat.ChatServices | ||||||||||||||||||||
|
|
@@ -23,8 +25,12 @@ public class ChatWorldBubbleService : IDisposable | |||||||||||||||||||
| private readonly ChatSettingsAsset chatSettings; | ||||||||||||||||||||
| private readonly IChatHistory chatHistory; | ||||||||||||||||||||
| private readonly ICommunityDataService communityDataService; | ||||||||||||||||||||
| private readonly IEventBus chatEventBus; | ||||||||||||||||||||
| private readonly ITranslationSettings translationSettings; | ||||||||||||||||||||
| private static readonly Color DEFAULT_COLOR = Color.white; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private readonly IDisposable translationSubscription; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public ChatWorldBubbleService( | ||||||||||||||||||||
| World world, | ||||||||||||||||||||
| Entity playerEntity, | ||||||||||||||||||||
|
|
@@ -33,7 +39,9 @@ public ChatWorldBubbleService( | |||||||||||||||||||
| NametagsData nametagsData, | ||||||||||||||||||||
| ChatSettingsAsset chatSettings, | ||||||||||||||||||||
| IChatHistory chatHistory, | ||||||||||||||||||||
| ICommunityDataService communityDataService) | ||||||||||||||||||||
| ICommunityDataService communityDataService, | ||||||||||||||||||||
| IEventBus chatEventBus, | ||||||||||||||||||||
| ITranslationSettings translationSettings) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| this.world = world; | ||||||||||||||||||||
| this.playerEntity = playerEntity; | ||||||||||||||||||||
|
|
@@ -43,8 +51,11 @@ public ChatWorldBubbleService( | |||||||||||||||||||
| this.chatSettings = chatSettings; | ||||||||||||||||||||
| this.chatHistory = chatHistory; | ||||||||||||||||||||
| this.communityDataService = communityDataService; | ||||||||||||||||||||
| this.chatEventBus = chatEventBus; | ||||||||||||||||||||
| this.translationSettings = translationSettings; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| chatHistory.MessageAdded += OnChatMessageAdded; | ||||||||||||||||||||
| translationSubscription = chatEventBus.Subscribe<TranslationEvents.MessageTranslated>(OnMessageTranslated); | ||||||||||||||||||||
| DCLInput.Instance.Shortcuts.ToggleNametags.performed += OnToggleNametagsShortcutPerformed; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -120,6 +131,7 @@ private void GenerateChatBubbleComponent(Entity e, ChatMessage chatMessage, Colo | |||||||||||||||||||
| if (!world.Has<NametagHolder>(e)) return; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| world.AddOrSet(e, new ChatBubbleComponent( | ||||||||||||||||||||
| chatMessage.MessageId, | ||||||||||||||||||||
| chatMessage.Message, | ||||||||||||||||||||
| chatMessage.SenderValidatedName, | ||||||||||||||||||||
| chatMessage.SenderWalletAddress, | ||||||||||||||||||||
|
|
@@ -134,10 +146,43 @@ private void GenerateChatBubbleComponent(Entity e, ChatMessage chatMessage, Colo | |||||||||||||||||||
| communityName ?? string.Empty)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private void OnMessageTranslated(TranslationEvents.MessageTranslated evt) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| MessageTranslation translation = evt.Translation; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (translation.State != TranslationState.Success || string.IsNullOrEmpty(translation.TranslatedBody)) | ||||||||||||||||||||
| return; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (!translationSettings.IsTranslationFeatureActive()) | ||||||||||||||||||||
| return; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // The message id encodes the sender wallet address, which the participant table maps to the live avatar entity. | ||||||||||||||||||||
| if (!ChatUtils.TryGetSenderWalletAddress(evt.MessageId, out string senderWalletAddress) | ||||||||||||||||||||
| || !entityParticipantTable.TryGet(senderWalletAddress, out var entry) | ||||||||||||||||||||
| || !world.IsAlive(entry.Entity)) | ||||||||||||||||||||
| return; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ref ChatBubbleComponent bubble = ref world.TryGetRef<ChatBubbleComponent>(entry.Entity, out bool exists); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Guard against the sender having since posted a newer message into the same bubble. | ||||||||||||||||||||
| if (!exists || bubble.MessageId != evt.MessageId) | ||||||||||||||||||||
| 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)) | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
// from ChatMessageFeedPresenter, line ~119 / 354
if (!translationSettings.IsTranslationFeatureActive()) …
if (!translationSettings.GetAutoTranslateForConversation(…)) …Fix:
Suggested change
|
||||||||||||||||||||
| return; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| bubble.ChatMessage = translation.TranslatedBody; | ||||||||||||||||||||
| bubble.IsTranslationUpdate = true; | ||||||||||||||||||||
| bubble.IsDirty = true; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public void Dispose() | ||||||||||||||||||||
| { | ||||||||||||||||||||
| DCLInput.Instance.Shortcuts.ToggleNametags.performed -= OnToggleNametagsShortcutPerformed; | ||||||||||||||||||||
| chatHistory.MessageAdded -= OnChatMessageAdded; | ||||||||||||||||||||
| translationSubscription.Dispose(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private void OnToggleNametagsShortcutPerformed(InputAction.CallbackContext obj) | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential
NullReferenceExceptionmessageId.IndexOf(':')throws ifmessageIdisnull.TranslationEvents.MessageTranslated.MessageIdis a struct field — its default isnull— 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 statica null guard is cheap insurance: