[Port] TTS #121
[Port] TTS #121
Conversation
WalkthroughЭтот запрос на извлечение вводит обширный набор изменений, сосредоточенных на интеграции функциональности синтеза речи (TTS) в различных компонентах приложения. Ключевые дополнения включают новые классы для управления TTS-системами, прототипами голосов и связанными событиями, а также улучшения существующих классов для поддержки выбора голоса и управления громкостью. Файлы локализации были обновлены для предоставления удобных для пользователя меток и опций для настроек TTS на нескольких языках. Кроме того, были реализованы миграции базы данных для учета новых атрибутов голоса в профилях персонажей. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 30
🧹 Outside diff range and nitpick comments (41)
Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl (2)
2-3: Consider standardizing the transliteration of similar names."Kseniya" and "Xenia" appear to be different transliterations of the same Russian name (Ксения). Consider standardizing the transliteration to avoid confusion:
- Either use "Kseniya" and "Kseniya2"
- Or use "Xenia" and "Xenia2"
1-3: Add documentation comments for voice entries.Consider adding FTL comments to document:
- The TTS provider/API these voices are associated with
- Any specific characteristics (e.g., accent, age, style)
- Usage recommendations
Example:
+# Russian male voice for general use tts-voice-name-eugene = Evgeniy +# Russian female voice, neutral style tts-voice-name-kseniya = Kseniya +# Russian female voice, alternative style tts-voice-name-xenia = XeniaContent.Shared/_White/TTS/Events/RequestTTSEvent.cs (1)
5-10: Add XML documentation for better API clarity.The implementation looks good, following proper serialization and immutability patterns. Consider adding XML documentation to describe the purpose and usage of this event.
[Serializable, NetSerializable] // ReSharper disable once InconsistentNaming +/// <summary> +/// Event raised to request text-to-speech processing for the specified text. +/// </summary> public sealed class RequestTTSEvent(string text) : EntityEventArgs { public string Text { get; } = text; }Resources/Prototypes/_White/Voice/tts_voices.yml (1)
1-17: Add newline at end of file.Add a newline character at the end of the file to comply with POSIX standards.
sex: Female - speaker: xenia \ No newline at end of file + speaker: xenia +🧰 Tools
🪛 yamllint
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
Content.Shared/_White/TTS/TTSComponent.cs (1)
5-8: Add XML documentation to improve code maintainability.Consider adding XML documentation to describe the component's purpose and usage.
+/// <summary> +/// Component that manages Text-to-Speech voice settings for an entity. +/// </summary> [RegisterComponent, AutoGenerateComponentState] // ReSharper disable once InconsistentNaming public sealed partial class TTSComponent : ComponentContent.Shared/_White/TTS/Events/PlayTTSEvent.cs (2)
5-7: Consider documenting the ReSharper suppression.While the suppression for TTS naming is valid, it would be helpful to add a brief XML comment explaining why this naming convention is acceptable in this context.
+/// <summary> +/// Event for playing Text-to-Speech (TTS) audio data. +/// </summary> [Serializable, NetSerializable] // ReSharper disable once InconsistentNaming public sealed class PlayTTSEvent(NetEntity uid, byte[] data, bool boostVolume) : EntityEventArgs
1-14: Consider implementing a mock TTS service for testing.Since you mentioned not having access to an API for verification, it would be beneficial to:
- Create an interface
ITTSServicethat this event will work with- Implement a mock service that generates simple tones or uses pre-recorded samples
- Add unit tests using the mock implementation
This would allow testing the TTS infrastructure without requiring API access.
Would you like me to help create a mock implementation and corresponding unit tests?
Content.Client/_White/TTS/TTSManager.cs (1)
6-10: Consider adding dependency validation.While the dependencies are correctly injected, consider adding validation in a constructor to ensure they are properly resolved before use.
public sealed class TTSManager { [Dependency] private readonly IClientNetManager _netMgr = default!; [Dependency] private readonly EntityManager _entityManager = default!; + + public TTSManager() + { + IoCManager.InjectDependencies(this); + + if (_netMgr == null) + throw new InvalidOperationException($"{nameof(IClientNetManager)} dependency not resolved"); + if (_entityManager == null) + throw new InvalidOperationException($"{nameof(EntityManager)} dependency not resolved"); + }Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs (1)
22-27: LGTM! Consider data preservation strategy.The Down migration correctly removes the column, but be aware that this will result in loss of voice preferences if the migration is rolled back. This is acceptable for the current scope, but consider documenting this behavior for maintainers.
Add a comment above the Down method:
/// <inheritdoc /> + /// <remarks> + /// Warning: Rolling back this migration will permanently delete all voice preferences. + /// </remarks> protected override void Down(MigrationBuilder migrationBuilder)Content.Shared/_White/TTS/TTSVoicePrototype.cs (2)
10-21: Consider strengthening Speaker property validation.The Speaker property is marked as required but initializes to an empty string, which seems contradictory. Consider adding validation or documentation to clarify the expected values.
- [ViewVariables(VVAccess.ReadWrite), DataField(required: true)] - public string Speaker { get; } = string.Empty; + [ViewVariables(VVAccess.ReadWrite), DataField(required: true)] + public string Speaker { get; }Also, consider adding XML documentation for the Sex property to clarify its usage in voice selection.
+ /// <summary> + /// Determines the gender characteristics of the voice for TTS generation. + /// </summary> [DataField(required: true)] public Sex Sex { get; }
28-32: Document SponsorOnly and BorgVoice properties.These properties lack documentation explaining their purpose and impact on voice selection/behavior. Consider adding XML documentation to clarify:
- When SponsorOnly is true, what features are restricted?
- What modifications does BorgVoice apply to the TTS output?
+ /// <summary> + /// Indicates whether this voice is restricted to sponsors only. + /// </summary> [DataField] public bool SponsorOnly { get; } + /// <summary> + /// Indicates whether this voice should be processed with robotic/synthetic effects. + /// </summary> [DataField] public bool BorgVoice { get; }Content.Shared/_White/TTS/MsgRequestTTS.cs (1)
8-11: Consider using a different message group for TTS requests.The
Commandmessage group is typically used for critical game commands. TTS requests might be better suited for a different group likeGameor a custom group, as they are not critical commands and might need different handling priorities.Also, the ReSharper suppression for naming suggests we should follow a more consistent naming convention. Consider renaming to something like
TextToSpeechRequestMessagefor clarity and consistency.Content.Shared/_White/TTS/TTSPitchRateSystem.cs (1)
39-40: Add validation and documentation to TTSPitchRate record.Consider adding validation and documentation to ensure only valid pitch and rate values are used.
// ReSharper disable once InconsistentNaming -public record TTSPitchRate(string Pitch = "medium", string Rate = "medium"); +/// <summary> +/// Represents the pitch and rate settings for TTS voice synthesis. +/// </summary> +/// <param name="Pitch">Voice pitch level (x-low, low, medium, high, x-high)</param> +/// <param name="Rate">Speech rate (x-slow, slow, medium, fast, x-fast)</param> +public record TTSPitchRate +{ + public string Pitch { get; } + public string Rate { get; } + + public TTSPitchRate(string pitch = "medium", string rate = "medium") + { + ArgumentOutOfRangeException.ThrowIfNotEqual(pitch, TTSValues.ValidPitches.Contains(pitch)); + ArgumentOutOfRangeException.ThrowIfNotEqual(rate, TTSValues.ValidRates.Contains(rate)); + Pitch = pitch; + Rate = rate; + } +}Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs (4)
1-9: Consider reorganizing the file structure.The namespace override comments suggest this file might be in a non-standard location. Consider moving the TTS-related code to a more appropriate location within the
Content.Client.Lobby.UIdirectory structure for better maintainability.
11-16: Add XML documentation to the public class.Adding documentation would improve code maintainability and help other developers understand the purpose and responsibilities of this class.
+/// <summary> +/// Manages the TTS (Text-to-Speech) functionality in the humanoid profile editor. +/// </summary> public sealed partial class HumanoidProfileEditor
27-41: Add error handling for dependency resolution.The initialization method should handle potential failures in dependency resolution and validate that required services are available.
private void InitializeVoice() { + try { _random = IoCManager.Resolve<IRobustRandom>(); _ttsManager = IoCManager.Resolve<TTSManager>(); _ttsSystem = IoCManager.Resolve<IEntityManager>().System<TTSSystem>(); _voiceList = _prototypeManager.EnumeratePrototypes<TTSVoicePrototype>().Where(o => o.RoundStart).ToList(); + } catch (Exception e) { + Logger.Error($"Failed to initialize TTS voice system: {e}"); + return; + }
69-76: Add error handling for TTS playback.The TTS playback logic should handle potential failures and provide feedback to the user.
private void PlayTTS() { if (Profile is null) return; _ttsSystem.StopCurrentTTS(PreviewDummy); - _ttsManager.RequestTTS(PreviewDummy, _random.Pick(_sampleText), Profile.Voice); + try + { + var text = _random.Pick(_sampleText); + _ttsManager.RequestTTS(PreviewDummy, text, Profile.Voice); + } + catch (Exception e) + { + Logger.Error($"Failed to play TTS: {e}"); + // TODO: Show error feedback to user + }Content.Shared/Humanoid/HumanoidAppearanceComponent.cs (1)
116-119: Add XML documentation for the Voice property.Consider adding XML documentation to explain the property's purpose and any constraints.
// WD EDIT START + /// <summary> + /// The TTS voice prototype used for this humanoid's speech. + /// </summary> [DataField, AutoNetworkedField] public ProtoId<TTSVoicePrototype> Voice { get; set; } = SharedHumanoidAppearanceSystem.DefaultVoice; // WD EDIT ENDContent.Client/Audio/ContentAudioSystem.cs (2)
33-33: LGTM! Consider adding documentation about the multiplier choice.The neutral multiplier (1f) is a good conservative choice for TTS volume control. Consider adding a brief comment explaining why this value was chosen, similar to how other audio systems typically provide normalized output that doesn't need additional amplification.
- public const float TTSMultiplier = 1f; // WD EDIT + // TTS engines typically provide normalized output, so we use a neutral multiplier + public const float TTSMultiplier = 1f;
33-33: Remove the "WD EDIT" comment.For consistency with the rest of the codebase, avoid using source control comments in the code.
Content.Client/Options/UI/Tabs/AudioTab.xaml (1)
51-65: Consider removing custom edit markers.The
<!--WD EDIT START-->and<!--WD EDIT END-->comments are unnecessary in version-controlled code as Git already tracks changes. These markers can become outdated and add noise to the codebase.- <!--WD EDIT START--> <BoxContainer Orientation="Horizontal" Margin="5 0 0 0"> <Label Text="{Loc 'ui-options-tts-volume'}" HorizontalExpand="True" /> <Control MinSize="8 0" /> <Slider Name="TtsVolumeSlider" MinValue="0" MaxValue="200" HorizontalExpand="True" MinSize="80 0" Rounded="True" /> <Control MinSize="8 0" /> <Label Name="TtsVolumeLabel" MinSize="48 0" Align="Right" /> <Control MinSize="4 0" /> </BoxContainer> - <!--WD EDIT END-->Content.Server/Entry/EntryPoint.cs (1)
1-1: Consider namespace convention for TTS implementation.The
_Whiteprefix in the namespace suggests this is an internal implementation. Consider whether this should be moved to a more standard namespace structure if it's intended to be a permanent feature.Content.Client/Entry/EntryPoint.cs (3)
1-1: Consider restructuring the TTS namespace.The underscore prefix in
_White.TTSnamespace suggests this might be a temporary or internal implementation. Consider moving it to a more standard namespace structure (e.g.,Content.Client.TTS) for better maintainability.
78-78: Remove unnecessary code comment.The "WD EDIT" comment is redundant as git history already tracks these changes. Consider removing it to maintain cleaner code.
- [Dependency] private readonly TTSManager _ttsManager = default!; // WD EDIT + [Dependency] private readonly TTSManager _ttsManager = default!;
Line range hint
78-174: Consider a more robust TTS architecture.The current implementation tightly couples the TTS system into the main client initialization path. Consider:
- Implementing a facade or service locator pattern to allow for different TTS providers
- Lazy initialization of TTS services to avoid impacting startup time
- Feature flagging to easily disable TTS if needed
- Clear separation between TTS interface and implementation to facilitate testing and provider switching
This would make the system more maintainable and testable, especially given the uncertainty around API access.
Content.Client/Options/UI/Tabs/AudioTab.xaml.cs (1)
Line range hint
171-197: Add TTS volume validation to UpdateChangesThe UpdateChanges method is missing validation for TTS volume changes. This means changes to the TTS volume won't properly update the UI state (Apply/Reset buttons).
Add TTS volume validation:
private void UpdateChanges() { var isMasterVolumeSame = Math.Abs(MasterVolumeSlider.Value - _cfg.GetCVar(CVars.AudioMasterVolume) * 100f / ContentAudioSystem.MasterVolumeMultiplier) < 0.01f; + var isTtsVolumeSame = + Math.Abs(TtsVolumeSlider.Value - _cfg.GetCVar(WhiteCVars.TtsVolume) * 100f / ContentAudioSystem.TTSMultiplier) < 0.01f; // ... other volume checks ... var isEverythingSame = isMasterVolumeSame && isMidiVolumeSame && isAmbientVolumeSame && isAmbientMusicVolumeSame && isAmbientSoundsSame && isLobbySame && isRestartSoundsSame && isEventSame && isAnnouncerDisableMultipleSoundsSame && isAdminSoundsSame && isLobbyVolumeSame - && isInterfaceVolumeSame && isAnnouncerVolumeSame; + && isInterfaceVolumeSame && isAnnouncerVolumeSame && isTtsVolumeSame;Also add the TTS volume label update:
AnnouncerVolumeLabel.Text = Loc.GetString("ui-options-volume-percent", ("volume", AnnouncerVolumeSlider.Value / 100)); + TtsVolumeLabel.Text = + Loc.GetString("ui-options-volume-percent", ("volume", TtsVolumeSlider.Value / 100)); AmbienceSoundsLabel.Text = ((int)AmbienceSoundsSlider.Value).ToString();Content.Client/Lobby/UI/HumanoidProfileEditor.xaml (1)
83-91: LGTM! The TTS UI implementation follows consistent patterns.The voice selection UI is well-integrated into the profile editor, maintaining consistent styling and layout with other sections. The implementation includes both voice selection and preview functionality.
Consider enhancing the playback control UX:
<BoxContainer HorizontalExpand="True"> <Label Text="{Loc 'humanoid-profile-editor-voice-label'}" /> <Control HorizontalExpand="True"/> <OptionButton Name="VoiceButton" HorizontalAlignment="Right" /> - <Button Name="VoicePlayButton" Text="▶" MaxWidth="80" /> + <Button Name="VoicePlayButton" Text="▶" MaxWidth="40" /> + <Button Name="VoiceStopButton" Text="⏹" MaxWidth="40" /> </BoxContainer>Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs (1)
48-56: Consider using constants for voice IDs.The voice IDs in
DefaultSexVoiceare hardcoded string literals. Consider extracting these into constants to maintain consistency withDefaultVoiceand to make updates easier.[ValidatePrototypeId<TTSVoicePrototype>] public const string DefaultVoice = "Eugene"; +[ValidatePrototypeId<TTSVoicePrototype>] +public const string MaleVoice = "Eugene"; +[ValidatePrototypeId<TTSVoicePrototype>] +public const string FemaleVoice = "Kseniya"; +[ValidatePrototypeId<TTSVoicePrototype>] +public const string UnsexedVoice = "Xenia"; public static readonly Dictionary<Sex, string> DefaultSexVoice = new() { - {Sex.Male, "Eugene"}, - {Sex.Female, "Kseniya"}, - {Sex.Unsexed, "Xenia"}, + {Sex.Male, MaleVoice}, + {Sex.Female, FemaleVoice}, + {Sex.Unsexed, UnsexedVoice}, };Content.Server.Database/Model.cs (1)
337-337: Consider standardizing the comment style.The "WD EDIT" comment style differs from the codebase's conventions. Consider removing it or adapting it to match the XML documentation style used elsewhere.
- public string Voice { get; set; } = null!; // WD EDIT + /// <summary> + /// The character's voice identifier for TTS. + /// </summary> + public string Voice { get; set; } = null!;Content.Server/Database/ServerDbBase.cs (1)
201-206: LGTM! Consider enhancing the documentation.The voice handling logic is correctly implemented with a proper fallback to default voice based on sex. However, consider adding XML documentation to explain the voice parameter's purpose and its relationship with the TTS feature.
Add XML documentation above the code block:
+/// <summary> +/// Assigns the voice for the character profile, falling back to a default voice based on sex if not specified. +/// This is used for the Text-to-Speech (TTS) feature. +/// </summary> // WD EDIT STARTContent.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (1)
1139-1146: Consider adding input validation.While the implementation follows the established pattern, consider adding validation for the
newVoiceparameter to ensure it's not null or empty and represents a valid voice option.private void SetVoice(string newVoice) { + if (string.IsNullOrEmpty(newVoice)) + return; + + // Verify that newVoice is a valid option + if (!_availableVoices.Contains(newVoice)) + return; + Profile = Profile?.WithVoice(newVoice); IsDirty = true; }Content.Client/_White/TTS/TTSSystem.cs (4)
21-21: Typo in variable name_enquedStreamsThe variable
_enquedStreamsappears to be misspelled. The correct spelling should be_enqueuedStreams. Correcting this will improve code readability and consistency.Apply this diff to fix the typo:
-private readonly Dictionary<EntityUid, Queue<AudioStreamWithParams>> _enquedStreams = new(); +private readonly Dictionary<EntityUid, Queue<AudioStreamWithParams>> _enqueuedStreams = new();Remember to update all occurrences of
_enquedStreamsin the code.
23-24: Replace magic number with existing constantThe
VoiceRangeconstant is hardcoded as10. Since the comment mentions it is the same asServer.ChatSystem.VoiceRange, consider using the existing constant directly to maintain consistency and avoid magic numbers.If accessible, you can replace it as follows:
-// Same as Server.ChatSystem.VoiceRange -private const float VoiceRange = 10; +private const float VoiceRange = Server.ChatSystem.VoiceRange;This ensures any future changes to
VoiceRangeare automatically reflected in your system.
44-47: Simplify complex conditional logic for clarityThe conditional statement in the
FrameUpdatemethod is complex and might be difficult to read and maintain. Consider refactoring it to improve readability.You could simplify the logic as follows:
if (Deleted(uid)) continue; if (audioComponent?.Running == true && audioComponent.Playing) continue; if (!_enqueuedStreams.TryGetValue(uid, out var queue) || !queue.TryDequeue(out var toPlay)) continue;Or combine conditions with explanatory variables:
bool isAudioPlaying = audioComponent?.Running == true && audioComponent.Playing; bool hasQueuedAudio = _enqueuedStreams.TryGetValue(uid, out var queue) && queue.TryDequeue(out var toPlay); if (Deleted(uid) || isAudioPlaying || !hasQueuedAudio) continue;This refactor enhances readability and maintainability.
11-11: Remove unnecessary ReSharper directiveThe comment
// ReSharper disable InconsistentNamingdisables naming consistency checks. It's generally best to adhere to consistent naming conventions for maintainability.If possible, refactor the code to comply with naming conventions and remove this directive:
-// ReSharper disable InconsistentNamingThis promotes code consistency and readability.
Content.Server/_White/TTS/TTSManager.cs (4)
57-57: Use consistent logging methods for error messagesAt line 57, you're using
_sawmill.Log(LogLevel.Error, ...), while elsewhere you use methods like_sawmill.Error(...). For consistency and clarity, consider using_sawmill.Error(...)here.Apply this diff to make the logging method consistent:
-_sawmill.Log(LogLevel.Error, nameof(TTSManager), "TTS Api url not specified"); +_sawmill.Error("TTS API URL not specified");
93-93: Update the 'CachedCount' metric when removing items from the cacheWhen an item is removed from the cache at line 93, the
CachedCountgauge is not decremented, leading to inaccurate cache size metrics.Adjust the
CachedCountwhen evicting items:_cache.Remove(keyToRemove); +CachedCount.Dec();
88-89: Throw a more specific exception type for HTTP errorsThrowing a generic
Exceptionmakes error handling less precise. Consider usingHttpRequestExceptionto provide better context for HTTP-related errors.Apply this change:
-throw new Exception($"TTS request returned bad status code: {response.StatusCode}"); +throw new HttpRequestException($"TTS request returned bad status code: {response.StatusCode}");
108-108: Improve the clarity of log messagesThe log messages at lines 108 and 114 could be rephrased for better readability:
- Line 108: "Timeout of request generation new sound..." could be "Timeout occurred while requesting new sound..."
- Line 114: "Failed of request generation new sound..." could be "Failed to generate new sound..."
Apply these changes:
// Line 108 -_sawmill.Warning($"Timeout of request generation new sound for '{text}' speech by '{speaker}' speaker"); +_sawmill.Warning($"Timeout occurred while requesting new sound for text '{text}' by speaker '{speaker}'");// Line 114 -_sawmill.Warning($"Failed of request generation new sound for '{text}' speech by '{speaker}' speaker\n{e}"); +_sawmill.Warning($"Failed to generate new sound for text '{text}' by speaker '{speaker}'. Exception: {e.Message}");Also applies to: 114-114
Content.Server/_White/TTS/TTSSystem.Sanitize.cs (1)
66-134: Consider makingWordReplacementdictionary read-onlyTo prevent accidental modification at runtime and enhance safety, consider making the
WordReplacementdictionary read-only.Apply this diff to make the dictionary read-only:
-private static readonly Dictionary<string, string> WordReplacement = new() +private static readonly IReadOnlyDictionary<string, string> WordReplacement = new Dictionary<string, string> { // Existing key-value pairs };Content.Server/_White/TTS/TTSSystem.cs (1)
131-165: Optimize emergency light lookup for better performanceIn the
OnAnnounceRequestmethod, an entity lookup is performed for each player to find nearby emergency lights. This could impact server performance, especially with many players.Consider optimizing the process by:
- Gathering all emergency lights once and mapping their positions.
- Broadcasting the announcement from a central entity or predefined positions.
- Caching the closest emergency light per area or player group.
This can reduce redundant calculations and improve overall performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (43)
Content.Client/Audio/ContentAudioSystem.cs(1 hunks)Content.Client/Entry/EntryPoint.cs(3 hunks)Content.Client/IoC/ClientContentIoC.cs(2 hunks)Content.Client/Lobby/UI/HumanoidProfileEditor.xaml(1 hunks)Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs(4 hunks)Content.Client/Options/UI/Tabs/AudioTab.xaml(1 hunks)Content.Client/Options/UI/Tabs/AudioTab.xaml.cs(4 hunks)Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs(1 hunks)Content.Client/_White/TTS/TTSManager.cs(1 hunks)Content.Client/_White/TTS/TTSSystem.cs(1 hunks)Content.IntegrationTests/Tests/Preferences/ServerDbSqliteTests.cs(1 hunks)Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.Designer.cs(1 hunks)Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs(1 hunks)Content.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.cs(1 hunks)Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.Designer.cs(1 hunks)Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs(1 hunks)Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs(1 hunks)Content.Server.Database/Model.cs(1 hunks)Content.Server/Database/ServerDbBase.cs(3 hunks)Content.Server/Entry/EntryPoint.cs(2 hunks)Content.Server/IoC/ServerContentIoC.cs(2 hunks)Content.Server/_White/TTS/TTSManager.cs(1 hunks)Content.Server/_White/TTS/TTSSystem.Sanitize.cs(1 hunks)Content.Server/_White/TTS/TTSSystem.cs(1 hunks)Content.Shared/Humanoid/HumanoidAppearanceComponent.cs(2 hunks)Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs(4 hunks)Content.Shared/Preferences/HumanoidCharacterProfile.cs(9 hunks)Content.Shared/_White/CVars.cs(1 hunks)Content.Shared/_White/TTS/Events/PlayTTSEvent.cs(1 hunks)Content.Shared/_White/TTS/Events/RequestTTSEvent.cs(1 hunks)Content.Shared/_White/TTS/MsgRequestTTS.cs(1 hunks)Content.Shared/_White/TTS/TTSComponent.cs(1 hunks)Content.Shared/_White/TTS/TTSPitchRateSystem.cs(1 hunks)Content.Shared/_White/TTS/TTSVoicePrototype.cs(1 hunks)Resources/Locale/en-US/_white/escape-menu/ui/options-menu.ftl(1 hunks)Resources/Locale/en-US/_white/escape-menu/ui/options.ftl(0 hunks)Resources/Locale/en-US/_white/preferences/ui/humanoid-profile-editor.ftl(1 hunks)Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl(1 hunks)Resources/Locale/ru-RU/_white/escape-menu/ui/options-menu.ftl(1 hunks)Resources/Locale/ru-RU/_white/escape-menu/ui/options.ftl(0 hunks)Resources/Locale/ru-RU/_white/preferences/ui/humanoid-profile-editor.ftl(1 hunks)Resources/Locale/ru-RU/_white/prototypes/voice/tts_voices.ftl(1 hunks)Resources/Prototypes/_White/Voice/tts_voices.yml(1 hunks)
💤 Files with no reviewable changes (2)
- Resources/Locale/en-US/_white/escape-menu/ui/options.ftl
- Resources/Locale/ru-RU/_white/escape-menu/ui/options.ftl
✅ Files skipped from review due to trivial changes (3)
- Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.Designer.cs
- Resources/Locale/en-US/_white/preferences/ui/humanoid-profile-editor.ftl
- Resources/Locale/ru-RU/_white/preferences/ui/humanoid-profile-editor.ftl
🧰 Additional context used
🪛 yamllint
Resources/Prototypes/_White/Voice/tts_voices.yml
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (57)
Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl (1)
1-3: LGTM! The FTL entries follow the correct format.
The voice name entries follow consistent naming patterns and proper FTL syntax.
Resources/Locale/ru-RU/_white/escape-menu/ui/options-menu.ftl (2)
5-5: LGTM! TTS volume label follows localization conventions.
The translation is accurate and follows the proper FTL syntax for UI labels.
7-7: Consider moving HUD theme changes to a separate PR.
The addition of the operative HUD theme localization appears unrelated to the TTS implementation scope of this PR. It would be cleaner to handle this in a separate PR focused on HUD theme changes.
Let's verify if there are other HUD theme related changes:
Resources/Locale/en-US/_white/escape-menu/ui/options-menu.ftl (2)
1-7: LGTM! Localization entries follow consistent patterns.
The localization entries are well-structured and follow consistent naming conventions. Each entry is clear, concise, and serves a specific UI purpose.
5-5: Verify TTS volume control integration.
The localization entry for TTS volume control looks good. Let's verify that this label is properly connected to the TTS volume slider in the UI.
✅ Verification successful
TTS volume control is properly integrated
The localization entry is correctly implemented and used in the UI:
- Connected to a slider control in
Content.Client/Options/UI/Tabs/AudioTab.xaml - Backed by a CVar system in
Content.Shared/_White/CVars.cs - Fully functional in the TTS system with volume control logic in
Content.Client/_White/TTS/TTSSystem.cs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the TTS volume label usage in UI files
# Expected: Find references to this localization key in UI configuration files
# Search for the localization key usage in XAML files
rg "ui-options-tts-volume" --type xaml
# Search for volume-related TTS configurations
ast-grep --pattern 'class $_ {
$$$
TTS$_Volume$_ {
$$$
}
$$$
}'
Length of output: 145
Script:
#!/bin/bash
# Let's try a broader search approach
# Search for TTS volume references in all files
rg "ui-options-tts-volume" -t all
# Search for any TTS-related UI files
fd -e xaml . | xargs rg -l "TTS"
# Search for volume control implementations
rg -i "tts.*volume" -g "!*.ftl"
Length of output: 1791
Resources/Prototypes/_White/Voice/tts_voices.yml (3)
1-17: LGTM! Voice definitions are well-structured.
The voice entries follow consistent naming conventions and provide clear localization keys. The structure includes all necessary attributes (type, id, name, sex, speaker) for each voice.
🧰 Tools
🪛 yamllint
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
1-17: Verify voice availability and consider fallback mechanism.
Since you mentioned not having API access for verification:
- Please confirm these voice IDs (eugene, kseniya, xenia) match the actual TTS service voice IDs
- Consider adding a fallback mechanism in case specific voices are unavailable
This will help prevent runtime issues if voice IDs don't match the TTS service offerings.
Would you like assistance in implementing a fallback mechanism?
🧰 Tools
🪛 yamllint
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
3-3: Verify localization key definitions.
Let's ensure the localization keys are properly defined in the locale files.
Also applies to: 9-9, 15-15
✅ Verification successful
Localization keys are properly defined
The localization keys for TTS voices are correctly defined in both English and Russian locale files:
tts-voice-name-eugene→ "Evgeniy"/"Евгений"tts-voice-name-kseniya→ "Kseniya"/"Ксения"tts-voice-name-xenia→ "Xenia"/"Сения"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if TTS voice localization keys are defined in locale files
# Expected: Find matching entries for each voice name in both en-US and ru-RU locale files
echo "Checking localization keys..."
for key in "tts-voice-name-eugene" "tts-voice-name-kseniya" "tts-voice-name-xenia"; do
echo "Looking for $key..."
rg "$key" "Resources/Locale/*/*/prototypes/voice/tts_voices.ftl"
done
Length of output: 1097
Script:
#!/bin/bash
# Let's first find where the locale files are actually located
echo "Searching for locale/translation files..."
fd -e ftl
# Also search for any files that might contain these voice-related translations
echo -e "\nSearching for voice-related translations..."
rg -i "tts-voice-name"
Length of output: 181457
Content.Shared/_White/TTS/TTSComponent.cs (1)
9-10: Consider making the default voice configurable and add validation.
The hardcoded default voice "Eugene" might need to be configurable through settings or constants. Additionally, consider adding validation to ensure the prototype exists.
Let's verify the existence of the "Eugene" voice prototype and other voice prototypes:
Content.Shared/_White/TTS/Events/PlayTTSEvent.cs (1)
1-4: LGTM: Proper namespace organization and imports.
The namespace follows the project's convention, and the required serialization import is present.
Content.Client/_White/TTS/TTSManager.cs (2)
1-4: LGTM: Dependencies and namespace are well-structured.
The code properly separates client/server concerns and includes necessary networking dependencies.
12-15: Verify network message registration.
Since API access isn't available for testing, ensure that:
MsgRequestTTSis properly defined in the shared code- The server-side handler for this message type exists
✅ Verification successful
Network message registration is properly implemented
The verification shows:
MsgRequestTTSis correctly defined in shared code with proper serialization- Server-side handler
OnRequestTTSis registered - Client-side registration matches the server implementation
- Message follows the expected pattern with appropriate properties (Uid, Text, VoiceId)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify MsgRequestTTS definition and handler
# Check for message definition
echo "Checking MsgRequestTTS definition..."
rg -l "class MsgRequestTTS"
# Check for message handler
echo "Checking for message handler..."
rg -l "RegisterNetMessage<MsgRequestTTS>"
Length of output: 351
Script:
#!/bin/bash
# Let's verify the content of these files to ensure proper implementation
# Check MsgRequestTTS definition
echo "=== MsgRequestTTS Definition ==="
cat Content.Shared/_White/TTS/MsgRequestTTS.cs
# Check server-side handler implementation
echo -e "\n=== Server Handler Implementation ==="
rg -A 10 "RegisterNetMessage<MsgRequestTTS>" Content.Server/_White/TTS/TTSSystem.cs
Length of output: 1686
Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs (3)
1-9: LGTM! Migration class structure follows best practices.
The migration is properly structured with appropriate namespace and inheritance.
1-29: Verify consistency with PostgreSQL migration.
The migration looks good overall, but let's ensure it's consistent with its PostgreSQL counterpart.
✅ Verification successful
Migration is consistent with PostgreSQL version
The migrations are consistent across both databases:
- SQLite uses
type: "TEXT"while PostgreSQL usestype: "text"(correct per-database type naming) - Both add the same
voicecolumn to theprofiletable with identical properties:- Not nullable
- Default value of empty string
- Same column name and table name
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Compare with PostgreSQL migration
echo "Searching for corresponding PostgreSQL migration..."
rg -l "20241105191142_TtsVoices"
# Check for any differences in column definition
rg "AddColumn.*voice"
Length of output: 250
Script:
#!/bin/bash
# Let's check the content of both migration files
echo "=== SQLite Migration ==="
cat Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs
echo -e "\n=== PostgreSQL Migration ==="
cat Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs
# Also check the designer file to be thorough
echo -e "\n=== PostgreSQL Migration Designer ==="
cat Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.Designer.cs
Length of output: 73075
11-19: Consider documenting voice format and reviewing default value.
While the migration is technically correct, consider the following suggestions:
- Add a comment documenting the expected format of the voice value (e.g., "voice-id", "language-code/voice-name")
- Verify if an empty string is the most appropriate default value, or if there should be a system default voice
Let's check if there's a default voice defined elsewhere:
✅ Verification successful
Migration is correctly implemented with appropriate defaults
The empty string default in the migration is appropriate as the codebase has proper handling:
- The system uses a prototype-based voice system with "Eugene" as the default voice
- There's sex-specific default voice mapping:
- Male: "Eugene"
- Female: "Kseniya"
- Unsexed: "Xenia"
- The voice format is validated through
TTSVoicePrototypeand proper fallbacks are implemented in the codebase
The migration's default empty string is safe as the application layer will automatically assign the appropriate default voice based on the character's sex if an invalid/empty value is encountered.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for default voice definitions
rg -l "DefaultVoice"
Length of output: 188
Script:
#!/bin/bash
# Let's check the content of these files to understand the default voice implementation
rg "DefaultVoice" -A 5 -B 5
Length of output: 2857
Script:
#!/bin/bash
# Let's check for TTSVoicePrototype to understand the voice format
rg "TTSVoicePrototype" -A 5 -B 5
Length of output: 11013
Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs (3)
5-6: Verify SQLite migration compatibility.
This Postgres migration needs a corresponding SQLite migration for development environments.
Let's check for the SQLite migration:
✅ Verification successful
Let me verify the content of both migration files to ensure they are properly synchronized:
SQLite migration is present and properly synchronized
The corresponding SQLite migration exists and matches the Postgres migration, with the only difference being the column type (text vs TEXT) which is the correct database-specific type for each platform. Both migrations:
- Add the same
voicecolumn to theprofiletable - Set the same constraints (not null with empty default)
- Include proper Down migration to drop the column
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for corresponding SQLite migration
fd -g "*TtsVoices.cs" -t f Content.Server.Database/Migrations/Sqlite
Length of output: 140
Script:
#!/bin/bash
# Compare both migration files
echo "=== Postgres Migration ==="
cat Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs
echo -e "\n=== SQLite Migration ==="
cat Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs
Length of output: 1805
13-18: 🛠️ Refactor suggestion
Reconsider the default value for the voice column.
The voice column is added with an empty string as default, but based on the AI summary, there's a SharedHumanoidAppearanceSystem.DefaultVoice constant. Consider using this default value instead of an empty string to maintain consistency with the application logic.
Let's verify the default voice value:
#!/bin/bash
# Search for DefaultVoice constant definition
rg -A 2 "DefaultVoice\s*=" Consider updating the migration to use the same default value:
migrationBuilder.AddColumn<string>(
name: "voice",
table: "profile",
type: "text",
nullable: false,
- defaultValue: "");
+ defaultValue: "default_voice"); // Replace with actual DefaultVoice value1-29: Verify the migration's compatibility with existing profiles.
The migration looks structurally sound with proper Up/Down methods. However, since this is adding a required column to an existing profiles table:
- Verify that the default value is appropriate for existing profiles
- Ensure the TTSManager/TTSSystem can handle profiles with the default voice value
- Consider adding data validation in the application layer
Let's check for profile handling in the codebase:
✅ Verification successful
Migration is safe with appropriate default voice handling
The verification shows that:
- The default voice value is properly defined as "Eugene" in
SharedHumanoidAppearanceSystem.DefaultVoice - The system has sex-specific default voices:
- Male: "Eugene"
- Female: "Kseniya"
- Unsexed: "Xenia"
- The TTSVoicePrototype system is properly implemented with required sex validation
- The HumanoidCharacterProfile and TTSSystem properly handle voice initialization
The empty string default in the migration is safe because:
- New profiles will use the system defaults
- Existing profiles will be populated with the empty string, which will fall back to the appropriate default voice based on the character's sex
- The TTSSystem includes validation logic to ensure voice-sex compatibility
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check how profiles are handled in relation to voice property
ast-grep --pattern 'class $_ {
$$$
voice $$$
$$$
}'
# Look for profile validation logic
rg -A 5 "ValidateProfile|ProfileValidation"
Length of output: 59
Script:
#!/bin/bash
# Let's try different approaches to find voice-related code and profile handling
# Search for voice-related code
rg -l "voice" --type cs
# Check profile-related files
fd "Profile" --extension cs
# Look for TTS-related implementations
rg -l "TTS|TextToSpeech" --type cs
# Check for profile table schema or related code
rg -A 5 "table: \"profile\"" --type cs
Length of output: 31484
Script:
#!/bin/bash
# Let's check the TTS implementation and default voice handling
# Check TTSVoicePrototype for default voice configuration
rg -A 10 "class TTSVoicePrototype" --type cs
# Check HumanoidCharacterProfile for voice property handling
rg -A 10 "voice.*=.*" Content.Shared/Preferences/HumanoidCharacterProfile.cs
# Look for TTSSystem voice initialization
rg -A 10 "InitializeVoice|DefaultVoice" --type cs
Length of output: 10175
Content.Shared/_White/TTS/TTSVoicePrototype.cs (3)
1-5: LGTM: Clean namespace structure and minimal imports.
The file structure follows project conventions and includes only necessary dependencies.
1-33: Verify error handling for unavailable TTS service.
Since the PR mentions lack of API access for verification, ensure that the TTS system gracefully handles scenarios where the voice service is unavailable.
✅ Verification successful
Based on the results, I can now generate the final response since I have sufficient information about the error handling implementation.
Error handling for TTS service is properly implemented ✓
The TTS system already includes comprehensive error handling for service unavailability:
- Handles missing API URL configuration with appropriate logging
- Implements request timeout (10 seconds) with proper error recovery
- Catches and handles network/service exceptions gracefully
- Returns null and logs warnings when the service is unavailable, allowing graceful degradation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for error handling in TTS implementation
# Expect: Error handling for service unavailability
# Search for error handling in TTS-related files
rg -A 5 'try|catch|throw|Error|Exception' --type cs -g '*TTS*.cs'
Length of output: 2638
6-9: Verify prototype ID format consistency.
The prototype ID "ttsVoice" should be verified against other prototypes in the codebase for naming convention consistency.
✅ Verification successful
The provided output seems to be from a different command than what was specified in the script. Let me search for prototype IDs to verify the naming convention.
Based on the search results, I can now provide a final response regarding the prototype ID format consistency:
The prototype ID "ttsVoice" follows the established naming convention.
Looking at the codebase's prototype IDs, they follow these consistent patterns:
- camelCase format (e.g., "customGhost", "artifactTrigger", "statusEffect")
- Descriptive and domain-specific names
- No spaces or special characters
The "ttsVoice" prototype ID adheres to these conventions, using camelCase and being descriptive of its purpose.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check other prototype IDs for naming convention consistency
# Expect: Other prototype attributes following similar naming patterns
# Search for all Prototype attributes
rg '\[Prototype\("([^"]+)"\)\]' -g '*.cs'
Length of output: 2638
Script:
#!/bin/bash
# Search for all Prototype attributes in CS files
rg '\[Prototype\("([^"]+)"\)\]' -g '*.cs' -A 1
Length of output: 27305
Content.Shared/_White/TTS/MsgRequestTTS.cs (1)
1-6: LGTM! Imports and namespace are well-organized.
All necessary dependencies are properly imported, and the namespace follows the project structure.
Content.Shared/_White/TTS/TTSPitchRateSystem.cs (2)
1-10: LGTM! Class structure follows entity system pattern.
The class is properly structured as an entity system with appropriate namespace organization and dependency imports.
33-36: LGTM! Method is concise and uses appropriate dictionary access.
The implementation correctly handles the case when the species is not found in the dictionary.
Content.Client/IoC/ClientContentIoC.cs (2)
1-1: LGTM!
The using directive is properly placed and follows the existing namespace organization pattern.
58-58: Consider interface-based registration and verify API integration.
A few concerns regarding the TTSManager registration:
- Other managers in this file typically register with interfaces (e.g.,
IoCManager.Register<IChatManager, ChatManager>()). Consider defining anITTSManagerinterface to maintain consistency and improve testability. - The comment "WD EDIT" might not follow project conventions.
- Since TTSManager uses HTTP client for API communication (as noted in the summary), and you mentioned not having API access for verification, this could lead to runtime issues.
Let's verify the TTSManager implementation:
Content.Server/IoC/ServerContentIoC.cs (2)
1-1: LGTM!
The using directive is properly placed and follows the existing namespace organization pattern.
69-69: Consider registering TTSManager with an interface for better dependency management.
The current registration binds to the concrete TTSManager type. For better maintainability and testability, consider:
- Defining an interface (e.g.,
ITTSManager) - Registering using
IoCManager.Register<ITTSManager, TTSManager>()
Additionally, since the PR mentions lack of API access for verification:
- Implement graceful fallback if TTS API is unavailable
- Add health checks or connection validation
- Consider making TTS features toggleable via configuration
Let's verify the TTSManager implementation:
Content.IntegrationTests/Tests/Preferences/ServerDbSqliteTests.cs (1)
48-48: Consider using system defaults and adding voice-specific test cases.
Instead of hardcoding "Eugene", consider using SharedHumanoidAppearanceSystem.DefaultVoice to align with the system's default values. Additionally, since voice is now validated against character sex, consider adding specific test cases to verify this constraint.
Let's verify the available voices and their validation rules:
Content.Shared/Humanoid/HumanoidAppearanceComponent.cs (2)
116-119: Verify voice validation logic.
The Voice property is added but there's no visible validation to ensure compatibility with the humanoid's species and gender. Ensure this is handled in the appearance system.
#!/bin/bash
# Check for voice validation logic in the appearance system
ast-grep --pattern 'class SharedHumanoidAppearanceSystem {
$$$
SetTTSVoice($$$) {
$$$
}
$$$
}'
# Look for voice-related validation code
rg -p "Voice.*valid|valid.*Voice" -g "*.cs"1-1: Verify the namespace convention.
The Content.Shared._White.TTS namespace uses an underscore prefix, which is unusual. Consider whether this follows the project's namespace conventions.
✅ Verification successful
Based on the search results, I can see that the _White namespace is consistently used across multiple files and appears to be an intentional namespace structure for a specific set of features, including TTS functionality. The underscore prefix, while unusual, is part of a broader pattern in this codebase.
The namespace convention appears to be intentional and consistent.
- The
_Whitenamespace is used systematically across multiple features (TTS, Animations, Guns, etc.) - It's properly referenced in both shared and implementation code
- The structure is consistently applied in both client and server components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other usages of underscore-prefixed namespaces
rg -g "*.cs" "namespace.*_.*" || echo "No other underscore-prefixed namespaces found"
# Check the structure of other shared components
fd -e cs -p "Content.Shared.*Component.cs"
Length of output: 108
Script:
#!/bin/bash
# Let's try a different approach to check namespace conventions
# Search for all namespaces in .cs files
rg -g "*.cs" "^namespace" --no-filename | sort -u
# Check for files in Content.Shared._White directory
fd -e cs . "Content.Shared._White"
# Look for other references to the TTS namespace
rg -g "*.cs" "Content\.Shared\._White\.TTS"
Length of output: 93100
Content.Client/Options/UI/Tabs/AudioTab.xaml (1)
51-65: LGTM: UI implementation follows established patterns.
The TTS volume control implementation maintains consistency with existing volume controls in terms of:
- Layout structure using
BoxContainer - Control spacing and margins
- Use of labels and sliders
- Naming conventions
Content.Server/Entry/EntryPoint.cs (1)
111-111:
Add error handling for TTS initialization.
Given that:
- The PR mentions lack of API access for verification
- TTS services typically require external API connectivity
- No visible error handling for initialization failures
Consider adding proper error handling to ensure server startup isn't affected if the TTS service is unavailable.
Here's a suggested approach:
- IoCManager.Resolve<TTSManager>().Initialize(); // WD EDIT
+ try
+ {
+ IoCManager.Resolve<TTSManager>().Initialize();
+ }
+ catch (Exception ex)
+ {
+ Logger.WarningS("tts", "Failed to initialize TTS service: {0}", ex);
+ // Consider adding a server configuration flag to track TTS availability
+ }Let's verify the error handling in the TTSManager class:
Content.Client/Entry/EntryPoint.cs (1)
174-174:
Add error handling for TTS initialization.
Given that:
- The PR mentions lack of API access for verification
- TTS services often require external dependencies
- The initialization could fail for various reasons (service unavailable, invalid credentials, etc.)
Consider adding error handling to gracefully handle initialization failures:
- _ttsManager.Initialize(); // WD EDIT
+ try
+ {
+ _ttsManager.Initialize();
+ }
+ catch (Exception ex)
+ {
+ _logManager.GetSawmill("tts").Error($"Failed to initialize TTS system: {ex}");
+ // Consider whether TTS is critical enough to throw or just continue without it
+ }Let's verify the error handling in the TTSManager implementation:
Content.Client/Options/UI/Tabs/AudioTab.xaml.cs (3)
130-130: LGTM: TTS volume configuration follows established pattern
The TTS volume configuration implementation correctly follows the same pattern as other audio controls, using appropriate scaling with TTSMultiplier.
157-157: LGTM: TTS volume reset implementation is consistent
The TTS volume reset logic correctly follows the established pattern of other audio controls.
1-2: Verify TTS-related dependencies
Let's verify the required TTS components are properly defined:
✅ Verification successful
TTS dependencies are properly configured and integrated
The verification shows all TTS-related components are properly defined and integrated:
WhiteCVars.TtsVolumeis defined in CVars.cs as a client-side configurationContentAudioSystem.TTSMultiplieris defined with a value of 1.0f- TTS UI elements are properly defined in AudioTab.xaml including the
TtsVolumeSlider - TTSSystem properly subscribes to volume changes through
WhiteCVars.TtsVolume
The imports in AudioTab.xaml.cs are valid and correspond to the actual implementations in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for TTS-related configurations and UI elements
# Verify WhiteCVars.TtsVolume definition
echo "Checking WhiteCVars.TtsVolume definition..."
rg -l "TtsVolume" --type cs
# Verify ContentAudioSystem.TTSMultiplier definition
echo "Checking ContentAudioSystem.TTSMultiplier definition..."
rg -l "TTSMultiplier" --type cs
# Verify TtsVolumeSlider UI definition
echo "Checking TtsVolumeSlider in XAML..."
rg -l "TtsVolumeSlider" --type xaml
Length of output: 632
Script:
#!/bin/bash
# Let's examine the actual implementations
# Check CVars.cs for TtsVolume definition
echo "=== Content.Shared/_White/CVars.cs TtsVolume definition ==="
rg "TtsVolume" Content.Shared/_White/CVars.cs -C 2
# Check ContentAudioSystem for TTSMultiplier
echo -e "\n=== Content.Client/Audio/ContentAudioSystem.cs TTSMultiplier definition ==="
rg "TTSMultiplier" Content.Client/Audio/ContentAudioSystem.cs -C 2
# Check AudioTab.xaml for TTS-related UI elements (using .xaml extension)
echo -e "\n=== Content.Client/Options/UI/Tabs/AudioTab.xaml TTS UI elements ==="
rg "Tts" Content.Client/Options/UI/Tabs/AudioTab.xaml -C 2
# Check TTSSystem implementation
echo -e "\n=== Content.Client/_White/TTS/TTSSystem.cs implementation ==="
rg "TtsVolume" Content.Client/_White/TTS/TTSSystem.cs -C 2
Length of output: 2141
Content.Shared/Preferences/HumanoidCharacterProfile.cs (3)
3-3: LGTM: Voice property and dependency added correctly.
The Voice property is properly integrated with serialization support and a default value.
Also applies to: 81-84
128-128: LGTM: Constructors updated consistently.
Voice parameter is properly integrated into both constructors, maintaining the class's immutable design pattern.
Also applies to: 148-148, 172-172
502-508: LGTM: Voice validation properly implemented.
The voice validation logic in EnsureValid follows the established pattern and includes appropriate fallback handling.
Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs (2)
403-403: LGTM!
The integration of SetTTSVoice in LoadProfile follows the established pattern and correctly batches updates by setting sync to false.
4-4: Verify namespace convention.
The underscore prefix in _White namespace suggests it might be a private or internal namespace. Consider following standard namespace naming conventions if this is intended to be a public API.
Content.Server.Database/Model.cs (1)
337-337: LGTM with migration verification needed.
The Voice property implementation follows the established patterns in the Profile class. However, since this is a non-nullable string property, ensure that the corresponding database migration includes a default value for existing records.
✅ Verification successful
Let me check the content of the migration files to verify the Voice column addition.
Property implementation and migration are properly handled
The Voice property implementation is correctly supported by migrations in both Postgres and SQLite databases. The migrations add the 'voice' column to the 'profile' table with:
- Non-nullable string type (TEXT/text)
- Empty string as default value for existing records
- Consistent naming convention (lowercase 'voice')
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for migration files that handle the Voice column addition
# Look for migration files that might contain Voice column definition
rg -l "Voice" --type-add 'migration:*.cs' -tmigration
Length of output: 2484
Script:
#!/bin/bash
# Check the content of TtsVoices migration files
rg -A 10 "Voice" Content.Server.Database/Migrations/*/20241105191*_TtsVoices.cs
Length of output: 2465
Content.Server/Database/ServerDbBase.cs (3)
232-232: LGTM! Parameter placement is appropriate.
The voice parameter is correctly positioned between sex and gender parameters in the constructor, maintaining a logical grouping of related characteristics.
271-271: LGTM! Voice property assignment is correct.
The voice property is properly assigned from the humanoid profile to the database profile, ensuring voice data persistence.
Line range hint 201-271: Verify database schema changes for voice support.
The code changes imply a new 'voice' column in the profile table. Let's verify the database schema changes are properly handled.
✅ Verification successful
Database schema changes for voice support are properly handled
The verification confirms that the voice-related changes are properly handled in the database schema:
- The
Profilemodel includes theVoiceproperty with proper nullability constraints - Database migrations exist for both SQLite and Postgres:
20241105191222_TtsVoices.cs(SQLite)20241105191142_TtsVoices.cs(Postgres)
- Both migrations properly add the
voicecolumn to theprofiletable with appropriate data types (TEXT/text) and constraints
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify database migration files for voice column
# Test: Search for migration files containing voice column changes
rg -l "voice" --type-add 'sql:*.sql' -t sql
# Test: Search for voice-related changes in entity configuration
ast-grep --pattern 'public string Voice { get; set; }'
Length of output: 249
Script:
#!/bin/bash
# Let's try a broader search for database migrations and profile-related files
# Search for migration files in common database migration paths
fd -e sql
# Search for Profile entity definition
rg -l "class Profile" --type cs
# Once we find the Profile class, let's examine its content
rg -A 5 -B 5 "class Profile" --type cs
Length of output: 6573
Script:
#!/bin/bash
# Let's examine the Profile model definition more thoroughly and look for voice-related properties
# Get the full Profile class definition
rg -A 50 "^ public class Profile" Content.Server.Database/Model.cs
# Search for any recent migrations that might contain voice-related changes
fd "202" Content.Server.Database/Migrations
Length of output: 16978
Script:
#!/bin/bash
# Let's examine the TtsVoices migration which seems relevant to voice changes
# Check both SQLite and Postgres migrations for voice-related changes
rg -A 10 "TtsVoices" Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs
rg -A 10 "TtsVoices" Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs
Length of output: 925
Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs (1)
835-841: LGTM! The Voice property is properly configured.
The Voice property is correctly added to the Profile entity with appropriate configuration:
- Required field with TEXT column type
- Follows EF Core naming conventions
- Properly integrated into the existing Profile entity model
Content.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.cs (1)
886-892: Property configuration looks good but consider default value.
The Voice property is correctly configured with appropriate column type and constraints. However, since it's marked as required, consider the following:
- Ensure that a migration plan exists for existing profiles in the database
- Consider providing a default value to handle existing records
Let's verify if there's a corresponding migration file:
✅ Verification successful
Migration properly handles existing records with default value
The migration file correctly addresses the concern by setting defaultValue: "" for the new required Voice property. This ensures existing profiles will have an empty string as the voice value, maintaining database consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for migration file that adds the voice column
fd --type f ".*Voice.*" Content.Server.Database/Migrations
Length of output: 362
Script:
#!/bin/bash
# Check the migration content to verify default value handling
cat Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs
Length of output: 840
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (3)
647-647: LGTM!
The placement of UpdateTTSVoicesControls() call is logical and consistent with the initialization order.
1134-1134: LGTM!
The placement of UpdateTTSVoicesControls() call is appropriate as voice options typically depend on the selected sex.
152-159: Verify the voice initialization implementation.
The voice initialization region is added in a logical place, but the implementation of InitializeVoice() is not visible in the provided code. Please ensure that:
- The method properly initializes voice-related UI controls
- It handles any potential errors during initialization
- It follows the same patterns as other initialization methods in the class
✅ Verification successful
Based on the extensive search results, I can now provide a complete analysis of the InitializeVoice() implementation and its related functionality:
The voice initialization implementation is properly structured and complete
The implementation shows that:
-
The method properly initializes voice-related UI controls by:
- Setting up the TTS system dependencies
- Loading available voice prototypes from the prototype manager
- Configuring the voice selection button and play button
- Handling voice selection and preview functionality
-
It handles errors appropriately by:
- Checking for null Profile before operations
- Validating voice compatibility with character sex
- Falling back to default voices when needed
-
It follows the same patterns as other initialization methods in the class by:
- Being called during the component's initialization phase
- Using dependency injection for required services
- Integrating with the profile editor's UI update cycle
The implementation is part of a larger TTS (Text-to-Speech) system that properly handles voice selection, preview, and persistence in character profiles.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Look for the InitializeVoice implementation
ast-grep --pattern 'private void InitializeVoice() {
$$$
}'
Length of output: 63
Script:
#!/bin/bash
# Let's try a broader search for any method containing 'Voice' in its name
ast-grep --pattern 'void $_Voice$_() {
$$$
}'
# Also search for any voice-related code using ripgrep for better context
rg -i "voice" "Content.Client/Lobby/UI/HumanoidProfileEditor" -A 5 -B 5
Length of output: 205
Script:
#!/bin/bash
# First, let's find the exact path of the file
fd "HumanoidProfileEditor.xaml.cs"
# Then search for voice-related code in the correct path
rg -i "voice" -A 5 -B 5
# Also search for the initialization method with a more flexible pattern
ast-grep --pattern 'InitializeVoice'
Length of output: 316377
Content.Client/_White/TTS/TTSSystem.cs (1)
12-130: Overall implementation effectively integrates TTS functionality
The TTSSystem class is well-structured and integrates the TTS functionality smoothly into the client. Initialization, shutdown, and frame update methods are appropriately overridden, and dependency injections are properly handled.
Content.Server/_White/TTS/TTSManager.cs (1)
114-114: Avoid logging detailed exception information
Including full exception details in logs may expose sensitive information. It's safer to log only the necessary information to identify the issue.
[security_issue]
Consider modifying the log statement:
-_sawmill.Warning($"Failed to generate new sound for text '{text}' by speaker '{speaker}'. Exception: {e.Message}");
+_sawmill.Warning($"Failed to generate new sound for text '{text}' by speaker '{speaker}'.");If detailed exception information is needed for debugging, consider logging it at a higher verbosity level or in a secure location.
Content.Server/_White/TTS/TTSSystem.Sanitize.cs (2)
38-48: Ensure Sanitize method is thoroughly tested
The Sanitize method performs multiple regex replacements and string manipulations. It's crucial to validate that it handles all expected input cases, including edge cases, to prevent potential issues during runtime.
Would you like assistance in creating unit tests to cover various scenarios for the Sanitize method?
16-18: 🛠️ Refactor suggestion
Consider using the appropriate culture for regex matching
The MatchedWordsRegex is using the culture "en-US", which may not handle Cyrillic characters correctly. Since the text includes Russian characters, consider changing the culture to "ru-RU" to ensure proper processing.
Apply this diff to update the culture setting:
[GeneratedRegex("(?<![a-zA-Zа-яёА-ЯЁ])[a-zA-Zа-яёА-ЯЁ]+?(?![a-zA-Zа-яёА-ЯЁ])",
- RegexOptions.IgnoreCase | RegexOptions.Multiline, "en-US")]
+ RegexOptions.IgnoreCase | RegexOptions.Multiline, "ru-RU")]
private static partial Regex MatchedWordsRegex();Likely invalid or redundant comment.
Content.Server/_White/TTS/TTSSystem.cs (1)
187-216: TTS generation logic is well-implemented
The GenerateTTS method correctly handles text sanitization, pitch, and rate parameters, and interacts effectively with the TTS manager to produce audio data.
Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.Designer.cs (1)
1-1818: Code Structure and Conventions Review: Looks Good
The migration is well-structured and follows Entity Framework Core conventions. The overall schema definitions and relationships are correctly implemented.
| var message = FormattedMessage.RemoveMarkup(args.Message); | ||
|
|
||
| var soundData = await GenerateTTS(uid, message, protoVoice.Speaker); | ||
| if (soundData is null) |
There was a problem hiding this comment.
Ensure comprehensive sanitization of TTS input text
While FormattedMessage.RemoveMarkup removes markup, additional sanitization might be necessary to prevent injection attacks or unintended behavior when sending user-generated text to the TTS API.
Verify that the Sanitize method effectively sanitizes the text to prevent security issues. Ensure that any potentially harmful inputs are appropriately handled before sending the text to the external TTS service.
| if (entities.Count == 0) | ||
| return; |
There was a problem hiding this comment.
Handle announcements when no emergency lights are nearby
Currently, if there are no emergency lights within range of a player, the TTS announcement is not delivered to them. This could result in important announcements being missed by players.
Consider modifying the code to ensure that announcements are still played to players even when no emergency lights are in range. You can default to the player's position or another appropriate entity.
Apply this diff to address the issue:
if (entities.Count == 0)
- return;
+{
+ // No emergency lights in range, play TTS directly to the player
+ RaiseNetworkEvent(new PlayTTSEvent(GetNetEntity(ev.Source), soundData, true), Filter.SinglePlayer(player), false);
+ continue;
+}Committable suggestion skipped: line range outside the PR's diff.
| var chosenWhisperText = _random.Pick(_whisperWords); | ||
| var obfSoundData = await GenerateTTS(uid, chosenWhisperText, protoVoice.Speaker); |
There was a problem hiding this comment.
Localize or generalize whisper obfuscation words
The _whisperWords array contains words in Cyrillic script, which may not be appropriate or recognizable to all players, especially those using different language settings.
Consider localizing the whisper obfuscation words or using neutral sounds (like "psst" or "shh") to ensure they are suitable and understandable for all players regardless of language settings.
Apply this diff to update the obfuscation words:
-private readonly string[] _whisperWords = ["тсс", "псс", "ччч", "ссч", "сфч", "тст"];
+private readonly string[] _whisperWords = { "psst", "shh", "hush", "sshh", "whisper" };Committable suggestion skipped: line range outside the PR's diff.
| .IsRequired() | ||
| .HasColumnType("text") | ||
| .HasColumnName("voice"); | ||
|
|
There was a problem hiding this comment.
Critical Issue: Non-nullable Voice column without a default value
The new Voice column in the profile table is defined as non-nullable (.IsRequired()) but does not specify a default value. This can cause migration failures when existing records do not have values for this column. To ensure a smooth migration, either set a default value or make the column nullable.
Apply this diff to set a default value:
b.Property<string>("Voice")
.IsRequired()
.HasColumnType("text")
+ .HasDefaultValue(string.Empty)
.HasColumnName("voice");Alternatively, make the column nullable:
b.Property<string>("Voice")
- .IsRequired()
.HasColumnType("text")
.HasColumnName("voice");Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 29
🧹 Outside diff range and nitpick comments (52)
Resources/Locale/ru-RU/_white/escape-menu/ui/options-menu.ftl (1)
Line range hint
4-8: Consider adding comments for translators.While the translations are correct, it would be helpful to add comments explaining what TTS stands for and its purpose. This helps future translators understand the context better.
Add comments like this:
+# TTS (Text-to-Speech) volume control label ui-options-tts-volume = Громкость TTS: +# HUD theme option for operative role ui-options-hud-theme-operative = ОперативникResources/Locale/en-US/_white/escape-menu/ui/options-menu.ftl (1)
1-7: LGTM with a minor suggestion for consistency.The localization entries are well-structured and follow proper Fluent syntax. They align perfectly with the TTS implementation objectives and maintain clear, unambiguous wording.
Consider standardizing the capitalization across menu items. Currently, "TTS Volume" uses title case while other entries use sentence case. For consistency, you might want to change it to:
-ui-options-tts-volume = TTS Volume: +ui-options-tts-volume = TTS volume:Content.Shared/_White/TTS/Events/RequestTTSEvent.cs (1)
5-10: LGTM! Consider adding XML documentation.The implementation is clean and follows best practices. Consider adding XML documentation to describe the purpose of the class and its property.
[Serializable, NetSerializable] // ReSharper disable once InconsistentNaming +/// <summary> +/// Event raised to request text-to-speech conversion of the specified text. +/// </summary> public sealed class RequestTTSEvent(string text) : EntityEventArgs { + /// <summary> + /// The text to be converted to speech. + /// </summary> public string Text { get; } = text; }Resources/Prototypes/_White/Voice/tts_voices.yml (1)
13-17: Consider distinguishing Xenia and Kseniya voices more clearly.While the configuration is technically correct, having two similarly named female voices (Xenia and Kseniya) might cause user confusion. Consider:
- Adding more descriptive localization keys that highlight their differences
- Including additional metadata to distinguish their use cases
Also, please add a newline at the end of the file to comply with YAML best practices.
sex: Female speaker: xenia +🧰 Tools
🪛 yamllint
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
Content.Shared/_White/TTS/TTSComponent.cs (1)
9-10: Consider making the default voice configurable.The default voice "Eugene" is hardcoded. Consider moving this to a configuration file or constants file to make it easier to modify without recompiling.
- public ProtoId<TTSVoicePrototype> Prototype { get; set; } = "Eugene"; + [DataField, AutoNetworkedField] + public ProtoId<TTSVoicePrototype> Prototype { get; set; } = DefaultVoices.Default;Add a new constants file:
namespace Content.Shared._White.TTS { public static class DefaultVoices { public const string Default = "Eugene"; } }Content.Shared/_White/TTS/Events/PlayTTSEvent.cs (1)
5-7: Add XML documentation for the event class.Since this is a public class that's part of the TTS system API, please add XML documentation to describe:
- The purpose of this event
- The parameters and their expected values
- Usage examples
[Serializable, NetSerializable] // ReSharper disable once InconsistentNaming +/// <summary> +/// Event raised to play TTS audio data for a specific entity. +/// </summary> +/// <param name="uid">The target entity's network ID</param> +/// <param name="data">The raw audio data to play</param> +/// <param name="boostVolume">Whether to amplify the audio playback</param> public sealed class PlayTTSEvent(NetEntity uid, byte[] data, bool boostVolume) : EntityEventArgsContent.Client/_White/TTS/TTSManager.cs (2)
6-10: Add XML documentation for better code maintainability.Consider adding XML documentation to describe the class purpose, dependencies, and their roles. This will improve code maintainability and help other developers understand the TTS implementation better.
// ReSharper disable once InconsistentNaming +/// <summary> +/// Manages client-side Text-to-Speech (TTS) requests, handling communication with the server +/// for TTS processing. +/// </summary> public sealed class TTSManager { + /// <summary> + /// Network manager for sending TTS requests to the server. + /// </summary> [Dependency] private readonly IClientNetManager _netMgr = default!; + /// <summary> + /// Entity manager for resolving entity references. + /// </summary> [Dependency] private readonly EntityManager _entityManager = default!;
12-15: Consider adding initialization safety checks.The
Initializemethod could benefit from:
- A flag to prevent multiple initializations
- Implementation of an interface (e.g., IInitializable) to make the initialization contract explicit
+ private bool _initialized; + public void Initialize() { + if (_initialized) + return; + _netMgr.RegisterNetMessage<MsgRequestTTS>(); + _initialized = true; }Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs (1)
13-18: Consider adding column constraints and documentation.The voice column implementation could benefit from the following improvements:
- Consider adding a maximum length constraint to prevent excessive storage usage
- Document the expected format/structure of the voice identifier
- Evaluate if an empty string is the most appropriate default value for voice identification
migrationBuilder.AddColumn<string>( name: "voice", table: "profile", type: "TEXT", - nullable: false, - defaultValue: ""); + type: "TEXT", + maxLength: 100, // Adjust based on your voice ID format + nullable: false, + defaultValue: "default", // Consider using a meaningful default voice ID + comment: "Stores the voice identifier for TTS playback");Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs (1)
13-18: Consider adding column description for better documentation.While the column definition is correct, adding a description would help other developers understand its purpose.
Add a column description after adding the column:
migrationBuilder.AddColumn<string>( name: "voice", table: "profile", type: "text", nullable: false, defaultValue: ""); +migrationBuilder.Sql( + "COMMENT ON COLUMN profile.voice IS 'Stores the TTS voice identifier for the profile';" +);Content.Shared/_White/TTS/TTSVoicePrototype.cs (2)
6-9: Add XML documentation for the class.While the class implementation is good, adding XML documentation would help other developers understand the purpose and usage of this prototype.
[Prototype("ttsVoice")] // ReSharper disable once InconsistentNaming +/// <summary> +/// Represents a voice prototype for the Text-to-Speech system. +/// This prototype defines the characteristics and configuration of a TTS voice. +/// </summary> public sealed class TTSVoicePrototype : IPrototype
28-32: Add XML documentation for configuration properties.The
SponsorOnlyandBorgVoiceproperties lack documentation explaining their purpose and impact.[DataField] + /// <summary> + /// Whether this voice is restricted to sponsors only. + /// </summary> public bool SponsorOnly { get; } [DataField] + /// <summary> + /// Whether this voice is specifically designed for Borg characters. + /// </summary> public bool BorgVoice { get; }Content.Shared/_White/TTS/MsgRequestTTS.cs (2)
8-11: Consider using a different message group for TTS requests.The
Commandmessage group is typically used for critical game commands. TTS requests are more like regular game events and might be better suited to a different message group to avoid potential prioritization issues.- public override MsgGroups MsgGroup => MsgGroups.Command; + public override MsgGroups MsgGroup => MsgGroups.Game;
1-30: Consider implementing rate limiting and caching mechanisms.As TTS requests can be resource-intensive:
- Implement a rate limiter to prevent spam
- Consider caching frequently used TTS responses
- Add a queue system for handling multiple requests
This will help manage server load and improve performance.
Content.Shared/_White/TTS/TTSPitchRateSystem.cs (3)
11-19: Document species-specific voice characteristics.Add XML documentation explaining the rationale behind each species' pitch/rate configuration to help maintain consistency as the system evolves.
+ /// <summary> + /// Maps species to their characteristic voice patterns. + /// Pitch values: x-low, low, medium, high, x-high + /// Rate values: x-slow, slow, medium, fast, x-fast + /// </summary> public readonly Dictionary<ProtoId<SpeciesPrototype>, TTSPitchRate> SpeciesPitches = new()
21-31: Enhance method documentation and readability.The method implementation is correct but could benefit from some improvements.
Consider applying these enhancements:
+ private static readonly TTSPitchRate DefaultPitchRate = new(); + + /// <summary> + /// Attempts to get the TTS pitch rate for an entity based on its species. + /// </summary> + /// <param name="uid">The entity to check</param> + /// <param name="pitch">The resulting pitch rate if found</param> + /// <returns>True if a species-specific pitch rate was found, false otherwise</returns> public bool TryGetPitchRate(EntityUid uid, [NotNullWhen(true)] out TTSPitchRate? pitch) { - if (!TryComp<HumanoidAppearanceComponent>(uid, out var humanoid)) - { - pitch = new TTSPitchRate(); - return false; - } + if (TryComp<HumanoidAppearanceComponent>(uid, out var humanoid)) + { + pitch = GetPitchRate(humanoid.Species); + return pitch != null; + } - pitch = GetPitchRate(humanoid.Species); - return pitch != null; + pitch = DefaultPitchRate; + return false; }
33-36: Add XML documentation for the GetPitchRate method.The implementation is correct, but documentation would improve maintainability.
+ /// <summary> + /// Gets the TTS pitch rate configuration for a specific species. + /// </summary> + /// <param name="protoId">The species prototype identifier</param> + /// <returns>The species-specific pitch rate if defined, null otherwise</returns> public TTSPitchRate? GetPitchRate(ProtoId<SpeciesPrototype> protoId)Content.Shared/_White/CVars.cs (1)
34-37: Consider defaulting TTS to disabled.For security and resource management, it's generally safer to have optional features disabled by default, especially for features that might involve external API calls.
-public static readonly CVarDef<bool> TtsEnabled = CVarDef.Create("tts.enabled", true, CVar.SERVERONLY); +public static readonly CVarDef<bool> TtsEnabled = CVarDef.Create("tts.enabled", false, CVar.SERVERONLY);Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs (3)
27-41: Add error handling and improve event handler organization.The initialization looks functional but could be more robust:
- Consider adding null checks after dependency resolution
- Event handlers could be separated into named methods for better maintainability
Consider refactoring to:
private void InitializeVoice() { _random = IoCManager.Resolve<IRobustRandom>(); _ttsManager = IoCManager.Resolve<TTSManager>(); _ttsSystem = IoCManager.Resolve<IEntityManager>().System<TTSSystem>(); + + if (_random is null || _ttsManager is null || _ttsSystem is null) + throw new InvalidOperationException("Failed to resolve TTS dependencies"); + _voiceList = _prototypeManager.EnumeratePrototypes<TTSVoicePrototype>().Where(o => o.RoundStart).ToList(); - VoiceButton.OnItemSelected += args => - { - VoiceButton.SelectId(args.Id); - SetVoice(_voiceList[args.Id].ID); - }; + VoiceButton.OnItemSelected += OnVoiceSelected; + VoicePlayButton.OnPressed += OnVoicePlayPressed; } - VoicePlayButton.OnPressed += _ => { PlayTTS(); }; +private void OnVoiceSelected(ItemSelectedEventArgs args) +{ + if (args.Id >= 0 && args.Id < _voiceList.Count) + { + VoiceButton.SelectId(args.Id); + SetVoice(_voiceList[args.Id].ID); + } +} + +private void OnVoicePlayPressed(ButtonEventArgs _) +{ + PlayTTS(); }
43-67: Improve code clarity and performance.The method works but has room for improvement:
- Magic number
1forfirstVoiceChoiceIdshould be a named constantFindIndexcould be replaced with a more efficient lookup- Voice selection logic could be more explicit
Consider refactoring to:
+private const int DefaultVoiceChoiceId = 1; private void UpdateTTSVoicesControls() { if (Profile is null) return; VoiceButton.Clear(); - var firstVoiceChoiceId = 1; + var firstVoiceChoiceId = DefaultVoiceChoiceId; + var currentVoiceId = -1; for (var i = 0; i < _voiceList.Count; i++) { var voice = _voiceList[i]; if (!HumanoidCharacterProfile.CanHaveVoice(voice, Profile.Sex)) continue; var name = Loc.GetString(voice.Name); VoiceButton.AddItem(name, i); - if (firstVoiceChoiceId == 1) + if (firstVoiceChoiceId == DefaultVoiceChoiceId) firstVoiceChoiceId = i; + + if (voice.ID == Profile.Voice) + currentVoiceId = i; } - var voiceChoiceId = _voiceList.FindIndex(x => x.ID == Profile.Voice); - if (!VoiceButton.TrySelectId(voiceChoiceId) && VoiceButton.TrySelectId(firstVoiceChoiceId)) + if (!VoiceButton.TrySelectId(currentVoiceId) && VoiceButton.TrySelectId(firstVoiceChoiceId)) SetVoice(_voiceList[firstVoiceChoiceId].ID); }
69-76: Add error handling and volume control.The playback method could be more robust:
- Add error handling for TTS playback
- Consider adding volume control
- Use localized sample text
Consider refactoring to:
private void PlayTTS() { if (Profile is null) return; _ttsSystem.StopCurrentTTS(PreviewDummy); - _ttsManager.RequestTTS(PreviewDummy, _random.Pick(_sampleText), Profile.Voice); + try + { + var sampleTextKey = _random.Pick(_sampleTextKeys); + var localizedText = Loc.GetString(sampleTextKey); + _ttsManager.RequestTTS( + PreviewDummy, + localizedText, + Profile.Voice, + volume: IoCManager.Resolve<IConfigurationManager>().GetCVar(CCVars.TTSVolume) + ); + } + catch (Exception e) + { + Logger.ErrorS("tts", $"Failed to play TTS: {e}"); + } }Content.Server/IoC/ServerContentIoC.cs (1)
69-69: Consider implementing an interface for TTSManagerThe current registration directly uses the concrete
TTSManagerclass. Given that TTS functionality relies on an external API (currently unavailable for testing), consider:
- Defining an interface (e.g.,
ITTSManager) to abstract the implementation- Updating the registration to use the interface
This would:
- Facilitate unit testing through mocking
- Improve maintainability
- Allow for easier API changes or alternative implementations
Example implementation:
- IoCManager.Register<TTSManager>(); // WD EDIT + IoCManager.Register<ITTSManager, TTSManager>(); // WD EDITContent.IntegrationTests/Tests/Preferences/ServerDbSqliteTests.cs (1)
48-48: Consider adding voice-specific test casesSince this is part of a TTS integration, consider adding specific test cases to verify voice-related database operations:
- Voice persistence
- Voice validation
- Default voice handling
Would you like me to help generate additional test cases for voice-related scenarios?
Content.Shared/Humanoid/HumanoidAppearanceComponent.cs (1)
116-119: Add XML documentation for the Voice property.The Voice property should be documented to explain its purpose and usage within the TTS system.
Consider adding documentation like this:
// WD EDIT START + /// <summary> + /// The TTS voice prototype assigned to this humanoid. + /// This determines the voice used for text-to-speech audio output. + /// </summary> [DataField, AutoNetworkedField] public ProtoId<TTSVoicePrototype> Voice { get; set; } = SharedHumanoidAppearanceSystem.DefaultVoice; // WD EDIT ENDContent.Client/Audio/ContentAudioSystem.cs (1)
33-33: Consider documenting the TTSMultiplier value choice.The implementation looks correct, but it would be helpful to document why 1.0f was chosen as the multiplier value, especially since other audio types use different multipliers (e.g., MasterVolumeMultiplier = 3f, MidiVolumeMultiplier = 0.25f).
Content.Client/Options/UI/Tabs/AudioTab.xaml (1)
52-64: Consider adding a tooltip for the TTS volume control.Other complex controls in the file (like AnnouncerDisableMultipleSoundsCheckBox) include tooltips to provide additional context. Consider adding a tooltip to explain the TTS volume control, especially since this is a new feature.
Add a tooltip attribute:
<Label Text="{Loc 'ui-options-tts-volume'}" + ToolTip="{Loc 'ui-options-tts-volume-tooltip'}" HorizontalExpand="True" />Content.Server/Entry/EntryPoint.cs (1)
1-1: Consider renaming the namespace to follow standard conventions.The underscore prefix in
_Whitenamespace suggests this might be a temporary implementation. Consider moving this to a more standard namespace structure (e.g.,Content.Server.TTSorContent.Server.Features.TTS) for better maintainability.Content.Client/Entry/EntryPoint.cs (3)
1-1: Consider relocating TTS namespace to follow standard conventions.The
_Whitenamespace prefix suggests a non-standard location. Consider moving the TTS-related code to a more conventional namespace likeContent.Client.TTSto maintain consistency with the project's architecture.
78-78: LGTM! Consider removing the redundant comment.The TTSManager dependency is correctly integrated using the established dependency injection pattern. However, the "WD EDIT" comment is redundant as git history already tracks these changes.
- [Dependency] private readonly TTSManager _ttsManager = default!; // WD EDIT + [Dependency] private readonly TTSManager _ttsManager = default!;
174-174: Consider adding error handling for TTS initialization.While the initialization placement is correct, consider adding error handling to gracefully handle TTS initialization failures. This is particularly important since the PR description mentions that API testing wasn't possible.
Also, the "WD EDIT" comment is redundant.
- _ttsManager.Initialize(); // WD EDIT + try + { + _ttsManager.Initialize(); + } + catch (Exception ex) + { + _logManager.GetSawmill("tts").Error($"Failed to initialize TTS: {ex}"); + // Optionally: Disable TTS features or show user notification + }Content.Client/Options/UI/Tabs/AudioTab.xaml.cs (2)
Line range hint
196-213: Add TTS volume label updateThe UpdateChanges method updates labels for all other volume controls but is missing the TTS volume label update.
Add the following code after the other volume label updates:
AnnouncerVolumeLabel.Text = Loc.GetString("ui-options-volume-percent", ("volume", AnnouncerVolumeSlider.Value / 100)); +TtsVolumeLabel.Text = + Loc.GetString("ui-options-volume-percent", ("volume", TtsVolumeSlider.Value / 100)); AmbienceSoundsLabel.Text = ((int)AmbienceSoundsSlider.Value).ToString();
Missing TTS volume update handling in code
The TTS volume slider UI is properly defined in the XAML, but there are missing pieces in the code-behind:
- The
UpdateChanges()method needs to:
- Add
isTtsVolumeSamecheck- Update
TtsVolumeLabeltext- The localization string
ui-options-tts-volumeneeds to be defined🔗 Analysis chain
Line range hint
1-213: Verify TTS UI layout in XAMLThe TTS volume control integration looks good in the code-behind, but we should verify the XAML layout.
Let's check the XAML file for proper TTS UI elements:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for TTS-related UI elements in the XAML file rg -A 5 "TtsVolume" --type xamlLength of output: 60
Script:
#!/bin/bash # Search for the XAML file in the repository fd -e xaml AudioTab # Once we find it, let's look for TTS-related elements rg -l "TtsVolume" $(fd -e xaml AudioTab) # Let's also see the content of the file to verify the UI layout cat $(fd -e xaml AudioTab)Length of output: 8890
Script:
#!/bin/bash # Let's verify if TtsVolumeLabel is properly handled in UpdateChanges method rg -A 5 "isTtsVolumeSame|TtsVolumeLabel" Content.Client/Options/UI/Tabs/AudioTab.xaml.cs # Also check if there's a localization string for TTS volume rg "ui-options-tts-volume" --type yamlLength of output: 129
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml (1)
83-91: LGTM! Consider enhancing the playback button UX.The voice selection UI is well-integrated and follows the established layout patterns. However, the playback experience could be improved.
Consider these UX enhancements:
- Add visual feedback for the play button's state (playing/stopped)
- Include a loading state for voice playback
- Show error feedback for failed playback attempts
Example implementation:
- <Button Name="VoicePlayButton" Text="▶" MaxWidth="80" /> + <Button Name="VoicePlayButton" Text="▶" MaxWidth="80" ToolTip="{Loc 'humanoid-profile-editor-voice-play-button-tooltip'}" />Content.Shared/Preferences/HumanoidCharacterProfile.cs (2)
3-3: LGTM! Consider adding XML documentation.The Voice property is properly implemented with DataField attribute for serialization. Consider adding XML documentation to describe the property's purpose and constraints.
[DataField] +/// <summary> +/// The TTS voice identifier associated with this character profile. +/// </summary> public string Voice { get; set; } = SharedHumanoidAppearanceSystem.DefaultVoice;Also applies to: 81-84
510-515: Simplify voice compatibility logic.The boolean expression in CanHaveVoice is complex and could be clearer. Consider restructuring for better readability.
public static bool CanHaveVoice(TTSVoicePrototype voice, Sex sex) { - return voice.RoundStart && sex == Sex.Unsexed || voice.Sex == sex || voice.Sex == Sex.Unsexed; + if (!voice.RoundStart) + return false; + + return voice.Sex == Sex.Unsexed || // Voice works for all sexes + sex == Sex.Unsexed || // Character can use any voice + voice.Sex == sex; // Voice matches character's sex }Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs (2)
305-314: Add parameter documentation.While the method has a summary, it's missing XML documentation for its parameters. Consider adding:
/// <summary> /// Set a humanoid mob's voice type. /// </summary> /// <param name="uid">The humanoid mob's UID.</param> +/// <param name="voiceId">The prototype ID of the TTS voice to set.</param> +/// <param name="sync">If true, immediately synchronize changes to the client.</param> +/// <param name="humanoid">Optional humanoid component instance for improved performance.</param> // ReSharper disable once InconsistentNaming
320-322: Consider validating the voice prototype.The method should verify if the voiceId corresponds to a valid TTSVoicePrototype.
-if (!TryComp<TTSComponent>(uid, out var comp) - || !Resolve(uid, ref humanoid)) +if (!TryComp<TTSComponent>(uid, out var comp) + || !Resolve(uid, ref humanoid) + || !_proto.HasIndex<TTSVoicePrototype>(voiceId)) return;Content.Server/Database/ServerDbBase.cs (1)
201-205: LGTM! Consider adding null check.The voice handling logic is well-implemented with proper fallback to default values based on sex. However, consider adding a null check before checking for empty string to make the code more robust.
- if (voice == string.Empty) + if (string.IsNullOrEmpty(voice))Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (1)
Line range hint
152-1146: Consider extracting TTS functionality into a separate component.The current implementation tightly couples TTS functionality with the profile editor. Consider:
- Creating a separate
TTSComponentto handle voice-related functionality- Using an interface (e.g.,
ITTSService) for better modularity and testability- Implementing an event system for voice changes
This would improve:
- Maintainability: TTS logic is isolated and easier to modify
- Testability: Voice functionality can be tested independently
- Reusability: TTS features can be used in other parts of the application
Content.Client/_White/TTS/TTSSystem.cs (4)
21-113: Correct the misspelling of_enquedStreamsto_enqueuedStreams.The variable
_enquedStreamsis misspelled. It should be_enqueuedStreamsto match the correct spelling of "enqueue" and to maintain consistency.Apply this diff to correct the spelling throughout the code:
- private readonly Dictionary<EntityUid, Queue<AudioStreamWithParams>> _enquedStreams = new(); + private readonly Dictionary<EntityUid, Queue<AudioStreamWithParams>> _enqueuedStreams = new(); ... - if (!_enquedStreams.TryGetValue(uid, out var queue)) + if (!_enqueuedStreams.TryGetValue(uid, out var queue)) ... - _enquedStreams[uid] = queue; + _enqueuedStreams[uid] = queue;
44-47: Simplify and clarify the conditional logic for better readability.The combined conditions in the
ifstatement are complex and might be hard to read. Breaking them into separate conditions enhances readability.Refactor the conditions as follows:
if (Deleted(uid) || audioComponent is not { Running: true, Playing: true }) continue; if (!_enqueuedStreams.TryGetValue(uid, out var queue)) continue; if (!queue.TryDequeue(out var toPlay)) continue;This makes each condition clear and improves the maintainability of the code.
64-65: Simplify entity usage by removing unnecessaryGetEntitycall.In
OnPlayTTS,ev.Uidis already anEntityUid. There's no need to callGetEntity, which can be omitted for simplicity.Update the code as follows:
- PlayTTS(GetEntity(ev.Uid), ev.Data, ev.BoostVolume ? _volume + 5 : _volume); + PlayTTS(ev.Uid, ev.Data, ev.BoostVolume ? _volume + 5 : _volume);This change streamlines the method call.
24-25: Consider makingVoiceRangeconfigurable for flexibility.Currently,
VoiceRangeis a constant value. Allowing it to be configurable enhances adaptability to different scenarios.Introduce a configuration variable:
private float _voiceRange = 10f; // Default value public override void Initialize() { _cfg.OnValueChanged(WhiteCVars.TtsVolume, OnTtsVolumeChanged, true); + _cfg.OnValueChanged(WhiteCVars.TtsVoiceRange, OnVoiceRangeChanged, true); SubscribeNetworkEvent<PlayTTSEvent>(OnPlayTTS); } +private void OnVoiceRangeChanged(float range) +{ + _voiceRange = range; +}Update
AudioParamsto use_voiceRangeinstead ofVoiceRange.Content.Server/_White/TTS/TTSManager.cs (3)
142-148: Simplify cache key generation to improve performanceUsing SHA256 hashing adds unnecessary overhead:
var key = $"{speaker}/{text}"; var keyData = Encoding.UTF8.GetBytes(key); var bytes = System.Security.Cryptography.SHA256.HashData(keyData); return Convert.ToHexString(bytes);If security isn't a concern, use the raw key directly:
- return Convert.ToHexString(bytes); + return key;
57-57: Use consistent logging methodsCurrently using
_sawmill.Loginstead of_sawmill.Error:_sawmill.Log(LogLevel.Error, nameof(TTSManager), "TTS Api url not specified");For consistency:
- _sawmill.Log(LogLevel.Error, nameof(TTSManager), "TTS Api url not specified"); + _sawmill.Error("TTS API URL not specified");
46-51: Enhance XML documentation for clarityThe summary and parameters can be more descriptive.
Suggested improvements:
- Update the summary to "Generates audio from the provided text using the TTS API."
- Clearly describe each parameter, including
pitch,rate, andeffect.Content.Server/_White/TTS/TTSSystem.Sanitize.cs (4)
7-7: Adhere to naming conventions instead of suppressing warningsAt line 7, the code suppresses the
InconsistentNamingwarning with a ReSharper directive. To improve code maintainability and readability, consider refactoring the code to adhere to the standard naming conventions rather than suppressing the warning.
172-173: Handle large numbers gracefully inNumberToTextmethodIn the
NumberToTextmethod at lines 172-173, when the value exceeds 1,000,000,000,000,000 (10^15), the method returns an empty string. This could lead to confusion or silent failures. Consider handling this case more gracefully by throwing an exception or returning a meaningful message indicating that the number is too large to process.
26-29: Add XML documentation comments for public methodsTo enhance code readability and maintainability, consider adding XML documentation comments to public methods such as
InitializeSanitize,Sanitize, andNumberToText. This will help other developers understand the purpose and usage of these methods.Also applies to: 38-48, 170-209
38-48: Implement error handling in theSanitizemethodThe
Sanitizemethod processes input text through multiple regex replacements, which may throw exceptions if unexpected inputs are encountered. Consider implementing try-catch blocks or input validation to handle potential exceptions and ensure the method fails gracefully.Content.Server/_White/TTS/TTSSystem.cs (1)
145-160: Optimize distance calculation when finding the closest emergency lightThe current implementation initializes
rangeas aVector2and usesLength()in distance calculations, which involves unnecessary square root computations. For better performance, initialize afloat minDistanceSqand useLengthSquared()for comparisons to avoid square roots.Apply this diff to optimize the distance calculation:
- var range = new Vector2(100f); + float minDistanceSq = float.MaxValue; foreach (var item in entities) { var itemSource = _xforms.GetWorldPosition(Transform(item)); var playerSource = _xforms.GetWorldPosition(Transform(player.AttachedEntity.Value)); - var distance = playerSource - itemSource; - if (range.Length() <= distance.Length()) + var distanceSq = (playerSource - itemSource).LengthSquared(); + if (distanceSq >= minDistanceSq) continue; - range = distance; + minDistanceSq = distanceSq; entity = item; }Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.Designer.cs (1)
1-1749: Auto-Generated File Should Not Be Modified ManuallyThis file is marked as auto-generated (
// <auto-generated />). Manual changes to this file may be overwritten by the Entity Framework tooling. Please ensure that any necessary changes are made in the migration file20241105191222_TtsVoices.csor in the model classes themselves.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (43)
Content.Client/Audio/ContentAudioSystem.cs(1 hunks)Content.Client/Entry/EntryPoint.cs(3 hunks)Content.Client/IoC/ClientContentIoC.cs(2 hunks)Content.Client/Lobby/UI/HumanoidProfileEditor.xaml(1 hunks)Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs(4 hunks)Content.Client/Options/UI/Tabs/AudioTab.xaml(1 hunks)Content.Client/Options/UI/Tabs/AudioTab.xaml.cs(4 hunks)Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs(1 hunks)Content.Client/_White/TTS/TTSManager.cs(1 hunks)Content.Client/_White/TTS/TTSSystem.cs(1 hunks)Content.IntegrationTests/Tests/Preferences/ServerDbSqliteTests.cs(1 hunks)Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.Designer.cs(1 hunks)Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs(1 hunks)Content.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.cs(1 hunks)Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.Designer.cs(1 hunks)Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs(1 hunks)Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs(1 hunks)Content.Server.Database/Model.cs(1 hunks)Content.Server/Database/ServerDbBase.cs(3 hunks)Content.Server/Entry/EntryPoint.cs(2 hunks)Content.Server/IoC/ServerContentIoC.cs(2 hunks)Content.Server/_White/TTS/TTSManager.cs(1 hunks)Content.Server/_White/TTS/TTSSystem.Sanitize.cs(1 hunks)Content.Server/_White/TTS/TTSSystem.cs(1 hunks)Content.Shared/Humanoid/HumanoidAppearanceComponent.cs(2 hunks)Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs(4 hunks)Content.Shared/Preferences/HumanoidCharacterProfile.cs(9 hunks)Content.Shared/_White/CVars.cs(1 hunks)Content.Shared/_White/TTS/Events/PlayTTSEvent.cs(1 hunks)Content.Shared/_White/TTS/Events/RequestTTSEvent.cs(1 hunks)Content.Shared/_White/TTS/MsgRequestTTS.cs(1 hunks)Content.Shared/_White/TTS/TTSComponent.cs(1 hunks)Content.Shared/_White/TTS/TTSPitchRateSystem.cs(1 hunks)Content.Shared/_White/TTS/TTSVoicePrototype.cs(1 hunks)Resources/Locale/en-US/_white/escape-menu/ui/options-menu.ftl(1 hunks)Resources/Locale/en-US/_white/escape-menu/ui/options.ftl(0 hunks)Resources/Locale/en-US/_white/preferences/ui/humanoid-profile-editor.ftl(1 hunks)Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl(1 hunks)Resources/Locale/ru-RU/_white/escape-menu/ui/options-menu.ftl(1 hunks)Resources/Locale/ru-RU/_white/escape-menu/ui/options.ftl(0 hunks)Resources/Locale/ru-RU/_white/preferences/ui/humanoid-profile-editor.ftl(1 hunks)Resources/Locale/ru-RU/_white/prototypes/voice/tts_voices.ftl(1 hunks)Resources/Prototypes/_White/Voice/tts_voices.yml(1 hunks)
💤 Files with no reviewable changes (2)
- Resources/Locale/en-US/_white/escape-menu/ui/options.ftl
- Resources/Locale/ru-RU/_white/escape-menu/ui/options.ftl
✅ Files skipped from review due to trivial changes (2)
- Resources/Locale/en-US/_white/preferences/ui/humanoid-profile-editor.ftl
- Resources/Locale/ru-RU/_white/preferences/ui/humanoid-profile-editor.ftl
🧰 Additional context used
🪛 yamllint
Resources/Prototypes/_White/Voice/tts_voices.yml
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (50)
Resources/Locale/ru-RU/_white/prototypes/voice/tts_voices.ftl (2)
1-1: LGTM!
The translation for Eugene (Евгений) is correct and follows proper FTL formatting.
2-3:
Review the voice naming consistency.
There appears to be an inconsistency in the Russian translations:
tts-voice-name-kseniya = Ксенияtts-voice-name-xenia = Сения
Both entries seem to represent the same name but with different spellings. The correct Russian spelling is "Ксения". Additionally, having two entries for the same name might cause confusion.
Consider the following changes:
- Correct the spelling of 'Сения' to 'Ксения'
- Consider removing one of the duplicate entries if they're meant to be the same voice
tts-voice-name-kseniya = Ксения
-tts-voice-name-xenia = Сения
+tts-voice-name-xenia = КсенияLet's check if these voice names are used consistently across the codebase:
Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl (1)
1-3: LGTM! The FTL entries follow proper formatting.
The entries are correctly structured following FTL conventions with consistent hyphenation in identifiers.
Resources/Locale/ru-RU/_white/escape-menu/ui/options-menu.ftl (1)
5-5: LGTM: TTS volume label translation is accurate.
The Russian translation "Громкость TTS:" correctly represents "TTS Volume:" while maintaining the technical acronym "TTS" in Latin alphabet, which is the standard practice in Russian UI translations.
Resources/Prototypes/_White/Voice/tts_voices.yml (3)
1-5: LGTM! Voice configuration for Eugene is well-structured.
The prototype includes all necessary fields with appropriate values and follows localization best practices.
7-11: LGTM! Voice configuration for Kseniya is well-structured.
The prototype follows the same consistent pattern and includes all required fields.
1-17: Verify localization files for voice names.
Let's ensure the localization keys are properly defined in both English and Russian locale files.
✅ Verification successful
Localization keys are properly defined in both locale files ✓
All TTS voice name keys (tts-voice-name-eugene, tts-voice-name-kseniya, tts-voice-name-xenia) are present and correctly defined in both:
- English (en-US) with appropriate English names
- Russian (ru-RU) with appropriate Cyrillic names
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if localization files exist and contain the voice name keys
# Test: Look for localization files and their content
echo "Checking English localizations..."
rg "tts-voice-name-(eugene|kseniya|xenia)" "Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl"
echo "Checking Russian localizations..."
rg "tts-voice-name-(eugene|kseniya|xenia)" "Resources/Locale/ru-RU/_white/prototypes/voice/tts_voices.ftl"
Length of output: 554
🧰 Tools
🪛 yamllint
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
Content.Shared/_White/TTS/TTSComponent.cs (2)
5-7: LGTM! Component declaration follows best practices.
The component is correctly registered and marked for auto-state generation. The sealed modifier prevents inheritance, which is appropriate for ECS components.
9-10: Verify the existence of "Eugene" voice prototype.
Need to ensure that the "Eugene" voice prototype is defined in the prototype system to prevent runtime errors.
✅ Verification successful
"Eugene" voice prototype is properly defined and used as default
The "Eugene" voice prototype is correctly defined in Resources/Prototypes/_White/Voice/tts_voices.yml with all required properties (id, name, sex, speaker). It's also consistently used as the default voice across the codebase, particularly in SharedHumanoidAppearanceSystem.cs where it's set as the default male voice.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for the "Eugene" voice prototype definition
# Expected: Should find a prototype definition for "Eugene" voice
# Look for prototype definition in YAML files
rg -l "id:\s*Eugene" --type yaml
# Look for voice prototype definitions
ast-grep --pattern 'class TTSVoicePrototype : IPrototype {
$$$
}'
Length of output: 153
Script:
#!/bin/bash
# Let's check the content of the TTS voices prototype file
cat Resources/Prototypes/_White/Voice/tts_voices.yml
# Also search for any references to Eugene in the codebase
rg "Eugene" -C 2
Length of output: 2162
Content.Shared/_White/TTS/Events/PlayTTSEvent.cs (1)
1-14: Verify network serialization security.
Since this event is network-serializable and contains raw byte data, we should verify:
- The serialization/deserialization implementation in the network protocol
- Any size limits enforced by the network layer
✅ Verification successful
Let me gather more specific information about the TTS systems and any size limits.
Based on the gathered information, I can now provide a final response:
Network serialization of TTS data appears secure with proper handling
The implementation shows:
- Server-side control of TTS generation and transmission
- Proper scoping of network events using filters (PVS, SinglePlayer)
- Data is only sent to relevant clients based on game logic (distance for whispers, PVS for normal speech)
- No direct client-to-client transmission
- Robust.NET's built-in network protocol handles serialization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for network protocol implementations and serialization handlers
echo "Searching for network protocol implementations..."
rg -l "INetChannel|IServerNetManager|NetworkManager" --type cs
echo "Searching for serialization handlers..."
rg -l "ITypeSerializer|SerializationManager" --type cs
echo "Searching for other TTS event handlers..."
rg -l "PlayTTSEvent" --type cs
Length of output: 3894
Script:
#!/bin/bash
# Check TTS system implementations for size limits and handling
echo "Checking TTS system implementations..."
rg -A 10 "PlayTTSEvent" Content.Server/_White/TTS/TTSSystem.cs Content.Client/_White/TTS/TTSSystem.cs
echo "Checking for network configuration and size limits..."
rg -l "MaxMessageSize|NetworkConfiguration|NetManager" --type cs
echo "Checking for byte array handling in network code..."
rg "byte\[\].*Data" --type cs
Length of output: 17881
Content.Client/_White/TTS/TTSManager.cs (1)
1-24: Consider enhancing the TTS architecture.
The current implementation could benefit from these architectural improvements:
- Add events/callbacks for TTS request status (success/failure)
- Implement request queuing to prevent flooding
- Add rate limiting to prevent abuse
- Consider adding a request cancellation mechanism
This would make the system more robust and maintainable.
Let's check if there are any rate limiting or queuing mechanisms elsewhere in the codebase:
Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs (2)
24-26: LGTM! Clean rollback implementation.
The Down migration correctly removes the voice column, providing a clean rollback path.
1-29: Verify TTS voice validation in the application layer.
Since this migration allows any string value in the voice column, ensure that:
- The application layer validates voice identifiers before storage
- There's proper error handling for invalid voice IDs during TTS playback
- The system gracefully handles migration of existing profiles
✅ Verification successful
Based on the search results, I can now provide a final response:
Voice validation and migration are properly handled in the codebase.
- Voice validation is enforced through:
TTSVoicePrototypewith required sex field and roundstart flagCanHaveVoicemethod that validates voice-sex compatibility- Default voices are provided via
DefaultSexVoicemapping
- Migration of existing profiles is handled by defaulting empty voices to sex-appropriate defaults in both the application layer (
HumanoidCharacterProfile) and database layer (ServerDbBase) - The system gracefully handles invalid voices by falling back to defaults:
Voice = SharedHumanoidAppearanceSystem.DefaultSexVoice[sex]
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for voice validation in profile-related code
# Look for voice validation logic in profile-related files
echo "Searching for voice validation in profile handlers..."
rg -l "voice.*valid" --type cs
# Check for TTS-related error handling
echo "Checking TTS error handling..."
rg -l "catch.*TTS|TTS.*Exception" --type cs
# Look for profile migration handling
echo "Checking profile migration handling..."
rg -l "migrate.*profile|profile.*migrate" --type cs
Length of output: 407
Script:
#!/bin/bash
# Let's try a broader search for TTS and voice-related code
# Search for TTS-related files and classes
echo "Searching for TTS-related files..."
fd -e cs | xargs rg -l "TTS|TextToSpeech" --type cs
# Search for voice-related code in a broader context
echo "Searching for voice-related code..."
rg -A 5 "voice" --type cs
# Look for profile-related validation
echo "Searching for profile validation..."
rg -A 5 "Profile.*Valid|ValidateProfile" --type cs
# Check for any profile-related classes and interfaces
echo "Searching for profile-related types..."
ast-grep --pattern 'class $_ { $$$ Profile $$$ }'
Length of output: 42348
Script:
#!/bin/bash
# Let's check for voice validation and migration handling in specific files
# Check TTSVoicePrototype for validation rules
echo "Checking TTSVoicePrototype for validation..."
rg -A 10 "class TTSVoicePrototype" --type cs
# Check HumanoidCharacterProfile's voice handling
echo "Checking voice handling in profile..."
rg -A 10 "EnsureValid.*voice|ValidateVoice" Content.Shared/Preferences/HumanoidCharacterProfile.cs
# Check for default voice handling
echo "Checking default voice handling..."
rg -A 5 "DefaultSexVoice" --type cs
# Check for voice-related migrations
echo "Looking for voice migration handling..."
rg -A 5 "Migration.*voice|voice.*Migration" --type cs
Length of output: 2932
Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs (2)
8-19: LGTM! Migration structure follows best practices.
The migration is well-structured with appropriate column type, constraints, and default value. The use of text type for the voice column allows for flexible voice identifiers.
13-18: Verify handling of existing profiles.
The migration adds a non-nullable column with an empty string default. Please ensure that:
- The application code properly handles empty voice values
- There's a mechanism to assign appropriate voice values to existing profiles
Let's check for any existing profiles that might be affected:
✅ Verification successful
Based on the code analysis, I can now generate the final response:
The empty voice value handling is properly implemented with fallback mechanisms.
The codebase safely handles empty/invalid voice values through multiple safeguards:
- The
EnsureValidmethod validates voice values and falls back to defaults based on sex if invalid - The UI (HumanoidProfileEditor) ensures only valid voices can be selected based on character's sex
- The TTSVoicePrototype system has proper validation through
CanHaveVoicechecks - Existing profiles will get default voices through
SharedHumanoidAppearanceSystem.DefaultSexVoicewhen validated
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing profile handling in the codebase
# Look for profile-related files and voice handling
fd -e cs -e json "Profile|Voice" --exec rg -l "voice|Voice|TTS|TextToSpeech" {}
Length of output: 1224
Script:
#!/bin/bash
# Let's check how the voice field is handled in the profile code and TTS system
rg -A 10 "voice" ./Content.Shared/Preferences/HumanoidCharacterProfile.cs
# Also check the TTS voice prototype implementation
cat ./Content.Shared/_White/TTS/TTSVoicePrototype.cs
# And check the profile editor UI handling
rg -A 10 "voice" ./Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs
Length of output: 5983
Content.Shared/_White/TTS/TTSVoicePrototype.cs (2)
1-4: LGTM! Clean and focused imports.
The imports are minimal and directly related to the functionality needed.
1-33: Consider adding validation and extensibility features.
A few architectural suggestions:
- Consider adding validation logic to ensure Speaker names are unique or follow a specific format.
- Think about adding a version field to support future voice format changes.
- Consider adding a
Priorityfield to control voice selection order in the UI. - You might want to add a
Tagscollection for flexible categorization beyond the current boolean flags.
Let's check if there are any similar prototype implementations we can learn from:
Content.Shared/_White/TTS/MsgRequestTTS.cs (1)
1-6: LGTM! Imports and namespace are well-organized.
The imports are relevant and the namespace follows the project structure.
Content.Shared/_White/TTS/TTSPitchRateSystem.cs (2)
1-10: LGTM! Class structure and imports are well-organized.
The system is properly integrated into the entity framework with appropriate imports and namespace organization.
1-40: Verify TTS integration points and add tests.
Since you mentioned not having an API for testing, consider:
- Adding unit tests that verify the pitch/rate mappings and validation logic
- Creating mock TTS service for integration testing
- Adding debug logging for pitch/rate values during development
Let's check for other TTS-related components that might need coordination:
✅ Verification successful
Based on the shell script results, I can now make an informed assessment:
-
The TTS system is well-integrated across the codebase with clear dependencies:
- TTSComponent and TTSPitchRateSystem are part of a larger TTS implementation
- The pitch/rate mappings are used by multiple systems including HumanoidAppearanceSystem
- Species definitions are referenced in various parts of the codebase
-
The species list in TTSPitchRateSystem matches the supported species in the codebase, as evidenced by numerous references to these species in various components and systems.
-
The pitch/rate values follow a standard format using SSML-compatible values ("x-low", "low", "medium", "high", "x-high" for pitch and "x-slow", "slow", "medium", "fast", "x-fast" for rate).
No issues found with TTS pitch/rate mappings
The implementation is consistent with the codebase's species support and uses standard SSML values. The default values of "medium" are appropriate for the base case.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all TTS-related files and their dependencies
echo "=== TTS-related files ==="
rg -l "TTS|TextToSpeech" --type cs
echo -e "\n=== HumanoidAppearanceComponent usage ==="
rg -A 5 "HumanoidAppearanceComponent" --type cs
echo -e "\n=== Species definitions ==="
fd -e yaml -e json . | xargs rg "SlimePerson|Arachnid|Dwarf|Human|Diona|Reptilian"
Length of output: 71305
Content.Shared/_White/CVars.cs (1)
39-42: Verify TTS API URL at runtime.
The empty default is good, but ensure there's runtime validation for:
- URL format validity
- Required HTTPS protocol for security
- Presence of the URL before enabling TTS
Content.Client/IoC/ClientContentIoC.cs (1)
1-1: LGTM!
The using directive is correctly placed and follows the existing pattern.
Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs (3)
1-9: LGTM: Dependencies and namespace are well-organized.
The imports are appropriate for TTS functionality, and the namespace aligns with the client-side UI location.
11-16: LGTM: Dependencies are properly initialized.
The required dependencies are correctly marked with default! and will be initialized in InitializeVoice().
1-77: Verify integration with TTS components.
Let's verify the integration with other TTS components to ensure consistency.
✅ Verification successful
Integration with TTS components is properly implemented
The code shows correct integration with all TTS components:
TTSManagercorrectly handles voice requests through network messagesTTSSystemproperly manages audio playback and queuing- Voice property is correctly integrated in
HumanoidCharacterProfilewith proper validation throughCanHaveVoice TTSVoicePrototypedefines all necessary properties for voice selection
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify TTS component integration
# Test 1: Check TTSManager initialization in EntryPoint
echo "Checking TTSManager initialization..."
rg "TTSManager" -A 5 "EntryPoint.cs"
# Test 2: Check TTSSystem audio control
echo "Checking TTSSystem implementation..."
rg "TTSSystem" -A 5 "ContentAudioSystem.cs"
# Test 3: Check HumanoidCharacterProfile voice property
echo "Checking profile voice property..."
rg "Voice.*get|set" "HumanoidCharacterProfile.cs"
Length of output: 557
Script:
#!/bin/bash
# Let's search more broadly for these components
# Find TTS-related files
echo "Finding TTS-related files..."
fd -t f -e cs TTS
# Check TTSManager implementation
echo "Checking TTSManager implementation..."
rg "class TTSManager" -A 5
# Check TTSSystem implementation
echo "Checking TTSSystem implementation..."
rg "class TTSSystem" -A 5
# Check HumanoidCharacterProfile voice property
echo "Checking voice property in profile..."
rg "Voice.*{.*get;" --type cs
Length of output: 3534
Script:
#!/bin/bash
# Let's check the voice property and integration points
# Check voice property in shared preferences
echo "Checking voice property in preferences..."
rg "Voice" "Content.Shared/Preferences/HumanoidCharacterProfile.cs"
# Check TTSVoicePrototype implementation
echo "Checking TTSVoicePrototype..."
cat "Content.Shared/_White/TTS/TTSVoicePrototype.cs"
# Check TTSSystem client implementation
echo "Checking client TTSSystem..."
cat "Content.Client/_White/TTS/TTSSystem.cs"
# Check client TTSManager
echo "Checking client TTSManager..."
cat "Content.Client/_White/TTS/TTSManager.cs"
Length of output: 6496
Content.Server/IoC/ServerContentIoC.cs (1)
1-1: LGTM!
The using directive is properly placed and follows the project's convention for Content.Server namespaces.
Content.IntegrationTests/Tests/Preferences/ServerDbSqliteTests.cs (1)
48-48: Verify voice value matches system defaults
The hardcoded voice value "Eugene" should align with the available voice options in the system. Consider using a constant or referencing the default voice from SharedHumanoidAppearanceSystem.DefaultVoice to maintain consistency with the production code.
✅ Verification successful
Based on the search results, I can now provide a definitive response about the voice value "Eugene":
Voice value "Eugene" is correctly defined as the default voice
The value "Eugene" is properly defined and used throughout the codebase:
- It is the default voice in
SharedHumanoidAppearanceSystem.DefaultVoice - It is defined as a TTS voice prototype in
Resources/Prototypes/_White/Voice/tts_voices.yml - It is specifically configured as a male voice that is available at round start
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the voice value exists in voice definitions
# Test: Search for voice definition files and verify "Eugene" is defined
rg -A 5 '"Eugene"' --type yaml
Length of output: 32
Script:
#!/bin/bash
# Let's try different approaches to find voice-related configurations
# 1. Search for voice-related files
fd -e yaml voice
# 2. Search for voice-related constants or configurations in code
rg -i "defaultvoice|voicelist|voices?" --type cs -A 5
# 3. Search specifically for Eugene in all files
rg "Eugene" -A 5
# 4. Look for voice-related prototypes
fd -e yaml prototype
Length of output: 104285
Content.Shared/Humanoid/HumanoidAppearanceComponent.cs (2)
1-1: LGTM!
The import is correctly placed and necessary for the TTS functionality.
116-119: Verify DefaultVoice constant exists.
The property uses SharedHumanoidAppearanceSystem.DefaultVoice as the default value.
Let's verify this constant exists:
✅ Verification successful
DefaultVoice constant exists and is properly defined
The constant DefaultVoice is defined in SharedHumanoidAppearanceSystem.cs as a string with value "Eugene". The property usage in HumanoidAppearanceComponent.cs is correct.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for DefaultVoice constant definition in SharedHumanoidAppearanceSystem
ast-grep --pattern 'class SharedHumanoidAppearanceSystem {
$$$
public const $_ DefaultVoice = $_;
$$$
}'
Length of output: 112
Script:
#!/bin/bash
# Let's first find the SharedHumanoidAppearanceSystem file
fd "SharedHumanoidAppearanceSystem"
# Then let's do a broader search for DefaultVoice in the codebase
rg "DefaultVoice" -A 2 -B 2
Length of output: 1488
Content.Client/Audio/ContentAudioSystem.cs (1)
33-33: Verify if 1.0f is the optimal value for TTS audio.
While the implementation is correct, the multiplier value might need adjustment based on how TTS audio sounds relative to other audio types in the system. Consider testing with different values to ensure optimal audio balance.
Some considerations for testing:
- Compare TTS volume against interface sounds (InterfaceMultiplier = 2f)
- Compare against announcer volume (AnnouncerMultiplier = 3f) since both are voice-related
- Test with different voice types and accents to ensure consistent volume levels
Content.Server/Entry/EntryPoint.cs (2)
111-111: Consider adding feature flags and testing strategy.
Given that the TTS API isn't available for testing:
- Consider wrapping the TTS functionality behind a feature flag (e.g.,
CCVars.TTSEnabled) to easily disable it if issues arise - Add integration tests with a mock TTS service
- Document the expected TTS API requirements and fallback behavior
Let's check if there are any existing feature flags or tests:
#!/bin/bash
# Description: Check for TTS-related configuration and tests
# Test 1: Look for TTS-related CVars
echo "Checking for TTS configuration..."
rg "TTS.*CVar"
# Test 2: Look for TTS-related tests
echo "Checking for TTS tests..."
fd -e cs -e cs.disabled --exec rg -l "TTS.*Test"111-111: 🛠️ Refactor suggestion
Add error handling and improve initialization.
The TTS initialization could be improved in several ways:
- Add error handling to gracefully handle initialization failures
- Replace the "WD EDIT" comment with meaningful documentation
- Verify proper IoC registration
Let's verify the TTSManager registration and potential dependencies:
Consider applying this improvement:
- IoCManager.Resolve<TTSManager>().Initialize(); // WD EDIT
+ try {
+ var ttsManager = IoCManager.Resolve<TTSManager>();
+ ttsManager.Initialize();
+ Log.Info("TTS system initialized successfully");
+ } catch (Exception ex) {
+ Log.Error($"Failed to initialize TTS system: {ex}");
+ // Consider whether TTS failure should be fatal or not
+ }Content.Client/Entry/EntryPoint.cs (1)
78-78: Verify TTSManager registration in IoC container.
Let's ensure that TTSManager is properly registered in the IoC container.
✅ Verification successful
TTSManager is properly registered in IoC container
The verification shows that TTSManager is correctly registered:
- Registered in client IoC container at
Content.Client/IoC/ClientContentIoC.cs - Has proper dependency injection attributes in implementation at
Content.Client/_White/TTS/TTSManager.cs - Successfully used as a dependency in
EntryPoint.csand other components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if TTSManager is registered in the IoC container
# Expected: Find TTSManager registration in ClientContentIoC.cs
# Test: Search for TTSManager registration
rg -l "TTSManager" "Content.Client/IoC/ClientContentIoC.cs"
# Test: Check the actual registration code
rg "TTSManager" -A 3 -B 3
Length of output: 6053
Content.Client/Options/UI/Tabs/AudioTab.xaml.cs (2)
2-2: LGTM: TTS initialization properly integrated
The TTS volume slider is correctly initialized and integrated into the UI controls list.
Also applies to: 44-44
130-130: Verify TTS volume configuration persistence
The TTS volume control follows the same pattern as other audio controls, but we should verify the configuration variable.
Let's check if the TTS volume configuration is properly defined:
✅ Verification successful
TTS volume configuration is properly implemented and persistent
The verification shows that:
- TTS volume CVar is properly defined in
Content.Shared/_White/CVars.cswithARCHIVEflag for persistence - The volume control is correctly integrated in
TTSSystem.cswith proper subscription/unsubscription to volume changes - The scaling using
TTSMultiplier(1.0f) inContentAudioSystemis correctly applied
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for TTS volume configuration definition
rg -A 2 "TtsVolume.*=.*new"
Length of output: 29
Script:
#!/bin/bash
# Let's try different search patterns to find TTS volume related configurations
# Search for TtsVolume in any context
rg -i "ttsvolume"
# Search for WhiteCVars class definition and its contents
ast-grep --pattern 'class WhiteCVars {
$$$
}'
# Search for TTSMultiplier constant
rg "TTSMultiplier"
Length of output: 1196
Script:
#!/bin/bash
# Let's check for TTSMultiplier definition and usage
rg -A 2 "TTSMultiplier.*=.*"
# Let's also check the audio system implementation
rg -A 5 "class ContentAudioSystem"
Length of output: 2695
Content.Shared/Preferences/HumanoidCharacterProfile.cs (2)
128-128: LGTM! Constructor changes are consistent.
The Voice parameter is properly integrated into both constructors, maintaining the class's immutable-like design pattern.
Also applies to: 148-148, 172-172
503-507: LGTM! Voice validation is properly implemented.
The voice validation logic correctly handles invalid cases and falls back to the default voice based on sex.
Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs (3)
47-56: LGTM! Well-structured default voice mappings.
The implementation is robust with:
- Proper prototype validation
- Immutable dictionary
- Complete coverage of all Sex enum values
403-403: LGTM! Proper integration with profile loading.
The TTS voice setting is well-integrated into the profile loading process, maintaining consistency with other profile attributes and properly using batched synchronization.
4-4: Verify namespace convention for _White.
The underscore prefix in the namespace Content.Shared._White.TTS suggests it might be a private or internal namespace. Please confirm if this follows the project's namespace conventions.
✅ Verification successful
_White namespace appears to be a well-established convention in this codebase
The search results show extensive and consistent usage of the _White namespace across the codebase, with over 100 files following this pattern. It's used systematically in all three main project areas (Shared, Server, and Client), suggesting this is an intentional and established namespace convention for this project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other usages of _White namespace to verify convention
rg -l "namespace.*_White" || rg -l "using.*_White"
Length of output: 6901
Content.Server.Database/Model.cs (1)
337-337: Consider database migration impact and add validation
The addition of a non-null Voice property without a default value could cause issues with existing records during database migration. Additionally, consider adding:
- Documentation to specify valid voice values
- Length constraints using [MaxLength] attribute
- Default value for backward compatibility
Consider this implementation:
- public string Voice { get; set; } = null!; // WD EDIT
+ [MaxLength(32)]
+ [Comment("The TTS voice identifier used for this profile")]
+ public string Voice { get; set; } = "default"; // Default voice for backward compatibilityContent.Server/Database/ServerDbBase.cs (3)
232-232: LGTM! Voice parameter correctly added.
The voice parameter is properly integrated into the HumanoidCharacterProfile constructor.
271-271: LGTM! Voice property correctly mapped.
The voice property is properly mapped from the humanoid profile to the database entity.
Line range hint 201-271: Verify database schema changes for voice property.
The voice property has been added to the profile conversion logic. Please ensure that:
- The database schema has been properly updated to include the voice column.
- Any necessary database migrations have been created.
Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs (1)
835-841: Verify migration strategy for the required Voice column.
The Voice property is correctly configured with appropriate type and naming conventions. However, since this is a required column being added to an existing table, ensure that:
- A migration strategy exists for existing profiles
- A default voice value is provided during migration
Let's verify the migration strategy:
✅ Verification successful
Migration strategy for Voice column is properly handled
The migration file correctly handles the addition of the required Voice column by providing an empty string as the default value for existing records, which is an acceptable approach for string columns. This ensures the migration will run smoothly without breaking existing profiles.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if a migration exists for the Voice column and verify the default value strategy
# Test: Look for migration files containing voice-related changes
rg -l "voice" --type cs
Length of output: 1548
Script:
#!/bin/bash
# Check the migration content to verify the default value strategy
cat Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs
Length of output: 836
Content.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.cs (1)
886-892: Verify migration handling for the required Voice column.
The addition of a required Voice column looks good, but since it's marked as required (not nullable), ensure that the corresponding migration provides a default value for existing Profile records to prevent any runtime issues.
Let's verify the migration handling:
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (1)
152-158: Verify the implementation of InitializeVoice().
The code adds a new region for voice functionality, but the implementation of InitializeVoice() is not visible in the changes. This method is crucial for initializing voice-related UI controls and functionality.
#!/bin/bash
# Description: Search for the InitializeVoice implementation
ast-grep --pattern 'private void InitializeVoice() {
$$$
}'Content.Server/_White/TTS/TTSSystem.Sanitize.cs (1)
33-35: Ensure _isEnabled is properly declared and initialized
The _isEnabled field is checked at line 33 but is not declared or initialized in this file. This may lead to a runtime error if _isEnabled is null or has an unexpected value. Verify that _isEnabled is properly declared and initialized elsewhere in the partial class or in the base class.
Run the following script to locate the declaration and initialization of _isEnabled:
✅ Verification successful
_isEnabled is properly declared and initialized via configuration
The field is correctly declared in TTSSystem.cs and initialized through configuration binding in the same file. Specifically:
- Declared as
private bool _isEnabledinTTSSystem.cs:37 - Initialized via configuration binding at
TTSSystem.cs:44usingWhiteCVars.TtsEnabled
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find where _isEnabled is declared and initialized.
# Search for the declaration of _isEnabled.
rg --type cs --no-heading --line-number 'bool _isEnabled'
# Search for assignments to _isEnabled.
rg --type cs --no-heading --line-number '_isEnabled\s*='
Length of output: 879
Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.Designer.cs (2)
18-19: Migration class TtsVoices is correctly defined
The migration class TtsVoices is properly defined and linked to the PostgresServerDbContext. This ensures that the database context is accurately targeted during the migration process.
857-860: Voice property successfully added to the Profile entity
The Voice property has been correctly added to the Profile entity with the appropriate configurations. This addition allows for storing voice attributes in humanoid profiles as intended.
| tts-voice-name-eugene = Evgeniy | ||
| tts-voice-name-kseniya = Kseniya | ||
| tts-voice-name-xenia = Xenia |
There was a problem hiding this comment.
💡 Codebase verification
Voice naming inconsistencies found in transliteration
The Russian locale reveals inconsistencies in transliteration:
- "Kseniya" (Ксения) and "Xenia" (Сения) are indeed different names in Russian
- However, "Xenia" appears to be incorrectly transliterated as "Сения" instead of "Ксения"
- This suggests a potential error in the Russian translation rather than a need to differentiate in English
Recommended changes:
- Keep the English transliterations as is since they correctly represent different forms
- Fix the Russian translation of "Xenia" to use "Ксения" for consistency
- Adding comments for clarity is still beneficial as suggested
🔗 Analysis chain
Consider improving clarity and user experience.
A few suggestions to enhance this implementation:
- Add comments explaining these are TTS voice options
- Consider differentiating "Kseniya" and "Xenia" more clearly as they might be confusing to users
- Verify transliteration consistency with the Russian locale
+# Text-to-speech voice options
tts-voice-name-eugene = Evgeniy
tts-voice-name-kseniya = Kseniya
-tts-voice-name-xenia = Xenia
+tts-voice-name-xenia = Xenia (Alt)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check transliteration consistency between locales
# Test: Compare with Russian locale entries
echo "Checking Russian locale entries:"
cat Resources/Locale/ru-RU/_white/prototypes/voice/tts_voices.ftl
Length of output: 235
| public sealed class RequestTTSEvent(string text) : EntityEventArgs | ||
| { | ||
| public string Text { get; } = text; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for the text parameter.
The text parameter should be validated to ensure it's not null or empty before setting the property.
-public sealed class RequestTTSEvent(string text) : EntityEventArgs
+public sealed class RequestTTSEvent : EntityEventArgs
{
- public string Text { get; } = text;
+ public string Text { get; }
+
+ public RequestTTSEvent(string text)
+ {
+ Text = string.IsNullOrEmpty(text)
+ ? throw new ArgumentException("Text cannot be null or empty.", nameof(text))
+ : text;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public sealed class RequestTTSEvent(string text) : EntityEventArgs | |
| { | |
| public string Text { get; } = text; | |
| public sealed class RequestTTSEvent : EntityEventArgs | |
| { | |
| public string Text { get; } | |
| public RequestTTSEvent(string text) | |
| { | |
| Text = string.IsNullOrEmpty(text) | |
| ? throw new ArgumentException("Text cannot be null or empty.", nameof(text)) | |
| : text; | |
| } | |
| } |
| { | ||
| public NetEntity Uid { get; } = uid; | ||
|
|
||
| public byte[] Data { get; } = data; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider defensive copying of byte array data.
Since Data is exposed as a public property, consider making a defensive copy to prevent modification of the original data.
- public byte[] Data { get; } = data;
+ public byte[] Data { get; } = (byte[])data.Clone();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public byte[] Data { get; } = data; | |
| public byte[] Data { get; } = (byte[])data.Clone(); |
| public sealed class PlayTTSEvent(NetEntity uid, byte[] data, bool boostVolume) : EntityEventArgs | ||
| { | ||
| public NetEntity Uid { get; } = uid; | ||
|
|
||
| public byte[] Data { get; } = data; | ||
|
|
||
| public bool BoostVolume { get; } = boostVolume; | ||
| } |
There was a problem hiding this comment.
Add input validation for constructor parameters.
The constructor should validate:
- That
datais not null and not empty - That
datasize is within reasonable limits to prevent memory issues
public sealed class PlayTTSEvent(NetEntity uid, byte[] data, bool boostVolume) : EntityEventArgs
{
+ private const int MaxDataSize = 1024 * 1024; // 1MB limit for audio data
+
+ public PlayTTSEvent(NetEntity uid, byte[] data, bool boostVolume) : this()
+ {
+ ArgumentNullException.ThrowIfNull(data);
+ if (data.Length == 0)
+ throw new ArgumentException("Audio data cannot be empty", nameof(data));
+ if (data.Length > MaxDataSize)
+ throw new ArgumentException($"Audio data exceeds maximum size of {MaxDataSize} bytes", nameof(data));
+
+ Uid = uid;
+ Data = data;
+ BoostVolume = boostVolume;
+ }
+
public NetEntity Uid { get; } = uid;
public byte[] Data { get; } = data;
public bool BoostVolume { get; } = boostVolume;
}Committable suggestion skipped: line range outside the PR's diff.
| public void RequestTTS(EntityUid uid, string text, string voiceId) | ||
| { | ||
| var netEntity = _entityManager.GetNetEntity(uid); | ||
| var msg = new MsgRequestTTS { Text = text, Uid = netEntity, VoiceId = voiceId }; | ||
| _netMgr.ClientSendMessage(msg); | ||
| } |
There was a problem hiding this comment.
Add input validation and error handling.
The RequestTTS method lacks several important validations and error handling:
- Parameter validation
- Text length limits
- Network error handling
- Voice ID validation
Consider implementing these safeguards:
- public void RequestTTS(EntityUid uid, string text, string voiceId)
+ public void RequestTTS(EntityUid uid, string text, string voiceId)
+ {
+ if (string.IsNullOrEmpty(text))
+ throw new ArgumentException("Text cannot be null or empty", nameof(text));
+
+ if (string.IsNullOrEmpty(voiceId))
+ throw new ArgumentException("Voice ID cannot be null or empty", nameof(voiceId));
+
+ if (text.Length > 1000) // Add appropriate limit
+ throw new ArgumentException("Text exceeds maximum length", nameof(text));
+
+ try
+ {
+ var netEntity = _entityManager.GetNetEntity(uid);
+ var msg = new MsgRequestTTS { Text = text, Uid = netEntity, VoiceId = voiceId };
+ _netMgr.ClientSendMessage(msg);
+ }
+ catch (Exception ex)
+ {
+ // Consider logging the error
+ throw new InvalidOperationException("Failed to send TTS request", ex);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void RequestTTS(EntityUid uid, string text, string voiceId) | |
| { | |
| var netEntity = _entityManager.GetNetEntity(uid); | |
| var msg = new MsgRequestTTS { Text = text, Uid = netEntity, VoiceId = voiceId }; | |
| _netMgr.ClientSendMessage(msg); | |
| } | |
| public void RequestTTS(EntityUid uid, string text, string voiceId) | |
| { | |
| if (string.IsNullOrEmpty(text)) | |
| throw new ArgumentException("Text cannot be null or empty", nameof(text)); | |
| if (string.IsNullOrEmpty(voiceId)) | |
| throw new ArgumentException("Voice ID cannot be null or empty", nameof(voiceId)); | |
| if (text.Length > 1000) // Add appropriate limit | |
| throw new ArgumentException("Text exceeds maximum length", nameof(text)); | |
| try | |
| { | |
| var netEntity = _entityManager.GetNetEntity(uid); | |
| var msg = new MsgRequestTTS { Text = text, Uid = netEntity, VoiceId = voiceId }; | |
| _netMgr.ClientSendMessage(msg); | |
| } | |
| catch (Exception ex) | |
| { | |
| // Consider logging the error | |
| throw new InvalidOperationException("Failed to send TTS request", ex); | |
| } | |
| } |
| catch (Exception e) | ||
| { | ||
| RequestTimings.WithLabels("Error").Observe((DateTime.UtcNow - reqTime).TotalSeconds); | ||
| _sawmill.Warning($"Failed of request generation new sound for '{text}' speech by '{speaker}' speaker\n{e}"); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid catching general exceptions
Catching Exception can mask unforeseen errors and make debugging difficult.
Instead, catch specific exceptions you expect:
- catch (Exception e)
+ catch (HttpRequestException e)Add any other specific exceptions that should be handled.
Committable suggestion skipped: line range outside the PR's diff.
| var textSanitized = Sanitize(text); | ||
| if (textSanitized == "") | ||
| return null; |
There was a problem hiding this comment.
Ensure Sanitize method is defined to prevent compilation errors
The method Sanitize is called but not defined within the provided code. This will result in a compilation error. Please ensure that the Sanitize method is either implemented in this class or properly imported from another module.
| return; | ||
|
|
There was a problem hiding this comment.
Fix logical error to prevent premature method exit
In the OnAnnounceRequest method, if a player does not have any emergency lights in range, the method uses return;, which exits the method entirely. This prevents other players from receiving the announcement. Instead, use continue; to skip to the next player.
Apply this diff to fix the issue:
if (entities.Count == 0)
- return;
+ continue;Committable suggestion skipped: line range outside the PR's diff.
| b.Property<string>("Voice") | ||
| .IsRequired() | ||
| .HasColumnType("TEXT") | ||
| .HasColumnName("voice"); |
There was a problem hiding this comment.
Adding Non-nullable 'Voice' Property May Cause Migration Issues
You have added a new non-nullable property Voice to the Profile entity without specifying a default value. When applying this migration to an existing database with existing profile records, this can cause a constraint violation since existing records will not have a value for this new non-nullable column.
Consider setting a default value for the Voice column to prevent migration issues:
b.Property<string>("Voice")
.IsRequired()
.HasColumnType("TEXT")
+ .HasDefaultValue("")
.HasColumnName("voice");Alternatively, if it's acceptable for the Voice property to be nullable, you can make it optional:
b.Property<string>("Voice")
- .IsRequired()
.HasColumnType("TEXT")
.HasColumnName("voice");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| b.Property<string>("Voice") | |
| .IsRequired() | |
| .HasColumnType("TEXT") | |
| .HasColumnName("voice"); | |
| b.Property<string>("Voice") | |
| .IsRequired() | |
| .HasColumnType("TEXT") | |
| .HasDefaultValue("") | |
| .HasColumnName("voice"); | |
| ``` | |
| Option 2 (making it nullable): | |
| ```suggestion | |
| b.Property<string>("Voice") | |
| .HasColumnType("TEXT") | |
| .HasColumnName("voice"); |
There was a problem hiding this comment.
Actionable comments posted: 35
🧹 Outside diff range and nitpick comments (40)
Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl (2)
2-3: Consider clarifying the distinction between Kseniya and Xenia voices.Since "Kseniya" and "Xenia" are variants of the same name (Ксения), consider:
- Adding comments to explain the difference between these voices (e.g., different accents, tones, or use cases)
- Using more distinctive names if they serve different purposes
1-3: Consider adding descriptive comments.To improve maintainability, consider adding FTL comments describing:
- The purpose of each voice
- Any specific use cases or limitations
- The source/provider of these TTS voices
Example:
+# Russian male voice for general use tts-voice-name-eugene = Evgeniy +# Russian female voice with standard accent tts-voice-name-kseniya = Kseniya +# Russian female voice with alternative accent tts-voice-name-xenia = XeniaResources/Locale/en-US/_white/escape-menu/ui/options-menu.ftl (1)
5-5: Consider adding a tooltip description for the TTS volume control.To improve user experience, consider adding a tooltip description that explains what TTS is and how this volume setting affects the system.
Add a new entry like:
ui-options-tts-volume = TTS Volume: +ui-options-tts-volume-tooltip = Adjusts the volume of Text-to-Speech voice outputContent.Shared/_White/TTS/Events/RequestTTSEvent.cs (1)
7-10: Consider adding documentation and input validation.The implementation could benefit from:
- XML documentation describing the purpose and usage
- Input validation for the text parameter
- Network safety constraints
Consider applying these improvements:
[Serializable, NetSerializable] // ReSharper disable once InconsistentNaming +/// <summary> +/// Event raised to request text-to-speech synthesis for the specified text. +/// </summary> public sealed class RequestTTSEvent(string text) : EntityEventArgs { + private const int MaxTextLength = 1000; // Adjust based on your requirements + + public RequestTTSEvent(string text) : this() + { + if (string.IsNullOrEmpty(text)) + throw new ArgumentException("Text cannot be null or empty", nameof(text)); + + if (text.Length > MaxTextLength) + throw new ArgumentException($"Text length exceeds maximum allowed length of {MaxTextLength}", nameof(text)); + + Text = text; + } + + /// <summary> + /// The text to be synthesized into speech. + /// </summary> public string Text { get; } = text; }Resources/Prototypes/_White/Voice/tts_voices.yml (1)
13-17: Add newline at end of file.The Xenia voice configuration is well-structured and consistent with other entries. However, the file is missing a newline at the end, which should be added to comply with YAML best practices.
Apply this diff to fix the issue:
id: Xenia name: tts-voice-name-xenia sex: Female speaker: xenia +🧰 Tools
🪛 yamllint
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
Content.Shared/_White/TTS/Events/PlayTTSEvent.cs (1)
5-7: Consider documenting the ReSharper suppression.While the code is correctly marked for serialization, the ReSharper suppression for naming could benefit from a brief explanation of why TTS abbreviation is acceptable in this context.
-// ReSharper disable once InconsistentNaming +// ReSharper disable once InconsistentNaming - TTS is a widely accepted acronym for Text-to-SpeechContent.Client/_White/TTS/TTSManager.cs (2)
1-4: Consider standardizing the namespace structure.The underscore prefix in the namespace
_Whitesuggests this might be a temporary or non-standard location. Consider moving this to a more standard location in the codebase (e.g.,Content.Client.TTS) for better maintainability.
12-15: Consider implementing a lifecycle interface.The
Initializemethod could benefit from:
- Implementing an interface (e.g.,
IPostInject) to ensure proper initialization timing- Adding error handling for network message registration
-public sealed class TTSManager +public sealed class TTSManager : IPostInject { [Dependency] private readonly IClientNetManager _netMgr = default!; [Dependency] private readonly EntityManager _entityManager = default!; - public void Initialize() + void IPostInject.PostInject() { + try { _netMgr.RegisterNetMessage<MsgRequestTTS>(); + } catch (Exception e) { + Logger.ErrorS("tts", e, "Failed to register TTS network message"); + } }Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs (1)
13-18: Consider adding a constraint for voice identifier validation.While the implementation is functional, consider adding a check constraint to ensure voice identifiers follow a valid format. This would prevent invalid voice identifiers from being stored in the database.
migrationBuilder.AddColumn<string>( name: "voice", table: "profile", type: "text", nullable: false, - defaultValue: ""); + defaultValue: "", + // Add a check constraint to ensure voice identifiers are valid + // Adjust the pattern based on your voice ID format + check: "voice ~ '^[a-zA-Z0-9_-]*$'");Content.Shared/_White/TTS/TTSVoicePrototype.cs (2)
10-14: Consider adding XML documentation for the Name property.While the properties are well-implemented, adding XML documentation for the Name property would improve code clarity, especially since it's a core identifier field.
[DataField] +/// <summary> +/// The display name of the TTS voice. +/// </summary> public string Name { get; } = string.Empty;
22-32: Consider implementing validation logic for voice selection.The boolean flags (
RoundStart,SponsorOnly,BorgVoice) suggest different voice categories that would need validation during character creation:
RoundStartaffects availability in character editorSponsorOnlyimplies premium feature checksBorgVoicesuggests special character type restrictionsConsider implementing a voice validation service that:
- Validates sponsor status for
SponsorOnlyvoices- Checks character type compatibility for
BorgVoice- Ensures voice availability based on
RoundStartand game stateContent.Shared/_White/TTS/MsgRequestTTS.cs (1)
1-30: Consider implementing message versioning for future compatibility.As TTS is a new feature that might evolve, consider adding version information to the message format. This would allow for:
- Backward compatibility with older clients
- Graceful degradation of functionality
- Easier future extensions (e.g., adding voice parameters, language settings)
Example implementation:
public sealed class MsgRequestTTS : NetMessage { + private const int CurrentVersion = 1; + private int _version = CurrentVersion; public override MsgGroups MsgGroup => MsgGroups.Command; public override void ReadFromBuffer(NetIncomingMessage buffer, IRobustSerializer serializer) { + _version = buffer.ReadInt32(); Uid = new NetEntity(buffer.ReadInt32()); Text = buffer.ReadString(); VoiceId = buffer.ReadString(); } public override void WriteToBuffer(NetOutgoingMessage buffer, IRobustSerializer serializer) { + buffer.Write(CurrentVersion); buffer.Write((int)Uid); buffer.Write(Text); buffer.Write(VoiceId); } }Content.Shared/_White/TTS/TTSPitchRateSystem.cs (1)
33-36: Add documentation and validation to GetPitchRate.The method would benefit from XML documentation and input validation.
Consider this enhancement:
+ /// <summary> + /// Retrieves the pitch rate configuration for a given species. + /// </summary> + /// <param name="protoId">The species prototype identifier</param> + /// <returns>The pitch rate configuration if defined, null otherwise</returns> public TTSPitchRate? GetPitchRate(ProtoId<SpeciesPrototype> protoId) { + if (protoId == default) + return null; return SpeciesPitches.GetValueOrDefault(protoId); }Content.Client/IoC/ClientContentIoC.cs (1)
58-58: Consider registering TTSManager with an interface.Most other services in this file are registered with their interfaces (e.g.,
IoCManager.Register<IChatManager, ChatManager>()). Consider creating anITTSManagerinterface to maintain consistency with the codebase's architecture and to improve testability through loose coupling.Example implementation:
- IoCManager.Register<TTSManager>(); // WD EDIT + IoCManager.Register<ITTSManager, TTSManager>();Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs (2)
27-41: Add error handling for dependency resolution.While the initialization logic is correct, it lacks error handling for cases where dependency resolution fails.
Consider adding null checks and error handling:
private void InitializeVoice() { + try { _random = IoCManager.Resolve<IRobustRandom>(); _ttsManager = IoCManager.Resolve<TTSManager>(); _ttsSystem = IoCManager.Resolve<IEntityManager>().System<TTSSystem>(); + } catch (Exception e) { + Logger.Error($"Failed to initialize TTS dependencies: {e}"); + return; + } _voiceList = _prototypeManager.EnumeratePrototypes<TTSVoicePrototype>().Where(o => o.RoundStart).ToList();
69-76: Enhance PlayTTS with error handling and cancellation support.The current implementation lacks error handling and doesn't support cancellation for long-running TTS requests.
Consider adding these improvements:
- private void PlayTTS() + private async Task PlayTTS(CancellationToken cancellationToken = default) { if (Profile is null) return; - _ttsSystem.StopCurrentTTS(PreviewDummy); - _ttsManager.RequestTTS(PreviewDummy, _random.Pick(_sampleText), Profile.Voice); + try + { + _ttsSystem.StopCurrentTTS(PreviewDummy); + await _ttsManager.RequestTTSAsync( + PreviewDummy, + _random.Pick(GetSampleText()), + Profile.Voice, + cancellationToken + ); + } + catch (OperationCanceledException) + { + // Handle cancellation gracefully + } + catch (Exception e) + { + Logger.Error($"Failed to play TTS: {e}"); + } }Content.IntegrationTests/Tests/Preferences/ServerDbSqliteTests.cs (1)
48-48: Consider adding voice-specific test cases.Since this is part of a TTS implementation, it would be beneficial to add specific test cases that verify voice-related database operations (e.g., saving/loading different voices, handling invalid voices).
Would you like me to help generate additional test cases for voice-related functionality?
Content.Shared/Humanoid/HumanoidAppearanceComponent.cs (1)
116-119: Consider grouping voice-related properties with other character attributes.While the implementation is correct, consider moving this property closer to related attributes like Gender, Age, or Sex for better code organization.
[DataField, AutoNetworkedField] public Sex Sex = Sex.Male; +[DataField, AutoNetworkedField] +public ProtoId<TTSVoicePrototype> Voice { get; set; } = SharedHumanoidAppearanceSystem.DefaultVoice; + [DataField, AutoNetworkedField] public Color EyeColor = Color.Brown;Content.Client/Options/UI/Tabs/AudioTab.xaml (2)
51-52: Remove custom edit markers.The "WD EDIT" comments might not follow project conventions. Consider removing these markers as version control already tracks changes.
- <!--WD EDIT START--> + - <!--WD EDIT END--> +Also applies to: 65-65
53-54: Consider adding a tooltip for the TTS volume control.Other volume controls like the announcer have tooltips to provide additional context. Consider adding a tooltip to explain the TTS volume control's purpose or impact.
- <Label Text="{Loc 'ui-options-tts-volume'}" HorizontalExpand="True" /> + <Label Text="{Loc 'ui-options-tts-volume'}" + ToolTip="{Loc 'ui-options-tts-volume-tooltip'}" + HorizontalExpand="True" />Content.Client/Entry/EntryPoint.cs (1)
174-174: Consider initializing TTS before chat system.The TTS initialization is placed after chat, UI, and preferences systems. Since TTS might be needed by the chat system, consider moving the initialization before
_chatManager.Initialize()to ensure TTS is ready when chat features are initialized._parallaxManager.LoadDefaultParallax(); _overlayManager.AddOverlay(new SingularityOverlay()); _overlayManager.AddOverlay(new FlashOverlay()); _overlayManager.AddOverlay(new RadiationPulseOverlay()); + _ttsManager.Initialize(); // WD EDIT _chatManager.Initialize(); _clientPreferencesManager.Initialize(); _euiManager.Initialize(); _voteManager.Initialize(); - _ttsManager.Initialize(); // WD EDITContent.Client/Options/UI/Tabs/AudioTab.xaml.cs (2)
Line range hint
192-193: Add TTS volume validation to UpdateChanges.The UpdateChanges method is missing TTS volume validation, which could lead to incorrect Apply/Reset button states when only TTS volume is changed.
Add TTS volume validation:
var isAnnouncerVolumeSame = Math.Abs(AnnouncerVolumeSlider.Value - _cfg.GetCVar(CCVars.AnnouncerVolume) * 100f / ContentAudioSystem.AnnouncerMultiplier) < 0.01f; + var isTtsVolumeSame = + Math.Abs(TtsVolumeSlider.Value - _cfg.GetCVar(WhiteCVars.TtsVolume) * 100f / ContentAudioSystem.TTSMultiplier) < 0.01f; var isAmbientSoundsSame = (int)AmbienceSoundsSlider.Value == _cfg.GetCVar(CCVars.MaxAmbientSources);Also update the isEverythingSame check:
var isEverythingSame = isMasterVolumeSame && isMidiVolumeSame && isAmbientVolumeSame && isAmbientMusicVolumeSame && isAmbientSoundsSame && isLobbySame && isRestartSoundsSame && isEventSame && isAnnouncerDisableMultipleSoundsSame && isAdminSoundsSame && isLobbyVolumeSame - && isInterfaceVolumeSame && isAnnouncerVolumeSame; + && isInterfaceVolumeSame && isAnnouncerVolumeSame && isTtsVolumeSame;And add the TTS volume label update:
AnnouncerVolumeLabel.Text = Loc.GetString("ui-options-volume-percent", ("volume", AnnouncerVolumeSlider.Value / 100)); + TtsVolumeLabel.Text = + Loc.GetString("ui-options-volume-percent", ("volume", TtsVolumeSlider.Value / 100)); AmbienceSoundsLabel.Text = ((int)AmbienceSoundsSlider.Value).ToString();
Line range hint
1-24: Consider implementing a systematic approach for adding new volume controls.To prevent similar integration gaps in the future, consider creating a helper class or extension method that handles all aspects of adding a new volume control:
- Initialization in constructor
- Event handler attachment/detachment
- Apply/Reset functionality
- Change validation
- Label updates
This would ensure consistent implementation and reduce the likelihood of missing steps when adding new audio features.
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml (1)
83-91: Enhance accessibility and user experience of voice controls.While the implementation follows the UI patterns well, consider these improvements:
<BoxContainer HorizontalExpand="True"> <Label Text="{Loc 'humanoid-profile-editor-voice-label'}" /> <Control HorizontalExpand="True"/> - <OptionButton Name="VoiceButton" HorizontalAlignment="Right" /> - <Button Name="VoicePlayButton" Text="▶" MaxWidth="80" /> + <OptionButton + Name="VoiceButton" + HorizontalAlignment="Right" + ToolTip="{Loc 'humanoid-profile-editor-voice-tooltip'}" /> + <Button + Name="VoicePlayButton" + Text="{Loc 'humanoid-profile-editor-voice-play-button'}" + ToolTip="{Loc 'humanoid-profile-editor-voice-play-tooltip'}" + MaxWidth="80" /> </BoxContainer>Consider adding these localization keys to your language files:
humanoid-profile-editor-voice-tooltip: Select a voice for your character humanoid-profile-editor-voice-play-button: ▶ humanoid-profile-editor-voice-play-tooltip: Preview selected voiceContent.Shared/Preferences/HumanoidCharacterProfile.cs (1)
Line range hint
539-558: Update GetHashCode to include Voice propertyThe GetHashCode method should be updated to maintain consistency with the Equals implementation.
Add Voice to the hash calculation:
public override int GetHashCode() { var hashCode = new HashCode(); hashCode.Add(_jobPriorities); hashCode.Add(_antagPreferences); hashCode.Add(_traitPreferences); hashCode.Add(_loadoutPreferences); hashCode.Add(Name); hashCode.Add(FlavorText); hashCode.Add(Species); hashCode.Add(Age); hashCode.Add((int)Sex); hashCode.Add((int)Gender); hashCode.Add(Appearance); hashCode.Add((int)SpawnPriority); hashCode.Add((int)PreferenceUnavailable); hashCode.Add(Customspeciename); + hashCode.Add(Voice); return hashCode.ToHashCode(); }Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs (2)
4-4: LGTM! Consider using enum.GetValues for sex mapping.The constants and imports are well-structured. The
DefaultSexVoicedictionary provides a good mapping of voices to sex types.Consider using enum.GetValues to ensure all Sex enum values are covered:
- public static readonly Dictionary<Sex, string> DefaultSexVoice = new() - { - {Sex.Male, "Eugene"}, - {Sex.Female, "Kseniya"}, - {Sex.Unsexed, "Xenia"}, - }; + public static readonly Dictionary<Sex, string> DefaultSexVoice = + Enum.GetValues<Sex>().ToDictionary( + sex => sex, + sex => sex switch + { + Sex.Male => "Eugene", + Sex.Female => "Kseniya", + Sex.Unsexed => "Xenia", + _ => "Xenia" // fallback + });Also applies to: 47-56
305-330: LGTM! Consider removing unnecessary ReSharper comment.The implementation is solid and follows the system's patterns for component updates. The method properly handles component resolution and synchronization.
The ReSharper disable comment for TTS naming can be removed as it's a commonly accepted acronym in the codebase.
- // ReSharper disable once InconsistentNaming public void SetTTSVoice(Content.Server.Database/Model.cs (1)
337-337: Add XML documentation for the Voice property.For consistency with other properties and to improve code maintainability, consider adding XML documentation explaining the purpose and constraints of this property.
+/// <summary> +/// The TTS voice identifier selected for this profile. +/// </summary> public string Voice { get; set; } = null!; // WD EDITContent.Client/_White/TTS/TTSSystem.cs (4)
20-20: Typo in variable name '_enquedStreams'—should be '_enqueuedStreams'The variable
_enquedStreamsis misspelled. The correct spelling is_enqueuedStreams. This typo is present in the declaration and all usages of the variable. Renaming it will improve code readability and prevent confusion.Apply this diff to correct the variable name:
-private readonly Dictionary<EntityUid, Queue<AudioStreamWithParams>> _enquedStreams = new(); +private readonly Dictionary<EntityUid, Queue<AudioStreamWithParams>> _enqueuedStreams = new();Ensure all references to
_enquedStreamsare updated accordingly:- if (!_enquedStreams.TryGetValue(uid, out var queue)) + if (!_enqueuedStreams.TryGetValue(uid, out var queue)) - _enquedStreams[uid] = queue; + _enqueuedStreams[uid] = queue; - foreach (var (_, queue) in _enquedStreams) + foreach (var (_, queue) in _enqueuedStreams)Also applies to: 104-112
83-89: Potential null reference in 'StopCurrentTTS' methodIn the
StopCurrentTTSmethod, there is a possibility of a null reference exception ifaudio.Owneris null. Although unlikely, it's good practice to safeguard against null references.Consider adding a null check before attempting to stop the audio:
if (audio.Owner != null) { _audioSystem.Stop(audio.Owner); }
114-121: Consider clearing '_currentlyPlaying' dictionary in 'ClearQueues' methodThe
ClearQueuesmethod only clears the_enqueuedStreamsdictionary. To ensure all audio references are properly disposed of during shutdown, consider clearing the_currentlyPlayingdictionary as well.Add the following line to clear
_currentlyPlaying:private void ClearQueues() { foreach (var (_, queue) in _enqueuedStreams) { queue.Clear(); } + _currentlyPlaying.Clear(); }
95-101: Handle the case when 'audio' is null after attempting to playIn the
EnqueueAudiomethod, if_audioSystem.PlayEntityreturnsnull, the method exits silently. Consider logging a warning or handling this case to aid in debugging potential audio playback issues.Add a log statement when
audiois null:if (!audio.HasValue) { + Logger.WarningS("tts", $"Failed to play audio for entity {uid}."); return; }This provides visibility into situations where audio playback fails.
Content.Server/_White/TTS/TTSSystem.Sanitize.cs (7)
139-140: Ensure compliance with code licensing and attribution.The
NumberConverterclass is sourced from an external website (https://codelab.ru/s/csharp/digits2phrase). Please verify that the code complies with your project's licensing requirements and that proper attribution is provided if necessary.
236-248: Simplify theGetDeclensionmethod for clarity.The logic within the
GetDeclensionmethod can be simplified to improve readability and maintainability. Consider refactoring the method to make the declension determination more straightforward.Apply this diff to simplify the method:
private static string GetDeclension(int val, string one, string two, string five) { - var t = val % 100 > 20 - ? val % 10 - : val % 20; - return t switch + int t = val % 100; + if (t >= 11 && t <= 14) + return five; + switch (t % 10) { - 1 => one, - 2 or 3 or 4 => two, - _ => five + case 1: + return one; + case 2: + case 3: + case 4: + return two; + default: + return five; }; }
144-156: Consider consolidating similar arrays to reduce duplication.The arrays
Frac20MaleandFrac20Femalehave many overlapping elements. To reduce code duplication, you might consolidate them into a single structure or use a more dynamic approach to handle gender-specific words.
66-134: Validate the entries in theWordReplacementdictionary.Review the
WordReplacementdictionary to ensure all entries are correct and necessary. Some entries might be duplicates or could be combined. Additionally, verify that the replacements align with the intended pronunciation in the TTS system.
10-24: Optimize the use ofGeneratedRegexattributes.While using
GeneratedRegexcan improve performance by compiling regex patterns at build time, ensure that the regex patterns are necessary and optimized. Complex patterns can impact build times and memory usage. Review each regex for efficiency and consider simplifying if possible.
170-209: Add comments and documentation toNumberToTextmethod.The
NumberToTextmethod contains complex logic for converting numbers to text. Adding detailed comments or XML documentation can help future developers understand the purpose and functionality of this method.
7-7: Follow consistent naming conventions.The comment
// ReSharper disable once InconsistentNamingis used to suppress a naming convention warning. It's generally advisable to adhere to consistent naming conventions throughout the codebase. Consider renaming the class or method to conform to the project's style guidelines instead of disabling the warning.Content.Server/_White/TTS/TTSSystem.cs (1)
53-54: Unregister network message handler on shutdown.The
MsgRequestTTSmessage is registered in theInitialize()method but is not unregistered when the system shuts down. This can lead to issues during hot-reloading or if the system is re-initialized. Ensure that the message handler is unregistered in theShutdown()method.Add the following
Shutdown()method:+public override void Shutdown() +{ + base.Shutdown(); + _netMgr.UnregisterNetMessage<MsgRequestTTS>(); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (43)
Content.Client/Audio/ContentAudioSystem.cs(1 hunks)Content.Client/Entry/EntryPoint.cs(3 hunks)Content.Client/IoC/ClientContentIoC.cs(2 hunks)Content.Client/Lobby/UI/HumanoidProfileEditor.xaml(1 hunks)Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs(4 hunks)Content.Client/Options/UI/Tabs/AudioTab.xaml(1 hunks)Content.Client/Options/UI/Tabs/AudioTab.xaml.cs(4 hunks)Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs(1 hunks)Content.Client/_White/TTS/TTSManager.cs(1 hunks)Content.Client/_White/TTS/TTSSystem.cs(1 hunks)Content.IntegrationTests/Tests/Preferences/ServerDbSqliteTests.cs(1 hunks)Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.Designer.cs(1 hunks)Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs(1 hunks)Content.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.cs(1 hunks)Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.Designer.cs(1 hunks)Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs(1 hunks)Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs(1 hunks)Content.Server.Database/Model.cs(1 hunks)Content.Server/Database/ServerDbBase.cs(3 hunks)Content.Server/Entry/EntryPoint.cs(2 hunks)Content.Server/IoC/ServerContentIoC.cs(2 hunks)Content.Server/_White/TTS/TTSManager.cs(1 hunks)Content.Server/_White/TTS/TTSSystem.Sanitize.cs(1 hunks)Content.Server/_White/TTS/TTSSystem.cs(1 hunks)Content.Shared/Humanoid/HumanoidAppearanceComponent.cs(2 hunks)Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs(4 hunks)Content.Shared/Preferences/HumanoidCharacterProfile.cs(9 hunks)Content.Shared/_White/CVars.cs(1 hunks)Content.Shared/_White/TTS/Events/PlayTTSEvent.cs(1 hunks)Content.Shared/_White/TTS/Events/RequestTTSEvent.cs(1 hunks)Content.Shared/_White/TTS/MsgRequestTTS.cs(1 hunks)Content.Shared/_White/TTS/TTSComponent.cs(1 hunks)Content.Shared/_White/TTS/TTSPitchRateSystem.cs(1 hunks)Content.Shared/_White/TTS/TTSVoicePrototype.cs(1 hunks)Resources/Locale/en-US/_white/escape-menu/ui/options-menu.ftl(1 hunks)Resources/Locale/en-US/_white/escape-menu/ui/options.ftl(0 hunks)Resources/Locale/en-US/_white/preferences/ui/humanoid-profile-editor.ftl(1 hunks)Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl(1 hunks)Resources/Locale/ru-RU/_white/escape-menu/ui/options-menu.ftl(1 hunks)Resources/Locale/ru-RU/_white/escape-menu/ui/options.ftl(0 hunks)Resources/Locale/ru-RU/_white/preferences/ui/humanoid-profile-editor.ftl(1 hunks)Resources/Locale/ru-RU/_white/prototypes/voice/tts_voices.ftl(1 hunks)Resources/Prototypes/_White/Voice/tts_voices.yml(1 hunks)
💤 Files with no reviewable changes (2)
- Resources/Locale/en-US/_white/escape-menu/ui/options.ftl
- Resources/Locale/ru-RU/_white/escape-menu/ui/options.ftl
✅ Files skipped from review due to trivial changes (5)
- Content.Client/Audio/ContentAudioSystem.cs
- Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.Designer.cs
- Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.Designer.cs
- Resources/Locale/en-US/_white/preferences/ui/humanoid-profile-editor.ftl
- Resources/Locale/ru-RU/_white/preferences/ui/humanoid-profile-editor.ftl
🧰 Additional context used
🪛 yamllint
Resources/Prototypes/_White/Voice/tts_voices.yml
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (61)
Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl (1)
1-3: LGTM! The FTL entries follow the correct format.
The voice name entries are properly structured and follow consistent naming conventions.
Resources/Locale/ru-RU/_white/escape-menu/ui/options-menu.ftl (1)
5-5: LGTM! Translations are accurate and well-formatted.
The new Russian translations for TTS volume ("Громкость TTS:") and operative HUD theme ("Оперативник") are grammatically correct and maintain consistency with technical terminology conventions. The translations appropriately correspond to their English counterparts and follow the Fluent (FTL) file format.
Also applies to: 7-7
Resources/Locale/en-US/_white/escape-menu/ui/options-menu.ftl (1)
5-5: LGTM! TTS volume label is clear and follows conventions.
The localization entry for TTS volume control is well-formatted and consistent with UI labeling standards.
Content.Shared/_White/TTS/Events/RequestTTSEvent.cs (1)
5-10: LGTM! The implementation follows good practices.
The class is properly designed as an immutable, serializable event with appropriate inheritance and sealing.
Resources/Prototypes/_White/Voice/tts_voices.yml (3)
1-5: LGTM! Eugene voice configuration is well-structured.
The configuration follows YAML best practices and uses consistent naming conventions.
7-11: LGTM! Kseniya voice configuration is consistent.
The configuration maintains consistency with the established pattern.
1-17: Verify TTS API compatibility with defined voices.
Since the PR description mentions that API testing wasn't possible, please ensure:
- The speaker IDs (
eugene,kseniya,xenia) match the actual TTS API's available voices - The voice configurations are validated against the API specifications
- Implement proper error handling for cases where voices are unavailable
Consider adding a validation step during application startup to verify TTS voice availability.
🧰 Tools
🪛 yamllint
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
Content.Shared/_White/TTS/TTSComponent.cs (3)
5-7: LGTM: Component architecture follows best practices.
The component is correctly registered and configured for state generation, following ECS patterns. The partial class designation allows for separation of concerns across multiple files.
9-10: Consider externalizing the default voice value.
While the implementation is correct, hardcoding the default voice "Eugene" directly in the component makes it less maintainable. Consider moving this to a configuration file or constants class.
Example approach:
- public ProtoId<TTSVoicePrototype> Prototype { get; set; } = "Eugene";
+ public ProtoId<TTSVoicePrototype> Prototype { get; set; } = TTSConstants.DefaultVoice;Let's verify if this voice prototype exists and if there are any other voice options:
1-11: Verify TTS API integration.
Since the PR description mentions the lack of an API for testing, we should ensure proper error handling is implemented for cases where the TTS service is unavailable.
Let's check for error handling in the TTS system:
✅ Verification successful
Based on the search results, I can now provide a final response regarding the TTS error handling:
Error handling is implemented for TTS service unavailability
The codebase has proper error handling for TTS service failures:
- The
TTSManager.cschecks if the API URL is configured before making requests - The TTS system has a fallback mechanism through CVars (
tts.enabledandtts.api_url) - Failed TTS requests are gracefully handled - the system continues to function without audio output
- The client-side implementation (
TTSSystem.cs) has volume checks and proper audio stream management - The server maintains a cache system with size limits to handle service interruptions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for TTS error handling
# Expected: Find try-catch blocks or error handling related to TTS API calls
# Search for TTS system implementation
ast-grep --pattern 'class TTSSystem {
$$$
}'
# Search for error handling patterns
rg -A 5 'try.*catch.*TTS'
Length of output: 76
Script:
#!/bin/bash
# Let's try a broader search for TTS-related files and their implementations
# Find all TTS-related files
fd -e cs TTS
# Search for any error handling patterns in these files
rg -l "catch|Error|Exception" $(fd -e cs TTS)
# Look for system implementations with broader pattern
ast-grep --pattern 'class $_ : ISystem {
$$$
}'
# Search for TTS-related method implementations
rg -A 10 "TTS" --type cs
Length of output: 60862
Content.Shared/_White/TTS/Events/PlayTTSEvent.cs (2)
1-4: LGTM: Proper namespace organization and imports.
The namespace follows the project's convention, and the required serialization import is present.
8-14: Verify data handling and consider adding validation.
The implementation looks correct, but there are some security and performance considerations:
- The byte[] Data property could potentially be very large, impacting network performance
- There's no validation on the data size or content
Consider:
- Adding size validation for the byte array
- Implementing data compression if the audio data is large
- Adding XML documentation to clarify the expected data format and size limits
Run this script to analyze similar event handlers in the codebase:
Content.Client/_White/TTS/TTSManager.cs (2)
6-10: LGTM! Well-structured class with proper dependency injection.
The class follows good practices:
- Sealed class for better performance
- Proper dependency injection
- Required dependencies are clearly marked
1-24: Verify server-side implementation and API integration plan.
The PR description mentions that no API is available for testing. Please clarify:
- How will the server handle these TTS requests?
- What is the plan for API integration?
- Are there any fallback mechanisms when TTS is unavailable?
✅ Verification successful
Server-side implementation is complete with proper API integration
The server-side implementation is well-structured with:
- Proper API handling through configurable URL (
WhiteCVars.TtsApiUrl) - Fallback mechanisms when TTS is unavailable:
- Checks for enabled state (
TtsEnabledCVar) - Validates API URL availability
- Message length limits
- Null checks on sound data generation
- Checks for enabled state (
- Additional features:
- Cache management
- Volume control
- Special handling for whispers and announcements
- Configurable speech rates and pitches
The implementation is ready for integration with any TTS API service by simply configuring the API URL through the CVar system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for server-side TTS implementation
echo "Searching for server-side TTS handler..."
rg -t cs "class.*TTS.*Handler"
echo "Searching for TTS API configuration..."
rg -t cs "TTSConfig|TTSSettings|TTSService"
Length of output: 256
Script:
#!/bin/bash
# Let's try a broader search for TTS-related files and implementations
echo "Searching for all TTS-related files..."
fd -e cs -e json "TTS"
echo "Searching for MsgRequestTTS handling..."
rg -t cs "MsgRequestTTS" -A 5
echo "Searching for any voice or TTS related network messages..."
rg -t cs "class.*Msg.*(?:TTS|Voice)"
Length of output: 3166
Script:
#!/bin/bash
# Let's check the server-side implementation and configuration
echo "Checking server TTS system implementation..."
cat Content.Server/_White/TTS/TTSSystem.cs
echo "Checking for configuration variables..."
rg -t cs "WhiteCVars.*Tts" -A 2
Length of output: 11055
Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs (5)
1-6: LGTM!
The file structure follows standard EF Core migration practices with appropriate imports and namespace organization.
7-9: LGTM!
The migration class follows proper naming conventions with timestamp prefix and descriptive suffix.
22-27: LGTM!
The Down migration correctly reverts the changes by dropping the voice column.
1-29: Verify integration with TTS feature implementation.
Since this migration is part of a larger TTS feature implementation, let's verify the consistency with related components:
✅ Verification successful
Let me gather more specific information about the profile voice implementation to ensure complete verification.
Let me check one final piece to verify the profile voice property implementation:
Migration is properly integrated with TTS feature implementation
The verification confirms that the migration adding the voice column to the profile table is well-integrated with the TTS feature:
- The
HumanoidCharacterProfileclass properly implements theVoiceproperty with a default value TTSVoicePrototypedefines the voice configuration structure- UI implementation in
HumanoidProfileEditor.TTS.cshandles voice selection and preview - Proper validation through
CanHaveVoicemethod ensures voice-sex compatibility - Database integration via
ServerDbBase.cscorrectly handles the voice profile property
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for related TTS implementations and profile modifications
echo "Checking for TTS-related components:"
rg -l "TTS|TextToSpeech" --type-add 'code:*.{cs,xaml}'
echo "\nChecking for profile voice property usage:"
rg "profile.*voice|voice.*profile" --type cs
Length of output: 2436
Script:
#!/bin/bash
# Check the HumanoidCharacterProfile implementation for voice property
echo "Checking HumanoidCharacterProfile voice implementation:"
rg -A 5 "Voice\s*{" Content.Shared/Preferences/HumanoidCharacterProfile.cs
echo "\nChecking TTSVoicePrototype definition:"
cat Content.Shared/_White/TTS/TTSVoicePrototype.cs
echo "\nChecking voice-related UI implementation:"
cat Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs
Length of output: 3786
Script:
#!/bin/bash
# Check HumanoidCharacterProfile for voice property
echo "Checking profile voice property:"
rg -A 3 "public\s+string\s+Voice" Content.Shared/Preferences/HumanoidCharacterProfile.cs
echo "\nChecking voice validation method:"
rg -A 5 "CanHaveVoice" Content.Shared/Preferences/HumanoidCharacterProfile.cs
Length of output: 973
11-19: Consider refining the voice column constraints.
While the migration will work, there are a few considerations:
- Using an empty string as default might not be the most appropriate choice for voice identifiers. Consider using a meaningful default voice value or allowing NULL with a proper application-level default.
- The TEXT column type without length constraints could potentially allow unnecessarily large values. Consider adding a maximum length constraint based on your expected voice identifier format.
Let's check the voice identifier format used in the codebase:
Consider applying these improvements:
migrationBuilder.AddColumn<string>(
name: "voice",
table: "profile",
type: "TEXT",
- nullable: false,
- defaultValue: "");
+ type: "TEXT",
+ maxLength: 100, // Adjust based on your voice ID format
+ nullable: true,
+ defaultValue: null);Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs (1)
1-29: LGTM! Well-structured migration with proper Up/Down methods.
The migration follows best practices:
- Proper naming convention for the timestamp prefix
- Clean reversible migration with corresponding Down method
- Consistent with PostgreSQL naming conventions (lowercase)
- Appropriate use of non-nullable column with default value
Let's verify if there are any existing profiles that might be affected by this migration:
✅ Verification successful
Based on the search results, I can see that the profile table has existed since the initial migration (20200929113117_Init.cs) and has undergone several migrations adding various columns, all following the same pattern of using non-nullable columns with default values. The new TTS voices migration follows this established pattern.
Looking at the recent migrations in chronological order:
- Height (Jan 2024)
- SpawnPriorityPreference (Feb 2024)
- CustomSpecieName (Oct 2024)
- TtsVoices (Nov 2024)
All these migrations handle existing profiles by providing default values, which is the correct approach.
Migration looks good and follows established patterns for handling existing profiles
The migration is safe because:
- It follows the same pattern as previous migrations for the profile table
- Uses a non-nullable column with empty string default value, consistent with other text columns
- Has proper Up/Down methods for reversibility
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing profiles in the database schema
# This helps assess the impact of adding a new non-nullable column
# Search for profile table references in migrations
rg -A 5 'table: "profile"' --type cs
Length of output: 27901
Content.Shared/_White/TTS/TTSVoicePrototype.cs (3)
1-9: LGTM! Well-structured class declaration.
The class is properly structured with appropriate namespace organization and follows C# naming conventions. The prototype attribute and interface implementation are correctly set up.
19-20: Ensure Speaker value is properly sanitized.
The Speaker property is used to identify the TTS voice and is marked as required. Since this value might be passed to external TTS APIs, ensure that:
- The value is properly sanitized before use
- There are appropriate length limits
- The value matches the expected format of your TTS provider
✅ Verification successful
Let me analyze the findings from the shell scripts:
- The
Speakerproperty inTTSVoicePrototype.csis used as:
- A required data field for TTS voice configuration
- Passed to TTS generation APIs for voice selection
- No validation or sanitization is performed on the value
-
The property is only used within the TTS system for voice identification and API calls, with no direct user input or external exposure.
-
The codebase shows extensive sanitization patterns for user-facing inputs, but this property is a configuration value loaded from prototypes rather than user input.
No security concerns found with the Speaker property implementation.
The property is appropriately used as a configuration identifier for TTS voices and doesn't require additional sanitization since it's:
- Loaded from controlled prototype definitions
- Used internally for voice identification
- Not exposed to or modified by user input
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for Speaker value validation
# Expected: Find validation or sanitization of Speaker values
# Look for Speaker validation
rg -B 2 -A 2 "Speaker.*validate|validate.*Speaker"
# Check for any TTS-related validation
ast-grep --pattern 'class $_ {
$$$
validate($$$) {
$$$
Speaker
$$$
}
$$$
}'
Length of output: 150
Script:
#!/bin/bash
# Let's broaden our search to understand the usage and validation context of Speaker
# Find all references to Speaker property
rg -B 3 -A 3 "Speaker\s*{"
# Look for any TTS-related validation or sanitization
rg -B 3 -A 3 "TTS.*validate|sanitize|clean"
# Find where Speaker is used
rg -B 2 -A 2 "\.Speaker"
# Check for any string validation utilities
fd -e cs -x rg -l "string.*validate|sanitize|clean" {}
Length of output: 449619
16-20: Verify Sex enum values match TTS requirements.
The Sex property is correctly marked as required, but we should verify that the Sex enum values align with the TTS system's gender options.
✅ Verification successful
Let me gather more information about how the Sex enum is used with TTS voices to verify the alignment.
Let me try one final verification to check the implementation details of TTS voice handling:
Sex enum values correctly align with TTS system requirements
The verification shows that:
- Sex enum has appropriate values (Male, Female, Unsexed)
- These values are properly mapped to default voices in
DefaultSexVoicedictionary - The
CanHaveVoicevalidation ensures compatibility by checking:- Voice's sex matches character's sex
- Voice is unsexed (works for any sex)
- Character is unsexed and voice is allowed at round start
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Sex enum definition and usage
# Expected: Enum values that match TTS system requirements
# Find the Sex enum definition
rg -A 10 "enum Sex"
# Find other usages of Sex enum with TTS
rg "Sex.*TTS|TTS.*Sex"
Length of output: 738
Script:
#!/bin/bash
# Check implementation of CanHaveVoice method and TTS voice usage
ast-grep --pattern 'CanHaveVoice($_, $_) {
$$$
}'
# Find any voice/TTS configuration files that may define gender requirements
rg -l "\.json" | xargs rg "voice|tts|sex|gender" -i
# Check for any TTS system configurations
fd -e json -e yaml | xargs rg "tts|voice" -i
Length of output: 698
Script:
#!/bin/bash
# Check TTSVoicePrototype usage and implementation
rg -A 5 "class TTSVoicePrototype"
# Find where Sex is used in relation to voice selection/validation
rg -B 2 -A 5 "CanHaveVoice.*Sex|Sex.*voice|voice.*Sex" -i
# Look for any voice restriction or validation logic
ast-grep --pattern 'if ($voice.$$ == $sex) {
$$$
}'
Length of output: 4836
Content.Shared/_White/TTS/MsgRequestTTS.cs (3)
1-6: LGTM: Imports and namespace are well-structured.
The imports are minimal and necessary for the functionality. The namespace follows the project's convention.
8-11: Verify the use of MsgGroups.Command for TTS requests.
Using MsgGroups.Command for TTS requests might have security implications. We should verify that:
- This matches other similar message types in the codebase
- Proper authentication/authorization is implemented in the handling system
✅ Verification successful
The previous search showed files using Command but didn't show the actual implementations. Let's get more context around the Command usage and look for TTS handlers to verify authorization.
The previous search showed that MsgGroups.Command is commonly used for various UI and system control messages. Let's look specifically for TTS handling code to verify the security measures.
Using MsgGroups.Command for TTS requests is appropriate
The verification shows that:
- MsgGroups.Command is consistently used across the codebase for UI and system control messages, including chat messages, preferences, and other user interactions
- TTS is implemented as a proper system (TTSSystem) with client and server components, following the same pattern as other game systems
- The message type matches similar user-initiated actions like chat messages (MsgChatMessage) which also use Command group
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other NetMessage implementations using Command group
rg -l "MsgGroups\.Command" | while read -r file; do
echo "=== $file ==="
rg "class.*NetMessage.*MsgGroups\.Command" "$file" -A 2
done
Length of output: 4066
Script:
#!/bin/bash
# Let's search for the actual MsgGroups.Command usage in these files
rg "MsgGroups\.Command" -B 2 -A 2
# Also search for any TTS-related message handlers to check authorization
rg -i "tts.*handler|handle.*tts" -B 2 -A 2
Length of output: 8219
Script:
#!/bin/bash
# Let's look for TTS-related system or handler implementations
rg -i "class.*tts.*system|class.*tts.*handler" -B 2 -A 5
# Also search for any access checks or authentication in TTS-related files
fd -e cs -g "*TTS*" -x rg -i "require.*|auth.*|access" {} -B 2 -A 2
Length of output: 2999
13-15:
Consider adding input validation for the Text property.
The Text property could be vulnerable to abuse through:
- Extremely long strings causing performance issues
- Special characters or control sequences affecting TTS behavior
- Potential memory issues with large messages
Consider implementing:
- Maximum length restrictions
- Character validation
- Rate limiting (in the handling system)
Content.Shared/_White/TTS/TTSPitchRateSystem.cs (2)
1-10: LGTM! Class structure and dependencies are well organized.
The system is properly structured with appropriate inheritance and necessary imports.
1-40: Verify integration with other TTS components.
The implementation looks solid but let's verify the integration points with other TTS components.
✅ Verification successful
Let me gather more information about the integration by checking the actual usage of TTSPitchRateSystem in the TTS processing flow.
Integration with TTS components is properly implemented
The verification shows that:
- TTSPitchRateSystem is correctly integrated into the server-side TTSSystem through dependency injection
- The pitch/rate values are properly retrieved and used during TTS processing in TTSSystem.cs
- The system works alongside other TTS components (TTSVoicePrototype, TTSManager) in a cohesive architecture
- The implementation follows the expected integration pattern where species-specific voice characteristics are applied during TTS processing
The code is well-integrated and ready for use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for related TTS components and their integration points
echo "Checking for TTS manager implementation..."
rg -l "TTSManager|TTSSystem" --type cs
echo "Checking for voice prototype usage..."
rg "TTSVoicePrototype" --type cs
echo "Checking for pitch/rate usage in audio processing..."
rg "Pitch.*Rate.*TTS|TTS.*Pitch.*Rate" --type cs
Length of output: 3828
Script:
#!/bin/bash
# Check how TTSPitchRateSystem is used in the TTS processing
echo "Checking TTSSystem implementation for pitch/rate usage..."
rg -A 10 "_ttsPitchRateSystem" --type cs
echo "Checking message handling related to TTS..."
ast-grep --pattern 'class TTSSystem {
$$$
public void $_(MsgRequestTTS $_) {
$$$
}
$$$
}'
Length of output: 2078
Content.Shared/_White/CVars.cs (2)
31-33: LGTM! Region declaration follows established pattern.
39-42: Consider adding URL validation.
While the empty string default is safe, it would be beneficial to add runtime validation to ensure the provided URL is well-formed when configured.
Content.Client/IoC/ClientContentIoC.cs (1)
1-1: LGTM!
The using directive is correctly placed and follows the existing pattern.
Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs (1)
1-17: LGTM! Field declarations and imports are well-structured.
The dependency injection setup and field declarations follow good practices.
Content.Server/IoC/ServerContentIoC.cs (1)
1-1: Verify the namespace convention for TTS implementation.
The namespace Content.Server._White.TTS uses an underscore prefix, which typically indicates internal or experimental features. Consider whether this is intentional or if the TTS implementation should be moved to a more standard namespace structure.
Let's check if this is a common pattern in the codebase:
✅ Verification successful
The _White namespace convention appears to be an established pattern in this codebase
The _White namespace is extensively used across both server and client components, containing various game systems and features. This appears to be an intentional architectural decision for this codebase, with numerous implementations following this pattern including animations, aspects, overlays, and other game systems. The TTS implementation follows this established convention.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other usages of _White namespace
# Expected: Find similar patterns to validate the namespace convention
# Search for files under _White namespace
fd -t f . "Content.Server/_White"
# Search for imports of _White namespace
rg "using.*_White"
Length of output: 11693
Content.IntegrationTests/Tests/Preferences/ServerDbSqliteTests.cs (1)
48-48: Consider using a constant for the default voice value.
While the change correctly adds voice support to the test profile, using a hardcoded string "Eugene" might make tests brittle. Consider using a constant from the voice system, such as SharedHumanoidAppearanceSystem.DefaultVoice.
Let's verify if "Eugene" is a valid voice option:
Content.Shared/Humanoid/HumanoidAppearanceComponent.cs (2)
1-1: LGTM: Import statement is correctly placed and necessary.
The import of the TTS namespace is required for the new Voice property type.
118-118: Verify the default voice configuration.
Please ensure that SharedHumanoidAppearanceSystem.DefaultVoice is properly configured and the prototype exists.
✅ Verification successful
Default voice configuration is properly set and exists
The verification confirms that:
- The default voice "Eugene" is properly defined in
tts_voices.ymlwith the correct prototype type - The
TTSVoicePrototypeclass is well-defined and matches the YAML structure - The default voices for all sexes (Eugene, Kseniya, Xenia) exist in the prototypes
- The prototype system is correctly configured with required fields matching the implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the default voice prototype is defined
# Test 1: Find the DefaultVoice constant definition
echo "Searching for DefaultVoice constant definition:"
rg -A 2 "DefaultVoice\s*=\s*" --type cs
# Test 2: Find the voice prototype definition
echo "Searching for voice prototype definition:"
fd -e yaml -e yml | xargs rg "type: TTSVoicePrototype"
Length of output: 600
Script:
#!/bin/bash
# Let's check the voice prototypes in the Resources directory
echo "Searching for voice-related YAML files:"
fd -e yaml -e yml | xargs rg -l "voice|Voice|TTS"
echo "Checking content of SharedHumanoidAppearanceSystem.cs for more context:"
rg -A 10 "DefaultVoice\s*=\s*" Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs
echo "Looking for TTSVoicePrototype class definition:"
rg -l "TTSVoicePrototype" --type cs
echo "Checking for voice-related prototypes in specific Resources paths:"
fd Prototypes -t d | xargs rg -l "voice|Voice|TTS"
Length of output: 7651
Script:
#!/bin/bash
# Let's check the TTS voice prototypes definition in the YAML file
echo "Checking TTS voices configuration:"
cat Resources/Prototypes/_White/Voice/tts_voices.yml
echo "Checking TTSVoicePrototype class definition:"
cat Content.Shared/_White/TTS/TTSVoicePrototype.cs
Length of output: 1359
Content.Client/Options/UI/Tabs/AudioTab.xaml (2)
51-65: Well-structured implementation following UI patterns.
The TTS volume control implementation maintains consistency with existing volume controls:
- Follows the established BoxContainer structure
- Uses consistent margins and spacing
- Adheres to naming conventions
- Properly integrates with surrounding controls
53-53: Verify the localization key exists.
The TTS volume label uses the localization key 'ui-options-tts-volume'. Ensure this key is defined in the localization files.
✅ Verification successful
Localization key 'ui-options-tts-volume' is properly defined
The key is correctly defined in the English localization file at Resources/Locale/en-US/_white/escape-menu/ui/options-menu.ftl with the value "TTS Volume:" and is also present in the Russian localization. The usage in AudioTab.xaml is valid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the TTS volume localization key in resource files
rg "ui-options-tts-volume" --type yaml --type json
Length of output: 50
Script:
#!/bin/bash
# Let's try searching in all files since the localization files might have different extensions
rg "ui-options-tts-volume" -l
Length of output: 198
Script:
#!/bin/bash
# Let's also check for common localization file patterns
fd -e resx -e yaml -e json -e po -e loc -e xaml Resources
Length of output: 59
Script:
#!/bin/bash
# Let's check if there's a specific localization directory
fd -t d -d 2 local
Length of output: 68
Script:
#!/bin/bash
# Let's check the content of these localization files to verify the key definition
rg "ui-options-tts-volume" -A 1 Resources/Locale/en-US/_white/escape-menu/ui/options-menu.ftl
Length of output: 130
Content.Server/Entry/EntryPoint.cs (2)
1-1: Verify namespace convention.
The namespace Content.Server._White.TTS uses an underscore prefix, which typically indicates internal implementation. Ensure this aligns with your project's namespace conventions.
Consider moving the TTS implementation to a more standard namespace structure if this is intended to be a long-term, maintainable feature rather than an internal implementation.
111-111: 🛠️ Refactor suggestion
Review TTSManager initialization and lifecycle management.
The TTSManager initialization has been added, but there are several important considerations:
- Error handling is missing for the initialization
- The initialization order might affect other services
- No visible cleanup/disposal handling in the
Disposemethod
Let's verify the TTSManager implementation:
Consider the following improvements:
IoCManager.Resolve<TTSManager>().Initialize(); // WD EDITto:
try
{
var ttsManager = IoCManager.Resolve<TTSManager>();
ttsManager.Initialize();
}
catch (Exception ex)
{
Logger.ErrorS("tts", ex, "Failed to initialize TTS manager");
}Also, if TTSManager implements IDisposable, add cleanup in the Dispose method:
protected override void Dispose(bool disposing)
{
_playTimeTracking?.Shutdown();
_dbManager?.Shutdown();
+ IoCManager.Resolve<TTSManager>()?.Dispose();
IoCManager.Resolve<ServerApi>().Shutdown();
}Content.Client/Entry/EntryPoint.cs (2)
78-78: Verify TTSManager registration in IoC container.
The dependency injection is correctly added. However, we should verify that TTSManager is properly registered in the IoC container.
✅ Verification successful
TTSManager dependency injection is properly registered
The TTSManager is correctly registered in the IoC container within Content.Client/IoC/ClientContentIoC.cs, ensuring the dependency injection will work as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if TTSManager is registered in ClientContentIoC
rg -A 5 "TTSManager" "Content.Client/IoC"
Length of output: 271
1-1: Verify TTS namespace dependencies.
The TTS namespace import suggests this is part of a White Dream Project customization. Let's verify the TTS implementation exists and follows the project's architecture.
✅ Verification successful
Let me gather more information about the TTS implementation to verify its architecture and dependencies.
Based on the gathered information, I can now provide the final response:
TTS namespace implementation verified and properly structured
The TTS implementation follows a clean client-server architecture with proper separation of concerns:
- Shared components and messages in
Content.Shared/_White/TTS/ - Server-side implementation in
Content.Server/_White/TTS/ - Client-side implementation in
Content.Client/_White/TTS/
The import in EntryPoint.cs is valid and necessary as it references the client-side TTS implementation that integrates with the game's audio and networking systems.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check TTS implementation structure
echo "Checking TTS implementation files..."
fd -e cs . -E "**/bin/**" -E "**/obj/**" | rg "_White/TTS"
echo "Checking TTS interface definition..."
ast-grep --pattern 'interface I?TTSManager {
$$$
}'
Length of output: 822
Script:
#!/bin/bash
# Check TTSManager implementations and their dependencies
echo "Checking TTSManager implementations..."
rg -A 10 "class TTSManager" Content.Client/_White/TTS/TTSManager.cs Content.Server/_White/TTS/TTSManager.cs
echo "Checking TTSSystem implementations..."
rg -A 10 "class TTSSystem" Content.Client/_White/TTS/TTSSystem.cs Content.Server/_White/TTS/TTSSystem.cs
echo "Checking shared interfaces and components..."
rg -A 5 "interface" Content.Shared/_White/TTS/*.cs
rg -A 5 "class TTSComponent" Content.Shared/_White/TTS/*.cs
Length of output: 4959
Content.Client/Options/UI/Tabs/AudioTab.xaml.cs (3)
157-157: LGTM! Reset implementation is correct.
The TTS volume reset implementation follows the established pattern and correctly applies the multiplier.
130-130: Verify TTSMultiplier constant value.
The TTS volume setting implementation follows the established pattern and correctly applies the multiplier. However, we should verify the TTSMultiplier constant value to ensure proper volume scaling.
✅ Verification successful
TTSMultiplier constant value is correctly defined as 1.0
The TTSMultiplier constant in Content.Client/Audio/ContentAudioSystem.cs is properly defined as 1f, which means it maintains the original volume scale without any additional amplification or reduction. This is a reasonable default value that allows the slider to directly control the TTS volume percentage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify TTSMultiplier constant value
rg -A 1 "TTSMultiplier\s*=\s*" --type cs
Length of output: 183
2-2:
Initialize TtsVolumeSlider in constructor.
While the import for TTS configuration is added, the TtsVolumeSlider initialization is missing from the constructor. This could lead to inconsistent initial state.
Add the following initialization in the constructor after the existing checkbox initializations:
AdminSoundsCheckBox.Pressed = _cfg.GetCVar(CCVars.AdminSoundsEnabled);
+TtsVolumeSlider.Value = _cfg.GetCVar(WhiteCVars.TtsVolume) * 100f / ContentAudioSystem.TTSMultiplier;Likely invalid or redundant comment.
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml (1)
85-89: Verify TTS system integration.
Please ensure:
- The voice options in
VoiceButtonare populated from the available TTS voices - The play button correctly triggers the TTS preview
- Voice selection is properly saved in the character profile
✅ Verification successful
TTS integration is properly implemented
The code shows complete integration with the TTS system:
- Voice options are populated via
UpdateTTSVoicesControls()with proper sex-based filtering - Voice selection is handled and saved through
VoiceButton.OnItemSelected - Play button triggers TTS preview via
PlayTTS()which uses both_ttsSystemand_ttsManager - Voice property is correctly managed in the profile with validation through
CanHaveVoice
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for voice-related event handlers and TTS integration
echo "Checking for voice button event handlers..."
rg -l "VoiceButton" --type cs
echo "Checking for TTS preview implementation..."
rg -l "VoicePlayButton" --type cs
echo "Checking for voice property in profile..."
rg -l "Voice.*Profile" --type cs
Length of output: 548
Script:
#!/bin/bash
# Let's examine the TTS integration implementation
echo "=== Examining TTS integration implementation ==="
rg -A 10 "VoiceButton|VoicePlayButton" Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs
echo -e "\n=== Checking for TTS manager/system ==="
rg -l "TTSManager|TTSSystem" --type cs
echo -e "\n=== Looking for voice profile properties ==="
rg "Voice.*Profile" Content.Client/_White/TTS/HumanoidProfileEditor.TTS.cs
Length of output: 2242
Content.Shared/Preferences/HumanoidCharacterProfile.cs (2)
81-84: LGTM: Voice property addition
The Voice property is correctly added with a default value from the system.
128-128: LGTM: Constructor and WithVoice implementation
The changes correctly maintain the immutable semantics of the class and follow the existing patterns for property modification.
Also applies to: 148-148, 172-172, 272-272
Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs (2)
403-403: LGTM! Voice initialization properly integrated.
The SetTTSVoice call is well-placed in the profile loading sequence and correctly uses batched updates.
305-330: Verify voice prototype availability.
Let's ensure that the default voice prototypes are properly registered in the prototype system.
✅ Verification successful
Let's cast a wider net to understand how TTS voices are defined and configured in the codebase. This will help us verify if the voice prototype system is properly implemented and where the voice definitions should be located.
The previous search revealed the TTS-related code structure. Let's dive deeper into the implementation details to understand how voices are defined and registered in the system.
From the previous results, we can see that the TTS system is properly implemented with TTSVoicePrototype class and "Eugene" as the default voice. Let's verify if and where the actual voice prototypes are defined in the YAML configuration files.
Voice prototypes are properly configured and available
The verification shows that:
- The TTS voice prototypes are correctly defined in
Resources/Prototypes/_White/Voice/tts_voices.yml - The default voice "Eugene" exists along with "Kseniya" and "Xenia"
- The prototypes follow the required structure defined in
TTSVoicePrototypeclass with all mandatory fields (id, name, sex, speaker)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the default voice prototypes exist
# Expected: Find prototype definitions for Eugene, Kseniya, and Xenia voices
# Search for voice prototype definitions
rg -l "type: TTSVoice" | while read -r file; do
echo "=== $file ==="
rg "id: (Eugene|Kseniya|Xenia)" "$file" -A 2
done
Length of output: 39
Script:
#!/bin/bash
# Let's try a broader search for TTS-related prototypes and configurations
# Search for TTS-related files and prototypes
echo "=== Searching for TTS prototype definitions ==="
rg -l "TTSVoice" --type-add 'yaml:*.yml' --type yaml
echo -e "\n=== Searching for TTSVoicePrototype references ==="
rg "TTSVoicePrototype"
echo -e "\n=== Looking for voice-related YAML configurations ==="
fd -e yml -e yaml | xargs rg "voice:" -l
Length of output: 2242
Script:
#!/bin/bash
# Let's examine the TTS implementation details
# Check the default voice constant and implementation
echo "=== Checking DefaultVoice implementation ==="
rg "DefaultVoice.*=" -A 2
# Examine the TTSVoicePrototype class definition
echo -e "\n=== TTSVoicePrototype class definition ==="
cat Content.Shared/_White/TTS/TTSVoicePrototype.cs
# Look for voice prototype registration or initialization
echo -e "\n=== Searching for prototype registration ==="
rg "IPrototypeManager.*TTS" -A 2
Length of output: 1523
Script:
#!/bin/bash
# Let's look for the TTS voice prototype definitions in Resources
# Search for ttsVoice prototype definitions
echo "=== Searching for ttsVoice prototype files ==="
fd -e yml -e yaml | xargs rg "type: ttsVoice" -l
# If found, examine their content
echo -e "\n=== Content of TTS voice prototype files ==="
fd -e yml -e yaml | xargs rg -l "type: ttsVoice" | while read -r file; do
echo "=== $file ==="
cat "$file"
done
# Look for any voice-related prototype loading
echo -e "\n=== Voice prototype loading references ==="
rg "LoadPrototypes.*ttsVoice" -A 2
Length of output: 979
Content.Server.Database/Model.cs (1)
337-337: Verify database migration for the Voice property.
The addition of a non-nullable property requires a database migration. Let's verify its existence and implementation.
Content.Server/Database/ServerDbBase.cs (3)
201-206: LGTM! Voice initialization looks good.
The voice initialization logic is well-implemented with proper fallback to default sex-based voice when empty.
232-232: LGTM! Voice parameter placement is appropriate.
The voice parameter is correctly positioned in the constructor, grouped logically with related attributes.
271-271: LGTM! Verify database migration for voice property.
The voice property assignment is correctly implemented. Since this adds a new field to the profile, ensure that the corresponding database migration has been created and tested.
✅ Verification successful
Based on the search results, I can see that there are database migrations for the voice property (20241105191142_TtsVoices.cs and 20241105191222_TtsVoices.cs) which were added to both Postgres and SQLite databases. These migrations add a "voice" column to handle the voice property in the database.
Database migration exists and has been properly implemented.
The voice property is being correctly:
- Stored in the database model (
Content.Server.Database/Model.cs) - Handled in the database migrations for both Postgres and SQLite
- Used in the profile conversion code (
Content.Server/Database/ServerDbBase.cs)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of database migration for voice property.
# Test: Search for migration files containing voice-related changes.
rg -l "voice" --glob "Migrations/*.cs"
Length of output: 187
Script:
#!/bin/bash
# Let's first check if the Migrations directory exists and its location
fd -t d "Migrations"
# Then search for migration files in the entire codebase
fd -e cs ".*Migration.*"
# Also check for any SQL-related files that might contain voice-related changes
fd -e sql .
# Finally, let's check the database context/model to see if voice property is defined
rg -i "voice" --type cs
Length of output: 35085
Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs (2)
835-841: LGTM! The Voice property is properly configured.
The Voice column is correctly added to the Profile table with appropriate configuration:
- Required field (non-nullable)
- TEXT column type
- Proper column name mapping
835-841: Consider database migration impact.
Since this adds a required column to an existing table:
- Ensure a default value is provided in the migration for existing records
- Consider making this nullable if voice selection is optional
- Plan for database rollback scenarios
Let's verify the migration strategy:
✅ Verification successful
Migration implementation addresses the concerns adequately
The migration files show proper handling of the database changes:
- A default empty string value is provided for existing records in both Postgres and SQLite migrations
- Rollback (Down) method is implemented to drop the column if needed
- Both database types (Postgres and SQLite) are properly covered with appropriate migrations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for related migration files and default value handling
# Test: Look for migration files that add this column
rg -l "AddColumn.*voice" --type cs
# Test: Check for default value assignment in migrations
rg "DefaultValue.*voice" --type cs
Length of output: 73
Script:
#!/bin/bash
# Let's try a broader search to find migration files and their content
# Find all migration files
fd ".*Migration.*\.cs$"
# Look for any voice-related changes in migration files
rg -i "voice" --type cs -C 5 Content.Server.Database/Migrations/
Length of output: 14639
Content.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.cs (1)
886-892:
Consider data migration strategy and column constraints.
The addition of a required Voice property raises several concerns:
- Making this column required (
IsRequired()) could break the migration if there are existing profile records, as they won't have a voice value. - The unconstrained
texttype without a maximum length could lead to storage inefficiency.
Consider these changes:
- Either make the column nullable initially or provide a default value for existing records:
- b.Property<string>("Voice")
- .IsRequired()
- .HasColumnType("text")
- .HasColumnName("voice");
+ b.Property<string>("Voice")
+ .IsRequired()
+ .HasColumnType("character varying(100)")
+ .HasDefaultValue("default_voice")
+ .HasColumnName("voice");- Add a comment documenting valid voice values:
+ // Valid values are defined in Content.Shared/Voice/VoicePrototype.cs
b.Property<string>("Voice")Let's verify the impact on existing records:
✅ Verification successful
Based on the gathered information, I can now provide the final response:
The Voice column implementation is correct and safe
The migration files show that a default empty string value is already being set for existing records:
migrationBuilder.AddColumn<string>(
name: "voice",
table: "profile",
type: "text",
nullable: false,
defaultValue: "");Additionally, the property in HumanoidCharacterProfile.cs has a default value from SharedHumanoidAppearanceSystem.DefaultVoice, ensuring new profiles will always have a valid voice value.
While the text column is unconstrained, this is acceptable as voice identifiers are typically short strings managed by the system, and the default value mechanism prevents any data migration issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing profile records that would be affected by this migration
# Test: Search for profile-related migrations and entity definitions
rg -A 5 "CreateTable.*profile|AddColumn.*profile"
Length of output: 51
Script:
#!/bin/bash
# Let's try a different approach to find migration files and profile-related schema
# First, find all migration files
fd ".*Migration.*\.cs$" Content.Server.Database/Migrations
# Then, let's look for profile table creation or modification
rg -l "Profile|profile" Content.Server.Database/Migrations
# Also check for any existing profile entity configuration
ast-grep --pattern 'entity.Property<string>("Voice")'
# And look for profile table creation
ast-grep --pattern 'CreateTable(
$$$
"profile"
$$$
)'
Length of output: 10490
Script:
#!/bin/bash
# Let's check the specific TTS voice migration files we found
cat Content.Server.Database/Migrations/Postgres/20241105191142_TtsVoices.cs
cat Content.Server.Database/Migrations/Sqlite/20241105191222_TtsVoices.cs
# Also check if there's a default voice value defined somewhere
rg -l "DefaultVoice|default.*voice" --type cs
Length of output: 1893
Script:
#!/bin/bash
# Let's check the default voice implementation in the found files
cat Content.Shared/Preferences/HumanoidCharacterProfile.cs | grep -A 10 -B 10 "DefaultVoice"
# Also check if there are any voice prototypes defined
rg -g "*.yml" "- type: voice" .
Length of output: 1098
Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs (1)
647-647: LGTM: Voice update call placement is correct.
The UpdateTTSVoicesControls call is appropriately placed in the SetProfile method to ensure voice settings are updated when the profile changes.
Content.Server/_White/TTS/TTSSystem.Sanitize.cs (2)
33-34: Verify that _isEnabled is properly defined and initialized.
The variable _isEnabled is used to check if the TTS system is active before transforming speech. Ensure that _isEnabled is declared and initialized within the TTSSystem class or its partial definitions to prevent NullReferenceException or logical errors.
Run the following script to confirm the definition and initialization of _isEnabled:
#!/bin/bash
# Description: Search for the definition and initialization of _isEnabled.
# Test: Find all declarations of _isEnabled in the codebase.
rg --type cs --no-heading --line-number "bool\s+_isEnabled" .
# Test: Find all assignments to _isEnabled.
rg --type cs --no-heading --line-number "_isEnabled\s*=" .16-18: Ensure correctness of the MatchedWordsRegex pattern.
The regex pattern in MatchedWordsRegex uses lookbehind and lookahead assertions to match words, which can have performance implications and may not behave as expected in all cases. Consider reviewing the regex to ensure it correctly matches words boundaries, especially with Unicode characters.
Run the following script to verify the regex behavior:
✅ Verification successful
The regex pattern in MatchedWordsRegex is correctly implemented for its use case.
The regex pattern is used in a text sanitization process specifically for a TTS (Text-to-Speech) system that handles both Latin and Cyrillic characters. The pattern:
- Correctly uses lookbehind/lookahead to match word boundaries
- Is compiled with
[GeneratedRegex]attribute for optimal performance - Is used in
Sanitize()method withReplace()to transform specific abbreviations into their spoken form using theWordReplacementdictionary - Handles both English and Russian text as evidenced by the character classes
[a-zA-Zа-яёА-ЯЁ]
The implementation is appropriate for its specific use case of processing text for text-to-speech conversion, particularly for handling abbreviations like "GPS" → "Джи Пи Эс" as shown in the WordReplacement dictionary.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Test the MatchedWordsRegex pattern against sample text to ensure correct word matching.
# Test: Create a sample text file with various Unicode words.
echo -e "тестовое слово\nпредложение с разными словами" > sample_text.txt
# Use dotnet script to test the regex pattern.
dotnet script -e "using System.Text.RegularExpressions; var text = System.IO.File.ReadAllText(\"sample_text.txt\"); var matches = TTSSystem.MatchedWordsRegex().Matches(text); foreach (Match match in matches) { Console.WriteLine(match.Value); }"
Length of output: 1333
Script:
#!/bin/bash
# Let's analyze the regex pattern usage in the codebase
rg -A 5 -B 5 "MatchedWordsRegex" --type cs
# Find where this regex is used
ast-grep --pattern 'MatchedWordsRegex().Matches($_)'
# Look for any test files related to this functionality
fd -e cs test.*tts -i
# Check the complete TTSSystem.Sanitize.cs file
fd -g "*TTSSystem.Sanitize.cs" --exec cat {}
Length of output: 9869
Content.Server/_White/TTS/TTSSystem.cs (1)
190-192:
Validate and sanitize input text to prevent security issues.
The Sanitize method is used to clean the input text before generating TTS audio. Ensure that this method robustly removes any malicious content or code injections that could lead to security vulnerabilities when processing user-generated input.
Please confirm that the Sanitize method properly handles all edge cases and potential threats.
| @@ -0,0 +1,3 @@ | |||
| tts-voice-name-eugene = Евгений | |||
| tts-voice-name-kseniya = Ксения | |||
| tts-voice-name-xenia = Сения | |||
There was a problem hiding this comment.
Fix spelling: "Сения" should be "Ксения"
The Russian translation for "Xenia" appears to be misspelled. In Russian, the name should be spelled as "Ксения" (with a 'К'), not "Сения".
-tts-voice-name-xenia = Сения
+tts-voice-name-xenia = Ксения📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tts-voice-name-xenia = Сения | |
| tts-voice-name-xenia = Ксения |
| tts-voice-name-eugene = Евгений | ||
| tts-voice-name-kseniya = Ксения | ||
| tts-voice-name-xenia = Сения |
There was a problem hiding this comment.
💡 Codebase verification
Voice name 'Xenia' has inconsistent translations
The localization files show an inconsistency in the Russian translation:
tts-voice-name-xeniais translated as "Сения" in ru-RU but should be "Ксения" to match the same name format astts-voice-name-kseniya- Both entries refer to the same name (Ксения/Kseniya/Xenia) but are translated differently in Russian
🔗 Analysis chain
Verify consistency across localization files
The voice names look good, except for the spelling issue noted above. Let's verify the consistency with other localization files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistency across localization files for these voice entries
# Test 1: Find all corresponding voice entries in other locale files
echo "Checking voice entries across all locale files..."
rg "tts-voice-name-(eugene|kseniya|xenia)" Resources/Locale/
# Test 2: Verify voice definitions in the prototype file
echo "Checking voice definitions in prototype file..."
cat Resources/Prototypes/_White/Voice/tts_voices.yml
Length of output: 1166
| tts-voice-name-eugene = Evgeniy | ||
| tts-voice-name-kseniya = Kseniya | ||
| tts-voice-name-xenia = Xenia |
There was a problem hiding this comment.
💡 Codebase verification
Voice prototypes need to be updated to match YAML configuration
The voice entries in the FTL file don't match their corresponding prototypes in the YAML configuration. The YAML file shows:
eugeneis missing its prototype definitionkseniyaandxeniahave proper prototype definitions with matching IDs and localization keys
Please ensure all voice entries in the FTL file have corresponding prototype definitions in the YAML configuration.
🔗 Analysis chain
Verify voice prototype configuration.
Let's ensure these voice entries match their corresponding prototypes in the YAML configuration.
Let me gather more information about the voice prototypes by searching in a different way.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the voice IDs in FTL file match the prototype definitions
# Test: Search for voice prototype definitions
echo "Checking voice prototypes in YAML files..."
rg -A 5 "id: (eugene|kseniya|xenia)" --type yaml
# Test: Verify corresponding Russian translations exist
echo "Checking Russian translations..."
rg "tts-voice-name-(eugene|kseniya|xenia)" Resources/Locale/ru-RU/
Length of output: 568
Script:
#!/bin/bash
# Search for TTS voice configurations in YAML files with broader pattern
echo "Searching for TTS voice configurations..."
rg -A 10 "^- type: TTSVoice" --type yaml
# Also check for any voice-related prototype definitions
echo "Searching for voice-related prototypes..."
rg -A 10 "voice" --type yaml Resources/Prototypes/
Length of output: 23305
| // ReSharper disable once InconsistentNaming | ||
| public void RequestTTS(EntityUid uid, string text, string voiceId) | ||
| { | ||
| var netEntity = _entityManager.GetNetEntity(uid); | ||
| var msg = new MsgRequestTTS { Text = text, Uid = netEntity, VoiceId = voiceId }; | ||
| _netMgr.ClientSendMessage(msg); | ||
| } |
There was a problem hiding this comment.
Add input validation and error handling.
The RequestTTS method needs several improvements for robustness:
- Input validation for text and voiceId
- Error handling for network operations
- Size limits for text to prevent network issues
- Documentation about the expected voiceId format
+ /// <summary>
+ /// Requests a TTS message to be played for the specified entity.
+ /// </summary>
+ /// <param name="uid">The entity to play the TTS for</param>
+ /// <param name="text">The text to convert to speech (max 1000 characters)</param>
+ /// <param name="voiceId">The voice identifier (format: language-voice, e.g., "en-US-1")</param>
+ /// <exception cref="ArgumentException">Thrown when inputs are invalid</exception>
public void RequestTTS(EntityUid uid, string text, string voiceId)
{
+ if (string.IsNullOrEmpty(text))
+ throw new ArgumentException("Text cannot be empty", nameof(text));
+ if (string.IsNullOrEmpty(voiceId))
+ throw new ArgumentException("VoiceId cannot be empty", nameof(voiceId));
+ if (text.Length > 1000)
+ throw new ArgumentException("Text exceeds maximum length of 1000 characters", nameof(text));
+
var netEntity = _entityManager.GetNetEntity(uid);
- var msg = new MsgRequestTTS { Text = text, Uid = netEntity, VoiceId = voiceId };
- _netMgr.ClientSendMessage(msg);
+ try {
+ var msg = new MsgRequestTTS { Text = text.Trim(), Uid = netEntity, VoiceId = voiceId };
+ _netMgr.ClientSendMessage(msg);
+ } catch (Exception e) {
+ Logger.ErrorS("tts", e, "Failed to send TTS request");
+ throw;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // ReSharper disable once InconsistentNaming | |
| public void RequestTTS(EntityUid uid, string text, string voiceId) | |
| { | |
| var netEntity = _entityManager.GetNetEntity(uid); | |
| var msg = new MsgRequestTTS { Text = text, Uid = netEntity, VoiceId = voiceId }; | |
| _netMgr.ClientSendMessage(msg); | |
| } | |
| /// <summary> | |
| /// Requests a TTS message to be played for the specified entity. | |
| /// </summary> | |
| /// <param name="uid">The entity to play the TTS for</param> | |
| /// <param name="text">The text to convert to speech (max 1000 characters)</param> | |
| /// <param name="voiceId">The voice identifier (format: language-voice, e.g., "en-US-1")</param> | |
| /// <exception cref="ArgumentException">Thrown when inputs are invalid</exception> | |
| public void RequestTTS(EntityUid uid, string text, string voiceId) | |
| { | |
| if (string.IsNullOrEmpty(text)) | |
| throw new ArgumentException("Text cannot be empty", nameof(text)); | |
| if (string.IsNullOrEmpty(voiceId)) | |
| throw new ArgumentException("VoiceId cannot be empty", nameof(voiceId)); | |
| if (text.Length > 1000) | |
| throw new ArgumentException("Text exceeds maximum length of 1000 characters", nameof(text)); | |
| var netEntity = _entityManager.GetNetEntity(uid); | |
| try { | |
| var msg = new MsgRequestTTS { Text = text.Trim(), Uid = netEntity, VoiceId = voiceId }; | |
| _netMgr.ClientSendMessage(msg); | |
| } catch (Exception e) { | |
| Logger.ErrorS("tts", e, "Failed to send TTS request"); | |
| throw; | |
| } | |
| } |
| public override void ReadFromBuffer(NetIncomingMessage buffer, IRobustSerializer serializer) | ||
| { | ||
| Uid = new NetEntity(buffer.ReadInt32()); | ||
| Text = buffer.ReadString(); | ||
| VoiceId = buffer.ReadString(); | ||
| } | ||
|
|
||
| public override void WriteToBuffer(NetOutgoingMessage buffer, IRobustSerializer serializer) | ||
| { | ||
| buffer.Write((int)Uid); | ||
| buffer.Write(Text); | ||
| buffer.Write(VoiceId); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to serialization methods.
The current implementation lacks error handling for:
- Buffer underflow during reading
- Invalid string formats
- Null value handling
Consider implementing this improved version:
public override void ReadFromBuffer(NetIncomingMessage buffer, IRobustSerializer serializer)
{
+ try {
+ if (buffer.LengthBytes <= 0)
+ throw new InvalidOperationException("Empty buffer");
Uid = new NetEntity(buffer.ReadInt32());
Text = buffer.ReadString();
+ if (string.IsNullOrEmpty(Text))
+ Text = string.Empty;
VoiceId = buffer.ReadString();
+ }
+ catch (Exception e)
+ {
+ throw new InvalidOperationException("Failed to read TTS message", e);
+ }
}Committable suggestion skipped: line range outside the PR's diff.
| [GeneratedRegex(@"(?<=[1-90])(\.|,)(?=[1-90])")] | ||
| private static partial Regex FractionalNumbersRegex(); |
There was a problem hiding this comment.
Correct the regex character range in FractionalNumbersRegex.
The character class [1-90] in the FractionalNumbersRegex pattern is incorrect. The range [1-90] does not represent the digits from 0 to 9 and may lead to unexpected behavior. It seems you intended to match digits from 0 to 9. Please update the character class to [0-9] to accurately match all digits.
Apply this diff to fix the regex pattern:
[GeneratedRegex(@"(?<=[1-90])(\.|,)(?=[1-90])")]
+[GeneratedRegex(@"(?<=[0-9])(\.|,)(?=[0-9])")]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [GeneratedRegex(@"(?<=[1-90])(\.|,)(?=[1-90])")] | |
| private static partial Regex FractionalNumbersRegex(); | |
| [GeneratedRegex(@"(?<=[0-9])(\.|,)(?=[0-9])")] | |
| private static partial Regex FractionalNumbersRegex(); |
| private void OnTransformSpeech(TransformSpeechEvent args) | ||
| { | ||
| if (!_isEnabled) | ||
| return; | ||
| args.Message = args.Message.Replace("+", ""); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extend sanitization to remove unwanted characters in OnTransformSpeech.
Currently, OnTransformSpeech only removes the "+" character. Consider extending this method to utilize the Sanitize function or to remove additional unwanted characters to ensure that all speech is appropriately sanitized before processing.
Apply this diff to enhance the sanitization:
private void OnTransformSpeech(TransformSpeechEvent args)
{
if (!_isEnabled)
return;
- args.Message = args.Message.Replace("+", "");
+ args.Message = Sanitize(args.Message);
}Committable suggestion skipped: line range outside the PR's diff.
| private async void OnEntitySpoke(EntityUid uid, TTSComponent component, EntitySpokeEvent args) | ||
| { | ||
| if (!_isEnabled || string.IsNullOrEmpty(_apiUrl) || args.Message.Length > MaxMessageChars) | ||
| return; | ||
|
|
||
| var voiceId = component.Prototype; | ||
| var voiceEv = new TransformSpeakerVoiceEvent(uid, voiceId); | ||
| RaiseLocalEvent(uid, voiceEv); | ||
| voiceId = voiceEv.VoiceId; | ||
|
|
||
| if (!_prototypeManager.TryIndex(voiceId, out var protoVoice)) | ||
| return; | ||
|
|
||
| var message = FormattedMessage.RemoveMarkup(args.Message); | ||
|
|
||
| var soundData = await GenerateTTS(uid, message, protoVoice.Speaker); | ||
| if (soundData is null) | ||
| return; | ||
|
|
||
| var ttsEvent = new PlayTTSEvent(GetNetEntity(uid), soundData, false); | ||
|
|
||
| // Say | ||
| if (!args.IsWhisper) | ||
| { | ||
| RaiseNetworkEvent(ttsEvent, Filter.Pvs(uid), false); | ||
| return; | ||
| } | ||
|
|
||
| // Whisper | ||
| var chosenWhisperText = _random.Pick(_whisperWords); | ||
| var obfSoundData = await GenerateTTS(uid, chosenWhisperText, protoVoice.Speaker); | ||
| if (obfSoundData is null) | ||
| return; | ||
| var obfTtsEvent = new PlayTTSEvent(GetNetEntity(uid), obfSoundData, false); | ||
| var xformQuery = GetEntityQuery<TransformComponent>(); | ||
| var sourcePos = _xforms.GetWorldPosition(xformQuery.GetComponent(uid), xformQuery); | ||
| var receptions = Filter.Pvs(uid).Recipients; | ||
|
|
||
| foreach (var session in receptions) | ||
| { | ||
| if (!session.AttachedEntity.HasValue) | ||
| continue; | ||
| var xform = xformQuery.GetComponent(session.AttachedEntity.Value); | ||
| var distance = (sourcePos - _xforms.GetWorldPosition(xform, xformQuery)).LengthSquared(); | ||
| if (distance > ChatSystem.VoiceRange * ChatSystem.VoiceRange) | ||
| continue; | ||
|
|
||
| EntityEventArgs actualEvent = distance > ChatSystem.WhisperClearRange | ||
| ? obfTtsEvent | ||
| : ttsEvent; | ||
|
|
||
| RaiseNetworkEvent(actualEvent, Filter.SinglePlayer(session), false); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add exception handling in async event handlers.
The methods OnEntitySpoke, OnAnnounceRequest, and OnRequestTTS are asynchronous event handlers marked as async void. Any exceptions thrown within these methods will not be caught, potentially leading to application crashes. Wrap the method bodies in try-catch blocks to handle exceptions gracefully and log any errors.
Apply the following changes to each method:
-private async void OnEntitySpoke(EntityUid uid, TTSComponent component, EntitySpokeEvent args)
+private async void OnEntitySpoke(EntityUid uid, TTSComponent component, EntitySpokeEvent args)
{
+ try
+ {
// Existing method body
+ }
+ catch (Exception ex)
+ {
+ Logger.ErrorS("tts", $"Exception in OnEntitySpoke: {ex}");
+ }
}Repeat similar try-catch blocks for OnAnnounceRequest and OnRequestTTS.
Also applies to: 111-165, 172-185
| if (entities.Count == 0) | ||
| return; |
There was a problem hiding this comment.
Fix premature method exit in player loop.
In the OnAnnounceRequest method, the return statement at line 142 will exit the entire method when no emergency lights are found for a player. This causes the announcement to be skipped for all subsequent players in the loop. To ensure all players receive the announcement when applicable, replace return with continue to skip to the next player.
Apply this diff to fix the issue:
if (entities.Count == 0)
- return;
+ continue;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (entities.Count == 0) | |
| return; | |
| if (entities.Count == 0) | |
| continue; |
|
Я бы предпочла чтобы он так и был открыт до того, как у нас появится АПИ чтобы протестить это. |
Кидаю тогда в драфт пока |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
Resources/Prototypes/_White/Voice/tts_voices.yml (1)
334-335: Добавьте пустую строку в конец файлаВ соответствии с рекомендациями YAML, файл должен заканчиваться пустой строкой.
Добавьте пустую строку в конец файла:
speaker: Borgs roundStart: false +🧰 Tools
🪛 yamllint (1.35.1)
[error] 335-335: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl(1 hunks)Resources/Locale/ru-RU/_white/prototypes/voice/tts_voices.ftl(1 hunks)Resources/Prototypes/_White/Voice/tts_voices.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
Resources/Prototypes/_White/Voice/tts_voices.yml
[error] 335-335: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
Resources/Locale/ru-RU/_white/prototypes/voice/tts_voices.ftl (2)
25-25: Обратите внимание на дублирование имени "Зина"
В файле присутствует два голоса с именем "Зина" (tts-voice-name-zina и tts-voice-name-zena). Это может вызвать путаницу у пользователей.
Рекомендуется использовать разные имена для разных голосов. Например:
-tts-voice-name-zena = Зина
+tts-voice-name-zena = Зена3-3:
Исправьте перевод имени "Xenia"
Имя "Xenia" переведено как "Сения", что является некорректным переводом. В русском языке правильным переводом является "Ксения".
Примените следующие изменения:
-tts-voice-name-xenia = Сения
+tts-voice-name-xenia = КсенияLikely invalid or redundant comment.
Resources/Locale/en-US/_white/prototypes/voice/tts_voices.ftl (1)
1-45: Английские переводы корректны
Все имена голосов корректно определены и соответствуют конфигурации в YAML файле.
| tts-voice-name-valentain = Валентин | ||
| tts-voice-name-chelsea = Челси | ||
| tts-voice-name-anna = Анна | ||
| tts-voice-name-jeanne = Джон |
There was a problem hiding this comment.
Исправьте перевод имени "Jeanne"
Имя "Jeanne" переведено как "Джон", что является некорректным. "Jeanne" - это женское имя (Жанна).
Примените следующие изменения:
-tts-voice-name-jeanne = Джон
+tts-voice-name-jeanne = Жанна📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tts-voice-name-jeanne = Джон | |
| tts-voice-name-jeanne = Жанна |
| speaker: m-1 | ||
|
|
||
| - type: ttsVoice | ||
| id: lalo |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Исправьте регистр ID голосов
Некоторые ID голосов начинаются с маленькой буквы, в то время как большинство использует заглавную букву. Необходимо соблюдать единый стиль именования.
Примените следующие изменения для соблюдения единого стиля:
- id: lalo
+ id: Lalo
- id: galo
+ id: Galo
- id: nacho
+ id: Nacho
- id: john
+ id: John
- id: lina
+ id: Lina
- id: zena
+ id: Zena
- id: lifty
+ id: Lifty
- id: valentain
+ id: Valentain
- id: chelsea
+ id: Chelsea
- id: anna
+ id: Anna
- id: jake
+ id: Jake
- id: rihter
+ id: Rihter
- id: wane
+ id: WaneAlso applies to: 74-74, 80-80, 92-92, 137-137, 149-149, 161-161, 179-179, 185-185, 191-191, 245-245, 251-251, 257-257
| id: Alfa | ||
| name: tts-voice-name-alfa | ||
| sex: Unsexed | ||
| speaker: u-1 |
There was a problem hiding this comment.
Исправьте дублирование speaker ID
Значение speaker "u-1" используется для нескольких разных голосов (Alfa, TrainingRobot, Kelthuzad). Это может привести к конфликтам при воспроизведении.
Рекомендуется использовать уникальные идентификаторы для каждого голоса.
Also applies to: 306-306, 313-313
Описание PR
Ну вроде должно работать. В любом случаи апи у меня для проверки нет.
Изменения
🆑 Spatison