migrate custom start path find to harmony.

[kianzarrin]

This issue is to focus on VehicleAI.StartPathFind part of #865

EDIT: I shall search for but not fix poteintial bad/stale code in the CustomAI. for those we can create issue[s] to discuss them.

[kianzarrin]

EDIT: I have modified my approach so the description bellow is out of date!

I read the custom AI code:

A) All *AI.StartPathFind() call different overloads of Pathfind.CreatePath()
B) Only *AI.StartPathFind() calls different overloads of Pathfind.CreatePath()
C) All overloads of Pathfind.CreatePath() eventually call the biggest overload of PathFind.CreatePath()
D) Its all done in simulation thread.
Therefore each *AIStartPathfFind() needs a prefix to store necessary data. The PathFind.CreatePath() overload C should be replaced with CustomPathManager.CustomCreatePath() using harmony prefix.

For example the picture bellow show the diff bellow:

A: ExtVehicleManager.Instance.OnStartPathFind is called in prefix
B: args should be initialized in prefix.
C: call to PathManager.CreatePath() is replaced with CustomPathManager.CustomCreatePath()

The TMPE redirects do have comments highlighting the non-stack code but they are not highlighting all the changes.

EDIT: The data stored in step B is going to be ignored if the if statement is not reached.

[kianzarrin]

stable path for bus AI is changed to true in this commit cdb0fbe
but I do not understand why. Does anyone know what stable path is for?

aubergine18Yesterday at 17:16
stable path seems to be don't randomize segments, I assume at junctions
so normal vehicles they'll occasionally take a random road at a junction, but that would be shit for busses - they'd be routing all over the place
hence stable path
i don't know why it's a parameter though, like, why not just check if vehicle is a bus and cache that somewhere
kian.zarrinYesterday at 20:56

from decompiler I can see:

this.m_ignoreCost = (this.m_stablePath || (this.m_pathUnits.m_buffer[(int)((UIntPtr)unit)].m_simulationFlags & 8) != 0);
// Does that mean with TMPE people are not discouraged by high bus ticket prices?

if (this.m_stablePath) b = 128;
else b = (byte)this.m_pathRandomizer.UInt32(1u, 254u);
this.ProcessItem(item, nodeID, num16, ref instance.m_segments.m_buffer[(int)num16], b, b, laneIndex4, lane6);
// this seems to be lane randomization

    if (!this.m_stablePath)
    {
        Randomizer randomizer = new Randomizer(this.m_pathFindIndex << 16 | (uint)item.m_position.m_segment);
        num8 *= (float)(randomizer.Int32(900, 1000 + (int)(instance.m_segments.m_buffer[(int)item.m_position.m_segment].m_trafficDensity * 10)) + this.m_pathRandomizer.Int32(20u)) * 0.001f;
    }
    float num11 = item.m_comparisonValue + num8 / (num4 * this.m_maxLength);
    if (!this.m_ignoreCost && ticketCost != 0)
    {
        num11 += (float)(ticketCost * this.m_pathRandomizer.Int32(2000u)) * 3.92156863E-07f;
    }
// so this one seems to be more general randomisation.

// don't know what this one does. there is not alternative for when `m_stablePath` is set to false.
if (lane3.m_laneType != NetInfo.LaneType.Pedestrian || item2.m_methodDistance < 1000f || this.m_stablePath){
    [...]
    item2.m_laneID = lane;
    item2.m_lanesUsed = (item.m_lanesUsed | lane3.m_laneType);
    this.AddBufferItem(item2, item.m_position);
}

I still do not understand why sable path is false in Vanilla game

[kianzarrin]

CarAi.StartPathfind changes lanetype

NetInfo.LaneType.Vehicle -> NetInfo.LaneType.Vehicle | NetInfo.LaneType.TransportVehicle

Commit: 8773045

