Skip to content
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

Language translator updates/fixes #509

Merged
merged 22 commits into from
Sep 20, 2019

Conversation

kvakvs
Copy link
Collaborator

@kvakvs kvakvs commented Aug 25, 2019

Fixes #493

  • On TM:PE language change
    • Language in options updates in main menu (before game is loaded) and during the game too
    • Tutorials update
    • Main TM:PE button tooltip is updated
  • All languages loaded at once
  • Translation class not static anymore
  • Loading CSV from Crowdin instead of multiple language files. The files now moved under Resources/Translations in the file tree
  • Added en-GB

@originalfoo originalfoo added Localisation Localised text and features UI User interface updates labels Aug 25, 2019
@originalfoo originalfoo added this to the 11.0 milestone Aug 25, 2019
@originalfoo
Copy link
Member

List of things I tried (issues listed later):

  • Changing game language 👍
  • Changing lang in mod options prior to loading a save 👍
  • Mod options updates from main menu 👍
  • Changing lang in mod options in-game 👍
  • Mod options updates in-game 👍
  • TM:PE "crown" button: Tooltip ❌
  • Menu panel: Tooltips on tool buttons 👍
  • Tooltips: Keyboard shortcut modifiers (eg. Shift) and special keys (eg. Backspace) ❌
  • Tutorial text (for several tools) ❌
  • Lane arrows panel 👍
  • Despawn tool: Confirmation dialog ❌
  • Timed Traffic Lights: Dialog panel 👍
  • Timed Traffic Lights: Overlays ❌
  • Speed limits: Main panel 👍
  • Speed limits: Default speeds panel 👍

That's everything I could find that had text to translate (did I miss anything?)...

