-
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
New Speed Limits UI #1168
New Speed Limits UI #1168
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
25707ba
to
3301b4b
Compare
It's draft, should we wait for the remaining todo's or you think it's ok to review? |
The TODOs changes should be small. |
I never care for draft PR and would rather review the final one. or is it really a draft in the sense that you want some feedback first before you want to proceed? |
You're welcome to not care for another few days, or can start reviewing right away. |
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 good, UI nicely separated from the tool 👍
Will test in-game...
Couple findings:
Error 210.3279831: GUI Error: System.NullReferenceException: Object reference not set to an instance of an object
at TrafficManager.U.HelperExtensions.GetScreenRectInGuiSpace (ColossalFramework.UI.UIComponent ctrl) [0x00000] in <filename unknown>:0
at TrafficManager.UI.SubTools.SpeedLimits.SpeedLimitsTool.CreateOverlayDrawArgs (Boolean interactive) [0x00000] in <filename unknown>:0
at TrafficManager.UI.SubTools.SpeedLimits.SpeedLimitsTool.RenderGenericInfoOverlay_GUI () [0x00000] in <filename unknown>:0
at TrafficManager.UI.TrafficManagerTool.OnToolGUIImpl (UnityEngine.Event e) [0x00000] in <filename unknown>:0
at CSUtil.Commons.Log.LogToFile(System.String log, LogLevel level)
at CSUtil.Commons.Log.Error(System.String s)
at TrafficManager.UI.TrafficManagerTool.OnToolGUIImpl(UnityEngine.Event e)
at TrafficManager.UI.TrafficManagerTool.OnToolGUI(UnityEngine.Event e)
at ToolBase.OnGUI()
One thing which once I realized about some time ago, now can't be unseen and would be nice to address it:
Other minor things to consider:
|
Added the notes to the TODO list, will begin fixing one by one now. |
Nope, after loading older savegame everything is ok, but once saved and reloaded any lane/segment I touched with speed limits tool is gone. Investigating... |
Line 573 in b703cc4
limit per lane dictionary is filled with Kmph, but then while saving they are treated as GameUnits
TMPE/TLM/TLM/Manager/Impl/SpeedLimitManager.cs Lines 922 to 926 in b703cc4
Lines 893 to 899 in b703cc4
here converting again to Kmph TMPE/TLM/TLM/State/Configuration.cs Lines 21 to 23 in b703cc4
Am I missing something here? |
Thanks for investigating |
That lane was changed in previous PR, probably by accident, since the code for loading and saving data is pretty much the same. I did 20? quick saves that's why values were so big. I haven't found any new issues other than those from previous comments😃 👍 |
…to have ResetToDefault type
…ts in speedlimits panel corrected too.
…ed, and hide on tool deactivate
…ated (rendering for another tool)
Added cheaper simple changes back. Layout changes are not cheap, the UI allows some of those tricks you desire, and doesn't allow some others, so it quickly becomes messy. |
The change size was growing, and i yet have to do some other work tonight. We can return to it later, probably, or redo it a bit cleaner. |
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.
Tested:
✔️ Loading/conversion of speed customisations from v11.5.2 and v11.6.1 saves
✔️ Vanilla + Workshop roads (including some weird roads I had subscribed)
✔️ Lane vs. segment
✔️ Inside/outside central 25 tile area
✔️ km/h vs. mph speeds
✔️ Setting default speeds
✔️ OSD collision transparency
✔️ Camera angles/zooms
✔️ Road vs. tracks
✔️ Short segments
✔️ Asymmetric roads
✔️ 1-way and 2-way roads/tracks
✔️ Esc exits tool
I've not tested save/load yet as another mod is currently breaking my saves. I've not tested monorail/tram-only networks or trolleybus. Those can be tested in future PR.
If Hold Alt to show defaults is not in this branch i can add it, the change set is short for that. |
It's in (tested via latest appveyor build) |
I mean holding Alt and editing defaults, and holding Alt in defaults and editing overrides. |
Regarding minor task in OP:
Maybe:
If leaving to later PR, add to #1221 to track. |
This introduces:
TO DO and Bugs:
Get rid of the tooltip and use existing tooltip helper class, problem when UI scale is different than 100%.