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

kianzarrin · 2020-02-20T16:59:29Z

Fixes #681

When user is hovering a vehicle restriction icon, appropriate lane is highlighted.
the highlight color is blue.
when user presses the icon, highlight color becomes purple to give visual feedback. I cannot use orange like other tools because that is the color of the selected segment overlay.
fixed Add lane highlight to Vehicle Restrictions tool #681 (comment) . Shift clicking an Icon only applies that Icon to the whole road .
delete key allows all vehicle (default state).

GUI panel changes:

Invert button is useless so I removed it.
holding shift will change the color of the related GUI buttons to blue as a cool visual feedback.
I believe -unlike speed limit tool - shift modifier key is not so useful here so i decided not to add a toggle button for it (in Speed Limit tool - Lane highlighting #709 I gave speed limit tool such a toggle). However for the sake of consistency with other tools, shift modifier key activates "entire road" mode.
for as long as delete key is pressed, Allow All button displays active(pressed) sprite.

Fixes #657
Fixes #667

~~EDIT: I have merged #709 branch with this so that I can use some shared code. This review will be in draft state until #709 is merged with trunk.~~

…on-highlight

…ghlight

originalfoo · 2020-02-20T18:52:01Z

Looking good so far!

kvakvs · 2020-02-22T10:46:19Z

A simple color effect maybe is possible?
Hovering displays current state of the hovered traffic type (red: currently disabled, green: enabled) and clicking briefly changes color and holds for a fraction of second in a new state?

…ghlight

kianzarrin · 2020-02-24T23:07:01Z

This is ready for review. Test result:

originalfoo · 2020-02-25T10:02:49Z

Just done a quick in-game test and it's looking good. Still need to test a few more things but so far no issues.

originalfoo

So far I've only found one issue with this PR, but it's a bit cumbersome to explain...

When a segment is selected, the vehicle restriction icons appear. (EDIT: this is not the problem heh)

If, while selecting a segment, the mouse is over the position of where one of those icons are going to appear upon segment selection, then upon segment becoming selected and icon appearing, the icon also registers the click and toggles itself.

In the image above, my mouse was over the position where the taxi icon would appear, so upon selecting the segment the taxi icon was toggled.

So segment selection should wait for mouse-up before enabling toggling of restrictions.

originalfoo · 2020-02-25T14:42:51Z

EDIT: Will create separate issue for this, it should be dealt with in separate PR.

~~Also, a very minor thing, but could we make distinction between hovered icon and non-hovered icon more obvious?~~

~~Suggestion:~~

~~Hovered icon 100% opaque (currently, it still has slight transparency if you look carefully at image above)~~
~~Non-hovered icons maybe a tiny bit smaller? (Or hovered icon a tiny bit larger?)~~

kvakvs

Code is looking good. One minor refactoring suggested.

TLM/TLM/UI/SubTools/VehicleRestrictionsTool.cs

kianzarrin · 2020-02-25T15:09:57Z

Hovered icon 100% opaque (currently, it still has slight transparency if you look carefully at image above)

@aubergine10 What about consistency with the rest of the tools. Should I do this all over?

Non-hovered icons maybe a tiny bit smaller? (Or hovered icon a tiny bit larger?)

@aubergine10 When a tool is not selected but overlay for that tool is turned on in the options, icons of that tool are always rendered but a bit smaller than usual. If I make non-hovered Icons smaller how do you distinguish them with other icons from other tools?

This requires a bit of decision making and consistency consideration so maybe create an issue about this?

kianzarrin · 2020-02-25T15:11:14Z

In the image above, my mouse was over the position where the taxi icon would appear, so upon selecting the segment the taxi icon was toggled.
@aubergine10 I might have introduced this bug. I did play with GetMouseButtonDown() and CheckClicked(). I will see if I can fix this.

originalfoo · 2020-02-25T15:20:12Z

Hovered icon 100% opaque (currently, it still has slight transparency if you look carefully at image above)

What about consistency with the rest of the tools. Should I do this all over?

Maybe as separate PR for the visual tweaks? I suspect there will already be quite a bit of variance between the tools and some of them won't really be applicable.

EDIT:

This requires a bit of decision making and consistency consideration so maybe create an issue about this?

Yes, good idea. I'll create an issue.

originalfoo · 2020-02-25T15:33:28Z

Created #739 to track ideas for hovered vs. unhovered overlay icon disambiguation.

kianzarrin · 2020-02-25T17:10:55Z

@aubergine10 I introduce the accidental click problem and then I fixed it. Also properly commented and documented relavent code so that it does not happen again and created issue #740

…s#740

originalfoo

Tested in-game, couldn't find any way to break it :)

LGTM 👍

Note: Did find one issue with tool, but unrelated to this PR: #741

kianzarrin · 2020-02-25T19:40:13Z

Can you please remove the change request @aubergine so that I can merge this?

krzychu124 · 2020-02-25T19:42:37Z

testing...

krzychu124

I'm getting error sometimes when clicking with shift

Error 456.5295541: GUI Error: System.InvalidOperationException: HashSet have been modified while it was iterated over
  at System.Collections.Generic.HashSet`1+Enumerator[System.UInt16].CheckState () [0x00000] in <filename unknown>:0 
  at System.Collections.Generic.HashSet`1+Enumerator[System.UInt16].MoveNext () [0x00000] in <filename unknown>:0 
  at TrafficManager.UI.SubTools.VehicleRestrictionsTool.ShowSigns (Boolean viewOnly) [0x00000] in <filename unknown>:0 
  at TrafficManager.UI.SubTools.VehicleRestrictionsTool.ShowGUIOverlay (ToolMode toolMode, Boolean viewOnly) [0x00000] in <filename unknown>:0 
  at TrafficManager.UI.TrafficManagerTool.OnToolGUI (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.OnToolGUI(UnityEngine.Event e)
   at ToolBase.OnGUI()

originalfoo · 2020-02-25T20:14:09Z

Hrm, upon checking the logs I was also getting that error - nothing shown on screen though:

Debug 1,165.2730200: NetService.PublishSegmentChanges(25862) called.
Debug 1,165.2753957: NetService.PublishSegmentChanges(36318) called.
Debug 1,165.2762314: NetService.PublishSegmentChanges(8990) called.
Error 1,165.2771216: GUI Error: System.InvalidOperationException: HashSet have been modified while it was iterated over
  at System.Collections.Generic.HashSet`1+Enumerator[System.UInt16].CheckState () [0x00000] in <filename unknown>:0 
  at System.Collections.Generic.HashSet`1+Enumerator[System.UInt16].MoveNext () [0x00000] in <filename unknown>:0 
  at TrafficManager.UI.SubTools.VehicleRestrictionsTool.ShowSigns (Boolean viewOnly) [0x00000] in <filename unknown>:0 
  at TrafficManager.UI.SubTools.VehicleRestrictionsTool.ShowGUIOverlay (ToolMode toolMode, Boolean viewOnly) [0x00000] in <filename unknown>:0 
  at TrafficManager.UI.TrafficManagerTool.OnToolGUI (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.OnToolGUI(UnityEngine.Event e)
   at ToolBase.OnGUI()

The NetService.PublishSegmentChanges trace was output 3 times before each logging of the GUI error.

TMPE.log

originalfoo

See error in log (comment above)

kianzarrin · 2020-02-26T15:16:18Z

@aubergine10
I tried returning it didn't work
I tried looking for what has changed during iteration, I can't find anything interesting.
I put some debug logs which suggest it is the foreach( lanes in sortedlanes) that is throwing the exception. but sortedlanes has not changed.

I am out of ideas.

This happens on master as well its not a bug that I have introduce. Lets create issue about this.
EDIT: created #744

originalfoo

LGTM 👍

The hidden error will be tackled at later date via #744

kianzarrin added 6 commits February 17, 2020 21:20

intermidiate commit

6c5119a

completed vehicle restriction lane highlighting.

3a9186d

Merge remote-tracking branch 'main/master' into 681-vehcile-restricti…

656c14e

…on-highlight

9556d1b

Merge branch '682-speedlimit-overlay' into 681-vehcile-restriction-hi…

131f6a6

…ghlight

changed highlight color to blue. polished code.

6df6730

kianzarrin self-assigned this Feb 20, 2020

kianzarrin added the enhancement Improve existing feature label Feb 20, 2020

kianzarrin requested review from krzychu124, kvakvs and originalfoo February 20, 2020 17:00

originalfoo changed the title ~~vehcile restriction lane highlight~~ Vehicle Restrictions tool - lane highlight and better UI panel Feb 24, 2020

originalfoo added UI User interface updates Usability Make mod easier to use VEHICLE RESTRICTIONS Feature: Vehicle restrictions labels Feb 24, 2020

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 25, 2020 00:49

Merge branch '682-speedlimit-overlay' into 681-vehcile-restriction-hi…

514012c

…ghlight

7c2170c

kianzarrin marked this pull request as ready for review February 24, 2020 22:59

originalfoo mentioned this pull request Feb 25, 2020

Shift route highlight for other tools #667

Closed

originalfoo requested changes Feb 25, 2020

View reviewed changes

kvakvs reviewed Feb 25, 2020

View reviewed changes

TLM/TLM/UI/SubTools/VehicleRestrictionsTool.cs Show resolved Hide resolved

originalfoo mentioned this pull request Feb 25, 2020

Improved contrast between hovered and unhovered overlay icons #739

Open

kianzarrin mentioned this pull request Feb 25, 2020

Improve handling OnClick events #740

Open

kianzarrin added 3 commits February 25, 2020 19:11

Consume check clicked in OnPrimaryClickedOverlay(). CitiesSkylinesMod…

0f408f6

…s#740

3f279c5

6e45cd3

originalfoo mentioned this pull request Feb 25, 2020

Vehicle restrictions overlay icons appear at base of hydro dam #741

Closed

originalfoo approved these changes Feb 25, 2020

View reviewed changes

kianzarrin requested a review from originalfoo February 25, 2020 19:39

kvakvs approved these changes Feb 25, 2020

View reviewed changes

krzychu124 requested changes Feb 25, 2020

View reviewed changes

originalfoo requested changes Feb 25, 2020

View reviewed changes

originalfoo mentioned this pull request Feb 26, 2020

lane connector should hide tracks that are not connected #734

Closed

6 tasks

Merge branch 'master' into 681-vehcile-restriction-highlight

13ca9b8

kianzarrin mentioned this pull request Feb 26, 2020

clicking on vehicle restriction handles throws hidden exception #744

Closed

originalfoo mentioned this pull request Feb 26, 2020

11.1.1 - Updated changelog and readme #745

Merged

originalfoo approved these changes Feb 26, 2020

View reviewed changes

krzychu124 approved these changes Feb 26, 2020

View reviewed changes

kianzarrin merged commit 51f9f9a into CitiesSkylinesMods:master Feb 26, 2020

kianzarrin deleted the 681-vehcile-restriction-highlight branch February 26, 2020 18:52

originalfoo modified the milestones: 11.6.0, 11.1.1 Jan 18, 2022

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

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

Uh oh!

Conversation

kianzarrin commented Feb 20, 2020 • edited by originalfoo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

originalfoo commented Feb 20, 2020

Uh oh!

kvakvs commented Feb 22, 2020

Uh oh!

kianzarrin commented Feb 24, 2020

Uh oh!

originalfoo commented Feb 25, 2020

Uh oh!

originalfoo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

originalfoo commented Feb 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kvakvs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kianzarrin commented Feb 25, 2020

Uh oh!

kianzarrin commented Feb 25, 2020

Uh oh!

originalfoo commented Feb 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

originalfoo commented Feb 25, 2020

Uh oh!

kianzarrin commented Feb 25, 2020

Uh oh!

originalfoo left a comment

Choose a reason for hiding this comment

Uh oh!

kianzarrin commented Feb 25, 2020

Uh oh!

krzychu124 commented Feb 25, 2020

Uh oh!

krzychu124 left a comment

Choose a reason for hiding this comment

Uh oh!

originalfoo commented Feb 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

originalfoo left a comment

Choose a reason for hiding this comment

Uh oh!

kianzarrin commented Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

originalfoo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kianzarrin commented Feb 20, 2020 •

edited by originalfoo

Loading

originalfoo left a comment •

edited

Loading

originalfoo commented Feb 25, 2020 •

edited

Loading

originalfoo commented Feb 25, 2020 •

edited

Loading

originalfoo commented Feb 25, 2020 •

edited

Loading

kianzarrin commented Feb 26, 2020 •

edited

Loading