-				if (CustomPathManager._instance.CreatePath((ExtVehicleType)vehicleType, vehicleID, ExtCitizenInstance.ExtPathType.None, out path, ref Singleton<SimulationManager>.instance.m_randomizer, Singleton<SimulationManager>.instance.m_currentBuildIndex, startPosA, startPosB, endPosA, endPosB, NetInfo.LaneType.Vehicle, info.m_vehicleType, 20000f, this.IsHeavyVehicle(), this.IgnoreBlocked(vehicleID, ref vehicleData), false, (vehicleData.m_flags & Vehicle.Flags.Spawned) != 0)) {
+				PathCreationArgs args;
+				args.extPathType = ExtCitizenInstance.ExtPathType.None;
+				args.extVehicleType = vehicleType;
+				args.vehicleId = vehicleID;
+				args.buildIndex = Singleton<SimulationManager>.instance.m_currentBuildIndex;
+				args.startPosA = startPosA;
+				args.startPosB = startPosB;
+				args.endPosA = endPosA;
+				args.endPosB = endPosB;
+				args.vehiclePosition = default(PathUnit.Position);
+				args.laneTypes = NetInfo.LaneType.Vehicle | NetInfo.LaneType.TransportVehicle;
+				args.vehicleTypes = info.m_vehicleType;
+				args.maxLength = 20000f;
+				args.isHeavyVehicle = this.IsHeavyVehicle();
+				args.hasCombustionEngine = this.CombustionEngine();
+				args.ignoreBlocked = this.IgnoreBlocked(vehicleID, ref vehicleData);
+				args.ignoreFlooded = false;
+				args.randomParking = false;
+				args.stablePath = false;
+				args.skipQueue = (vehicleData.m_flags & Vehicle.Flags.Spawned) != 0;
+
+				if (CustomPathManager._instance.CreatePath(out path, ref Singleton<SimulationManager>.instance.m_randomizer, args))

Seems like an accident. Not sure.

[kianzarrin]

BusAI does not call OnStartPathFind() unlike some AIs. Is this intentional or a mistake?
EDIT: BusAI lacks the code bellow:

//Ambulance AI
 ExtVehicleManager.Instance.OnStartPathFind(vehicleID, ref vehicleData, emergencyVehType);

Its either not necessary or its unfinished work. some other AI's also don't call OnStartPathFind()

[kianzarrin]

CargoTruckAI sets args.extVehicleType = ExtVehicleType.CargoVehicle;
why not ExtVehicleType.CargoTruck ?

The vanilla code (as well as redirected code) use generic types. So it makes sense that ExtVehicleType would also be generic. But I am not sure.

NetInfo.LaneType laneTypes = NetInfo.LaneType.Vehicle | NetInfo.LaneType.CargoVehicle;
VehicleInfo.VehicleType vehicleTypes = VehicleInfo.VehicleType.Car | VehicleInfo.VehicleType.Train | VehicleInfo.VehicleType.Ship | VehicleInfo.VehicleType.Plane;

[originalfoo]

m_stablePath

// Does that mean with TMPE people are not discouraged by high bus ticket prices?

I suspect that "cost" isn't about ticket prices, but actually "penalty cost" of pathfinding (eg. pathfinding costs based on congestion, traffic lights, junctions, speed limts, etc).

It would be great if we could rename "cost" in that context to "penalty" to disambiguate from ticket prices.

// this seems to be lane randomization

and

// so this one seems to be more general randomisation

Yup. Good for other vehicle types as it increases overall road usage, but bad for busses.

It's like normally the pathfinder will try and find "the best route for this vehicle type, avoiding delays, preferring faster roads, with some randomisation to make traffic look more organic".

But for busses you don't want that. The user is specifying a route stop by stop and they expect the bus to take direct route between the stops (but strongly preferring bus lanes if they are present).

I suspect in vanilla the 'stable path' thing has less noticeable effect, because there are less traffic customisations in vanilla. With all the extra stuff TM:PE adds, the need for "direct path with preference of bus lanes" becomes much more important.

CarAi.StartPathfind

If I remember correctly, most of the other vehicle AIs are based from the Car AI? If you mentally rename "CarAI" to "RoadVehicleAI" it will help reasoning.

BusAI

BusAI does not call OnStartPathFind() unlike some AIs. Is this intentional or a mistake?

Is that an event? A link to code would be useful. What does calling OnStartPathFind() usually trigger?

CargoTruckAI

CargoTruckAI sets args.extVehicleType = ExtVehicleType.CargoVehicle; why not ExtVehicleType.CargoTruck ?

I suspect this is because the vehicle is the cargo. Conceptually the game isn't routing a vehicle from A to B, it's routing cargo from A to B, but it can only route vehicles so vehicles are the cargo. And because cargo can "travel" via different modes, such as air, rail, ship or road, it has to be thought of (and pathed) as a generic "cargo vehicle" rather than specific mode of transport.

Take an outside ship conneciton for example, the game is essentially spawning trucks that 'merge' in to a ship and then the "truck ship" finds a cargo port and dissassembles back in to trucks. It still bakes my brain how it works. @krzychu124 can likely explain that part of the game far better than I can.

[kianzarrin]

In the code bellow CargoTruck AI checks endPos instead of startPos. I do not understand why:

commit c1fc56f
Author: @krzychu124 krzychu124@gmail.com
Added missing vanilla pathfinding parts, updated ExtVehicle type other

-	if (!startPosFound || startAltDistSqrA < startDistSqrA) {
+	if (!startPosFound || (startAltDistSqrA < startDistSqrA && 
+	    (Mathf.Abs(endPos.x) > 4800f || Mathf.Abs(endPos.z) > 4800f))) { // WHY ENDPOS!!!!!?????
 		startPosA = startAltPosA;
 		startPosB = startAltPosB;
 		startDistSqrA = startAltDistSqrA;
 		startDistSqrB = startAltDistSqrB;
 	}
...
-	if (!endPosFound || endAltDistSqrA < endDistSqrA) {
+	if (!endPosFound || (endAltDistSqrA < endDistSqrA && (Mathf.Abs(endPos.x) > 4800f || Mathf.Abs(endPos.z) > 4800f))) {
 		endPosA = endAltPosA;
 		endPosB = endAltPosB;
 		endDistSqrA = endAltDistSqrA;
 		endDistSqrB = endAltDistSqrB;
 	}

Vanilla CS code is:

if (!flag || (num3 < num && (Mathf.Abs(startPos.x) > 4800f || Mathf.Abs(startPos.z) > 4800f)))
{
	startPosA = position;
	startPosB = position2;
	num = num3;
	num2 = num4;
}
		if (!flag2 || (num7 < num5 && (Mathf.Abs(endPos.x) > 4800f || Mathf.Abs(endPos.z) > 4800f)))
{
	endPosA = position3;
	endPosB = position4;
	num5 = num7;
	num6 = num8;
}

@krzychu124 did you made a mistake?
Also change log sais:

Updated: Tweaked values in CargoTruckAI path finding (thanks to pcfantasy for improvement suggestion)
It would be nice to understand why did you change 4800 to 8000 so that I can explain it in a comment ... it would no longer be a mystery.

[krzychu124]

yep, that endPos looks like a bug. Regards to value, it was a long ago, but I think we've noticed that one of pcfantasy mods patched our code with good results, probably I asked him why and we've decided to change it too.

I think that stablePath indicates that path is based on user defined route - PT (offset is set to 128 by PF, because line stop is always in the middle of segment). Only citizens use stable paths, because one of requirements is transition from Pedestrian lane.

[kianzarrin]

@krzychu124 maybe if I replace endPos with startPos then we don't need to replace 4800 with 8000 anymore :p

[kianzarrin]

@pcfantasy can you please remind us of why we needed to change 4800 to 8000 here #895 (comment)
EDIT: did you have a problem with vanilla game?

[kianzarrin]

@aubergine10 I suspect that "cost" isn't about ticket prices, but actually "penalty cost" of pathfinding (eg. pathfinding costs based on congestion, traffic lights, junctions, speed limts, etc).

I did include this code in my original comment but I did not emphesize it!

    if (!this.m_ignoreCost && ticketCost != 0)
    {
        num11 += (float)(ticketCost * this.m_pathRandomizer.Int32(2000u)) * 3.92156863E-07f;
    }

So based on this code it does look like it has something to do with the ticket price. It might not be a bad idea to test if users are deterred by prices in vanilla VS TMPE. @thebugfixnet Would you be interested to test something like this?

[kianzarrin]

@aubergine10 If I remember correctly, most of the other vehicle AIs are based from the Car AI? If you mentally rename "CarAI" to "RoadVehicleAI" it will help reasoning.

Several problems with your logic:

• RoadVehicleAI does not exist.
• sub-classes of CarAI have overridden StartPathFind() without calling the base function.
• The Vanilla game does NOT use the additional NetInfo.LaneType.TransportVehicle

Therefore I suspect this might have been added by mistake probably while copy-pasting code. But it could be intentional too. Who knows ...

EDIT: I updated the comment with regards to BusAI.

I suspect this is because the vehicle is the cargo

That makes a lot of sense :)

[chameleon-tbn]

@aubergine10 I suspect that "cost" isn't about ticket prices, but actually "penalty cost" of pathfinding (eg. pathfinding costs based on congestion, traffic lights, junctions, speed limts, etc).

I did include this code in my original comment but I did not emphesize it!

    if (!this.m_ignoreCost && ticketCost != 0)
    {
        num11 += (float)(ticketCost * this.m_pathRandomizer.Int32(2000u)) * 3.92156863E-07f;
    }

So based on this code it does look like it has something to do with the ticket price. It might not be a bad idea to test if users are deterred by prices in vanilla VS TMPE. @thebugfixnet Would you be interested to test something like this?

Yes, i can test. Just provide me the test scenario and what exactly i should have an eye on.

[originalfoo]

Not sure if it's related or relevant, but NetLane also has a ticket cost (maybe for toll lanes?)

[kianzarrin]

@aubergine10 Not sure if it's related or relevant, but NetLane also has a ticket cost (maybe for toll lanes?)

seems like it:

EDIT: I hope this is not why those rediculous park scams are possible: https://youtu.be/lg-GrWiKO2E
I don't think so anyway because people do not use BusAI when going to park.

[chameleon-tbn]

Used a Vanilla save with 155k CIMs (growing)
Deleted Metro/Train-Lines, no Taxi
24 Buslines, a lot of busses, 150% budget

Price: 1$ with Ticket Price Customizer
2,810 CIMs a week / 188 Tourists a week after 4 weeks

Price: 10000$ with Ticket Price Customizer
2,851 CIMs a week / 201 Tourists a week after 4 weeks
+250k$ a week 😄

3rd test:
added toll booths to the 10 most used Bus routes with max. toll Price
2,859 CIMs a week / 197 Tourists a week after 4 weeks

[originalfoo]

Why on earth are cims ingoring ticket cost? That makes a whole bunch of city/district policies practically useless.

[chameleon-tbn]

You View it from the CIMs View. but you have to View it as a City Planner: you loose Money with these Rules, Change the amount of private vehicles on the Road and Reduce the noise Pollution. You're Not making CIMs more clever....

[originalfoo]

Were any transport-affecting city policies set when you did your tests? Because if not, then the ticket price and, by extension, all policies relating to it, are entirely useless - literally no effect on transport prefs.

Increasing ticket price of a transport line should make it less desireable. But if a change as significant as $0.01 vs. $100.00 has zero effect on transport prefs, then it seems something is probably wrong.

Would be interesting to perform similar test on toll booths - obviously they don't affect public transport (which is correct and logical). However, for private traffic is there any difference between a cheap toll booth and an expensive toll booth. Would need A/B split testing approach on same road to remove any other factors such as distance, speeds, etc.

[chameleon-tbn]

@aubergine10 No, no city policies set at all. No Mods that change game mechanics except TMPE and Ticket Price Customizer (only optical mods i had still active, all other unsubbed). It is a bare vanilla city.

I agree that it should be as you expect. But in my tests it did not...

Regarding to toll price:

I've tested today in my town and build one other roads as alternative to the road with a toll both... i just checked optical how much traffic was on the roads.... the shorter way got the toll booth... I make the toll free roads longer and longer step by step.... Even with beeing up to ~25% longer: there where a few cars using this longer road than w/o toll booth. I've tested it with lowest toll and highest toll: I would say just a minimal change in the amount of vehicles taking the longer but cheaper route.

But it was not that at max. standard possible toll all cars where using the alternative route. Only if i just split the road and have nearly the same length (maybe) 80% took the cheaper way.

At toll booth it is working - but in my opinion not strong enough....

[kianzarrin]

@victoriacity
He says that 8000 is to make it compatible with 81 tiles
4800 only works for 25 tiles
I guess he's not logged in to VPN for days
So he wasn't checking any non-Chinese social network

@krzychu124 yep, that endPos looks like a bug.

So the conclusion is all I need is a simple transpiler that changes 4800 to 8000 :)

EDIT: but is it TMPE's job to make CS 81 tiles compatible? now that we have Harmony maybe we do not need this. is there any other mod that makes this 8000 replacement?

[pcfantasy]

@kianzarrin

I remember that there is a bug related to this 4800 but not sure now.

The bug is , if a industrial building(maybe outside 25tile I am not sure now) is under an airport line, then there are no cargo trucks import to this building.

After change this to 8000, everything is fine.

[kianzarrin]

@pcfantasy thanks for the info
Does that mean the 81 tiles mods do not work well without TMPE?

[pcfantasy]

@pcfantasy thanks for the info
Does that mean the 81 tiles mods do not work well without TMPE?

Yes, I think so.

[Sipke82]

Why on earth are cims ingoring ticket cost? That makes a whole bunch of city/district policies practically useless.

I guess those policies just fiddle with the probability that the CIM uses Public transport =)