Issues:

  • Hungarian (Magyar) language recently added (Add Hungarian language #492) isn't included in the language drop-down in mod options...?
  • Also, some German translations seem missing:
    image
  • TM:PE button tool tip (German lang selected) in English:
    image
  • Should keybinds also be translated (German lang selected):
    image image
  • Tutorial advisor text did not localise (tried German and Chinese):
    image
  • Despawn tool confirmation dialog: Yes/No buttons not translated (tried Chinese and German):
    image
  • Timed traffic lights overlays - I am aware the yellow "Auto" panel is an image and only has few translations so ignore that. However the "CHANGE MODE" panel seems not to translate (as far as I know that is normal text component so should translate):
    image

@originalfoo
Copy link
Member

originalfoo commented Aug 26, 2019

Comparing to current master branch as requested:

  • Changing game language 👍
  • Changing lang in mod options prior to loading a save 👍
  • Mod options updates from main menu 👍
  • Changing lang in mod options in-game 👍
  • Mod options updates in-game 👍
  • TM:PE "crown" button: Tooltip ❌ - same issue as PR
  • Menu panel: Tooltips on tool buttons 👍
  • Tooltips: Keyboard shortcut modifiers (eg. Shift) and special keys (eg. Backspace) ❌ - same issue as PR
  • Tutorial text (for several tools) 👍 - this works in master
  • Lane arrows panel 👍
  • Despawn tool: Confirmation dialog ❌ - same issue as PR
  • Timed Traffic Lights: Dialog panel 👍
  • Timed Traffic Lights: Overlays ❌ - same issue as PR
  • Speed limits: Main panel ❌ - 'unlimited speed` locale key visible
  • Speed limits: Default speeds panel 👍

So, to summarise, there are two differences between current master branch and your PR #509:

  1. Tutorial Advisor panels are translating properly (at least when language changed in-game) in the master version.
  2. Noticed the "unlmited speed" caption on speeds panel was showing locale key (tested German language):
    image

Note: Regarding point 2 above ("unlimited speed" locale key showing), I might have missed similar issue in the PR build.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Aug 26, 2019

The advisor text update looked like a real bug, unless it is reset somewhere, it would fail adding same keys over again with an exception. One had to remove the existing keys first, and that code was not there in master.

@originalfoo
Copy link
Member

The one thing I've not tested is changing language at main menu, then loading game.

Each time I changed language in main menu, tested mod options from main menu, then reset to English then loaded game, and then started changing language in-game to test UI stuff.

So I'll repeat my tests but this time changing language from main menu to see if I can replicate the issues @krzychu124 was having in master branch, and also see if that fixes tutorial text in-game in the PR branch.

@originalfoo
Copy link
Member

originalfoo commented Aug 26, 2019

Ok, just replicated the tutorial text issue that @krzychu124 was having.

Steps to reproduce:

  1. master branch
  2. Launch Cities.exe
    • Do NOT alter language from main menu
  3. Load a save game
  4. Check tutorial - it is in whatever language TM:PE (or game if applicable) was set to when Cities.exe loaded
  5. In-game, change language in TM:PE mod options (didn't try game language but assume same issue)
  6. Tutorial panels do not change their language

So it seems that, in master branch, changing language from main menu before loading a save somehow fixes the bug with tutorial text when later changing languages in-game.

If you don't change the language at main menu, then subsequent in-game language changes don't update the tutorial panels.

EDIT: That was the only difference I noticed from earlier testing.

I will now retest the PR version to see what happens with same steps.

@originalfoo
Copy link
Member

originalfoo commented Aug 26, 2019

Just tested the PR again, and got the tutorial text working using same steps as comment above!

  1. PR Language translator updates/fixes #509 branch
  2. Launch Cities.exe
    • Do NOT alter language from main menu
  3. Load a save game
  4. Check tutorial - it is in whatever language TM:PE (or game if applicable) was set to when Cities.exe loaded
  5. In-game, change language in TM:PE mod options (didn't try game language but assume same)
  6. Tutorial panels change language properly!

So it seems you've inverted the problem compared to master branch 🤣

The Hungarian (Magyar) language seems to be appearing in mod options again (at least in-game). I will do some testing to see if it's there when I access them via main menu. EDIT: It's there in main menu; not sure why it wasn't appearing before :/

I re-tested the "unlimited speed" caption on speeds panel in the PR build - it shows "No limit" English text for some languages (tested German, Chinese, Polish). When I switched to Magyar (Hungarian) it showed correct translation. So I assume we're just missing translations for the other languages.

Note; We should check the Speed_limit_unlimited locale key exists in the lang files. I will do that shortly.

@originalfoo
Copy link
Member

Speed_limit_unlimited is missing from several of the locale text files.

Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See notes listed above.

@originalfoo originalfoo added the under-review A pull request has been created and is currently being reviewed label Aug 26, 2019
@kvakvs
Copy link
Collaborator Author

kvakvs commented Aug 29, 2019

Ok, until we have determined the translation tool to use, be it Crowdin or something else, or an offline tool, i am not going to address any missing translations. Only those things which are broken by coding or can be fixed by coding.

@originalfoo
Copy link
Member

Did you manage to sort the issue with the tutorial text?

@kvakvs
Copy link
Collaborator Author

kvakvs commented Aug 29, 2019

The tutorial panel is not refreshed that is why it seems like it doesn't change language, and honestly it shouldn't because it is already on screen. If we want it to change language the moment we change TM:PE language, we have to reactivate the tutorial on the same page again. If you click anything in TM:PE menu, the new language is shown.
Also yes, if a translation is not found, it resorts to English, and then it resorts to the Translation Key. So test with languages other than English.

TM:PE button changes its text, i added an update call for that 4 days ago.
TTL "Light mode" button is a texture, always English and only localized for JP and PL languages.
TTL "Counter" button is a texture, always English and only localized for JP and PL languages.


No action taken

Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some feedback above below.

TLM/TLM/UI/Translation.cs Outdated Show resolved Hide resolved
TLM/TLM/UI/Translation.cs Outdated Show resolved Hide resolved
TLM/TLM/UI/Translation.cs Outdated Show resolved Hide resolved
TLM/TLM/UI/Translation.cs Outdated Show resolved Hide resolved
@originalfoo
Copy link
Member

Tested in game and it works great. The pre-existing issues are still there, but IMO we can fix up most of that stuff over time as we overhaul bits of UI.

Can confirm that missing translations now show in English rather than locale keys which is a good enhancement for end users.

Changing languages also seems faster than the old implementation too :)

I don't know if it's possible, but I suspect there are lots of dead strings in the lang file that aren't used any more (eg. stuff from much earlier versions of the mod). Is there a way to identify obsolete strings and/or cull them?

@kvakvs
Copy link
Collaborator Author

kvakvs commented Aug 30, 2019

One way to find dead strings would be to filter all source files for Translate.GetString calls, extract those strings and create the Ultimate List of Every Used Key.
That'd be like 2 commands in Linux shell.
And then finding the intersection can probably be done in Excel. Open the translations CSV, paste the keys we've just extracted, and figure out how to intersect 2 sets of data in Excel.
Or just sort them and put side to side.

@originalfoo
Copy link
Member

I think it's worth doing, as this is ideal point to do it. That way we know we're stating with new format and also a cleaned set of translations.

It would likely also be a good point, seeing all those keys, to do the switch to better key format?

@kvakvs
Copy link
Collaborator Author

kvakvs commented Aug 30, 2019

Switching key format is a tedious task, it needs searching for every key and deciding where it'll go and what name it gets. Decide on format and do at least those from TTL, screaming for a rename

@kvakvs
Copy link
Collaborator Author

kvakvs commented Aug 30, 2019

102 unique strings

Translation.GetString("...")
Translation.GetString("Adaptive_step_switching")
Translation.GetString("Add")
Translation.GetString("Add_junction_to_timed_light")
Translation.GetString("Add_step")
Translation.GetString("After_min._time_has_elapsed_switch_to_next_step_if")
Translation.GetString("Allow_all_vehicles")
Translation.GetString("avg._flow")
Translation.GetString("avg._wait")
Translation.GetString("Ban_all_vehicles")
Translation.GetString("Cancel")
Translation.GetString("Clear_Traffic")
Translation.GetString("Copy")
Translation.GetString("default") + ")
Translation.GetString("Default_speed_limit") + ":")
Translation.GetString("Default_speed_limits")
Translation.GetString("Delete")
Translation.GetString("Deselect_all_nodes")
Translation.GetString("Display_speed_limits_mph")
Translation.GetString("down")
Translation.GetString("Driving_to_another_parking_spot")
Translation.GetString("Driving_to_a_parking_spot")
Translation.GetString("Edit")
Translation.GetString("Enable_test_mode_(stay_in_current_step)")
Translation.GetString("Entering_vehicle")
Translation.GetString("Extreme_long_green/red_phases")
Translation.GetString("Extreme_short_green/red_phases")
Translation.GetString("flow_ratio")
Translation.GetString("Hide_counters")
Translation.GetString("High")
Translation.GetString("incoming")
Translation.GetString("Incoming_demand")
Translation.GetString("Invert")
Translation.GetString("Keybind_category_Global")
Translation.GetString("Keybind_category_LaneConnector")
Translation.GetString("Keybind_conflict")
Translation.GetString("Keybind_Exit_subtool")
Translation.GetString("Keybind_lane_connector_delete")
Translation.GetString("Keybind_lane_connector_stay_in_lane")
Translation.GetString("Keybind_toggle_TMPE_main_menu")
Translation.GetString("Keybind_toggle_traffic_lights_tool")
Translation.GetString("Keybind_use_junction_restrictions_tool")
Translation.GetString("Keybind_use_lane_arrow_tool")
Translation.GetString("Keybind_use_lane_connections_tool")
Translation.GetString("Keybind_use_priority_signs_tool")
Translation.GetString("Keybind_use_speed_limits_tool")
Translation.GetString("Kilometers_per_hour")
Translation.GetString("Lane")
Translation.GetString("Lane_Arrow_Changer_Disabled_Connection")
Translation.GetString("Lane_Arrow_Changer_Disabled_Highway")
Translation.GetString("Long_green/red_phases")
Translation.GetString("Looking_for_a_parking_spot")
Translation.GetString("Low")
Translation.GetString("Max._Time:")
Translation.GetString("Miles_per_hour")
Translation.GetString("min/max")
Translation.GetString("Min._Time:")
Translation.GetString("Moderate_green/red_phases")
Translation.GetString("Node_is_level_crossing")
Translation.GetString("NODE_IS_TIMED_LIGHT")
Translation.GetString("Node") + " " + t + "\n")
Translation.GetString("Outgoing_demand")
Translation.GetString("Paste")
Translation.GetString("Remove_junction_from_timed_light")
Translation.GetString("Remove_this_citizen")
Translation.GetString("Remove_this_vehicle")
Translation.GetString("Remove_timed_traffic_light")
Translation.GetString("Road_type") + ":")
Translation.GetString("Rotate_left")
Translation.GetString("Rotate_right")
Translation.GetString("Save")
Translation.GetString("Save") + " & " + Translation.GetString("Apply")
Translation.GetString("Segment")
Translation.GetString("Select_junction")
Translation.GetString("Select_nodes")
Translation.GetString("Select_nodes_windowTitle")
Translation.GetString("Sensitivity")
Translation.GetString("Setup_timed_traffic_light")
Translation.GetString("Short_green/red_phases")
Translation.GetString("Show_counters")
Translation.GetString("Show_lane-wise_speed_limits")
Translation.GetString("Skip")
Translation.GetString("Speed_limits")
Translation.GetString("Speed_limit_unlimited")
Translation.GetString("Start")
Translation.GetString("State")
Translation.GetString("Stop")
Translation.GetString("Switch_view")
Translation.GetString("Thinking_of_a_good_parking_spot")
Translation.GetString("Timed_traffic_lights_manager")
Translation.GetString("Traffic_Manager_detected_incompatible_mods")
Translation.GetString("Unsubscribe")
Translation.GetString("up")
Translation.GetString("Using_public_transport")
Translation.GetString("Vehicle_restrictions")
Translation.GetString("Vehicles_may_enter_blocked_junctions")
Translation.GetString("Very_long_green/red_phases")
Translation.GetString("Very_short_green/red_phases")
Translation.GetString("View")
Translation.GetString("wait_ratio")
Translation.GetString("Walking")
Translation.GetString("Walking_to_car")

