Speed Limit tool - Lane highlighting #709

kianzarrin · 2020-02-17T09:02:33Z

fixes #682

highlight the affected lanes when user hovers speed limit sprite.

highlight color is blue. when mouse button is pressed the highlight is emboldened and becomes orange.
If in per lane mode one lane is highlighted. holding shift highlights that lane on the entirety of the road.
if in directional mode EDIT:all lanes in that direction are highlighted. they highlights are much thicker so that they would merge together as one. holding shift will highlight all affected lanes on that road. appropriate side of the segment is highlighted.

EDIT: I made major changes:

~~TrafficManagerTool has a new method to render only one side of a segment see screenshots~~
restored priority sings tool dust bin sprite from old TMPE. I renamed it to clear.png and put it in a central place to be accessed from multiple places. (Priority Signs: Interaction model is whack #77)
added new button to restore speed limits to default value. it uses clear.png sprite.
created back-end to reset speed limit to default state.
the UI panel has a new toggle to apply changes to entire road. This was handled with shift modifier (which still works)
pressing shift will toggle "entire road" mode. Pressing the toggle has the same effect. if the toggle is active, shift will deactivate multi-lane mode. When user pressed/releases shift, the toggle gets checked/unchecked to give visual feedback.
ShowPerLane toggle works in the same way as "entire road" toggle but with CTRL key.
The text of the toggles show the modifier key.

EDIT: I forgot to mention I introduced a couple methods/properties to Lane/Sagement traverse data structures and put some commonly used code in there.

Test:

I tested both with LHT and RHT
I tried swaping start and end node of segments
I tired inverting the segments.

changed highlight color

originalfoo · 2020-02-17T10:40:34Z

It went a bit bonkers on LHT map (mouse was over the icon at top of pic, but lanes at bottom of pic were highlighted):

originalfoo · 2020-02-17T10:42:12Z

Interestingly, when I hold down Ctrl key for lane-mode, it was highlighting correct lanes.

Seems the inversion issue is only affecting direction-mode.

originalfoo · 2020-02-17T10:44:31Z

In cases where two icons are hovered at same time in lane-mode, only one lane gets highligted:

I use the little trick of hovering the overlap in icons to set both at same time quite often.

EDIT: Same in direciton mode:

originalfoo · 2020-02-17T10:47:33Z

I suspect this is due to direction inversion issue on LHT maps - it's one way roundabout, in direction mode:

…lDirection instead of laneInfo.m_direction

kianzarrin · 2020-02-18T10:18:32Z

I replaced m_direction with m_finalDirection. This is fixed now.

I need to develop a test check list because I keep forgetting to test in all possible circumstances.

kvakvs

The amount of if (something) conditions is getting out of hand.
Is not a blocker for this PR but there should be a way to reduce nesting and reduce the cyclomatic complexity (amount of code branching to keep in head while reading) for the future code.

kvakvs · 2020-02-18T12:34:42Z

TLM/TLM/UI/Helpers/SegmentLaneMarker.cs

@@ -9,6 +9,7 @@ internal class SegmentLaneMarker {
        }

        internal Bezier3 Bezier;
+        internal float size = 1.1f;


is this a const? Can it change?

it does change.

kvakvs · 2020-02-18T12:42:21Z

TLM/TLM/UI/SubTools/SpeedLimits/SpeedLimitsTool.cs

@@ -200,12 +312,15 @@ public SpeedLimitsTool(TrafficManagerTool mainTool)
                }

                // no speed limit overlay on selected segment when in vehicle restrictions mode
