CitiesSkylinesMods / TMPE

Cities: Skylines Traffic Manager: President Edition
https://steamcommunity.com/sharedfiles/filedetails/?id=1637663252
MIT License
576 stars 85 forks source link

Lane arrow tool: Alt+click only updates half segment #548

Closed originalfoo closed 4 years ago

originalfoo commented 4 years ago

I have this road (just thrown together for testing):

image

The section of road in middle (highlighted above with Move It mod) is a single segment.

I open lane arrow tool and Alt+Click the segment in middle which gives this:

image

It's only updated one end of the segment (on the right). I was expecting it to update both ends?

If I Alt+Click a second time, it updates the other end:

image

originalfoo commented 4 years ago

@kianzarrin is this intended behaviour?

kianzarrin commented 4 years ago

Yes this is intended behavior. what if the user wants to update one end not the other? Also clicking on the road brings up the the control panel only for half of the road so the UI is consisten.

originalfoo commented 4 years ago

Ok, that makes sense.

Based on the tooltip, as an end-user I was expecting both ends of segment to update:

tooltip

Alt+Click: Separate turning lanes for segment

Does not convey what actually happens:

Separate turning lanes for nearest end of segment, unless they are already separate in which case do the other end as well.

I'm not sure if there's any way to clarify the situation without making the tooltip excessively long.

As there will always need to be two clicks to do both ends, would it be worth considering an alternative interaction model?

Specifically, what if Alt+Click would toggle separate turning lanes for nearest end of segment? Tool tip would then become:

Alt+Click: Toggle separate turning lane(s) for nearest end of segment

The benefits of this approach would be:

What do you think?

originalfoo commented 4 years ago

Note: Changes to interaction model would also impact #547

kianzarrin commented 4 years ago

@aubergine10

As there will always need to be two clicks to do both ends, would it be worth considering an alternative interaction model?

Wow! you have your junctions way too close to each other. that is not good! I never need to separate turning lanes for both ends since there is only one end connected anyway.

Some people do have junctions way too close to each other so I think it does not hurt to provide some clarification. Maybe I can highlight only half of the segment? Or provide more info on tutorial or the "?" button? or let the user to figure it out experimentally?

originalfoo commented 4 years ago

Maybe I can highlight only half of the segment?

Yes, this would help to highlight the incoherence of the interaction model.

Let's map this out:

The interaction model is incoherent.

Now, let's see what would happen if we changed it slightly so it toggles the half being clicked.

And if they want to do the other half, just move the mouse over it and it works exactly same way as first half. No more guesswork, and no need to switch to completely different interaction model if they make a mistake or later change their mind.

Wow! you have your junctions way too close to each other

My roads have very few junctions, and I avoid cross roads like the plague, preferring staggered junctions instead. ;) And if congestion occurs, I usually simplify my road network rather than making it more complex, I ask myself "What would America do?", and then do the opposite of that.

kianzarrin commented 4 years ago

@aubergine10 how is this? Screenshot (72)

originalfoo commented 4 years ago

That looks much better! Very clear which end of the road will be updated when user clicks.

originalfoo commented 4 years ago

In fact, that half-road highlight would be useful for normal lane connector arrow tool too. Because that seems to work same way.

For example, with lane connector arrow tool if I click where the 'green blob' is on this image, nothing happens, because it can't work out which end I want to edit. So your 'highlight selected half' approach would be a good improvement for this too:

image

kianzarrin commented 4 years ago

From https://github.com/krzychu124/Cities-Skylines-Traffic-Manager-President-Edition/pull/564#issuecomment-557828254 :

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:

  • [x] I want to improve the colors again
  • [x] Hovering highlights half segment but selection highlights the hole thing. this is inconsistent.
  • [x] null null-able exception occurs when I select one segment but ALT-hover over another (possibly #563).
  • [x] 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.

I forgot that I had done this intentionally out of the fear that segments that are too short (eg created by anarchy) can cause issues.

  • 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.
    
    /* I have used IsMiddle() so that if the segment is not connected to a junction on the other end 
  • then I highlight the full segment.*/ if(bStartNode & !IsMiddle(segment.m_endNode)) bezier = bezier.Cut(0, 0.5f); if (!bStartNode && !IsMiddle(segment.m_startNode)) bezier = bezier.Cut(0.5f, 1);
    
    But now this creates a new issue: there is a disconnect between the highlighted area and where the mouse needs to hover to highlight the desired segment end. To solve this I can iether:
  • get rid of the isMiddle()
  • change hoveredNodeID to the other end of hoveredSegmentID if hoveredNodeId is not a junction.

I like the last approach although it is harder to implement its more user friendly.

EDIT: I also need something more sophisticated than IsMiddle() because end of road is neither a junction nor has the middle flag.

originalfoo commented 4 years ago

change hoveredNodeID to the other end of hoveredSegmentID if hoveredNodeId is not a junction.

I like this approach but would do it slightly differently.

For example, in #391, kvakvs used white selectors to provide suggestions to user about where they could click.

So in case where user hovers over wrong half, could we highlight the other half (and maybe any other nearby clickable halves too?) in white letting them know where to click? So as they move mouse around it could highlight nearby clickable halves, or even highlight all clickable halves on the screen?

One thing is for sure: We have to have ability to handle very short segments as they are surprisingly common based on screenshots and savegames I've seen shared by the community.

I also need something more sophisticated than IsMiddle() because end of road is neither a junction nor has the middle flag.

There's load of info on various available flags (from asset creator perspective at least) here: https://cslmodding.info/asset/network/#flags

Also, an asset creator who did extensive research in to flags is whisperwalk on Steam: https://steamcommunity.com/id/whisperwalk/myworkshopfiles/?appid=255710

kianzarrin commented 4 years ago

For example, in #391, kvakvs used white selectors to provide suggestions to user about where they could click.

kvakvs situation is a bit different than mine. If I highlight all nearby clickable segments with white, wouldn't it look messy like Christmas?

kianzarrin commented 4 years ago

My color codes: light blue = hovered ALT=>strong blue (alpha =true) orange = clicked/selected ALT=> strong blue (alpha=true) red = not select-able this is consistent with rest of the UI. This wiki is the related

If there is a junction only at one end of a segment: A) full segment is highlighted B) hovering over both half of the segments highlights/selects/modifies the half with the junction. So input and output are consistent now.

also fixed the nullable error I got. it was indeed my fault.

I am going to show to you what I did to the colors in a animated picture because too many pictures are confusing

ezgif com-optimize

originalfoo commented 4 years ago

If there is a junction only at one end of a segment:
A) full segment is highlighted

I would still find that confusing, as only one end will get updated; so show the end that gets updated (half segment highlight) not the full segment would be better IMO.

I am going to show to you what I did to the colors

Do we have color codes or a class containing them or something? I'd like to add details to style guide.

Ideally we should have something along these lines so we're specifying states and the colors are determined by those states (that way update to the color for a state updates everything that uses that state):

highlight.color = StateColors.hover; // altHover, click, invalid, etc....
kianzarrin commented 4 years ago

@aubergine10

I would still find that confusing, as only one end will get updated; so show the end that gets updated (half segment highlight) not the full segment would be better IMO.

How is it confusing?! I am suspecting you missed the fact that I only do this in circumstance that is not confusing. Please look at the GIF bellow and tell me if it looks confusing in the GIF: ezgif com-crop

kianzarrin commented 4 years ago

@aubergine10

Do we have color codes or a class containing them or something? I'd like to add details to style guide.

Timed traffic lights defines the colors like this so its coming from skylines DLLs. ~~private GUIStyle layout = new GUIStyle { normal = { textColor = new Color(1f, 1f, 1f) } }; private GUIStyle layoutRed = new GUIStyle { normal = { textColor = new Color(1f, 0f, 0f) } }; private GUIStyle layoutGreen = new GUIStyle { normal = { textColor = new Color(0f, .8f, 0f) } }; private GUIStyle layoutYellow = new GUIStyle { normal = { textColor = new Color(1f, .7f, 0f) } };~~ EDIT: DrawNodeCircle(cameraInfo, nodeId, GetToolColor(warning, false), alpha);

originalfoo commented 4 years ago

Highlighting the full segment just looks a bit weird. It's an inconsistency; sometimes half segment gets highlighted (which is really good as it makes it very clear which end is being altered) but other times its' whole segment. For consistency, IMO, it would be better to always keep it half segment, even when the other end of segment doesn't have junction; it just makes it clearer "this end of the segment is where changes will happen".

kianzarrin commented 4 years ago

@aubergine10

Highlighting the full segment just looks a bit weird. It's an inconsistency; sometimes half segment gets highlighted (which is really good as it makes it very clear which end is being altered) but other times its' whole segment.

as you can see from the GIF The highlight always looks like a sausage which is cut at one end while the other end is half circle. The length of the sausage can vary though. Therefore its always clear which end is being altered.

The concept of segment is supposed to be hidden from the user ( I for one didn't know segments existed until I started modding). All the user sees is that part of the road is being selected. I don't see the confusion.

The problem with your approach is that in cases where the segment is small - and as you told me it happens a lot - it is very difficult to chose the desired segment end. I remember it was really annoying. My approach fixes this problem

In any case @aubergine10 if you want me to highlight half segment then that's what I shall do and create a new ticket about the difficulty of selecting short segments and different ways of solving it.

originalfoo commented 4 years ago

as you can see from the GIF The highlight always looks like a sausage which is cut at one end while the other end is half circle. The length of the sausage can vary though. Therefore its always clear which end is being altered.

Ok, I think I get it now. My understanding is as follows:

In that case, if my understanding is correct, then yes, I agree with the UI.

The concept of segment is supposed to be hidden from the user

I disagree. Users build, upgrade and bulldoze roads (and all other network based entities) segment-by-segment. Most TM:PE tools are per-segment (or 'between segments'), so it is a persistent factor of the game that cannot be avoided.

kianzarrin commented 4 years ago

@aubergine10

In that case, if my understanding is correct, then yes, I agree with the UI.

Yes your understanding is correct.

originalfoo commented 4 years ago

BTW, when I was testing #564, my brain was yearning for that toggle functionality I mentioned earlier.

So, if I Alt/Ctrl+Click to separate out the turning lane, clicking a second time should revert (return that lane to whatever vanilla would normally be).

Is there any chance of squeezing that in to #564?

kianzarrin commented 4 years ago

@aubergine10

Is there any chance of squeezing that in to #564?

This is already tackling multiple issues. Not a good idea to make it bigger. The revert functionality will be handled in issue #537 phase 2: implement state machine. or as part of #568 .