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

Ensure a valid langauge is always used #579

Merged
merged 6 commits into from
Dec 2, 2019

Conversation

originalfoo
Copy link
Member

@originalfoo originalfoo commented Nov 29, 2019

This fixes an issue where user might select a new language, then revert to older version of mod that doesn't have that language; it would try using the previously selected language, not find it, and crash.

image

This PR ensures default language will be used in any case where selected language is not available.

Note, however, this will obviously only fix the issue from version 11.0-alpha11 and above. There is no way to retroactively fix it on older versions of the mod.

This fixes an issue where user might select a new language, then revert to older version of mod that doesn't have that language; it would try using the previously selected language, not find it, and crash. The update ensures default language will be used in any case where selected language is not available.
@originalfoo originalfoo added Localisation Localised text and features technical Tasks that need to be performed in order to improve quality and maintainability 11 ALPHA TM:PE v11 alpha edition labels Nov 29, 2019
@originalfoo originalfoo added this to the 11.0 milestone Nov 29, 2019
@originalfoo originalfoo self-assigned this Nov 29, 2019
@krzychu124
Copy link
Member

Unfortunately your fix doesn't change anything 😉 You can make quick test: change manually your current language code to xx in Global config and run the game 😃

@originalfoo
Copy link
Member Author

Ah, I should have also checked the method below... will update PR in a few moments after doing some extra testing.

@krzychu124
Copy link
Member

krzychu124 commented Nov 29, 2019

I suggest adding new method to Translation class which will set current code to null if (currentCode != null && currentCode != "en" && GetValidLanguageId(currentCode) == "en") Save and reload Global config and call this method as first in method which builds options menu

@originalfoo
Copy link
Member Author

Here is stack trace, will update PR once fixed.

KeyNotFoundException: The given key was not present in the dictionary.
  at System.Collections.Generic.Dictionary`2[System.String,System.Collections.Generic.Dictionary`2[System.String,System.String]].get_Item (System.String key) [0x00000] in <filename unknown>:0 
  at TrafficManager.UI.Localization.LookupTable.Get (System.String lang, System.String key) [0x00000] in <filename unknown>:0 
  at TrafficManager.UI.Localization.LookupTable.Get (System.String key) [0x00000] in <filename unknown>:0 
  at TrafficManager.State.OptionsGeneralTab.T (System.String key) [0x00000] in <filename unknown>:0 
  at TrafficManager.State.OptionsGeneralTab.MakeSettings_General (TrafficManager.UI.Helpers.ExtUITabstrip tabStrip) [0x00000] in <filename unknown>:0 
  at TrafficManager.State.Options.MakeSettings (UIHelperBase helper) [0x00000] in <filename unknown>:0 
  at TrafficManager.TrafficManagerMod.OnSettingsUI (UIHelperBase helper) [0x00000] in <filename unknown>:0 
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0 
Rethrow as TargetInvocationException: Exception has been thrown by the target of an invocation.
  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0 
  at System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) [0x00000] in <filename unknown>:0 
  at OptionsMainPanel.AddUserMods () [0x00000] in <filename unknown>:0 
UnityEngine.DebugLogHandler:Internal_LogException(Exception, Object)
UnityEngine.DebugLogHandler:LogException(Exception, Object)
UnityEngine.Logger:LogException(Exception, Object)
UnityEngine.Debug:LogException(Exception)
OptionsMainPanel:AddUserMods()
OptionsMainPanel:CreateCategories()
OptionsMainPanel:Awake()
UnityEngine.Object:Internal_CloneSingle(Object)
UnityEngine.Object:Instantiate(GameObject)
ColossalFramework.UI.UIDynamicPanels:Init(UIView)
ColossalFramework.UI.UIView:Awake()

Also updated global config docs in wiki

@krzychu124
Copy link
Member

It might look a bit different on current Labs and Stable 😉

If not, do extra validation and correct the global config language.
@originalfoo
Copy link
Member Author

originalfoo commented Nov 29, 2019

In latest commit, I made following changes:

  • Internal IsLanguageCodeVerified, defaults to false, tracks whether language is validated in the current TM:PE session
  • When calling GetCurrentLanguage(), if we've not yet verified the language code it does all the relevant checks, updating GlobalConfig.Instance.LanguageCode in the process (to a valid language code)
    • Note: It does not update the .xml file on disk at this point; although it will if user makes other changes to mod options; this way if user switches back to more recent version, there is chance they will retain their chosen language
    • Subsequent calls to GetCurrentLanguage() shortcut to just returning GlobalConfig.Instance.LanguageCode as they know it's been validated by that point

@originalfoo
Copy link
Member Author

Also, I didn't get any errors on TM:PE STABLE (and I assume LABS will be similar).

I think the issue only occurs on TM:PE 11.0-alpha7 to 11.0-alpha10. In other words, the error was specific to the new localisation system, as far as I can tell.

Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@krzychu124 krzychu124 self-requested a review November 29, 2019 20:11
@originalfoo
Copy link
Member Author

@kvakvs While I'm in here editing stuff, what is the purpose of GetMenuWidth() in Translation.cs? And how were the magic numbers derived? I'd like to add a code comment just to clarify that for anyone editing in future.

@kvakvs
Copy link
Collaborator

kvakvs commented Nov 30, 2019

@aubergine10 This is legacy function, I did not create it. It is used by DebugMenuPanel.[Start, CreateTextField, CreateButton]

Found an event that's fired on game locale change (thanks Klyte45), and hooked in to General options tab code for when user alters langauge setting in TM:PE options, so now the validation checking only occurs when necessary.
@originalfoo
Copy link
Member Author

Committed some changes that hopefully fix the outstanding issues.

It now works as follows:

  • When first used, it will do full verification
  • If user changes game locale, it will first check if TM:PE is using game language, and if so it will do the game language-specific verification
  • If user changes TM:PE language setting, it will update as applicable (game language or specific TM:PE language) doing verification in the process
  • After any change to language (game or TM:PE) the current, verified language code, is cached so translation .Get() calls no longer incur per-call verification overheads.

Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

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

Looks and works good now 👍

@originalfoo
Copy link
Member Author

@kvakvs any comments before I merge this?

@kvakvs
Copy link
Collaborator

kvakvs commented Dec 2, 2019

The version jump from alpha4 to alpha11 is confusing, otherwise i'm ok with that

@originalfoo originalfoo merged commit ad8aa6d into master Dec 2, 2019
@originalfoo originalfoo deleted the pr-handle-missing-language branch December 2, 2019 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
11 ALPHA TM:PE v11 alpha edition Localisation Localised text and features technical Tasks that need to be performed in order to improve quality and maintainability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants