-
-
Notifications
You must be signed in to change notification settings - Fork 375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple languages & Language adaptation #224
base: development
Are you sure you want to change the base?
Conversation
It's almost as if you read my mind, I was about to add this myself. I'll review all the changes (already spotted a few things I'd like changed) in a bit. |
} | ||
else | ||
{ | ||
data = JsonConvert.DeserializeObject<Dictionary<string, Hashtable>>(jsonFile); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please only try/catch the necessary code. Only the JSON conversion could crash here, so that's what needs to be caught.
#if CLIENT | ||
vMenuClient.Notify.Error("An error occurred while processing the languages.json file. Language will set to English. Please correct any errors in the languages.json file."); | ||
#endif | ||
Debug.WriteLine($"[vMenu] json exception details: {e.Message}\nStackTrace:\n{e.StackTrace}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notify.Error()
already prints an error in the console, so you only need to log this to the console on the server side here.
} | ||
#endregion | ||
|
||
#region Get saved locations from the locations.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is there an additional indentation level here?
@@ -11,6 +11,9 @@ | |||
using static vMenuClient.CommonFunctions; | |||
using static vMenuShared.ConfigManager; | |||
using static vMenuShared.PermissionsManager; | |||
using System.Collections; | |||
using vMenuShared; | |||
using CitizenFX.Core.Native; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change this to:
using static CitizenFX.Core.Native.API;
using static CitizenFX.Core.Native.API; | ||
using static vMenuClient.CommonFunctions; | ||
using static vMenuShared.ConfigManager; | ||
using static vMenuShared.PermissionsManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is all this really necessary?
/// <param name="text"></param> | ||
private void DumpLanguages([FromSource] Player source, string text) | ||
{ | ||
if (IsPlayerAceAllowed(source.Handle, "vMenu.DumpLanguages.Dump") || IsPlayerAceAllowed(source.Handle, "vMenu.Everything") || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not need to be cheater checked if you make this a server command. It's also not a destructive action so there should be no need to have this be cheater proof.
@@ -1800,15 +1801,15 @@ public void UpdateMods(int selectedIndex = 0) | |||
index = 0; | |||
} | |||
|
|||
MenuListItem tireSmoke = new MenuListItem("Tire Smoke Color", tireSmokes, index, $"Choose a ~y~tire smoke color~s~ for your vehicle."); | |||
MenuListItem tireSmoke = new MenuListItem(LM.Get("Tire Smoke Color"), tireSmokes, index, $"Choose a ~y~tire smoke color~s~ for your vehicle."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description is missing a translation ;)
{ | ||
Label = $"~h~{MainMenu.Version}~h~" | ||
}; | ||
MenuItem credits = new MenuItem("About vMenu / Credits", "vMenu is made by ~b~Vespura~s~. For more info, checkout ~b~www.vespura.com/vmenu~s~. Thank you to: Deltanic, Brigliar, IllusiveTea, Shayan Doust and zr0iq for your contributions."); | ||
MenuItem credits = new MenuItem(LM.Get("About vMenu / Credits"), LM.Get("vMenu is made by ~b~Vespura~s~. For more info, checkout ~b~www.vespura.com/vmenu~s~. Thank you to: Deltanic, Brigliar, IllusiveTea, Shayan Doust and zr0iq for your contributions.")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Credits shouldn't need to be translated or possibly removed.
@@ -20,6 +20,7 @@ public class NoClip : BaseScript | |||
private static int Scale { get; set; } = -1; | |||
private static bool FollowCamMode { get; set; } = false; | |||
|
|||
private static LanguageManager LM = new LanguageManager(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a bit useless? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to add a translation for the NoClip button, but it crashed, seemingly because it was loaded earlier than the main menu.
@@ -0,0 +1,1631 @@ | |||
{ | |||
"american": {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also please split these languages up in different files, that way maintaining custom language versions is much easier for people when a new language is added or updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. When I first tried to support multiple languages, it was split into multiple files.
Additionally, please make an option to allow other languages than just the game languages to be added. So the user can select a language they want regardless of game language. Make this option default to the game's language. |
Thank you so much! 😆 Umm...In actually I am not good at C#, so I use the Google to learn how to write the code xD |
@kasuganosoras For just googling bits this isn’t half bad lol |
Any news about this? Has this been implemented in the new version yet? |
Well, that deescalated. Still needed. |
Currently none of the requested changes have been made yet. There is another PR that also implements multiple languages. Whichever one gets updated first and has the best implementation will be used, the other will unfortunately be closed. |
Added new feature
Added Languages
Added Command
dumped_text.json
, request permissionvMenu.DumpLanguages.Dump
)Added Class
Added Files
Locale file format
Example:
Locale use in code example
Thank you for creating this awesome menu!