CitiesSkylinesMods / TMPE

Cities: Skylines Traffic Manager: President Edition
https://steamcommunity.com/sharedfiles/filedetails/?id=1637663252
MIT License
571 stars 85 forks source link

Delete/upgrade a road and vehicles despawn rather than reroute #86

Closed originalfoo closed 5 years ago

originalfoo commented 5 years ago

chris_852 Broke my game. Once I deleted roads and rebuild them, traffic would disappear once it got to the newly constructed road segments. After I removed this mod, traffic returned to normal.

What should happen: Traffic reaching the point where road was deleted should reroute (ideally deleting roa should cause rerouting for any traffic destined for that road)

Temporary workaround

Current advice to end users: Wait a few minutes and traffic should return to normal

See: Troubleshooting guide

originalfoo commented 5 years ago

I've confirmed this bug.

Steps to reproduce:

  1. Start new map
  2. Build a square-shaped zoneable road
  3. Zone a bunch of residential, etc
  4. When cars start showing up regularly, delete a chunk of road

What happens:

Not yet tested: Pedestrians, cyclists, busses, trucks, etc.

What should happen:

Mod settings:

I have >100 mods installed, including IPT2, RealTime, etc. Will do some testing without those and report back.

originalfoo commented 5 years ago

After a few seconds, vehicles did start re-routing (they did u-turns at end of road; cars and trucks tested)

However, when I replaced road, vehicles still did u-turns in the same place (cars and trucks tested).

A while later, they started using road normally again.

Seems vehicles affected by changes to road aren't being updated as soon as expected. They seem to be lagging behind. When road is first removed, that lag is causing cars (possibly also trucks, will test) to despawn.

originalfoo commented 5 years ago

Pedestrians and service vehicles seem to be repathing fast, other vehicle types (including trucks) lag by a few seconds.

I tried changing mod options (eg. disabling the things I had selected from above, setting sim accuracy to very low, vehicle restriction aggression to low) but it made no difference.

Image of my test setup below: testing

originalfoo commented 5 years ago

Tagging https://github.com/VictorPhilipp/Cities-Skylines-Traffic-Manager-President-Edition/issues/236

originalfoo commented 5 years ago

Related discussion on Steam - note the date, I think this issue could possibly be due to changes in the old TMPE around that time so we might be able to track it down to a commit?

https://steamcommunity.com/workshop/filedetails/discussion/583429740/1748980761804012244/

originalfoo commented 5 years ago

Just had another similar report on Steam - it also happens when roads are updated.

originalfoo commented 5 years ago

FYI: Reports of this longstanding issue are piling up on Steam workshop page, most recently:

can you tell me where to find the solution for cars disappearing on new road, improved road, destroyed and rebuilt road and public transport lines please

As a random guess, TMPE is likely listening or detouring stuff related to network create, delete, upgrade, etc., so that it can keep it's internal traffic metrics stuff up-to-date and remove any related customisations and so on. In doing so, is it failing to trigger events (or some other stuff) that vanilla does?

krzychu124 commented 5 years ago

Thanks, I will try to find where is the problem. It's hard to debug that. :-/

krzychu124 commented 5 years ago

I have hard time with reproducing this issue... Sometimes I can see similar situation than yours but sometimes no matter what am I doing I can't provoke vehicles to despawn...

I can see that after road upgrade paths disappearing from path visualizer and sometimes ends just before upgraded segment but when vehicles approach this new end of path their status flag is changed to WaitingPath, their path is recalculated and they continue travel using upgraded segment.

I've had one issue after upgrading segments. Vehicles stopped just before upgraded segment and they won't move. I've noticed that they don't have path rendered in Path Visualizer view at all or their path stopped at point just before updated segment. ModTools shows that their m_path id was constantly changing and when their block counter hits 100 they were despawned... I tried to debug that and those vehicles constantly invalidates every path created for them. I couldn't reproduce that case after creating second new game with the same settings and similar road configuration

With regards to removing segment, vehicles are despawned only if there is no path to their destination e.g. if you delete segment before and behind vehicle, it will despawn.

Path recalculation can lag because in general when you disable Advanced Vehicle AI their path seems to be recalculated only when they arrive to path end point (if you remove segment that can cause path cutting, only at this point vehicle will try to call for path recalculation) Heavy trucks are excluded from Dynamic Lane Selection so even with enabled Advanced Vehicle AI they don't switch lanes and recalculate paths.

I still can't figure out if path is separated on parts when is calculated (to help DLS path change) or one time on just before vehicle spawn only.

I will try to check what part of code was changed since first report of this issue and compare current code to vanilla and after that disable redirection of some of methods

