The International Simutrans Forum

The only problem I'm having is constructing adequate test-cases for the routing behaviour. It seems to do what it should in all cases I've tried. But in particular I'm not sure how to test the code in haltestelle_t::rebuild_destinations() that adds stops to "warenziele" ordered by distance. I couldn't see how this affects routing, with or without my patch applied.

void convoi_t::advance_schedule() {
   // check if the convoi should switch direction
   if( fpl->is_mirrored() ) {
      if( fpl->get_aktuell()==0  ) {
         reverse_schedule = false;
      }
      else if( fpl->get_aktuell()==fpl->get_count()-1 ) {
         reverse_schedule = true;
      }
   }
   // advance the schedule cursor
   if( reverse_schedule ) {
      fpl->advance_reverse();
   } else {
      fpl->advance();
   }
}

A minor suggestion : maybe it's more convenient to p**** arguments by reference (instead of pointers) to schedule_t::increment_index(). In that way, you don't need to take the address of the variables, and later deference them again in the function body.

Besides, I would also suggest to relocate the reverse flag from convoys to schedules, as each convoy has its own copy of the schedule. In that way, the following code :could be encapsulated inside schedules. Convoy code should not know how mirrorring is implemented, or decide whether to advance schedule index normally or in reverse.

Conceptually, bidirectionality (both the directive to enforce it by alternating initial direction of individual convoys, as well as the actual state of it) is a property of lines only. They should not appear in the schedule copies of individual convoys (and definitely not in the schedules of lineless convoys). Thus I would suggest to move the bidirectionality flag from schedules to lines.

I think some special handling is necessary with bidirectional (but not mirrorred) lines and reversing.

For instance, what if a bidirectional line has only one convoy? Then this line is actually unidirectional. Warenziele entries would then have to be in line with the direction (normal or reverse) of the single convoy. And if its direction is reversed later, warenziele needs to be updated.

...

Unlike the mirror flag, setting the bidirectional flag of a line/schedule alone does not automatically make it bidirectional. Setting the bidirectional flag only indicates the wish to enforce bidirectionality; actual directionality depends on the directions of convoys ****ociated with the line. Thus, some extra logic is needed to keep track of the actual directionality of a line, and increment schedule counter whenever the directionality changes.

On the other hand, it is also necessary to increment schedule counter when the player reverses the direction of a lineless convoy.

I preferred to p**** by address just because it makes it more obvious that the variables will be changed, but I should've checked what the style was in other places . I've updated this and as you say it's cleaner as such.

If I move it to part of the schedule then there is the question of if the direction is not the same, is it the same schedule?

In particular, I would probably leave the direction out of sprintf_schedule and sscanf_schedule, but I'm not sure whether this would be desired in all cases.

True, the bidirectional flag should be part of the line. In terms of the UI though, even for lineless convoys you can "promote" their schedule to a line, so I'd need to find some better place of editing this. So far methods of editing the line just give you the schedule to edit, so in terms of the UI lines and schedules are almost identical.

Similarly for the direction flag, I couldn't see how to easily add it to the schedule editing UI without adding another row to the buttons at the top.

So I'm not too sure where it could be edited from.

I could possibly add another row for the active direction, and hide the "bidirectional" button for individual convoys. Then the bidirectional flag could sensibly be added to the line, and the direction flag to the schedule. I do like setting the direction from the convoy info window though.

Ahh yeah you're right... thinking about it I'll probably need to keep track of the served directions on a per-goods-type basis too, to catch a situation like all the buses going one way and all the mail vans going the other.

Well this will be my challenge for this weekend .

If I understand it correctly, sprintf_schedule() and sscanf_schedule() are for converting schedules to/from strings, a means of p****ing schedules when they are updated. It doesn't seem to be harmful at all to include direction as well. In sum, you may treat direction as a transient state much like aktuell. The advantage of putting the direction flag in schedule_t is that, you can encapsulate all processing and relevant data inside the same cl****, and you don't need to p**** the direction flag every time.