uni-directional lane connections #1211

kianzarrin · 2021-12-02T11:09:10Z

fixes #784. please see issue for more details.
Edit: as a happy coincident this change also fixes the issue with lane connector tool that got confused with bidirectional lanes.

every connection made by lane connector used to be bidirectional. Now its uni-directional unless user creates connection both ways.

highlights:

moved connection data out of obsolete Flags class and into LaneConnectionManager class.
we now need to differentiate between HasConnections() and HasOutGoingConnections().
for the sake of backward compatibility, when I load legacy save I restore the connections both ways.
I did not change the Routing Manager except for renaming a variable name to be more accurate.

code design: There are 3 layers:
UI layer (Lane Connector Tool) : no modification was needed.
Middle layer (Lane Connection Manager) : performs all the validations and checks before passing calls to base layer
Base Layer (used to be Flags but now is ConnectionData): handles getting/setting lane connection data only

build: https://ci.appveyor.com/api/projects/krzychu124/tmpe/artifacts/TMPE.zip?branch=bidirectional-fix-784

the commits are a mess and I was unable to fix them. just review all the commits together.

TODO:

create disabled backward lane connections to speed up the search in HasConnections
move some stuff from ConnectionData abstraction layer to lane connection manager
? use dictionary instead of array.
test all

…inesMods/TMPE into bidirectional-fix-784

kianzarrin · 2021-12-02T11:10:10Z

The code is complete. this is draft just because I want to perform a few more tests.

TLM/TLM/Manager/Impl/LaneConnectionManager.cs

kianzarrin · 2021-12-02T20:36:36Z

OK I done my tests. deleting other segments and bidirectional lanes work well.

For some reason the value of `Options.disableDespawning` was historically inverted during de/serialization. This PR fixes that. It's based in part on new `Version` field that Kian put in PR #1211.

krzychu124

Another round of questions :)

krzychu124 · 2022-03-12T22:59:23Z

TLM/TLM/Manager/Impl/LaneConnectionManagerData/LaneEnd.cs

+                return (int)(LaneId * 2);
+            else
+                return (int)(LaneId * 2 + 1);
+        }


Here is generated version:

public override int GetHashCode() { unchecked { return ((int)LaneId * 397) ^ StartNode.GetHashCode(); } }

Add also equals and Comparer to speed it up even more (autogenerated):

public bool Equals(LaneEnd other) { return LaneId == other.LaneId && StartNode == other.StartNode; } public override bool Equals(object obj) { return obj is LaneEnd other && Equals(other); } public static bool operator ==(LaneEnd left, LaneEnd right) { return left.Equals(right); } public static bool operator !=(LaneEnd left, LaneEnd right) { return !left.Equals(right); } private sealed class LaneIdStartNodeEqualityComparer : IEqualityComparer<LaneEnd> { public bool Equals(LaneEnd x, LaneEnd y) { return x.LaneId == y.LaneId && x.StartNode == y.StartNode; } public int GetHashCode(LaneEnd obj) { unchecked { return ((int)obj.LaneId * 397) ^ obj.StartNode.GetHashCode(); } } } public static IEqualityComparer<LaneEnd> LaneIdStartNodeComparer { get; } = new LaneIdStartNodeEqualityComparer();

Plus make struct : IEquatable<LaneEnd>

why would manually implementing them would speed up code? the auto-generated code seems fine.

krzychu124 · 2022-03-12T23:01:36Z

TLM/TLM/Manager/Impl/LaneConnectionManagerData/ConnectionDataBase.cs

+    using TrafficManager.Util;
+    using TrafficManager.Util.Extensions;
+
+    internal class ConnectionDataBase : Dictionary<LaneEnd, LaneConnectionData[]> {


Add constructor to use correct Comparer since the whole thing is a Dictionary:

public ConnectionDataBase(): base(LaneEnd.LaneIdStartNodeComparer) { }

TLM/TLM/Manager/Impl/LaneConnectionManagerData/LaneEnd.cs

TLM/TLM/Manager/Impl/LaneConnectionManagerData/LaneConnectionData.cs

TLM/TLM/Manager/Impl/LaneConnectionManagerData/ConnectionDataBase.cs

krzychu124 · 2022-03-12T23:32:56Z

TLM/TLM/Manager/Impl/LaneConnectionManager.cs

-            int nodeArrayIndex = startNode ? 0 : 1;
-            return Flags.laneConnections[laneId][nodeArrayIndex];
+            var key = new LaneEnd(laneId, startNode);
+            if (connectionDataBase_.TryGetValue(key, out var targets)) {


I would replace var with the concrete type here :)

TLM/TLM/Manager/Impl/LaneConnectionManager.cs

TLM/TLM/State/Configuration.cs

originalfoo · 2022-03-13T01:46:13Z

TLM/TLM/State/Configuration.cs

@@ -257,6 +287,13 @@ public class ExtCitizenData {
            }
        }

