reset lane arrows

[kianzarrin]

fixes #632
Added interface to reset lane arrows back to default. Ignores lanes with outgoing lane connections.

I remove lane flags, call NetAI to recalculate lanes and finally push lane changes.

UI to delete lane arrows : select lane arrow tool -> select segment end -> press Reset all button.

Tests passed:

• I tested with segment having lane connections on some lane or on all lanes. In the latter case the reset button should not show. In the first case resetting lane arrows ignores lanes with lane connections.
• I tested it with incoming and/or outgoing lane arrows on the same lane. My code can successfully distinguish between them
• I tested it with city to see cars obey reset lane arrows.

EDIT: Can someone please add this crowdin key?

 FILE:LaneRouting
"Button:Reset" => "Reset"

[kvakvs]

Minor notes.
Not tested, just quick code review.

[kvakvs]

Just the previous minor naming comments.
Otherwise looking good.

[kianzarrin]

@kvakvs Not like it matters, but do you need sorted lanes for that? Or is there a cheaper way to get all lanes?

No I don't need the lanes to be sorted but I don't have another choice.
segment.Info does have m_sortedLanes but it has not been put to use in GetSortedLanes() (optimization idea?). You are right though: this is not a performance critical part of the code. there are usually only 3 to sort lanes anyways.

[kianzarrin]

@kvakvs Also making name a bit longer is ok. IsHeadingTowardsStartNode something like this?
IsHeadStartNode stands for : is the head of the lane an start node? Or: Is the head start node?
in the doc I worded it a bit differently.
what do you think? should I change it?

[kvakvs]

@kianzarrin Name it with the correct meaning, of course, i missed there "and" in the function doc. Just don't shorten it if a few more letters make it more clear.

[originalfoo]

Segment highlight flickers when pressing reset button:

[originalfoo]

I go in to lane arrow tool and select this segment:

Looks correct. Then I click reset and initially freak because "hang on, that road can't go that way"...

But, wait.... Clicking the reset button actually selected a different road.

If I've got a segment selected, and I click reset, the already-selected segment should remain selected. I suspect the issue is related to the flickering observed in last comment.

Also, I think the whole lane arrow UI needs updating at some point. Are we making a rod for our own backs by adding reset via the UI? If we change to a new UI approach at later date, is it going to be weird for users if that reset button no longer exists?

[originalfoo]

See comments above re: flickering and also selection changing unexpetedly.

[kianzarrin]

@aubergine10 fixed it.
Unity says:

public static Rect Window(int id, Rect screenRect, GUI.WindowFunction func, string text, params GUILayoutOption[] options);

Returns Rect The rectangle the window is at. This can be in a different position and have a different size than the one you passed in.

but apparently the return value does not work when a button is being pressed.

Damn if I did not have my HotReload patch it would have taken me ages !

[originalfoo]

Will test later this evening.

Does "Reset all" mean...

• "for this segment" ? If so, less confusing caption would be "Reset"
• "for this junciton" ? If so, less confusing caption would be "Reset junction"?

[kianzarrin]

Reset all means reset everything you can see in the GUI.
OK i will change it to Reset.

[originalfoo]

This is working great now! Tested from end-user perspective, not really looked at code.

LGTM 👍

[originalfoo]

BTW, if you have inclination to do so, some additional enhancements:

1. Allow user to simply press Delete / Backspace to reset the arrows for selected segment -- would be good to have this shortcut work consistent across all tools.
2. Maybe horizontally center the Reset button?

[kianzarrin]

@aubergine10

BTW, if you have inclination to do so, some additional enhancements:

1. Allow user to simply press `Delete` / `Backspace` to reset the arrows for selected segment -- would be good to have this shortcut work consistent across all tools.

we have a GUI for resting here so there is not that much of a need. I like the idea of resting with delete but the Reset button has to give visual feedback that the user has pressed del. As you know from the other review I don't know how to use hotkeys so move it to #568

2. Maybe horizontally center the `Reset` button?

When I imagine the Reset button at the center, it may give the impression that it only belongs to the center lane. That is why I decided to put it on the side.

[originalfoo]

TBH I think at some point the lane arrows can get an overhaul so let's leave it as it is for now.

Once @kvakvs or @krzychu124 have done code review we can merge this.

[kvakvs]

Assume that entire tool workflow will change, you've seen some prototypes in the past, and assume that tool window will be redone too. Don't bother too much now.
I will keep all existing logic code where possible, but the UI will be thrown "oota windah", and made anew.

[kvakvs]

Minor notes.

[kianzarrin]

New translation is working.

[originalfoo]

This looks good, let me know if it's ok to merge - if so I'll merge today and, when #662 is merged, publish to LABS as 11.1-beta1 so users can start testing it.

[kianzarrin]

Thanks @aubergine10 please merge it.

I also added delete hotkey. very simple code using Input.KeyDown(Delete).
pressing del will turn reset green as a visual feedback. same thing happens if you click green.

More sophisticated code can be handheld as part of #568. In any case the days of this GUI is numbered.

[originalfoo]

Just noticed there's some coments from kvakvs - did those get implemented?

[krzychu124]

I am going to test it later today too :)

[kianzarrin]

All has comments from kvakvs has been addressed now.

[krzychu124]

Small issue - spam in log 🙂

[originalfoo]

@kianzarrin Can you remove the log spam as per @krzychu124 comment above

Then this can be merged.

[krzychu124]

👍