Multilanguage support #682
Conversation
This way we can add localized strings more easily.
Otherwise we might get different results after each compile which are hard to track for real changes
Summary of ChangesHello @sven-n, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers comprehensive multi-language support, allowing the application to present information and messages in a player's preferred language. It establishes the necessary infrastructure within the data model and game logic to handle localized strings, moving away from hardcoded text. This change significantly improves the accessibility and user experience for a global player base, making the game more adaptable to different linguistic preferences. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive localization system across the application, primarily affecting game logic and chat commands. Key changes include adding a LanguageIsoCode property to the Account entity, implementing a ModelResourceProvider for localized captions and descriptions, and integrating PlayerMessage.resx for storing player-facing messages. The Player class now includes Culture information and new methods (GetLocalizedMessage, ShowLocalizedBlueMessageAsync) to display localized messages. Many hardcoded strings in game logic (e.g., mini-game messages, MU Helper messages, duel messages, item consumption feedback) and chat commands have been replaced with calls to these new localization methods, ensuring that messages are displayed in the player's preferred language. Additionally, a new /language chat command has been added to allow players to change their language. The PlugIn attributes across numerous files have been refactored to use the Display attribute for plugin metadata, improving consistency and potentially enabling better UI integration. Error handling in chat commands has been updated to leverage LocalizableExceptionBase for localized error messages. Review comments highlighted issues such as an incorrect method signature for GetLocalizedMessage (fixed to use params object?[]), an illogical condition in SetStatChatCommandPlugIn (attribute.MaximumValue < 0 changed to arguments.Amount < 0), a copy-paste error in a PlugIn attribute description (EndDuelWhenLeavingDuelMapPlugIn), and the need to localize a quest progress message in QuestMonsterKillCountPlugIn.
| public string GetLocalizedMessage(string resourceName, params ReadOnlySpan<object?> formatArguments) | ||
| { | ||
| if (formatArguments.Length > 0) | ||
| { | ||
| return string.Format(PlayerMessage.ResourceManager.GetString(resourceName, this.Culture) ?? string.Empty, formatArguments); | ||
| } | ||
|
|
||
| return PlayerMessage.ResourceManager.GetString(resourceName, this.Culture) ?? string.Empty; | ||
| } |
There was a problem hiding this comment.
The method signature public string GetLocalizedMessage(string resourceName, params ReadOnlySpan<object?> formatArguments) is invalid. The params keyword cannot be used with ReadOnlySpan<T>. Additionally, string.Format does not have an overload that accepts a ReadOnlySpan<T>. This will cause a compilation error. You should change the signature to use params object?[] instead.
public string GetLocalizedMessage(string resourceName, params object?[] formatArguments)
{
if (formatArguments.Length > 0)
{
return string.Format(PlayerMessage.ResourceManager.GetString(resourceName, this.Culture) ?? string.Empty, formatArguments);
}
return PlayerMessage.ResourceManager.GetString(resourceName, this.Culture) ?? string.Empty;
}| if (attribute.MaximumValue < 0 || arguments.Amount > attribute.MaximumValue) | ||
| { | ||
| await player.ShowMessageAsync(e.Message).ConfigureAwait(false); | ||
| await player.ShowLocalizedBlueMessageAsync(nameof(PlayerMessage.InvalidStatValueRange), arguments.StatType, attribute.MaximumValue).ConfigureAwait(false); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The condition attribute.MaximumValue < 0 seems incorrect. It's likely a typo and should be arguments.Amount < 0 to check if the provided amount is negative, which would be invalid. The current check against attribute.MaximumValue doesn't make sense in this context, especially since the error message refers to a valid range starting from 0.
if (arguments.Amount < 0 || arguments.Amount > attribute.MaximumValue)
{
await player.ShowLocalizedBlueMessageAsync(nameof(PlayerMessage.InvalidStatValueRange), arguments.StatType, attribute.MaximumValue).ConfigureAwait(false);
return;
}| /// </summary> | ||
| [PlugIn(nameof(EndDuelWhenLeavingDuelMapPlugIn), "Updates the state of the weather of each hosted map in a random way.")] | ||
| [PlugIn] | ||
| [Display(Name = nameof(EndDuelWhenLeavingDuelMapPlugIn), Description = "Updates the state of the weather of each hosted map in a random way.")] |
There was a problem hiding this comment.
The description in the Display attribute seems to be incorrect. It says "Updates the state of the weather of each hosted map in a random way.", but the plugin is for ending a duel when a player leaves the duel map. This seems to be a copy-paste error.
[Display(Name = nameof(EndDuelWhenLeavingDuelMapPlugIn), Description = "Ends the duel when a player leaves the duel map.")]|
|
||
| await player.ShowMessageAsync(message).ConfigureAwait(false); | ||
| // TODO: Localize | ||
| await player.ShowBlueMessageAsync(message).ConfigureAwait(false); |
There was a problem hiding this comment.
The quest progress message is constructed manually and not localized, as noted by the TODO. You should move this string to the PlayerMessage.resx resource file and use ShowLocalizedBlueMessageAsync to display it. This will improve maintainability and make the message localizable.
For example, you could add a QuestMonsterKillProgress resource string like "{0}: {1} killed ({2}/{3})" and then change the code to use it.
await player.ShowLocalizedBlueMessageAsync(nameof(PlayerMessage.QuestMonsterKillProgress), questState.ActiveQuest.Name, monster.Definition.Designation, requirementState.KillCount, killRequirement.MinimumNumber).ConfigureAwait(false);
Account