-                DrawSpeedLimitHandles(
+                hover|= DrawSpeedLimitHandles(


Missing a space.

kianzarrin · 2020-02-18T14:20:13Z

@kvakvs The amount of if (something) conditions is getting out of hand.

"cyclomatic complexity" ... interesting concept.

how should I do this though? move branches into sub methods?

Note that when the user holds shift, FPS drops because the code goes though a big ass slow for loop. as an optimization effort I created a separate code path for when shift is not pressed so that in most cases FPS is high. I haven't performed any benchmark though. I could do some caching too but the FPS seems fine on my slow computer so I didn't bother.

kvakvs · 2020-02-18T14:29:30Z

If you later revisit this code, yes, please split logic into functions and reduce the nesting.
You will notice that a lot of values are carried between calls and those represent state of this logic flow and maybe deserve their own storage class or struct. The resulting code will be simpler and that is good.

originalfoo · 2020-02-18T16:07:30Z

There's another performance hit with speed limits -- and maybe other tools, haven't checked -- in that to facilitate user being able to 'drag a speed' along a series of segments... this: #388 (not relevant to this PR but something to consider for future performance tuning)

originalfoo

Tested in-game, LGTM from end-user perspective 👍

originalfoo · 2020-02-18T17:57:06Z

Need to do some more testing on this; found some issues with parking lane higlight in other PR and need to see if issues also apply to speed limits.

originalfoo · 2020-02-18T18:30:13Z

Retested, only one minor issue found - if two signs hovered at same time, only one lane is highlighting (yet both lanes would be updated if I clicked):

originalfoo

If two icons hovered at same time, please highlight all affected lanes not just lanes for one icon.

kianzarrin · 2020-02-20T10:36:23Z

improved highlighting of segment side:

kianzarrin · 2020-02-20T10:38:55Z

@aubergine10 If two icons hovered at same time, please highlight all affected lanes not just lanes for one icon.

this is more like a bug (you view it as a trick). In any case I increased the distance of the sprites so they do not overlap any more.

originalfoo · 2020-02-20T14:00:59Z

I'd suggest Reset instead of Clear ("Clear" is a bit ambiguous; might give impression the speed limit is being removed rather than reset to whatever its default is).

kvakvs · 2020-02-20T14:03:33Z

Do not bake keys into translation strings on Crowdin, just do + "[Del]" for now and i can deal with it later.

kianzarrin · 2020-02-20T15:49:04Z

@aubergine10 ! I renamed clear to to default. The screenshot are a commit too old.

@kvakvs Do not bake keys into translation

Of course . if you look at the code you will notice its not part of the translation.

originalfoo · 2020-02-20T17:44:03Z

Is there a way to constrain the highlight to where the lanes are - currently it goes over pavement and sometimes cuts in to other side of road too:

Road in screenshots is asymmetric road from UK Roads - Small Roads

originalfoo · 2020-02-20T17:47:59Z

Sometimes it looks a bit weird, eg. when apply to whole road and there's a corner in the road:

originalfoo · 2020-02-20T17:53:22Z

2-lane 1-way road, only half road highligted (direction mode):

originalfoo · 2020-02-23T23:54:36Z

@kianzarrin Did you get chance to update the direction-mode lane highlights? Everything else in this PR is good to go.

kianzarrin · 2020-02-24T14:28:49Z

I restored the old lane highlighting method.

On bend nodes I am either going to get this (currently this is what we get):

or this:

Unless if I do some super clever stuff which is a can of worms I do not want to open right now.

kianzarrin · 2020-02-24T14:31:01Z

I tested this on LHT with invert flag on and off and both directions of start/end nodes.

originalfoo · 2020-02-24T14:42:13Z

For the corners, I prefer the gap rather than the weird overlap.

Will test latest commits now...

originalfoo · 2020-02-24T15:44:33Z

Wrong translation key on the reset button in speed palette?

Other than that, looks good.

kianzarrin · 2020-02-24T21:58:59Z

Crowdin translations are in and i changed Checkbox:Default to Button:Default
@aubergine10 are you happy?

originalfoo

LGTM 👍

kianzarrin added 3 commits February 15, 2020 22:58

lane highlight for speed limits.

b2b8a5d

Merge remote-tracking branch 'main/master' into 682-speedlimit-overlay

164f74d

improved direction.

cbf5f83

changed highlight color

originalfoo mentioned this pull request Feb 17, 2020

Parking Restrictions tool - Lane highlighting #708

Merged

originalfoo added Overlays Overlays, data vis, etc. SPEED LIMITS Feature: Speed limits UI User interface updates Usability Make mod easier to use labels Feb 17, 2020

originalfoo assigned kianzarrin Feb 17, 2020

originalfoo self-requested a review February 17, 2020 12:36

originalfoo added this to the 11.1.1 milestone Feb 17, 2020

fixed speedLimitTool LHT highlight direction by using laneInfo.m_fina…

711c666

…lDirection instead of laneInfo.m_direction

kvakvs approved these changes Feb 18, 2020

View reviewed changes

originalfoo changed the title ~~speedlimit lane highlight~~ Speed Limit tool - Lane highlighting Feb 18, 2020

originalfoo approved these changes Feb 18, 2020

View reviewed changes

originalfoo added the DO NOT MERGE YET Don't merge this PR, even if approved, until further notice label Feb 18, 2020

originalfoo requested changes Feb 18, 2020

View reviewed changes

originalfoo removed the DO NOT MERGE YET Don't merge this PR, even if approved, until further notice label Feb 18, 2020

kianzarrin added 2 commits February 19, 2020 14:38

9b3bcb4

2ff2a34

fixed clear sign

49a33ef

added clear.png

c096475

kianzarrin mentioned this pull request Feb 20, 2020

Vehicle Restrictions tool - lane highlight and better UI panel #721

Merged

This was referenced Feb 24, 2020

UI: Highlight lane when hovering over icons associated with lane #33

Closed

Changes to long roads with lots of junctions #138

Closed

kianzarrin added 2 commits February 24, 2020 16:29

restored old method of highlighing.

2648a62

Merge branch 'master' into 682-speedlimit-overlay

1412b20

kianzarrin added 2 commits February 24, 2020 16:37

merged with trunk

3b03434

fixed merge

a23fdad

kianzarrin requested a review from originalfoo February 24, 2020 14:42

added crowdin translations

3b5c658

originalfoo approved these changes Feb 24, 2020

View reviewed changes

kianzarrin merged commit 3d5a286 into CitiesSkylinesMods:master Feb 24, 2020

kianzarrin deleted the 682-speedlimit-overlay branch February 24, 2020 22:58

This was referenced Feb 25, 2020

Shift route highlight for other tools #667

Closed

Multiple Set speed limit calls when holding mouse button while changing speed limit #388

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Speed Limit tool - Lane highlighting #709

Speed Limit tool - Lane highlighting #709

kianzarrin commented Feb 17, 2020 •

edited

originalfoo commented Feb 17, 2020

originalfoo commented Feb 17, 2020 •

edited

originalfoo commented Feb 17, 2020 •

edited

originalfoo commented Feb 17, 2020

kianzarrin commented Feb 18, 2020

kvakvs left a comment

kvakvs Feb 18, 2020

kianzarrin Feb 18, 2020

kvakvs Feb 18, 2020

kianzarrin commented Feb 18, 2020

kvakvs commented Feb 18, 2020

originalfoo commented Feb 18, 2020

originalfoo left a comment

originalfoo commented Feb 18, 2020

originalfoo commented Feb 18, 2020

originalfoo left a comment

kianzarrin commented Feb 20, 2020

kianzarrin commented Feb 20, 2020

originalfoo commented Feb 20, 2020

kvakvs commented Feb 20, 2020

kianzarrin commented Feb 20, 2020

originalfoo commented Feb 20, 2020 •

edited

originalfoo commented Feb 20, 2020

originalfoo commented Feb 20, 2020

originalfoo commented Feb 23, 2020

kianzarrin commented Feb 24, 2020

kianzarrin commented Feb 24, 2020

originalfoo commented Feb 24, 2020 •

edited

originalfoo commented Feb 24, 2020

kianzarrin commented Feb 24, 2020 •

edited

originalfoo left a comment

Speed Limit tool - Lane highlighting #709

Speed Limit tool - Lane highlighting #709

Conversation

kianzarrin commented Feb 17, 2020 • edited

originalfoo commented Feb 17, 2020

originalfoo commented Feb 17, 2020 • edited

originalfoo commented Feb 17, 2020 • edited

originalfoo commented Feb 17, 2020

kianzarrin commented Feb 18, 2020

kvakvs left a comment

Choose a reason for hiding this comment

kvakvs Feb 18, 2020

Choose a reason for hiding this comment

kianzarrin Feb 18, 2020

Choose a reason for hiding this comment

kvakvs Feb 18, 2020

Choose a reason for hiding this comment

kianzarrin commented Feb 18, 2020

kvakvs commented Feb 18, 2020

originalfoo commented Feb 18, 2020

originalfoo left a comment

Choose a reason for hiding this comment

originalfoo commented Feb 18, 2020

originalfoo commented Feb 18, 2020

originalfoo left a comment

Choose a reason for hiding this comment

kianzarrin commented Feb 20, 2020

kianzarrin commented Feb 20, 2020

originalfoo commented Feb 20, 2020

kvakvs commented Feb 20, 2020

kianzarrin commented Feb 20, 2020

originalfoo commented Feb 20, 2020 • edited

originalfoo commented Feb 20, 2020

originalfoo commented Feb 20, 2020

originalfoo commented Feb 23, 2020

kianzarrin commented Feb 24, 2020

kianzarrin commented Feb 24, 2020

originalfoo commented Feb 24, 2020 • edited

originalfoo commented Feb 24, 2020

kianzarrin commented Feb 24, 2020 • edited

originalfoo left a comment

Choose a reason for hiding this comment

kianzarrin commented Feb 17, 2020 •

edited

originalfoo commented Feb 17, 2020 •

edited

originalfoo commented Feb 17, 2020 •

edited

originalfoo commented Feb 20, 2020 •

edited

originalfoo commented Feb 24, 2020 •

edited

kianzarrin commented Feb 24, 2020 •

edited