+        public const int CURRENT_VERSION = 1;


Change to 2 due to me robbing v1 in PR #1463

done
I suppose previous intention was to manage version using this:

private const string DATA_ID = "TrafficManager_v1.0"; private const string VERSION_INFO_DATA_ID = "TrafficManager_VersionInfo_v1.0";

Now we have inconsistent code.

Yeah, that older string-based way of doing it was grim; your int Version approach is far easier to work with... beause it's actually usable lol.

I guess if we moved to XML we need to also change the data ID.

originalfoo · 2022-03-13T02:12:42Z

TLM/TLM/Manager/Impl/LaneConnectionManager.cs

-
-            return Flags.laneConnections[sourceLaneId] != null &&
-                   Flags.laneConnections[sourceLaneId][nodeArrayIndex] != null;
+            return HasOutgoingConnections(sourceLaneId, IsHeadingTowardsStartNode(sourceLaneId));


Although not touched by this PR yet, there might be some issues with IsHeadingTowardsStartNode() to consider as it checks specifically for NetInfo.Direction.Forward (taking in to account inverted etc).

What happens if direction is Both

Or any of the new Avoid things? - as detailed in Issues with station tracks #1337 (comment)

Also, that method uses foreach which is about 2x slower than for loop IIRC?

its only used by lane arrow manager.

its not performance critical

I would leave it like it is. Issue with Avoid flags was before and normal roads where user can manager lane connections are not available so I'm not sure if it's worth to improve that in this PR. IMO a bit out of scope

kianzarrin · 2022-03-13T10:45:11Z

@krzychu124 if the code is acceptable can you please approve it before more merge conflicts appear?

originalfoo · 2022-03-13T18:02:27Z

There will be merge conflict after #1465 which fixes issue with the new Version property in SerializableDataExtension

krzychu124 · 2022-03-13T19:21:29Z

And also @aubergine10 planned to do a bit more stuff in 11.6.5.2 (probably some of them will be pushed for later anyways) before releasing TEST as stable and moving to next "major version".
I don't plan any changes to lane connector code, and if there will be any necessary (for supporting Aircrafts) your PR will be merged first anyways.
Personally, I don't plan any fixes for 11.6.5.2 as I'm focused on support for Airports DLC/Aircrafts in general

originalfoo · 2022-03-13T20:06:24Z

IMO we should be looking to push 11.6.5.1 to STABLE as soon as we're happy it's got no major issues.

I can change 11.6.5.2 milestone to 11.6.6.0 for this PR and continuation of options refactor.

Does this PR facilitate what @Elesbaan70 is planning for the displaced traffic lights stuff?

Elesbaan70 · 2022-03-13T20:18:48Z

Does this PR facilitate what @Elesbaan70 is planning for the displaced traffic lights stuff?

It does not. But based on further discussions on Discord, that may be moot, so I'd say it's up to @krzychu124 to decide if that matters.

kianzarrin · 2022-03-13T20:57:00Z

@krzychu124 Can I merge now?

krzychu124

ok

kianzarrin added 12 commits December 1, 2021 23:11

fix: routing manager checks if target lane is connected to source.

47d9b49

2300a75

456f42f

lane connections are now unidirectional

32def46

bug fixes

bc89c30

HasConnections() and HasOutgoingConnections() are different

1685d17

bugfix: removing lane connection

62e12d3

polish

eec184d

e15db8f

lane connections are now unidirectional

d5766e4

HasConnections() and HasOutgoingConnections() are different

cf28318

Merge branch 'bidirectional-fix-784' of https://github.com/CitiesSkyl…

3b7093e

…inesMods/TMPE into bidirectional-fix-784

kianzarrin added BUG Defect detected LANE ROUTING Feature: Lane arrows / connectors labels Dec 2, 2021

