Compatibility with Sunset Harbor

[krzychu124]

Compatibility with Sunset Harbor

• fixed PathUnits.m_vehicleTypes error,
• added Trolleybuses AI,
• fixed priorities to cover Trolleybuses
• bumped version

[krzychu124]

ops... forgot to update CI...

[kianzarrin]

I compared the code to other custom AI.
There is a ton of dormant code in other AIs.
there is a lot of repeated code.

This is just a comment. no need to make any changes in this PR.

[kianzarrin]

in the decompiler I noticed there is a line like this:

RoadBaseAI.ClickNodeButton() 
if ((info.m_vehicleTypes & (VehicleInfo.VehicleType.Car | VehicleInfo.VehicleType.Tram | 
VehicleInfo.VehicleType.Trolleybus)) != VehicleInfo.VehicleType.None)

I compared it to CustomRoadBaseAI where we have

TMPE/TLM/TLM/Custom/AI/CustomRoadBaseAI.cs

Line 42 in 89f622a

if ((info.m_vehicleTypes & (VehicleInfo.VehicleType.Car | VehicleInfo.VehicleType.Tram)) ==

So you might want to add trolly bus in CustomClickNodeButton too. Not sure if it is important or if the code is even used or not. from the name I get the impression the UI could be in trouble.

[kianzarrin]

What is the deal with StockPathFind.cs? its not even compiled. is it only for information? If that is the case it is now outdated. maybe delete it or put a comment that it is outdated ? preferably with an issue number.

speaking of the devil are there any issues tracking redundant AI/pathfind code? if there is its worth referencing the issue in the code
// TODO [issue #123] this code is redundant/outdated

[kianzarrin]

its worth to use ExtVehicleManager.VEHICLE_TYPES in

TMPE/TLM/TLM/Manager/Impl/ExtSegmentManager.cs

Line 112 in 89f622a

VehicleInfo.VehicleType.Tram | VehicleInfo.VehicleType.Metro |

and

TMPE/TLM/TLM/Manager/Impl/ExtSegmentEndManager.cs

Line 323 in 89f622a

VehicleInfo.VehicleType.Tram | VehicleInfo.VehicleType.Metro |

ExtVehicleManager.VEHICLE_TYPES will need trollybus

[kianzarrin]

I looked at several other places in the code and I can see that it works for as long as there will be no trolly-only custom road assets.
@krzychu124 is that intentional?

[originalfoo]

What is the deal with StockPathFind.cs?

Each time there is game update we decompile the stock pathfinder and diff it to the StockPathFind.cs to see what's changed.

[originalfoo]

I looked at several other places in the code and I can see that it works for as long as there will be no trolly-only custom road assets.

There are definitely trolly only roads being worked on.

[kianzarrin]

@aubergine10 There are definitely trolleybus only roads being worked on.

EDIT: we should do some testing after a trolley only road is released. I don't know how to create an asset ( i fail every time i try) otherwise I would have created one myself for testing. This review should be merged in before a trolleyroad road asset is released and then when a trolleybus only road is created we come back to this.

@krzychu124 Please search for tram everywhere except for TramBaseAI and add trolley-bus where-ever you see tram (and deem it to be necessary) . There are not too many places so it will be easy.

Lane connector also does not work for trolley only roads.

[krzychu124]

@kianzarrin ok will do, also I've just noticed that nullpointer fix cause some error on second load, so possibly breaks hot-loading...

[originalfoo]

I'll ask around the creator communities to get clearer picture of what they're working on and rough ETA on when it might land in workshop.

[krzychu124]

I need to figure out what is wrong with PathManager and it will be ready for merge.
Maybe it's worth bump version number to 11.2.1? What do you think?

[originalfoo]

Yup, we can refer to the PR in both sets of release notes, and just have one release for 11.2.1

[krzychu124]

I think NullPointerException was fixed.
Already tested cases:

• enter + exit Map/Asset Editor,
• hotreload while in main menu,
• hotreload from in-game,
• reload savegame being in-game,
• "back-to-menu and load different savegame" scenario

I didn't noticed any issues or errors. Would be good if someone could confirm these results.
@aubergine10 Is there any pending issue to solve with regards to Sunset Harbor (or something simple that we can fit in update)?

[kianzarrin]

LGTM

[kianzarrin]

There is a ton of dormant code in other AIs.
there is a lot of repeated code.

This has not been addressed but I don't consider it as part of this review so I approved.

[originalfoo]

@kianzarrin can you create issue to track that domant code so we remember to sort it later?

[originalfoo]

LGTM 👍

[krzychu124]

@kianzarrin I'm not sure what do you mean by a lot of repeated/dormant code, but we will fix it in next PR. Thanks for review 😉

[kianzarrin]

@krzychu124 dormant ... I made a mistake. I don't understand how redirection framework works.

repeated ... yes. for example trolley bus custom AI looks like a copy of tram custom AI with a couple of minor modifications. Ideally repeated code should be shared.

[krzychu124]

Ideally repeated code should be shared.

No, because you will have hard time to spot a difference when they(CO) change anything