-
Notifications
You must be signed in to change notification settings - Fork 3
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
Adds improvements to localization system #58
Conversation
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.
Hey @HalfOfBilly great work 🚀
I think localization is now more cleaner and easier to understand. I still don't like arrays in .json
files but it's not a dealbreaker.
I've added just few suggestions.
|
||
def update_project_settings(self) -> None: | ||
project_godot_path = os.path.join(self.godot_project_path, 'project.godot') | ||
|
||
if not os.path.exists(project_godot_path): | ||
self.log_callback(get_localized(self.language, 'Console_Convertor_Settings_Error_GD_NotFound')) | ||
self.log_callback(get_localized("Console_Error_MissingGodotFile")) | ||
self.log_callback(get_localized("")) |
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.
Remove this line
|
||
def get_button(self, button_name): | ||
return self.buttons.get(button_name.lower()) | ||
return self.buttons.get(button_name)#.lower()) |
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.
Remove commented code
def get_localized(language, key): | ||
def get_localized(key): | ||
# Find active language | ||
with open('Current Language', 'r') as file: |
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 suppose this can cause slowness before we implement #57. I guess it's ok to have this approach for now.
@@ -263,11 +262,29 @@ def insert_formatted(content): | |||
text_widget.configure(state="disabled") | |||
|
|||
def setup_conversion_settings(self): | |||
settings = (set(item for sublist in get_localized(self.language, 'Settings_Categories_Contents') for item in sublist)) # merges all setting categories into a single array | |||
#settings = (set(item for sublist in get_localized("Settings_Categories_Contents") for item in sublist)) # merges all setting categories into a single array |
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.
Remove commented code
|
||
for idx, (label, command, icon) in enumerate(paths): | ||
Game_Engine = get_localized(f"Menu_{label}") |
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.
You can rename label
variable to engine_name
?
Just as a reference, this is continuation of #56. @HalfOfBilly I like to reference old/previous PR/issue just in case when someone is going through git history to find what it's needed easier. |
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.
Looks much much cleaner! Good job!
Revisions to localization system as per @rammba 's feedback: