Dead end connection #1613

kianzarrin · 2022-07-15T16:09:49Z

fixes #1213: Connecting a lane to itself creates dead end

it looks like this:

Note: as it was the case already: if a lane cannot have a possible target, then laneEnd node maker will not be displayed.

data base:

I store it as a lane connection to the same lane.

lane arrows:

no lane arrows for dead ends
BONUS: lane arrows are reset when all outgoing lane connections are removed form the lane.

when user hovers over same lane :

it draws a lane curve to the same lane:
any other existing lane connections would hide to indicate what is the post condition of the action.
no mouse cursor is displayed to avoid confusion.

kvakvs · 2022-07-16T11:13:40Z

Tested and i love it.
The mouse cursor on loop lane does not change to (+)

split overlay code into smaller methods use better render order overDrawCode is more readable and consistent.

kianzarrin · 2022-08-12T22:02:57Z

The mouse cursor on loop lane does not change to (+)

its intentional. should it change to + ? Creating dead end removes all existing connections.

kvakvs · 2022-08-12T22:06:30Z

The mouse cursor on loop lane does not change to (+)

its intentional. should it change to + ? Creating dead end removes all existing connections.

Maybe use some road sign like 🚫⛔ (easy to confuse with minus) or ❌or something

kianzarrin · 2022-08-12T22:32:22Z

@kvakvs Maybe use some road sign like 🚫⛔ (easy to confuse with minus) or ❌or something

I think there is no need for mouse sign in here. it only makes things more confusing.

kvakvs

Posting updated X texture plus editable SVG source, please drop them together in the same directory

kvakvs · 2022-08-14T20:33:52Z

TLM/TLM/Manager/Impl/LaneConnection/LaneConnectionSubManager.cs

@@ -423,6 +452,15 @@ public class LaneConnectionSubManager :
            return true;
        }

+        private void AssertLane(uint laneId, bool startNode) {


As this function ends with Assert call, is that a debug assert? And in release will it do the calculations but no assert?
Declare this function as [Conditional("DEBUG")]?

its only called when adding/removing lane connections so its not performance critical. therefore I decided to include it in release build. you never know .... it might help with debugging user errors.

Should I make it conditional?

I mean the assert function is marked debug only. i believe it is. So in release this code does nothing, just calculates values and returns.

Assert is not marked as debug.
AssertNotNone is the only one that is marked as debug. its inconsistent with all the rest of the assert functions.
but that does not matter. I think its a good idea to execute assert in release mode here.

I removed the [Conditional("DEBUG")] from AssertNotNone() and instead added #if DEBUG around the usage. so now its consistent

kvakvs · 2022-08-14T20:36:40Z

TLM/TLM/Manager/Impl/LaneConnection/LaneConnectionSubManager.cs

-            if (!HasOutgoingConnections(laneId, startNode)) {
+            const bool RESET = false; // reset lane arrows when last lane connection is removed.
+
+            if (RESET && !HasOutgoingConnections(laneId, startNode)) {


This if (false && is probably a source of a compiler warning

it does not (and should not) cause compiler warnings. its a legit thing to do.
we do that all the time when logging:

#if DEBUG private bool verbose_ => DebugSwitch.LaneConnections.Get(); #else private const bool verbose_ = false; #endif if(verbose_ ) ....

but I understand if you feel like its unnecessary code

kvakvs · 2022-08-14T20:46:57Z

DaEgi01

just some small things. can't speak much for the logic itself since i m not deep enough in the details. krzy should def. check it out.

DaEgi01 · 2022-08-25T12:22:26Z

TLM/TLM/Manager/Impl/LaneConnection/LaneConnectionSubManager.cs

-            canConnect =
-                IsDirectionValid(ref sourceNetLane, sourceLaneInfo, nodeId, true) &&
-                IsDirectionValid(ref targetNetLane, targetLaneInfo, nodeId, false);
+            canConnect = IsDirectionValid(ref sourceNetLane, sourceLaneInfo, nodeId, true);


ternary expression for the win

notice the &= operator. won't fit well with ternary.

true! I didn't see that at first.
wouldn't it be better to encapsulate the logic in the isdirectionvalid method itself?
since the method with the change now only does a "90% job".

TLM/TLM/Manager/Impl/LaneConnection/LaneConnectionSubManager.cs

DaEgi01 · 2022-08-25T12:45:52Z

TLM/TLM/Manager/Impl/LaneConnection/LaneConnectionSubManager.cs

-            if (sourceLaneId == targetLaneId) {
-                return false;
-            }
+            bool deadEnd = sourceLaneId == targetLaneId;


i'd move it down where its used first. just a small thing.

DaEgi01 · 2022-08-25T12:47:56Z

TLM/TLM/Manager/Impl/LaneConnection/LaneConnectionSubManager.cs

@@ -392,8 +393,36 @@ public class LaneConnectionSubManager :
                }
            }

+            var connections = GetLaneConnections(sourceLaneId, sourceStartNode);


this could be its own method right?

what can be its own method?

basically the whole block that was added.
than again, I can't think of a good name, probably because these are actually two different things that are happening here based on deadEnd. I'd do it the other way around.
if (deadEnd)
removeNonDeadEndConnections()
else
removeDeadEndConnection()
and move the laneId loop inside.

or something like that.

it would be two one line functions (unless if you count in the verbose log). Not sure this is a good idea.

DaEgi01 · 2022-08-25T12:51:51Z

TLM/TLM/Manager/Impl/LaneConnection/LaneConnectionSubManager.cs

-            if (!HasOutgoingConnections(laneId, startNode)) {
+            const bool RESET = false; // reset lane arrows when last lane connection is removed.
+
+            if (RESET && !HasOutgoingConnections(laneId, startNode)) {


I also dont get the point of this code. i understand it for #ifdef but whats the point here?

cool I remove it.

DaEgi01 · 2022-08-25T19:07:30Z

TLM/TLM/Manager/Impl/LaneConnection/LaneConnectionSubManager.cs

@@ -423,6 +452,15 @@ public class LaneConnectionSubManager :
            return true;
        }

+        private void AssertLane(uint laneId, bool startNode) {


probably AssertDeadEndConnection would be a better method name.

I think i should also add IsValidWithSegment() too. i mean if we are asserting stuff we might as well do it right.

krzychu124 · 2022-09-21T22:47:10Z

@kianzarrin dead end icon is not visible if you close tool and open again.

avoided code duplication to prevent such issues in future.

…r selected lane.

krzychu124

Code looks ok, I hadn't got a chance to test it in game yet.

kianzarrin · 2022-09-27T08:55:03Z

Code looks ok, I hadn't got a chance to test it in game yet.

should I wait?

krzychu124 · 2022-09-27T09:16:23Z

Yeah, you can wait, I'll do a quick test later today. Do you have any other plans with this feature or something else?

kianzarrin · 2022-09-27T09:31:03Z

nope this is the last one in the series of the lane connection EPIC issue #1479.
There are also few optional issues over there about minor ascetics improvements but I can't fix them.

krzychu124

Backspace/Delete does not reset lane arrows on dead-end lanes just like removing dead-end manually does

kianzarrin · 2022-09-29T10:31:17Z

OK I reset all lane arrow for node regardless of whether or not it had lane connection.

krzychu124

Everything seems to be working now

dead end

c62e1d8

kianzarrin self-assigned this Jul 15, 2022

kianzarrin added 3 commits July 15, 2022 19:10

e43fdda

fill color for dead end lane curve

60919b3

e41dd64

lane connection code cleanup

4f961a0

split overlay code into smaller methods use better render order overDrawCode is more readable and consistent.

kianzarrin mentioned this pull request Jul 17, 2022

lane connection code cleanup #1615

Merged

kianzarrin added 2 commits July 17, 2022 19:11

c533aae

merge

58fa219

kianzarrin changed the base branch from master to LCM-Cleanup July 17, 2022 19:46

kianzarrin added 3 commits July 17, 2022 22:51

e03aaef

1680031

render dead ends for all nodes

c36c4b0

Base automatically changed from LCM-Cleanup to master August 11, 2022 20:38

kianzarrin added 2 commits August 12, 2022 19:21

merge

b3bee3e

fixed merge

bd1ab6a

don't create lane transition to same lane.

2792e1a

bug fix: don't remove dead end cache when connecting to dead end lane.

d8c4e6e

kianzarrin requested review from krzychu124, kvakvs and chameleon-tbn August 12, 2022 22:33

kianzarrin marked this pull request as ready for review August 12, 2022 22:33

kvakvs approved these changes Aug 14, 2022

View reviewed changes

kianzarrin added 2 commits August 15, 2022 00:24

updated x texture and svg

b418f7b

bugfix: load dead end conncetion

a92d189

bugfix: show dead end sign on one way roads when refreshing node.

78816d9

DaEgi01 self-requested a review August 20, 2022 11:19

DaEgi01 reviewed Aug 25, 2022

View reviewed changes

kianzarrin added 2 commits September 7, 2022 22:44

pr fixes

9559eb6

Merge branch 'master' into LCM-dead-end

23beaee

kianzarrin requested a review from DaEgi01 September 7, 2022 22:36

krzychu124 added this to the 11.7.1 milestone Sep 14, 2022

kianzarrin added 3 commits September 23, 2022 00:12

Merge branch 'master' into LCM-dead-end

eb48b16

fixed car dead ends are not rendered after reentering the tool

78fefac

avoided code duplication to prevent such issues in future.

fixed hiding lane connections that would be removed when hovering ove…

c207794

…r selected lane.

krzychu124 approved these changes Sep 26, 2022

View reviewed changes

krzychu124 changed the title ~~dead end connection~~ Dead end connection Sep 26, 2022

krzychu124 self-requested a review September 28, 2022 21:09

krzychu124 requested changes Sep 28, 2022

View reviewed changes

kianzarrin added 3 commits September 29, 2022 12:48

Merge branch 'master' into LCM-dead-end

34b4ee7

reset selected lane when resting node to avoid minor visual glitch.

f6e8709

reset lane arrows when clearing node lane connections.

a9c4e95

kianzarrin requested a review from krzychu124 September 29, 2022 10:30

krzychu124 approved these changes Sep 29, 2022

View reviewed changes

kianzarrin merged commit 5ae6ed4 into master Sep 30, 2022

kianzarrin deleted the LCM-dead-end branch September 30, 2022 10:22

krzychu124 mentioned this pull request Oct 2, 2022

What's new 11.7.1.0 #1664

Merged

1 task

Dead end connection #1613

Dead end connection #1613

Conversation

kianzarrin commented Jul 15, 2022 • edited Loading

kvakvs commented Jul 16, 2022

kianzarrin commented Aug 12, 2022

kvakvs commented Aug 12, 2022

kianzarrin commented Aug 12, 2022

kvakvs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kianzarrin Aug 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvakvs commented Aug 14, 2022

DaEgi01 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DaEgi01 Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzychu124 commented Sep 21, 2022

krzychu124 left a comment

Choose a reason for hiding this comment

kianzarrin commented Sep 27, 2022

krzychu124 commented Sep 27, 2022

kianzarrin commented Sep 27, 2022

krzychu124 left a comment

Choose a reason for hiding this comment

kianzarrin commented Sep 29, 2022

krzychu124 left a comment

Choose a reason for hiding this comment

kianzarrin commented Jul 15, 2022 •

edited

Loading

kianzarrin Aug 14, 2022 •

edited

Loading

DaEgi01 Aug 25, 2022 •

edited

Loading