[Fixes v6.1] Tolerance level in step_passagiere()

[knightly]

James,

I have fixed 3 bugs and made some minor optimisations in step_passagiere(). I have already uploaded my commit to Github, so please check it out when you have time.

The bugs are :

(1)

When calculating tolerance levels, the format of your formulas is simrand(max) + min. This is wrong, and may result in a figure above max. The proper format should be simrand(max - min + 1) + min. The " + 1 " is needed because simrand(X) will return random values in the range of [ 0 , (X-1) ], inclusive.


(2)

You only use a single tolerance variable to store the tolerance levels of the alternative destinations. Thus, the value stored in tolerance will always be the tolerance level for the last destination. When you are evaluating the different destinations, your code always compare journey time against the tolerance level of the last destination. I have fixed it by adding a member variable for tolerance in the destination structure.


(3)

You misplaced the break in the following code, causing the identity check among start halts and end halts to be incomplete when there are multiple start halts :

unsigned ziel_count = 0;
for (uint h = 0; h < dest_plan->get_haltlist_count(); h++)
{
   halthandle_t halt = dest_list[h];
   if (halt->is_enabled(wtyp))
   {
      ziel_count++;
      for(int i = start_halts.get_count(); i >= 0; i--)
      {
         if(start_halts.get_count() > i && halt == start_halts[ i ])
         {
            can_walk_ziel = true;
            start_halt = start_halts[ i ];
            haltestelle_t::erzeuge_fussgaenger(welt, start_halts[ i ]->get_basis_pos3d(), pax_left_to_do);
         }
      }
         break; // because we found at least one valid step ...
   }
}

And due to this bug, you added the following check, which is necessary when the abovementioned identity check is incomplete :

if(route_good == good)
{
   pax.arrival_time = welt->get_zeit_ms();

   // All passengers will use the quickest route.
   start_halt = start_halts[best_start_halt];

   if(start_halt == pax.get_ziel())
   {
      // Without this, where there are overlapping stops, passengers
      // for nearby destinations accumulate at a stop, destined for
      // that same stop, and never leave.
      can_walk_ziel = true;
      goto walk; // Not ideal, I know. Can anyone suggest anything better that does not increase overhead?
   }

I have fixed the bug and removed the check which is no longer necessary now.


Knightly

[jamespetts]

Thank you very much for your fixes - most helpful. Sorry that I haven't replied to your PMs yet - have been busy. I shall have a look at those again in a minute.

As to fix no. 3, I am not sure that I fully understand what you mean: the tolerance level is the tolerance of the passengers, not of the destinations. Thus, a single packet of passengers with multiple destinations will have the same level of tolerance no matter whether they are going to their primary, secondary or tertiary (etc.) destination. The idea is that the tolerance level represents the overall journey time above which the passengers no longer think travelling worthwhile at all. It would make no sense if they were prepared to travel for longer to get to their second choice destination than their first choice destination (when they would only be going to their second choice destination if their first choice destination is unreachable, or is only reachable by means that exceed their journey time tolerance).

Thank you again for your help :-)

[knightly]

James,

The tolerance level is still the tolerance level of pax of course. I think you have forgotten that you have different tolerance levels for different ranges? Journey time for destinations generated with different ranges has to be compared against tolerance level of the matching range, right?

Your original code also calculates different tolerance levels for different range with randomisation. If you take a look at what I have changed, you will realise that what I am doing is in line with your original intention.

Knightly

[knightly]

James,

I can see that you have changed the code for tolerance. This is not a problem. But can you please refrain from such long and obscure program statement, such as the line below?

Code Select Expand


const uint16 tolerance = wtyp != warenbauer_t::passagiere ? 0 : range == local ? simrand(welt->get_einstellungen()->get_max_local_tolerance()) + welt->get_einstellungen()->get_min_local_tolerance() : range == midrange ? simrand(welt->get_einstellungen()->get_max_midrange_tolerance()) + welt->get_einstellungen()->get_min_midrange_tolerance() : simrand(welt->get_einstellungen()->get_max_longdistance_tolerance()) + welt->get_einstellungen()->get_min_longdistance_tolerance();


I am sorry. I don't have the patience to decode and understand such unclear code.

Edit :

If you have reviewed my code for tolerance level, you should know that I have already added static const variables for the various tolerance levels in the beginning part of step_passagiere(). Please make use of them, instead of repeatedly calling welt->get_einstellungen()->get_something() again and again.

Knightly

[jamespetts]

Knightly,

thank you for your comments. I have, as you suggested, changed to using your variables rather than calling the methods repeatedly. As to the long line of code: I have now broken it down into multiple lines. I have kept it semantically identical so that I can still use const, but added whitespace for clarity.

[knightly]

James, thank you very much for changing it   It would be even nicer if you can enclose each ternary operator with parentheses.