Medians #243

andreharv · 2019-03-20T13:44:43Z

So I'm not sure if I am doing this request in the right place but I have modified LaneConnectorTool and RouteManager to restrict crossing over medians to change lanes. Essentially groups of lanes separated by space (over 1m) or a median will be grouped together. I also made it so that trains cannot change lanes outside of junctions. This is just a first draft so it is subject to some additions.

Fixes #89

…ween them that is not a vehicle lane or there is more than a meter of space between them.

… ideal user-creations.

FireController1847 · 2019-03-27T00:11:46Z

I did this too, is there any way you can fix your indents to be tabs instead of spaces? Then it should fix this weird issue where it changes the whole file

VictorPhilipp · 2019-03-30T01:26:39Z

TLM/TLM/Manager/Impl/RoutingManager.cs

+                uint nextLaneId = nextFirstLaneId;
+                byte nextLaneIndex = 0;
+                //ushort compatibleLaneIndicesMask = 0;
+                int[] laneGroups = new int[nextSegmentInfo.m_lanes.Length];


The underlying question is: "Can I access lane k from lane i?" or as a predicate: canAccess(i, k) -> bool
If I understand correctly your approach to answer this question is:

Determine lane group of source lane end.

For each candidate lane end of the target segment: Determine lane group

Check if lane group of source lane end matches target lane group
a. if true: allow lane transition
b: if false: disallow
("lane end" = lane with node context)

The approach requires a minimum of m * n computations (m = number of source lanes, n = number of target lanes). It could possibly be improved by exploiting transitivity but that might also be a bit of overkill. But we should at least be careful not to use more computations than needed.

VictorPhilipp · 2019-03-30T01:26:46Z

TLM/TLM/Manager/Impl/RoutingManager.cs

+                    && (l.m_vehicleType & LaneConnectionManager.VEHICLE_TYPES) != VehicleInfo.VehicleType.None).OrderBy(l => l.m_position).ToList();
+                List<NetInfo.Lane> sortedLanes = nextSegmentInfo.m_lanes.Where(l => (l.m_laneType & LaneConnectionManager.LANE_TYPES) != NetInfo.LaneType.None
+                    && (l.m_vehicleType & LaneConnectionManager.VEHICLE_TYPES) != VehicleInfo.VehicleType.None).OrderBy(l => l.m_position).ToList();
+


Issues:

Linq/Collections have terrible performance on Mac. We should avoid using them whenever it's possible.

Duplicate code (refactor to method sortLanesBy...?)

Lanes are already getting sorted by outer similar index (using Linq ^^) on line 751. Can we reuse this calculation / move the logic there? (Edit: No, that's probably not a good idea. Your references to laneInValidGroup are at the right spots now)

Note: Both lists are sorted by m_position but that does not make them comparable. m_position cannot be used to compare lanes from different segments. We need some kind of node context here (I once dubbed "segment&node"="segment end" and here we would need something like a "lane end")

Why do we store NetInfo.Lane here? lane index might be sufficient.

VictorPhilipp · 2019-03-30T01:26:51Z

TLM/TLM/Manager/Impl/RoutingManager.cs

+                List<NetInfo.Lane> sortedLanes = nextSegmentInfo.m_lanes.Where(l => (l.m_laneType & LaneConnectionManager.LANE_TYPES) != NetInfo.LaneType.None
+                    && (l.m_vehicleType & LaneConnectionManager.VEHICLE_TYPES) != VehicleInfo.VehicleType.None).OrderBy(l => l.m_position).ToList();
+
+                if (prevSegmentInfo == nextSegmentInfo) {


This is true only if source (=previous) segment is of the same type as target (=next) segment. But what about segments of different type but which are very similar (e.g. two-lane road with median vs. two-lane road with median and trees)?

VictorPhilipp · 2019-03-30T01:26:54Z

TLM/TLM/Manager/Impl/RoutingManager.cs

+                    && (l.m_vehicleType & LaneConnectionManager.VEHICLE_TYPES) != VehicleInfo.VehicleType.None).OrderBy(l => l.m_position).ToList();
+
+                if (prevSegmentInfo == nextSegmentInfo) {
+                    if (!nextIsJunction) {


I think isNextRealJunction (number of connected segments == 2) would fit better

VictorPhilipp · 2019-03-30T01:27:00Z

TLM/TLM/Manager/Impl/RoutingManager.cs

+                        for (byte prevLaneInx = 1; prevLaneInx <= prevLaneSortedIndex && prevLaneId != 0; prevLaneInx++) {
+                            NetInfo.Lane laneInfo = prevSortedLanes[prevLaneInx];
+                            if ((laneInfo.m_vehicleType & VehicleInfo.VehicleType.Car) == VehicleInfo.VehicleType.None) {
+                                prevLaneGroup++;


every non-car lane gets a distinct lane group here. This might make it impossible for trains to switch lanes at railway switches (e.g. at nodes between ground and elevated tracks).

Is this relevant for trains anyway?

IIRC isn't there some 4-lane train track assets being worked on, which will include bypass tracks for stations (can't remember exact specifics, but just flagging it up in case it's relevant).

VictorPhilipp · 2019-03-30T01:27:04Z

TLM/TLM/Manager/Impl/RoutingManager.cs

+                            if (laneInfo.m_finalDirection != prevSortedLanes[prevLaneInx - 1].m_finalDirection) {
+                                prevLaneGroup++;
+                            } else {
+                                var gap = laneInfo.m_position - prevSortedLanes[prevLaneInx - 1].m_position - (0.5f * prevSortedLanes[prevLaneInx - 1].m_width) - (0.5f * laneInfo.m_width);


Is a gap really a good indicator for lane accessibility? Why not checking whether adjacent lanes have a compatible vehicle type?

And if that's not sufficient: I believe roads with medians have some kind of m_canCrossLanes flag. But I don't know whether asset creators have used it.

IMO the m_canCrossLanes flag is not related to what this PR is trying to achieve. I discussed that flag with a few road asset authors - they tried using it but it didn't seem to suffice. From what they were saying the flag determines whether vehicle can cross oncoming traffic to get to building on other side of road, and the flag seems mainly aimed at the small or medium roads. It doesn't seem to have any effect on vehicles ability, or lack of, to cross medians. See #89 for examples of some of the roads in question.

Ok, then forget this flag. xD It would not work for multiple medians either.
Using gaps as an estimator might be a good idea eventually. But I think we should make the minimum gap configurable in TMPE_GlobalConfig.xml

Yes, it will need some trial and error to get the correct gap size. That being said, I would imagine the width of a vanilla median is a good reference point - @andreharv could you confirm?

Also, is there a way to find out what's "in" a gap? For example, if it's pavement of some kind, we know it's definitely a line not to cross. But if it's empty space (like dashed line strip down the middle of a road) that would be useful for the emergency vehicle stuff that's being developed (eg. emergency vehicles could use it, or cars could use it as temporary parking to allow emergency vehicles to get through).

BTW, there are some unusual roads that might make good test cases listed in #34 and #30.

Thinking more on this, the m_canCrossLanes flag would also need to respect the medians, so vehicles can only cross oncoming traffic if m_canCrossLanes == true and there are no medians in the way.

Aside: Would it be useful to have a page detailing the various road flags and what they do, from perspective of TMPE? CSL Modding Info website has the information from asset creator perspective, but I've seen little to nothing from the code/modding perspective as to what effects these flags actually have in pathfinder and vehicle AIs.

Also, is there a way to find out what's "in" a gap?

Like I said: We could check whether adjacent lanes have different vehicle types assigned and use that as a primary estimator. But when an impassable track is not represented by a NetLane, we need to use a fallback mechnasim (like the gap check).

Could you clarify what you mean by 'different vehicle types assigned'? Is that vehicle restrictions added by TMPE, or vehicle classes defined by base game, or something else?

I would agree in the case of, for example, a pedestrian lane (which are sometimes present on medians) but if it was just different type of road vehicle it might cause false positives. Also, we have to view this from the perspective of other types of vehicles - for example, pedestrians perceive road lanes and train tracks to be impassible medians.

Also for gap checks, is there any way to cache the info per prefab so we can avoid having to do loads of checking at runtime? If I'm reading the code correctly (which may not be the case as I'm still total noob at C#) it looks like it's trying to determine where medians are while pathfinding rather than doing some one-off up-front processing when a level is loaded. If we detect a gap, we could add a fake 'median lane' or something?

Could you clarify what you mean by 'different vehicle types assigned'? Is that vehicle restrictions added by TMPE, or vehicle classes defined by base game, or something else?

That's coming from the base game. Each lane info has a lane type (m_laneType) and (m_vehicleType) assigned. A regular car lane is represented by Vehicle/Car (lane type/vehicle type), a bus lane by TransportVehicle/Car, a tram track by Vehicle/Tram and a sidewalk by Pedestrian/None.

but if it was just different type of road vehicle it might cause false positives

When could you get false positives? If asset creators model those lanes, checking vehicle types of adjacent lanes should work. If not, we have to check for gaps.

Also for gap checks, is there any way to cache the info per prefab so we can avoid having to do loads of checking at runtime?

👍

VictorPhilipp · 2019-03-30T01:27:08Z

TLM/TLM/Manager/Impl/RoutingManager.cs

+                                    prevLaneGroup++;
+                                }
+                            }
+                        }


Calculating lane groups should be performed beforehand (maybe in RecalculateSegment, or in a new intermediate method between RecalculateSegment and RecalculateLaneEndRoutingData, or in LaneConnectionManger, or even in a new manager). The method RecalculateLaneEndRoutingData is executed twice (for start & end node) for every source lane and only the grouping of source lanes would now be performed 2*n*n times (n may can reach up to 262144, so 2 * 262144 * 262144 = 137,438,953,472 times).

VictorPhilipp · 2019-03-30T01:27:26Z

TLM/TLM/Manager/Impl/RoutingManager.cs

+                            }
+                        }
+                    }
+                }