kianzarrin requested review from kvakvs, DaEgi01 and krzychu124 December 2, 2021 11:09

kianzarrin self-assigned this Dec 2, 2021

originalfoo reviewed Dec 2, 2021

View reviewed changes

TLM/TLM/Manager/Impl/LaneConnectionManager.cs Outdated Show resolved Hide resolved

originalfoo reviewed Dec 2, 2021

View reviewed changes

TLM/TLM/Manager/Impl/LaneConnectionManager.cs Outdated Show resolved Hide resolved

kianzarrin added 2 commits December 2, 2021 19:24

pr changes

4bd37e1

fixed removing incoming connections when another segment is deleted.

6adbc8c

kianzarrin marked this pull request as ready for review December 2, 2021 20:32

kianzarrin requested a review from originalfoo December 2, 2021 20:36

kianzarrin added 3 commits December 2, 2021 22:38

54cb28d

d2cdeb6

fixed save/load and legacy load

265013f

kianzarrin added this to the 11.7.0 milestone Dec 3, 2021

26f9656

originalfoo mentioned this pull request Mar 12, 2022

Un-invert Options.disableDespawning data byte #1463

Merged

krzychu124 reviewed Mar 12, 2022

View reviewed changes

originalfoo reviewed Mar 13, 2022

View reviewed changes

TLM/TLM/Manager/Impl/LaneConnectionManager.cs Outdated Show resolved Hide resolved

originalfoo reviewed Mar 13, 2022

View reviewed changes

kianzarrin added 4 commits March 13, 2022 03:48

review changes

52313d2

7a6a226

Merge branch 'master' into bidirectional-fix-784

71ee627

merge version

730790b

originalfoo reviewed Mar 13, 2022

View reviewed changes

kianzarrin added 2 commits March 13, 2022 12:31

Merge remote-tracking branch 'origin/master' into bidirectional-fix-784

94355cc

57284b6

kianzarrin requested a review from krzychu124 March 13, 2022 10:44

merge

b7a9e8f

originalfoo modified the milestones: 11.7.0, 11.6.5.2 Mar 13, 2022

krzychu124 approved these changes Mar 17, 2022

View reviewed changes

Merge branch 'master' into bidirectional-fix-784

01ddc25

kianzarrin merged commit 41322ab into master Mar 19, 2022

kianzarrin deleted the bidirectional-fix-784 branch March 19, 2022 03:17

originalfoo modified the milestones: 11.6.6.0, 11.6.5.2 Mar 19, 2022

originalfoo mentioned this pull request Mar 21, 2022

What's New in TM:PE 11.6.5.2 #1480

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

uni-directional lane connections #1211

uni-directional lane connections #1211

kianzarrin commented Dec 2, 2021 •

edited

kianzarrin commented Dec 2, 2021

kianzarrin commented Dec 2, 2021

krzychu124 left a comment

krzychu124 Mar 12, 2022

kianzarrin Mar 13, 2022

krzychu124 Mar 12, 2022

krzychu124 Mar 12, 2022

originalfoo Mar 13, 2022

kianzarrin Mar 13, 2022

originalfoo Mar 13, 2022

kianzarrin Mar 13, 2022

originalfoo Mar 13, 2022

kianzarrin Mar 13, 2022

kianzarrin Mar 13, 2022

krzychu124 Mar 13, 2022

kianzarrin commented Mar 13, 2022 •

edited

originalfoo commented Mar 13, 2022

krzychu124 commented Mar 13, 2022

originalfoo commented Mar 13, 2022

Elesbaan70 commented Mar 13, 2022

kianzarrin commented Mar 13, 2022

krzychu124 left a comment

uni-directional lane connections #1211

uni-directional lane connections #1211

Conversation

kianzarrin commented Dec 2, 2021 • edited

kianzarrin commented Dec 2, 2021

kianzarrin commented Dec 2, 2021

krzychu124 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

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

kianzarrin commented Mar 13, 2022 • edited

originalfoo commented Mar 13, 2022

krzychu124 commented Mar 13, 2022

originalfoo commented Mar 13, 2022

Elesbaan70 commented Mar 13, 2022

kianzarrin commented Mar 13, 2022

krzychu124 left a comment

Choose a reason for hiding this comment

kianzarrin commented Dec 2, 2021 •

edited

kianzarrin commented Mar 13, 2022 •

edited