@kvakvs
Copy link
Collaborator Author

kvakvs commented Sep 14, 2019

@aubergine10 the stuff's ready for testing
Except a few strings which could be moved elsewhere I am quite happy with it.
Missing translations/edits can be addressed later.

@originalfoo
Copy link
Member

Looking good from quick glance. I need to do some more testing and will report back any issues I find.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Sep 19, 2019

what are the outstanding issues? I am not fixing pre-existing issues. We've fixed the string names and missing translations issues. And some testing was done too. What we do with it now? Can we deliver or fix and deliver?
I've noticed some new translations been suggested, we can approve these and I can update the strings (easy download and unzip in Resources/Translations).

@originalfoo
Copy link
Member

I might have botched some merge conflicts on my side, but noticed the issues below - can anyone else confirm?

Lane connector button tooltip:
image

Toggle despawn button tool tip:
image

Junction restrictions tool tip:
image

Gameplay options:
image

Policies options:
image

Overlays options:
image

Maintenance tab:
image

Everything else I checked looked OK (in English at least).

@kvakvs
Copy link
Collaborator Author

kvakvs commented Sep 19, 2019

These can be discovered visually by looking into exported Excel tables. English column should never be empty, because it will be shown if the other language column is empty. If both other language and English columns were empty, you will get the key back, sometimes even with that ¶ (pilcrow) sign.

