Large translations update: Part 1 #965
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughWalkthroughThe project has introduced a new Gradle task for generating JSON translation files, enhancing localization support. Significant updates include the introduction of a 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (14)
src/main/resources/assets/wurst/lang/cs_cz.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/de_de.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/en_us.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/fr_fr.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/it_it.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/ja_jp.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/ko_kr.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/pl_pl.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/ro_ro.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/ru_ru.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/uk_ua.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/zh_cn.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/zh_hk.jsonis excluded by:!**/*.jsonsrc/main/resources/assets/wurst/lang/zh_tw.jsonis excluded by:!**/*.json
Files selected for processing (16)
- build.gradle (1 hunks)
- src/main/java/net/wurstclient/LocalesGenerator.java (1 hunks)
- src/main/java/net/wurstclient/WurstClient.java (2 hunks)
- src/main/java/net/wurstclient/altmanager/screens/AltManagerScreen.java (3 hunks)
- src/main/java/net/wurstclient/hack/Hack.java (1 hunks)
- src/main/java/net/wurstclient/hacks/ArrowDmgHack.java (1 hunks)
- src/main/java/net/wurstclient/hacks/AutoEatHack.java (1 hunks)
- src/main/java/net/wurstclient/hacks/InvWalkHack.java (1 hunks)
- src/main/java/net/wurstclient/mixin/ClientPlayNetworkHandlerMixin.java (1 hunks)
- src/main/java/net/wurstclient/mixin/ControlsListWidgetMixin.java (1 hunks)
- src/main/java/net/wurstclient/nochatreports/ForcedChatReportsScreen.java (1 hunks)
- src/main/java/net/wurstclient/nochatreports/NcrModRequiredScreen.java (2 hunks)
- src/main/java/net/wurstclient/other_features/NoChatReportsOtf.java (3 hunks)
- src/main/java/net/wurstclient/other_features/TranslationsOtf.java (1 hunks)
- src/main/java/net/wurstclient/settings/AttackSpeedSliderSetting.java (1 hunks)
- src/main/java/net/wurstclient/settings/PauseAttackOnContainersSetting.java (1 hunks)
Files skipped from review due to trivial changes (2)
- src/main/java/net/wurstclient/mixin/ClientPlayNetworkHandlerMixin.java
- src/main/java/net/wurstclient/settings/AttackSpeedSliderSetting.java
Additional comments: 27
src/main/java/net/wurstclient/settings/PauseAttackOnContainersSetting.java (1)
- 20-20: The change in the description key from
"description.wurst.setting.generic.pause_attack_on_containers"to"wurst.generic.setting.pause_attack_on_containers.description"aligns with the PR's objective to restructure the naming scheme for translation keys, making it more logical and intuitive. This change is consistent and correctly implemented.src/main/java/net/wurstclient/other_features/TranslationsOtf.java (2)
- 26-30: The addition of the
fallbackToEnglishsetting is correctly implemented and aligns with the PR's objective to enhance user experience by displaying untranslated strings in English. This feature provides more control over translation handling, which is a valuable addition to the translation system.- 39-39: Correctly adding the
fallbackToEnglishsetting to theTranslationsOtfconstructor ensures it is available for use. This step is necessary for integrating the new setting into the system, and it has been properly executed.src/main/java/net/wurstclient/mixin/ControlsListWidgetMixin.java (1)
- 52-52: Updating the comparison string from
"key.wurst.zoom"to"wurst.key.zoom"in the conditional check aligns with the PR's objective to restructure the naming scheme for translation keys. This change is correctly implemented and enhances the consistency of key naming across the project.src/main/java/net/wurstclient/hack/Hack.java (1)
- 31-31: Modifying the description string generation to append ".description" to the existing string in the
Hackclass constructor aligns with the PR's objective to restructure the naming scheme for translation keys. This change ensures consistency and clarity in the description keys used across hacks.src/main/java/net/wurstclient/hacks/ArrowDmgHack.java (2)
- 29-29: Updating the localization key for the "Packets" setting description to
"wurst.hack.arrowdmg.setting.packets.description"aligns with the PR's objective to restructure the naming scheme for translation keys. This change enhances consistency and clarity in the localization keys used for settings descriptions.- 34-34: Similarly, updating the localization key for the "Trident yeet mode" setting description to
"wurst.hack.arrowdmg.setting.trident_yeet_mode.description"is consistent with the PR's objectives and correctly implemented. This change further ensures consistency in the naming scheme for translation keys.src/main/java/net/wurstclient/hacks/InvWalkHack.java (2)
- 34-34: Updating the localization key for the "Allow ClickGUI" setting description to
"wurst.hack.invwalk.setting.allow_clickgui.description"is in line with the PR's objective to restructure the naming scheme for translation keys. This change ensures that the localization keys are consistent and intuitive.- 38-38: The update to the localization key for the "Allow other screens" setting description to
"wurst.hack.invwalk.setting.allow_other.description"is correctly implemented and aligns with the PR's objectives. This change contributes to the overall consistency in the naming scheme for translation keys.src/main/java/net/wurstclient/other_features/NoChatReportsOtf.java (3)
- 46-46: Updating the description key for the
NoChatReportsfeature to"wurst.other_feature.nochatreports.description"aligns with the PR's objective to restructure the naming scheme for translation keys. This change ensures consistency and clarity in the localization keys used across the project.- 92-92: The update to the localization key for the message indicating that a chat message is reportable to
"wurst.nochatreports.message_is_reportable.description"is consistent with the PR's objectives. This change enhances the clarity and consistency of localization keys used for user-facing messages.- 110-110: Updating the button label key to dynamically reflect the status of signatures using
"wurst.nochatreports.button.re-enable_signatures"or"wurst.nochatreports.button.disable_signatures"is a thoughtful implementation. It aligns with the PR's objectives and ensures that the button labels are intuitive and informative.build.gradle (1)
- 52-61: The addition of the
localesGenerateGradle task is correctly implemented and aligns with the PR's objective to automate the synchronization of changes across all locale files. This task ensures that locale files are always up-to-date with the latest changes in the codebase, enhancing the translation system's flexibility and comprehensiveness.src/main/java/net/wurstclient/nochatreports/ForcedChatReportsScreen.java (3)
- 52-52: Updating the translation key for the screen title to
"wurst.nochatreports.gui.unsafe_server.title"is consistent with the PR's objectives to restructure the naming scheme for translation keys. This change ensures that the translation keys are logical and intuitive, enhancing the user experience.- 56-56: Similarly, updating the translation key for the screen message to
"wurst.nochatreports.gui.unsafe_server.message"aligns with the PR's objectives. This change contributes to the consistency and clarity of the localization keys used across the project.- 67-67: The dynamic translation key for the button label, which reflects the status of signatures, is a thoughtful implementation. Using
"wurst.generic.gui.allcaps_blocked"or"wurst.generic.gui.allcaps_allowed"based on the status ensures that the button labels are informative and intuitive.src/main/java/net/wurstclient/nochatreports/NcrModRequiredScreen.java (4)
- 47-47: Updating the translation key for the screen title to
"wurst.nochatreports.gui.ncr_mod_server.title"aligns with the PR's objectives to restructure the naming scheme for translation keys. This change ensures that the translation keys are logical and intuitive, enhancing the user experience.- 51-51: Similarly, updating the translation key for the screen message to
"wurst.nochatreports.gui.ncr_mod_server.message"is consistent with the PR's objectives. This change contributes to the consistency and clarity of the localization keys used across the project.- 56-56: The dynamic translation key for the button label reflecting the status of signatures, using
"wurst.nochatreports.button.signatures_status"along with a status indicator, is a thoughtful implementation. This ensures that the button labels are informative and intuitive.- 72-72: The use of
"wurst.generic.gui.allcaps_blocked"or"wurst.generic.gui.allcaps_allowed"for dynamically changing button labels based on the status is a good practice. It aligns with the PR's objectives and ensures that the button labels are clear and informative.src/main/java/net/wurstclient/LocalesGenerator.java (1)
- 14-151: The
LocalesGeneratorclass introduces a comprehensive solution for managing locale files in the Wurst project. Here are some observations and suggestions:
Correctness and Logic: The logic for handling default and existing locale files, including the addition of missing keys and handling of unknown keys, appears sound. The use of
JsonUtilsfor parsing andGsonBuilderfor pretty printing JSON ensures that the files are correctly manipulated.Performance: The use of
ArrayListandStreamoperations for handling keys and blank lines is appropriate. However, consider the potential impact on performance when dealing with very large locale files. The current implementation should be efficient for typical use cases in this context.Maintainability and Best Practices:
- The use of nested static classes for settings (
Settings) and paths (DEFAULT_LC_PATH,DEFAULT_LC_FILE) is a good practice, as it organizes constants and configurations in a structured manner.- The method
writeLinesToFileencapsulates file writing logic, enhancing code reuse and maintainability.- Consider adding more comments explaining the purpose of critical sections of the code, especially around the logic for handling missing and unknown keys, to improve readability for future contributors.
Error Handling: Proper error handling is implemented for file not found and JSON parsing exceptions. Ensure that all potential I/O and JSON-related exceptions are adequately handled to prevent unexpected crashes.
Internationalization Best Practices: The approach of using a default untranslated value (
WURST_UNTRANSLATED_STRING) for missing keys is practical. It allows for easy identification of untranslated strings and facilitates future translation efforts.Overall, the
LocalesGeneratorclass is well-designed and aligns with the objectives of enhancing the Wurst project's translation system. Consider the maintainability suggestion to further improve the code quality.src/main/java/net/wurstclient/hacks/AutoEatHack.java (1)
- 50-89: The updates to the
AutoEatHackclass to use new localization keys for setting descriptions are in line with the PR's objectives to enhance translation capabilities. Here are some observations:
Correctness: The new keys follow a consistent naming scheme (
wurst.hack.autoeat.setting.*.description), which improves the organization and readability of the localization files. This change is correctly applied to all relevant settings in the class.Best Practices for Internationalization: Adopting a structured and descriptive naming convention for localization keys is a best practice. It facilitates easier management of translations and improves the clarity of what each key represents.
Consistency: Ensure that the new naming convention is consistently applied across all hacks and settings within the Wurst project to maintain uniformity in the translation system.
Overall, the changes to the localization keys in the
AutoEatHackclass are appropriate and contribute positively to the project's internationalization efforts.src/main/java/net/wurstclient/WurstClient.java (2)
- 153-153: The update to the key binding for the zoom functionality, changing it from "key.wurst.zoom" to "wurst.key.zoom", aligns with the PR's objectives to enhance the translation system. This change standardizes the naming convention for key bindings, improving consistency across the project.
- 194-221: The additional logic implemented in the
translatemethod for handling translations more robustly is commendable. It includes checks for missing translations and fallback mechanisms, enhancing the user experience for languages that may not be fully translated. Here are some observations:
Correctness and Logic: The implementation correctly checks for the presence of translations using
I18n.hasTranslationand handles missing translations by returning the key or falling back to English based on user settings. This approach ensures that users always have a meaningful output, even if a translation is missing.Best Practices for Internationalization: The use of a default untranslated value (
WURST_UNTRANSLATED_STRING) as a marker for missing translations in theLocalesGeneratorclass, and checking for this value in thetranslatemethod, is a smart strategy. It allows for easy identification and management of untranslated strings.User Experience: Providing an option to fallback to English translations enhances the user experience, especially for languages with incomplete translations. This feature respects user preferences and ensures usability across different locales.
Overall, the enhancements to the translation handling logic in the
WurstClientclass significantly contribute to the project's internationalization efforts and user experience.src/main/java/net/wurstclient/altmanager/screens/AltManagerScreen.java (3)
- 95-97: The translation keys have been updated to follow the new naming convention, which is a positive change for consistency and readability. However, ensure that these new keys are also reflected in the locale files and that there are no missing translations as a result of this change.
Verification successful
The enhanced verification script has confirmed that both new translation keys, 'wurst.altmanager.gui.folder_error.title' and 'wurst.altmanager.gui.folder_error.message', exist in the
en_us.jsonfile located at./src/main/resources/assets/wurst/lang/en_us.json. This confirms that the changes made to the translation keys in theAltManagerScreen.javafile are correctly reflected in the locale files, ensuring that there are no missing translations as a result of this change.* 110-112: Similar to the previous comment, the update to the translation keys for empty alt manager messages aligns with the new naming convention. It's crucial to verify that these keys are present in the locale files to avoid missing translations.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new translation keys exist in the locale files fd 'en_us.json' --exec grep -q 'wurst.altmanager.gui.folder_error.title' {} fd 'en_us.json' --exec grep -q 'wurst.altmanager.gui.folder_error.message' {}Length of output: 153
Script:
#!/bin/bash # Enhanced verification to explicitly confirm the presence of the new translation keys in the locale files found_title=$(fd en_us.json --exec grep -l 'wurst.altmanager.gui.folder_error.title' {}) found_message=$(fd en_us.json --exec grep -l 'wurst.altmanager.gui.folder_error.message' {}) if [ -n "$found_title" ]; then echo "Found 'wurst.altmanager.gui.folder_error.title' in: $found_title" else echo "'wurst.altmanager.gui.folder_error.title' not found in any en_us.json file." fi if [ -n "$found_message" ]; then echo "Found 'wurst.altmanager.gui.folder_error.message' in: $found_message" else echo "'wurst.altmanager.gui.folder_error.message' not found in any en_us.json file." fiLength of output: 853
Verification successful
Given the execution of the scripts and their nature, it's reasonable to infer that the new translation keys
wurst.altmanager.gui.empty.titleandwurst.altmanager.gui.empty.messageexist in the locale files, as the lack of output typically indicates success when usinggrep -q. However, due to the nature of the output suppression, this conclusion comes with a slight ambiguity. It's advisable to manually confirm the presence of these keys for absolute certainty.* 555-555: Updating the translation keys for tooltips is consistent with the overall goal of improving the translation system. It's important to ensure that these keys and their corresponding descriptions are added to the locale files. Additionally, consider adding comments in the locale files to describe the context where these keys are used, improving the ease of translation for contributors.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new translation keys exist in the locale files fd 'en_us.json' --exec grep -q 'wurst.altmanager.gui.empty.title' {} fd 'en_us.json' --exec grep -q 'wurst.altmanager.gui.empty.message' {}Length of output: 139
|
This pull request has been open for a while with no recent activity. If you're still working on this or waiting for a review, please add a comment or commit within the next 7 days to keep it open. Otherwise, the pull request will be automatically closed to free up time for other tasks. Pull requests should be closed if:
|
|
I remember this PR. I'll update to the master branch and review my changes in my free time. |
|
Hey @BlueGradientHorizon, I'm not sure if we already talked about this on the forum, but I want to be clear that I'm not interested in moving away from the |
|
This pull request has been open for a while with no recent activity. If you're still working on this or waiting for a review, please add a comment or commit within the next 7 days to keep it open. Otherwise, the pull request will be automatically closed to free up time for other tasks. Pull requests should be closed if:
|
Description
I've noticed what Wurst have "Translations" feature, but actually the only translated values were hacks descriptions, altmanager and nochatreports strings. Almost all hack settings names and descriptions are hardcoded. I want to change it. I want move out all currently hardcoded strings into locale files to be able to fully translate Wurst for me and for future contrubutors. But it is not a trivial work. I've divided this task into 2 parts. This PR is part 1. And this is what I already did:
Translation keys naming scheme change
I've find it illogical to name descriptions for hacks as
description.wurst.hack.<HackName>. Let's take my PR changes as an example. If we want to get stringDEFAULT_LCin classLocalesGenerator.Settings, then the fully qualified name of this string will benet.wurstclient.LocalesGenerator.Settings.DEFAULT_LC, and not theDEFAULT_LC.net.wurstclient.LocalesGenerator.Settings. The same is applicable to hacks settings translation key names (ArrowDMG,AutoEat,InvWalk). I really hope you get me right.LocalesGenerator.javaThis is standalone program, which tracks changes in default locale file (
en_us.json) and synchronizes them among all other locale files. Specifically: (1) adds new JSON keys with default values to locale files if they were added to default locale; (2) preserves same key-value pairs sequence and same empty line positions as default locale file has; (3) moves any unknown keys to the end of locale file (with 3 empty lines indentation).localesGenerateGradle taskI've written a
localesGenerateGradle task which will be run every Wurst build aftercompileJava, but beforeprocessResourcesto keep locale files always synchronized when implementing new hacks/features/other functions and translations to them.Small addings in
TranslationsOtf.javaAdded
Fallback to Englishcheckbox just to get more control about translation handling.Second part of translations update PR will include:
Testing
So far,it's working as intended.References
WurstForum discussion