originalfoo commented 5 years ago

I'm pretty sure I had DLS turned off when I was testing, but will do another check.

There is also possibility that it's different mod that's causing the issue, so I will also try and do test with other mods disabled (especially some of the most likely ones, such as MOM, IPT2, DSL3, MoveIt, etc).

krzychu124 commented 5 years ago

While digging in code and testing I think I found why Traffic Routes view is clearing paths after segment upgrade or removal.

Steps to show the problem:

  1. open Traffic Routes view
  2. select any segment to visualize connected paths
  3. close Traffic Routes view
  4. upgrade that previously selected segment
  5. open again Traffic Routes - no matter what segment you choose you won't see any route for a while.

I've checked vanilla and I see the same result.

Explanation (selected segment - last segment selected before closing Traffic Routes):

Now I have to check if that path from Traffic Routes (PathVisualizer) is somehow related with path which was found for vehicle by pathfinder (maybe they sharing ID or something)

originalfoo commented 5 years ago

I assume the traffic routes info view is just grabbing the path associated with vehicles (or iterating all vehicles to find any that cross selected segment). So if vehicle path breaks, so to does traffic route path.

krzychu124 commented 5 years ago

Yup, every path visible in Traffic Route is built from vehicles current path (m_path) and next parts of full route are retrieved via m_path.nextPathUnit until finished(?nexPathUnit == 0 or reached destination)

Edit: I've removed redirection of NetManager (notifiers of segment change) and after loading save everything was working until first segment change (desync of segments collection). Vehicles stopped jin front of changed segment and they are started spamming to get new path. Due to removal of original segment(and turned off synchronization) there was no valid path to reach destination (one-way to enter the city). Some vehicles were despawned one segment before last valid segment, other just stopped in front of invalid segment and then waited until despawn (m_blockCounter > 100 [simulation frames, I think])

krzychu124 commented 5 years ago

If any mod override NetManager::UpdateSegment(ushort lineID, ushort segmentID, ref NetSegment data) or NetManager::FinalizeSegment(ushort segment, ref NetSegment data) our pathfinding logic will stop working due to segments collection desync.

originalfoo commented 5 years ago

I think that was one of the key drivers to move to Harmony for patching. Apparently it has something that detects if something else has changed same stuff.

krzychu124 commented 5 years ago

Yes and theoretically both patches can run one after another depends on priority.

BTW yesterday I've been testing that harmony branch and it seems that it's working. I didn't check yet if all methods has been moved to harmony patcher but I haven't found any issue with it (no null pointers, arrays out of bounds etc.). That branch is a little bit outdated and lacks few commits related to industry update but on the other hand it has a few classes which were moved to appropriate manager classes to increase code consistency and readability.

I think I could update it with latest stuff if I'll find some time to put it to test on heavily modded games :wink: I think that might solve our problems with those type of errors.

originalfoo commented 5 years ago

In #101, some code changes have been tested to overcome issues of vehicles reaching a dead-end (allow them to choose any outgoing lane from the dead-end node)...

Upon further testing, it seems it might fix (or certainly improve) this #86 issue as well.

                bool isStockUturnPoint = (nextNode.m_flags & (NetNode.Flags.End | NetNode.Flags.OneWayOut)) != NetNode.Flags.None;

                bool relaxedLaneRouting =
                    m_isRoadVehicle &&
                    ((!m_queueItem.spawned || (m_queueItem.vehicleType & (ExtVehicleType.PublicTransport | ExtVehicleType.Emergency)) != ExtVehicleType.None) &&
                    (laneTransitions[k].laneId == m_startLaneA || laneTransitions[k].laneId == m_startLaneB)) || isStockUturnPoint;
krzychu124 commented 5 years ago

Great!

originalfoo commented 5 years ago

Just done some more testing with a few different road layouts and road sizes:

So, I think that minor bit of code (thanks @pcfantasy) seems to have solved the problem - I'm guessing some problem with u-turns at dead-ends was causing repathing to fail which was causing despawning.

Note: I'm building with master branch; I've not checked to see if there are other changes in there since the initial testing I did for this issue.

krzychu124 commented 5 years ago

Looks like it's working for me

BTW I am using branch + pull request instead of directly pushing to master

originalfoo commented 5 years ago

If you've added to a branch, you might as well send in PR :)

If we can get the incompatible mod checking done (even basic version - we can enhance later if needs be) I think it would be good time to push another release to steam.

krzychu124 commented 5 years ago

I've created very simple checking two days ago. I will make PR and you will see if it's worth push it to 1.10.16 I have to cleanup my mess first 😉