The International Simutrans Forum

 

Author Topic: [CODE] incorrect for statements in private car routing save data  (Read 665 times)

0 Members and 1 Guest are viewing this topic.

Offline Phystam

  • Devotee
  • *
  • Posts: 489
  • Pak256.Ex developer
    • Pak256 wiki page
  • Languages: JP, EN, EO
[CODE] incorrect for statements in private car routing save data
« on: February 18, 2020, 06:48:06 AM »
There is an error in simcity.cc:2385 and :2405,
Code: [Select]
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...

Offline freddyhayward

  • Devotee
  • *
  • Posts: 386
  • Languages: EN
Re: [CODE] incorrect for statements in private car routing save data
« Reply #1 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.

Offline Freahk

  • Devotee
  • *
  • Posts: 1257
  • Languages: DE, EN
Re: [CODE] incorrect for statements in private car routing save data
« Reply #2 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.

Offline Phystam

  • Devotee
  • *
  • Posts: 489
  • Pak256.Ex developer
    • Pak256 wiki page
  • Languages: JP, EN, EO
Re: [CODE] incorrect for statements in private car routing save data
« Reply #3 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.
« Last Edit: February 21, 2020, 09:48:53 AM by Phystam »

Offline freddyhayward

  • Devotee
  • *
  • Posts: 386
  • Languages: EN
Re: [CODE] incorrect for statements in private car routing save data
« Reply #4 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.

Offline Freahk

  • Devotee
  • *
  • Posts: 1257
  • Languages: DE, EN
Re: [CODE] incorrect for statements in private car routing save data
« Reply #5 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.

If 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)
« Last Edit: February 21, 2020, 10:17:24 AM by Freahk »

Offline Phystam

  • Devotee
  • *
  • Posts: 489
  • Pak256.Ex developer
    • Pak256 wiki page
  • Languages: JP, EN, EO
Re: [CODE] incorrect for statements in private car routing save data
« Reply #6 on: February 21, 2020, 10:13:57 AM »
Almost inifinte loop xD

Offline freddyhayward

  • Devotee
  • *
  • Posts: 386
  • Languages: EN
Re: [CODE] incorrect for statements in private car routing save data
« Reply #7 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.

Offline Freahk

  • Devotee
  • *
  • Posts: 1257
  • Languages: DE, EN
Re: [CODE] incorrect for statements in private car routing save data
« Reply #8 on: February 22, 2020, 01:14:45 PM »
Sounds buggy. From my understanding routes should be frequently recalculated, no matter any situations.

Offline jamespetts

  • Simutrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 20207
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: [CODE] incorrect for statements in private car routing save data
« Reply #9 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.

Offline jamespetts

  • Simutrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 20207
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: [CODE] incorrect for statements in private car routing save data
« Reply #10 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.