Issues:

Duplicate code

Assuming every source lane has 2 target lanes on average: n * 2 extra computations needed (max. 262144 * 2 = 524288) which gives us a total maximum of 2*n*n + n*2 = 137,439,477,760 required calculations.

lol that looks like I was only refering the curly brackets but I was talking about the whole block :-D

VictorPhilipp · 2019-03-30T01:29:15Z

TLM/TLM/Manager/Impl/RoutingManager.cs

-								if (prevIsMergeLane && Constants.ServiceFactory.NetService.CheckLaneFlags(nextLaneId, NetLane.Flags.Merge)) {
+                                    }
+                                }
+                                bool laneInValidGroup = laneGroups[sortedLanes.IndexOf(nextLaneInfo)] == prevLaneGroup;


Now lane groups of different segments are compared. This will not always yield a correct result because m_position has been used to order lanes -> results are not comparable

indexOf & indexOf(Pointer): performance?

VictorPhilipp · 2019-03-30T01:37:59Z

TLM/TLM/Manager/Impl/RoutingManager.cs

+                                        // lane can be used by all vehicles that may disregard lane arrows
+                                        transitionType = LaneEndTransitionType.Relaxed;
+                                        if (numNextRelaxedTransitionDatas < MAX_NUM_TRANSITIONS) {
+                                            if (nextIsJunction || laneInValidGroup)


ok: vehicles that ignore lane arrows should not ignore medians

VictorPhilipp · 2019-03-30T01:38:29Z

TLM/TLM/Manager/Impl/RoutingManager.cs

-									if (numNextForcedTransitionDatas < MAX_NUM_TRANSITIONS) {
-										nextForcedTransitionDatas[numNextForcedTransitionDatas].Set(nextLaneId, nextLaneIndex, transitionType, nextSegmentId, isNextStartNodeOfNextSegment);
+                                    if (numNextForcedTransitionDatas < MAX_NUM_TRANSITIONS) {
+                                        if (nextIsJunction || laneInValidGroup)


don't know, might pose a problem. Do trains need to care for medians?

As long as trains are encouraged to remain on their same track (as implemented in v10.17) there shouldn't be a problem? Except at ends of stations that have 'bypass' tracks?

VictorPhilipp · 2019-03-30T01:40:06Z

TLM/TLM/Manager/Impl/RoutingManager.cs

+                                        nextCompatibleOuterSimilarIndices[numNextCompatibleTransitionDatas] = nextOuterSimilarLaneIndex;
+                                        compatibleLaneIndexToLaneConnectionIndex[numNextCompatibleTransitionDatas] = currentLaneConnectionTransIndex;
+                                        //compatibleLaneIndicesMask |= POW2MASKS[numNextCompatibleTransitionDatas];
+                                        if (nextIsJunction || laneInValidGroup)


ok: regular road vehicles should not ignore medians

VictorPhilipp · 2019-03-30T01:40:56Z

TLM/TLM/UI/SubTools/LaneConnectorTool.cs

+                NetInfo.Lane[] lanes = NetManager.instance.m_segments.m_buffer[segmentId].Info.m_lanes;
+                uint laneId = NetManager.instance.m_segments.m_buffer[segmentId].m_lanes;
+                NetInfo info = NetManager.instance.m_segments.m_buffer[segmentId].Info;
+                List<NetInfo.Lane> sortedLanes = lanes.Where(l => (l.m_laneType & LaneConnectionManager.LANE_TYPES) != NetInfo.LaneType.None


Issues:

Duplicate code (same as in RoutingManager)

Linq in GUI code = 💀 for macs ^^

VictorPhilipp · 2019-03-30T01:44:21Z

TLM/TLM/UI/SubTools/LaneConnectorTool.cs

+                            if (gap > 1 ) {
+                                laneGroups[laneIndex]++;
+                            }
+                        }


Duplicate code

Also, this check could/should be performed in LaneConnectionManager. No game logic in GUI classes, please. GUI should be stupid such that we can throw it away when those brave guys do a GUI overhaul. :-)

VictorPhilipp · 2019-03-30T01:47:47Z

TLM/TLM/UI/SubTools/LaneConnectorTool.cs

+                        bool isSource = false;
+                        bool isTarget = false;
+                        if (connManager.GetLaneEndPoint(segmentId, !isEndNode, laneIndex, laneId, laneInfo, out isSource, out isTarget, out pos)) {
+                            int sortedLaneIndex = sortedLanes.IndexOf(laneInfo);


how about something like byte LaneConnectionManager.GetLaneEndGroup(ushort segmentId, ref NetSegment segment, ushort nodeId, ref NetNode node, byte laneIndex, NetInfo.Lane laneInfo, uint laneId)?

(we may not need all of the parameters I mentioned ^^)

VictorPhilipp · 2019-03-30T01:48:30Z

TLM/TLM/UI/SubTools/LaneConnectorTool.cs

+                foreach (NodeLaneMarker laneMarker2 in nodeMarkers) {
+                    if (!laneMarker2.isTarget)
+                        continue;
+                    if (laneMarker1.laneGroup != laneMarker2.laneGroup)


Currently, the lane group value is not comparable among different segments.

You will ask: "what to do to make it comparable?"

It's a bit complicated...

NetInfo.Direction nextDir = isNextStartNodeOfNextSegment ? NetInfo.Direction.Backward : NetInfo.Direction.Forward; NetInfo.Direction nextDir2 = !nextSegIsInverted ? nextDir : NetInfo.InvertDirection(nextDir); [...] if ((nextLaneInfo.m_finalDirection & nextDir2) != NetInfo.Direction.None) { // next is incoming lane [...] }

Also check RoutingManager.IsIncomingLane and IsOutgoingLane.

VictorPhilipp

Please see my comments. :-)
And of course: Feel free to ask if something is unclear or if I wrote anything stupid ^^

andreharv · 2019-03-30T12:00:43Z

Thank you soooo much! This allows me to better understand the flow structure of TMPE. I have to admit that without getting into too much of the guts this was sort of a seat of the pants attempt.
I definitely want to maximize time efficiency. If there is a way to trim the exponential stuff to log or even linear time, would be a great plus. Good to know about linq too. Do you just use 'for' statements?
Also I did find out the hard way about the nextDir and nextDir2. I will keep that in mind for times when people find it necessary to bring two roads together from opposite directions. Which leads me to...

-I am currently working on refining rail rules (I will branch that off till we get these settled). The idea there is that trains cannot change lanes like cars do...because they are on tracks. Therefore, I designed it that each lane is in it's own lanegroup. The rules that I came up with sum up to the principle that the only time rail vehicles can ever change lanes is when entering or leaving a station (and even then I would like for the transition cost to be lower for the train to stay in it's lane). I would also like them to follow highway rules at split and simple junctions (which I haven't gotten around to implementing yet. I am not sure if there would be a (train per lane) option with a highway rules option or if they should all be bundled together.

I laid low on the methods as I noticed you tended to keep things simple yet loooooong which gives a slight performance edge. However, duplicate code is not good. I will revise.

I will respond to each of your comments specifically as I address them in the code.

sorry formatting skills are suspect here.

originalfoo · 2019-03-30T12:13:18Z

@andreharv there were some changes made to trains changing tracks in the 1.10.17 release of TMPE - see #236 and #230

~~@VictorPhilipp Should there also be a mention of that in the release notes?~~ Doh, train updates were part of the 1.10.17 release and were included in release notes.

andreharv · 2019-03-30T12:22:31Z

Ok I will review those as well.

VictorPhilipp · 2019-03-30T15:06:42Z

Ok I will review those as well.

I increased lane changing costs at railway switches. That should make trains avoid unnecessary lane changes. Commit: 2b7db7a

originalfoo · 2019-03-30T16:25:15Z

@andreharv You mentioned in one of your commits that you'd done 'additional testing with less than ideal user creations' - could you provide links to those assets as we'll need to test with them prior to any public release of this PR.

andreharv · 2019-03-31T11:57:51Z

Metrobus
4-Lane One-Way Road with Trees
CSUR Ave 10DR41
CSUR Ave 10DR7
Royal Parade (extra wide Tram Avenue with bike lane and Trees)
CSUR Sep 222
CSUR St 8DR13
12L arterial w/ isolated 2L left-turn bay

DaEgi01 · 2021-04-08T00:26:28Z

Closing this PR for now, since there was no progress for more than a year.
In case this is still relevant PR can be reopened.

andreharv added 2 commits March 16, 2019 15:30

Prevent cars from changing vehicle lanes if there is another lane bet…

7e9fe33

…ween them that is not a vehicle lane or there is more than a meter of space between them.

Refined algorithms after doing some additional testing with less than…

ce5dd25

… ideal user-creations.

VictorPhilipp changed the base branch from 230-trains-maintain-their-lanes to master March 20, 2019 16:59

VictorPhilipp self-requested a review March 20, 2019 17:00

originalfoo added the enhancement Improve existing feature label Mar 21, 2019

VictorPhilipp reviewed Mar 30, 2019

View reviewed changes

VictorPhilipp requested changes Mar 30, 2019

View reviewed changes

FireController1847 mentioned this pull request Apr 1, 2019

Cars All Choose the Same Lane Causing Congestion #263

Open

FireController1847 assigned originalfoo and VictorPhilipp Apr 16, 2019

FireController1847 changed the title ~~89 medians~~ Medians May 14, 2019

FireController1847 added the stale Gathering cobwebs label May 14, 2019

originalfoo removed their assignment May 14, 2019

VictorPhilipp removed the stale Gathering cobwebs label Jun 3, 2019

VictorPhilipp added this to the 11.0 milestone Jun 6, 2019

originalfoo modified the milestones: 11.0, Soon Jun 10, 2019

kvakvs mentioned this pull request Jun 25, 2019

Undo the dreadful tab indent in .editorconfig #392

Merged

VictorPhilipp modified the milestones: Eventually, 11.1 Jul 18, 2019

This was referenced Aug 25, 2019

Median detection #503

Closed

[EPIC] Medians / Separation lanes #504

Closed

originalfoo added the stale Gathering cobwebs label Aug 30, 2019

originalfoo mentioned this pull request Feb 3, 2020

Make m_CanCrossLanes work intelligently #646

Closed

originalfoo modified the milestones: 11.1, 11.2 Feb 7, 2020

This was referenced Feb 10, 2020

V11 - Could not load type 'TrafficManager.Traffic.Data.VehicleState' from assembly 'TrafficManager` #649

Closed

Don't allow lane connections between tracks disconnected due to MaxTurnAngle #684

Merged

originalfoo removed this from the 11.2 milestone Feb 26, 2020

DaEgi01 closed this Apr 8, 2021

This was referenced Dec 6, 2021

An option to enhance the behavior of the Can Cross Lanes road asset flag #1220

Closed

[EPIC] Crossing the Lane/Road #1223

Open

originalfoo added this to the Abandoned milestone Jan 18, 2022

Medians #243

Medians #243

Conversation

andreharv commented Mar 20, 2019 • edited by FireController1847 Loading

FireController1847 commented Mar 27, 2019

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • 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

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

VictorPhilipp Mar 30, 2019 • edited Loading

Choose a reason for hiding this comment

VictorPhilipp left a comment • edited Loading

Choose a reason for hiding this comment

andreharv commented Mar 30, 2019 • edited Loading

originalfoo commented Mar 30, 2019 • edited Loading

andreharv commented Mar 30, 2019

VictorPhilipp commented Mar 30, 2019

originalfoo commented Mar 30, 2019

andreharv commented Mar 31, 2019

DaEgi01 commented Apr 8, 2021

andreharv commented Mar 20, 2019 •

edited by FireController1847

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp Mar 30, 2019 •

edited

Loading

VictorPhilipp left a comment •

edited

Loading

andreharv commented Mar 30, 2019 •

edited

Loading

originalfoo commented Mar 30, 2019 •

edited

Loading