News:

Simutrans Wiki Manual
The official on-line manual for Simutrans. Read and contribute.

[Fix & Optm. v3.12] rebuild_connexions() & add_connexion()

Started by knightly, May 25, 2009, 05:13:01 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

knightly

James,

I have made 2 commits and pushed them to my forked branch. Please take a look when you have time. The 2 commits are related to the following issues.


(A) Two bugs in rebuild_connexions() :


(1)
In the following part of code :

Quote
tmp_halt = haltestelle_t::get_halt(welt, fpl->eintrag[ i ].pos, besitzer_p);
if(!tmp_halt.is_bound())
{
   if(i_am_public)
   {
      // Public halts can connect to all other halts, so try each.
      spieler_t* sp = cnv->get_besitzer();
      tmp_halt = haltestelle_t::get_halt(welt, fpl->eintrag[ i ].pos, sp);
   }
   else
   {
      // Try a public player halt
      spieler_t* sp = welt->get_spieler(0);
      tmp_halt = haltestelle_t::get_halt(welt, fpl->eintrag[ i ].pos, sp);
   }
}

You have mistaken about the index of public player. Public player is #1, and #0 is the human player. I have fixed this in rebuild_connexions() as well as in notify_halts_to_rebuild_connexions().


(2)
There is a subtle bug which I don't know how it comes about but which does happen in unusual circumstances. To reproduce, set up a pax-only halt A and another halt B which supports both pax and mail, then operate a mail convoy between these 2 stops. If you look at the halt details of A, you will see a mail connexion; you won't see any mail connexion at B though. It is very unlikely to happen, but if it does, it will be misleading. I have fixed this by checking if the ware category is enabled at the halt before proceeding with connexions construction.


(B) Performance issues :


(3)
Similar code block as quoted in point (1) recurs twice in rebuild_connexions() and twice in add_connexion(). I have created a separate function to build a halt list for a given schedule, so that the same procedure does not need to be done more than once. This helps to save a little bit of processing time, and make the code clearer by factoring out repetitive code.


(4)
In add_connexion(), you first loop through the halts in the schedule of a line/convoy. Then for each halt, you loop through the same schedule again in a nested loop to calculate the journey distance from self halt to the current halt. Every time, you start your inner-loop search from the first halt in the schedule, and the number of iterations needed may be larger than the number of halts in the schedule, because you need to locate the self halt first before you start counting distance.

I have re-written the code and completely eliminated the inner search loop, by starting the loop at the next halt after the (1st) self halt, and incrementally adding up journey distance in each iteration, and resetting journey distance to 0 whenever another self halt is encountered. In this way, the same effects can be achieved in (n-1) iterations for a schedule with n halts.


(C) Miscellaneous issues :


(5)
In force_all_halts_paths_stale(), paths are made stale not by calling force_paths_stale() now, but by simply setting paths_timestamp[c] to 0, to speed up a little bit. Actually, by setting to 0 is already enough, for it will be reset to the current base_pathing_counter whenever paths are recalculated.


(6)
I have made some changes to prevent reconstruction of paths/connexions when opening and closing a schedule window without making any change. I have also added back paths/connexions reconstruction notification in convoi_t::set_line(), because the relevant section of code in set_schedule() is only executed on lineless convoys.

jamespetts

These are now incorporated into 3.13. Knightly - thank you very much for your work :-)
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.