Half overlay

[kianzarrin]

fixes #548 :
When hovering over a segment with the lane arrow tool only half of the segment is highlighted letting the user know which half is being modified.

fixes #555: I took the opportunity to fix this related issue.
I Removed buggy/redundant/weird code .
node highlights have the average radius of all connected segments see #555 (comment)

EDIT by aubergine: Just adding image of the 'half overlay' so reviewers know what to look for:

EDIT: fixed a bug regarding Alt+Click on a (LHT / RHD) map (see ,#564 (comment))

[originalfoo]

Found one small issue (with normal lane arrow tool selected):

That half of the segment does not contain any outgoing lanes (it's the 'incoming end' of a one way road).

While clicking has no effect, it would be better if the overlay was not displayed when the segment half has no outgoing lanes (ie. tool not applicable).

Edit: Alternatively, it could show red 'half overlay' instead of the usual blue color. Not sure which would be best approach (if tool not applicable: no overlay, or different color)?

[kianzarrin]

Found one small issue (with normal lane arrow tool selected):
While clicking has no effect, it would be better if the overlay was not displayed when the segment half has no outgoing lanes (ie. tool not applicable).
Edit: Alternatively, it could show red 'half overlay' instead of the usual blue color. Not sure which would be best approach (if tool not applicable: no overlay, or different color)?

Wow great observation! This was already the problem without my half segment highlights. Its a Good idea to fix it as part of PR.

I like the Idea of the red overlay too. currently I already use red for when segment selected and ALT is down in my previous commit. So I think I need to change that too.

if (altDown && HoveredSegmentId == SelectedSegmentId)
    color = MainTool.GetToolColor(true, true);

[originalfoo]

I already use red for when segment selected and ALT is down in my previous commit

Looks orange to me (when Alt is held down while hovering segment, or Ctrl is held down while hovering a node).

[originalfoo]

With this update I see only the highlighted half of the segment gets updated when Alt+Click which is much better.

I wonder if it would be worth adding that toggle feature I discussed in #548? So Alt+Click toggles the lane between turning lane or vanilla lane depending on it's current state.

[originalfoo]

Possible bug with Alt+Click: I'm on a left hand traffic (LHT / RHD) map, so this turning lane choice seems wrong (it would cut across oncoming traffic):

Although maybe I'm not understanding it correctly?

In UK, the dedicated turning lane is usually the one of the left, with the lane to the right going ahead+right. Here are some examples...

At a junction:

At a roundabout (where 'turning lane' means 'you must exit roundabout at next junction'):

We even cut up bus lanes to make room for turning lanes:

I suspect it may be different in other countries (ie. not purely dependent on the driving side). As such, a mod option may be beneficial at some point? (would have to think carefully how to word it)

[originalfoo]

Further to my comment above about which side should get the turning lane, I noticed that if there are three lanes to choose from vanilla automatically chooses these lane arrows:

And in that situation an Alt+Click does change the left lane in to a turning lane (which seems correct):

(sorry, I know this is getting off topic of the PR, but thought I'd mention it just in case)

[originalfoo]

Maybe when holding Alt key, we should highlight the half-lane that will be affected (make it really obvious to use which lane will change), rather than the half-segment?

See #391 (comment) for example of lane highlighting.

[kianzarrin]

Maybe when holding Alt key, we should highlight the half-lane that will be affected (make it really obvious to use which lane will change), rather than the half-segment?

I am not sure if that is practical. The calculations that needs to be done in each game click will be too much (unless if i cache it I guess). In another PR maybe.

Mean while I have done this:
Pictures below respectively:
hover
ALT hover
hover (or ALT hover) with no outgoing lanes
selected segment
selected segment + ALT hover

hmmm. I am going to change the color of alt hover to be strong blue and different from selected segment.

[kianzarrin]

I ended up changing the alpha:
hover:

ALT hover:

[kianzarrin]

@aubergine10 Sorry I missed some of your messages

Looks orange to me (when Alt is held down while hovering segment, or Ctrl is held down while hovering a node).

Not any more. I already fix this.

I wonder if it would be worth adding that toggle feature I discussed in #548? So Alt+Click toggles the lane between turning lane or vanilla lane depending on it's current state.

I suspect it may be different in other countries (ie. not purely dependent on the driving side). As such, a mod option may be beneficial at some point? (would have to think carefully how to word it)

In the last PR we talked about state machine. I created #567 to address it.

Possible bug with Alt+Click: I'm on a left hand traffic (LHT / RHD) map, so this turning lane choice seems wrong (it would cut across oncoming traffic):

Nice catch! that is a bug. ~~I think its more fitting to fix it in #567 . ~~
EDIT: scrap that. I fixed it.

EDIT:

left hand traffic (LHT / RHD)

Damn it I have been using LHD in my code to refer to British roads.:

bool lhd = LaneArrowManager.Instance.Services.SimulationService.LeftHandDrive;

in my defense its the same terminology as the interface:

        public bool LeftHandDrive =>
            Singleton<SimulationManager>.instance.m_metaData.m_invertTraffic
            == SimulationMetaData.MetaBool.True;

[originalfoo]

Damn it I have been using LHD in my code to refer to British roads

I regularly make similar mistake, confusing the four terms: LHT, RHD, RHT, LHD

See my comment in #567 regarding 'nearside' and 'farside' terminology which is driving direction agnostic.

[kianzarrin]

After playing around with the highlights I can still spot some caveats. Please put this review on halt until I fix them. I will continue the discussion about colors and other stuff in #548.
So far I have the following problems:

• I want to improve the colors again
• Hovering highlights half segment but selection highlights the hole thing. this is inconsistent.
• null exception occurs when I select one segment but ALT-hover over another (possibly GUI Error: System.NullReferenceException: Object reference not set #563.
• The length of the highlighted segment is not always half the segment, this can confuse the user because when he hovers the mouse over the would-be highlighted area of the segment, the highlight does not come up.

EDIT: sorry for the inconvenience.

[originalfoo]

null exception occurs when I select one segment but ALT-hover over another (possibly #563).

I'm not sure it is as user is reporting the exception when save loads or, if not then, when saving a game. Also, we've had similar error reported (see #566) for TM:PE STABLE v10.20 which doesn't even contain these features. I suspect something in base game or steam common distro has changed which is triggering some of the errors reported.

That being said, good catch on the null exception when hovering other half of selected segment!

[originalfoo]

Ah, I see - the other end of that short node in your GIF doesn't actually have lane arrows, despite being "junction node", because it's 1-way street. Good catch!

So: Only half-highlight if there are lane arrows at both end of segment, otherwise highlight whole segment.

[krzychu124]

Yes, both segments are 1-way streets, but different hover behavior, which might annoy user and provoke missclicks - you can't click on red highlighted segment (OK) but on that weird blue, sure, then lane arrows window will show up near the other node(I am pretty sure user won't expect that).

[kianzarrin]

@krzychu124

Yes, both segments are 1-way streets, but different hover behavior, which might annoy user and provoke missclicks - you can't click on red highlighted segment (OK) but on that weird blue, sure, then lane arrows window will show up near the other node.

Good catch. I will fix it. :)

[originalfoo]

Would it be worth removing the red highlight also?

For color vision impaired users, distinguishing between red and blue might be difficult (I'd have to read through my notes on #287 to check).

Maybe instead of red highlight, just don't highlight at all. That way it's simple UX: If it highlights, you can click it to do something, otherwise it's not relevant to the selected tool.

[kianzarrin]

@aubergine10

For color vision impaired users, distinguishing between red and blue might be difficult (I'd have to read through my notes on #287 to check).

Good idea! Makes my job easier too :) . In any case I will fix the problem @krzychu124 raised because fixing it also makes my code shorter.

One more case. What if we have a junction with one way out. changing lane arrows don't make any sense as there is only one direction to go to. So maybe we shouldn't highlight it?

I don't know how to take U turn into consideration.

That said it is possible to have lane arrows toward wrong direction for example :
step 1- manually override lane arrows to go to a road.
step 2- delete that road.

in such circumstances you can use lane arrow tool to fix the arrows.

It would be nice if we could automatically remove lane arrows toward lane direction. Also we should not allow the user to put lane arrows toward wrong directions.

So talking all that into consideration do you think I should allow selection of cases where there is only one way out?

[kvakvs]

Code looks good.
Did not try in game.

[originalfoo]

One more case. What if we have a junction with one way out. changing lane arrows don't make any sense as there is only one direction to go to. So maybe we shouldn't highlight it?

Agreed. Possibly related issues: #344

I don't know how to take U turn into consideration.

U-turns are a mess at the moment, there's like 2 or 3 ways to do them (off the top of my head: Lane connectors, and junction restrictions). We should only have one way to implement u-turns. I think there's already some issues tracking that. Possibly related issues: #364, #347, #365, #293

That said it is possible to have lane arrows toward wrong direction for example :

Yup, this is a general problem with all the tools. We need to have a think as to how to handle that situation better and, in particular, make sure the user realises their road changes just screwed up their customisations. Possibly related issues: #368, #457, #43

Will test this PR again later this evening if time permits.

[originalfoo]

Should train tracks be excluded? They have no visible lane arrows so any changes made would be invisible to user, which would result in difficult to diagnose problems with rail routing. I suspect this is more general issue with the lane arrows tool.

Same with tram tracks:

And monorail:

And metro:

There's some flags (#513 for example) that indicates what vehicles can use the network, so you could filter to just those that allow cars?

[originalfoo]

Regarding that stuff above about filtering out non-car networks, that could be a separate PR if desired as it's general issue of the lane arrow tool and not specific to the work you've been doing.

The remaining comments from @krzychu124 are more important for this PR.

[originalfoo]

See my comment above.

[kianzarrin]

@aubergine10

The remaining comments from @krzychu124 are more important for this PR.

Yes that one is fixed already.

Regarding that stuff above about filtering out non-car networks, that could be a separate PR if desired as it's general issue of the lane arrow tool and not specific to the work you've been doing.

Good idea. This PR is already growing out of control. Created #585.

If there are no bugs in this code lets merge this in before the code gets out of date.

[kianzarrin]

I am ready for the next round of this PR

[originalfoo]

LGTM from usability perspective 👍

Usability relating to other networks being tracked separately in #585.

@krzychu124 will still need to check over the stuff he commented on, which should now be fixed, prior to merge.

[krzychu124]

Finally 😃 👍