@originalfoo
Copy link
Member

Fixed issue with junction restrictions tooltip (was missing Tooltip: prefix on translation site) and updated csv files to latest build which seems to have resolved all the other issues.

Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@originalfoo
Copy link
Member

@kvakvs
Copy link
Collaborator Author

kvakvs commented Sep 20, 2019

In ModConflicts "Checkbox:Ignore disabled mods"
In SpeedLimits "General.Theme.Option:..." (multiple strings), "General.Dropdown:Road signs theme mph"
have the key copied in some of the languages. I am not sure why this is happening, we've done multiple transforms of the translations on the way like exporting, reimporting, moving and renaming strings, so probably there's a bug in Crowdin somewhere which we could report. Anyway, the key is copied in some languages there and those columns should be cleared, or the string possibly could be restored from the legacy strings files.

I think one can do some simple Excel filter highlighting cells in every line if cell in column A has same value. I can do it later today, unless you feel like doing it sooner.
P.S. Might as well remove legacy translation TXT files, if anyone ever needs to recover pieces from them, they are in Git.
P.P.S. I think Crowdin does this if the translation is empty string, the original key is copied. I now have enabled "Skip untranslated strings" and let's see how it affects the export.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Sep 20, 2019

String "Toolip:Toggle main menu" has a typo in the key.

@originalfoo
Copy link
Member

They are easy to spot so I'll just edit the csv files

@originalfoo
Copy link
Member

Looks like changing options on crowdin solved it - just rebuilt and the duplicates seem to be removed.

Pushed a commit with latest versions, also removed old lang files.

Let me know if there are any remaining issues.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Sep 20, 2019

I am very happy with how it turned out.
Wait for krzychu124 ? Or merge away? :)

@originalfoo
Copy link
Member

I think merge, it's had lots of testing.

@kvakvs kvakvs merged commit 1773b4d into CitiesSkylinesMods:master Sep 20, 2019
@krzychu124
Copy link
Member

Yeah, I've tested it yesterday and surprisingly I didn't spotted any issue 😉

TianQiBuTian added a commit to TianQiBuTian/Cities-Skylines-Traffic-Manager-President-Edition that referenced this pull request Oct 4, 2019
kvakvs pushed a commit that referenced this pull request Oct 4, 2019
* Fix #509 Lane arrow Error info
* Remove csproj file changed.
TianQiBuTian added a commit to TianQiBuTian/Cities-Skylines-Traffic-Manager-President-Edition that referenced this pull request Oct 4, 2019
And delete one superfluous "full stop"
kvakvs pushed a commit that referenced this pull request Oct 12, 2019
* Fix #509 Main menu Despawning text
And delete one superfluous "full stop"
* Revert CSV file.
* Updated translations
* Update translations; Fix translation keys; New string for keybind not set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Refactor code, remove old code, improve maintainability Localisation Localised text and features UI User interface updates under-review A pull request has been created and is currently being reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Text doesn't change when language updated
3 participants