[originalfoo]

but is it TMPE's job to make CS 81 tiles compatible?

Updates to 81 Tiles are extremely unlikely; BP is no longer active.

[DaEgi01]

Will it affect players who play with 9/25 tiles only?

[originalfoo]

Could it be made dependent on whether 81 Tiles mod is active?

[DaEgi01]

sure, would be a little slower due to method call instead of constant, but maybe worth it. no idea what that value actually affects though.

[originalfoo]

Looking at Kian's updated patch code, we only need to know whether 81 Tiles is present at time of transpiling (see TLM/TLM/Patch/_VehicleAI/_CargoTruckAI/StartPathFindPatch.cs in the linked PR #943). So it's one-time check per load.

[krzychu124]

PostVanAI has the same logic, should we change it there also?

[originalfoo]

I didn't see the transpiler in the current PR for PostVanAI...? It is essentially cargo, so I assume so.

[krzychu124]

It's not cargo(small vans), PostVanAI derives from CarAI, not CargoTruckAI, but its StartPathFind() is literally the same as CargoTruckAI

[kianzarrin]

CustomShipAI is the only one that does not use skip queue. Is this an oversight?

taggin @krzychu124

[kianzarrin]

As for the 81 tiles max start/end pos:

• I guess the code would be slower because startPathFind is called for vehicles beyond 25 tiles.
• TMPE does not currently use max pos of 8000 for Post vans. That means it might not work with 81 tiles. I can easily apply the same transpiler to PostVanAI. I don't even need duplicate code for that I just call the same transpiler :)
• One option is to move the code to 81 tiles mod (or whatever else it is called) .

[kianzarrin]

@aubergine10 I am almost certain that this is a mistake #895 (comment)

apart from the fact that lane type all of a suddent was changed ... look at the decompiled vanilla code:

if (PathManager.FindPathPosition( NetInfo.LaneType.Vehicle | NetInfo.LaneType.TransportVehicle, ...);

...
	
if (Singleton<PathManager>.instance.CreatePath(NetInfo.LaneType.Vehicle,  ...);

So it seems that the @Victor-Philipp has confused the arguments passed to FindPathPosition() with CreatePath()

[kianzarrin]

@aubergine10 I an idiot! BusAI StablePath value DOES math the CS vanilla code! Nothing to worry about there!

[originalfoo]

The maxPos transpiler might be required for garbage service, particularly the garbage transfer trucks added in Subset Harbor which are designed for long-haul transfers.

[originalfoo]

@kianzarrin can this issue be closed (IIRC we have fully migrated to harmony now)?

[originalfoo]

Looks like this was completed by #1089