-
Notifications
You must be signed in to change notification settings - Fork 85
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
Road sign themes reloadable + Canadian signs #1215
Road sign themes reloadable + Canadian signs #1215
Conversation
-#if DEBUG guards for release Config version bump Reverted DebugIf to accept a lambda instead of string Copied and adapted UIScaler logic from Kian's ModTools SpeedLimits manager uses intent structs for set speed limit action
…to have ResetToDefault type
…ts in speedlimits panel corrected too.
…ed, and hide on tool deactivate
…ated (rendering for another tool)
…cessible if action type is Unlimited or SetOverride
…erground to InGameUtil.cs
…ections; small optimisations
…ngular vs square signs
… apply speed limit changes, its for preview only
…ver multiple signs)
…ckbox creation fixed
@@ -146,11 +146,12 @@ or ItemClass.SubService.PublicTransportMetro | |||
laneIndex++; | |||
} | |||
|
|||
if (validLanes > 0) { | |||
meanSpeedLimit = meanSpeedLimit.Scale(1.0f / validLanes); | |||
switch (validLanes) { |
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.
maybe not switch case if two cases only? :D
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.
Converted to return with validLanes == 0 ? null : ...
private static UIDropDown _roadSignsMphThemeDropdown; | ||
private static int _roadSignMphStyleInt; | ||
|
||
// [Obsolete("To be removed")] |
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.
meh .. plz remove :D
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.
Removing
@@ -338,24 +371,57 @@ public static class OptionsGeneralTab { | |||
} | |||
} | |||
|
|||
private static void OnRoadSignsMphThemeChanged(int newRoadSignStyle) { | |||
// private static void OnRoadSignsMphThemeChanged(int newRoadSignStyle) { |
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.
meh, plz remove
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.
Removing
|
||
private string PathPrefix; | ||
|
||
public bool SupportsKmph; |
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 are these public fields that can be changed from the outside?
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.
Changed to readonly, they are accessed from General options tab.
mip: true); | ||
TexturesKmph.Add(speedLimit, resource ? resource : TexturesKmph[5]); | ||
} | ||
public static RoadSignTheme ActiveTheme { |
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.
lazy loading getter suck .. but ok .. same same before.
mip: true); | ||
TexturesMphUK.Add(speedLimit, resourceUk ? resourceUk : TexturesMphUK[5]); | ||
} | ||
public const string GERMAN_KM_SIGNS = "Kmph_Germany"; |
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 this public and the rest not?
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.
It is used as default fallback theme in the Options General tab
break; | ||
} | ||
} | ||
// TODO: Split loading here into dynamic sections, static enforces everything to stay in this ctor |
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 static ctor?
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.
The class is static.
The solution would be to make it a regular class and access via SpeedLimitsTextures.Instance
a bit longer and slower.
Oh, and I noticed one more bug: cannot change language in main menu NullReferenceException: Object reference not set to an instance of an object
at TrafficManager.State.OptionsGeneralTab.OnLanguageChanged (Int32 newLanguageIndex) [0x00000] in <filename unknown>:0
at UIHelper+<AddDropdown>c__AnonStorey2.<>m__0 (ColossalFramework.UI.UIComponent c, Int32 sel) [0x00000] in <filename unknown>:0
at ColossalFramework.UI.UIDropDown.OnSelectedIndexChanged () [0x00000] in <filename unknown>:0
at ColossalFramework.UI.UIDropDown.set_selectedIndex (Int32 value) [0x00000] in <filename unknown>:0
at ColossalFramework.UI.UIDropDown.PopupSelectedIndexChanged (ColossalFramework.UI.UIComponent component, Int32 si) [0x00000] in <filename unknown>:0
at ColossalFramework.UI.UIListBox.OnSelectedIndexChanged () [0x00000] in <filename unknown>:0
at ColossalFramework.UI.UIListBox.set_selectedIndex (Int32 value) [0x00000] in <filename unknown>:0
at ColossalFramework.UI.UIListBox.OnMouseDown (ColossalFramework.UI.UIMouseEventParameter p) [0x00000] in <filename unknown>:0
at ColossalFramework.UI.UIInput+MouseHandler.ProcessInput (IInputTranslator translator, Ray ray, ColossalFramework.UI.UIComponent component, Boolean retainFocusSetting) [0x00000] in <filename unknown>:0
at ColossalFramework.UI.UIInput.ProcessMouseInput () [0x00000] in <filename unknown>:0
at ColossalFramework.UI.UIInput.Update () [0x00000] in <filename unknown>:0 |
…for crash on language change from main menu
Not sure if in scope of this PR (can move to the scope creep issue if preferred) but... Still testing this; I'm finding the choice of speed unit vs. sign theme to be uncomfortable. IMO it would be better with a single drop down list. In mod options:
And on the speed palette, clicking the speed unit button could show the drop down list? |
@aubergine10 Yes the drop down list is listed in your suggestions in #1221 |
Yup, that would be fine so long as units are clearly shown in the list items. |
… for MPH theme; Updated strings
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.
fc66449 @kvakvs have you tested that? It still does not work because in the main menu there is no instance of ModUI 😉
Also this when switching between Km/h and Mph
NullReferenceException: Object reference not set to an instance of an object
at TrafficManager.U.Panel.BaseUWindowPanel.OnDestroy () [0x00000] in <filename unknown>:0
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'm 99.9% sure you should destroy Window gameObject at TMPE/TLM/TLM/UI/SubTools/SpeedLimits/SpeedLimitsTool.cs Lines 135 to 145 in f2b571f
|
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.
Finally! XD
Features
NOTE: This is based on Speed Limits branch so the change set may look very large.
⚠ REVIEW SHORT DIFF AFTER SPEED LIMITS ARE MERGED
TODO