The International Simutrans Forum

Simutrans Extended => Simutrans-Extended bug reports => Simutrans-Extended development => Simutrans-Extended closed bug reports => Topic started by: Phystam on February 18, 2020, 06:48:06 AM

Title: [CODE] incorrect for statements in private car routing save data
Post by: Phystam on February 18, 2020, 06:48:06 AM
There is an error in simcity.cc:2385 and :2405,

for (uint32 i = 0; i++; i < 2)
{
  uint32 private_car_routes_count = private_car_routes[i].get_count();
  file->rdwr_long(private_car_routes_count);
...

Please fix it, but probably this correction causes a incompatibility of saved data...
Title: Re: [CODE] incorrect for statements in private car routing save data
Post by: freddyhayward on February 21, 2020, 05:20:03 AM
I had a look at this and fixed it in my travel-time-congestion branch. It did indeed break all saves since the private car routes update, but I fixed this by running the old code on saves from that version.
Title: Re: [CODE] incorrect for statements in private car routing save data
Post by: Sirius on February 21, 2020, 09:01:50 AM
What does this code actually do?
Run until the uint32 becomes false aka 0, thus when it overflows after 2^32 iterations??
And then? Is attempts to access each index, where each index above 2 is out of range?
I would expect this to 1. grow the save quite a lot and 2. cause unexpected stuff in the code, where an immediate segfault would even be the best case, so I am wondering this didn't cause such huge issues yet and I guess I am missing something.
Title: Re: [CODE] incorrect for statements in private car routing save data
Post by: Phystam on February 21, 2020, 09:33:09 AM
Actually, this route count was not saved. When checking the loop condition first, the i is set as 0(false), since i++ is calculated after checking. If it were ++i, the increment would be calculated before checking, and it would cause an infinite loop.
Title: Re: [CODE] incorrect for statements in private car routing save data
Post by: freddyhayward on February 21, 2020, 09:42:15 AM
Phystam is right: stepping through on gdb, the for loop condition would fail on the first time, every time. what it was meant to do was save both tables, the processed and non-processed routes.
Title: Re: [CODE] incorrect for statements in private car routing save data
Post by: Sirius on February 21, 2020, 10:03:24 AM
Ah sure, obviously it's 0 at start xD

So fixing the code and using it only in later versions should work and would be the cleanest.
Effectively, we don't save references to private car routes stored all arond the world, thus we might want to perform a cleanup, resetting all private routes around the whole world when loading a game from that specific version.
That might also explain strange behavior in some places on stephenson-siemens where private cars were driving very strange routes.

Quote from: Phystam on February 21, 2020, 09:33:09 AMIf it were ++i, the increment would be calculated before checking, and it would cause an infinite loop.
Actually not true. We would get false at overflow (where we likely won't arrive due to segfaults)
Title: Re: [CODE] incorrect for statements in private car routing save data
Post by: Phystam on February 21, 2020, 10:13:57 AM
Almost inifinte loop xD
Title: Re: [CODE] incorrect for statements in private car routing save data
Post by: freddyhayward on February 21, 2020, 11:05:22 AM
Updated, since your suggestion is far more sensible than what I did. However from testing in debug mode, the problem with routing seems to be that in the effort to save performance, the routes are updated very conservatively -- effectively not at all apart from when two towns are linked by road for the first time. Remember the chorborough bypass that no one was using except in heuristic mode? I built a road tunnel linking All Petingstone and Peache, which effectively linked All Petingstone to all other mainland towns for the first time. After waiting and waiting, all the new routes *from* All Petingstone to the other towns went via the chorborough bypass, however none of the existing congested routes through chorborough were modified, and routes *towards* All Petingstone were never even created.
Title: Re: [CODE] incorrect for statements in private car routing save data
Post by: Sirius on February 22, 2020, 01:14:45 PM
Sounds buggy. From my understanding routes should be frequently recalculated, no matter any situations.
Title: Re: [CODE] incorrect for statements in private car routing save data
Post by: jamespetts on March 01, 2020, 01:26:34 PM
I should note that this code affects city storage of private car routes only, which I am currently working on abolishing in order to deal with the excess memory consumption issue.
Title: Re: [CODE] incorrect for statements in private car routing save data
Post by: jamespetts on March 22, 2020, 12:02:43 PM
The abolition of city storage of private car routes has now been deployed in the master branch and is in the latest nightly build. I believe therefore, that this bug is no longer relevant.