Text only | Text with Images

The International Simutrans Forum

Development => Patches & Projects => Incorporated Patches and Solved Bug Reports => Topic started by: knightly on November 05, 2009, 11:01:08 am

Title: [patch] In rebuild_destinations()
Post by: knightly on November 05, 2009, 11:01:08 am
This small patch (against r2848 relative to the trunk folder) fixes a small bug :
Quote
   // might be a new halt to add
   if(  !warenziele_by_stops.is_contained(wzs)  ) {
      warenziele_by_stops.append(wzs);
      non_identical_schedules_flag[ctg] = true;
   }
ctg should actually be add_catg_index[ctg].

It also changes the warenziele building loops so as to reduce the number of iterations required.
Title: Re: [patch] In rebuild_destinations()
Post by: Spike on November 05, 2009, 11:32:46 am
We must invent a new "Code diver" batch for you :)

Edit: Thanks for finding all these quirks and fixing them!
Title: Re: [patch] In rebuild_destinations()
Post by: prissi on November 05, 2009, 03:20:47 pm
Yeah, this code review is indeed extremely helpful. Thank you.
Title: Re: [patch] In rebuild_destinations()
Post by: knightly on November 05, 2009, 03:42:40 pm
It is just a small contribution, and it is my honour to be able to help :)

@ Prissi

Sorry for asking, but may I know why there are 2 loops that increment ctg??

Quote
for(  uint8 ctg=0;  ctg<add_catg_index.get_count();  ctg ++  ) {
   if(
      (add_catg_index[ctg]==0  &&  halt->get_pax_enabled())  ||
      (add_catg_index[ctg]==1  &&  halt->get_post_enabled())  ||
      (add_catg_index[ctg]>=2  &&  halt->get_ware_enabled())
   ) {
      do {
         warenzielsorter_t wzs( halt, stop_count, add_catg_index[ctg] );
         // might be a new halt to add
         if(  !warenziele_by_stops.is_contained(wzs)  ) {
            warenziele_by_stops.append(wzs);
            non_identical_schedules_flag[ctg] = true;
         }
      } while(  add_catg_index[ctg++]>=2  &&  ctg<add_catg_index.get_count()  );
   }
}

Does the inner do-while loop serve some special purpose?
Title: Re: [patch] In rebuild_destinations()
Post by: prissi on November 05, 2009, 04:16:18 pm
Because if goods are enabled, this means all goods are accepted there. p****=0, mail=1, all other goods>=2. THus the additional test above is not needed.
Title: Re: [patch] In rebuild_destinations()
Post by: knightly on November 05, 2009, 04:49:38 pm
However, the category indices in both add_catg_index (of lineless convoys) and goods_catg_index (of lines) are not sorted. Bulk goods may be processed before pax/mail, and if unfortunately pax/mail are not supported by the halt, warenziele will be wrong. Thus, there is no guarantee that the inner do-while loop works correctly under all circumstances.

Incidentally, you may just declare add_catg_index as a minivec instead of vector like a line's goods_catg_index.
Text only | Text with Images