Timetable based separation patch

[MagicBuzz]

I just released a new version of the patch.

To make it easy, I just rewritten everything, and added a control that automatically reorder the shared order list according the actual vehicle position : its "previous" vehicle will be now automatically the vehicle that is in front of it.

As a result, the separation is quite immediate now except in some rare cases (you add vehicles just before the first vehicle... they will have a big lateness to catch up in order to use proper separation.

-- Edit : Fixed the issue with boats when looking for the entering station (as a boats does not stops in a station but beside)

As you can see with the test savegame, this patch just helps to to ensure good ratings with less vehicles.

[MagicBuzz]

About the network support, I wonder if I should save the "std::list<Vehicle *> shared_order_previous_vehicles;" I added to to the Station class into the "static const SaveLoad _station_desc[]" array...

This list stores the last vehicle of each shared order list that deserved it while it is the first order station.

It is used after to reorder the shared order chain, that directly impacts the time a vehicle waits to a station.
How does the network sync works ? I read in the passenger destination patch something about the SaveLoad options for a class, so I wonder if I should store it as well...

Could someone help me testing that ?

[l_Blue_l]

You bet me to it. You have made a dynamic list of vehicles positions in chain. I am working on a way to create a vehicle position list by useing the vehicles current order but it still needs work.

On the subject of networking i have no idea. Personaly i hide from it . Good luck on working it all out.

[MagicBuzz]

I am working on a way to create a vehicle position list by useing the vehicles current order but it still needs work.

That looks like the feature I did in fact.

When a vehicle arrives at the order 0 station :
- It looks for the last vehicle of the same shared order that visited the station
- It updates the vehicle shared order chain to be the next vehicle after this one
- It updates the station to tell he is the last one from the list that visited the station

=> By this way, the shared order chain is always sorted according the current vehicle order on the route.

This works only for vehicles using shared order and timetable. But if needed it could be changed to work with any shared order vehicle. Also, it works only with the order 0 station. But it could also be changed to work with any visited station.

Currently, the last issue I have with my patch is about the order 0 : if it's not a station where the vehicle is ordered to stop, it doesn't work (no crash, there is just not separation). I have to add a new function to the vehicle class "FirstStopStationOrder()" that could return a DestinationID. With that, I'll be able to make the patch working whatever are the vehicle order. The last issue will be for vehicles that have only waypoints as orders, and just stop at station because they are on the path. But with this kind of route, te timetable is just useless as well, so there isn't a real problem.

[MagicBuzz]

Ok, let's go for a new update

Now, the separation and order chain are correctly computed when the first order isn't a station where the vehicle actually stops : all these tests will be done at the first actual station stop.

[MagicBuzz]

I just did a few test with a multiplayer game.
As I'm alone, I wasn't able to test a lot, but I didn't experience any problem after 3 years, so it seems to work

[l_Blue_l]

I am working on a way to create a vehicle position list by useing the vehicles current order but it still needs work.

That looks like the feature I did in fact.

Yes it is i was just commenting on the way i have implermented my version which has no need for a Station lists. Not 100% sure on how your version works but mine would beable to initialise a fairly accurate list without the need for std::list<Vehicle *>::.

Instead of std::list<Vehicle *>:: i used the current_order_indexs.

Instead of something like this

Code: Select all

+			Station *st = GetStation(this->current_order.dest);
+			std::list<Vehicle *>::iterator iter;
+			Vehicle *vehicle_to_remove = NULL;
+			for (iter = st->shared_order_previous_vehicles.begin(); iter != st->shared_order_previous_vehicles.end(); ++iter) {
+				Vehicle *previous = *iter;
+				if (this->IsVehicleInSharedOrdersList(previous)) {
+					/**
+					 * This vehicle is in the shared order list.
+					 * If it's not this previous one, we have to reorder the chain
+					 */
+					if (this->prev_shared != previous && this->prev_shared != NULL) {

You could do something like this

Code: Select all

UpdateTimetable()
{
VehicleOrderID index;
{
  //Get last order index
  if (this->vur_order_index == 0){
    index = this->number_of_vehicles_orders - 1;
  }
  else {
    index = this->cur_order_index -1;
  }
  // This Vehicle has passed the Vehicle that was infront of it.   
  if (this->prev_shared->cur_order_index == index) {
    // Change vehicles possition
  }
}

I have also done a way to initialise an accurate Vehicle list from the Vehicles current order. This is done by sorting the Vehicle from the cur_order_index then sub sorting the vehicles with the same cur_order_index with there current_travel_time.

[MagicBuzz]

Well, your system only checks if the vehicle passed its previous vehicle, not it the last vehicle that visited the station was its previous.
Your system should end up to the same result, but it would requiere a lot of time : if you just swap vehicles in the chain, they won't be at the right position before several visits.

Your cur_order_index based list interrests me. How does it work ? How to you keep it up to date ? I chose the last visitors list because I wanted to same some CPU cycles, as the wait_time tests are quite CPU consumers, I didn't add another heavy process for chain order.

Anyway. I just added a small test in order to avoid using the IsVehicleInSharedOrdersList() function that consumes some CPU cycles for nothing most of the time.

Code: Select all

			/// Find the previous vehicle of the shared order that visited the station
			Station *st = GetStation(this->current_order.dest);
			std::list<Vehicle *>::iterator iter;
			Vehicle *vehicle_to_remove = NULL;
			for (iter = st->shared_order_previous_vehicles.begin(); iter != st->shared_order_previous_vehicles.end(); ++iter) {
				Vehicle *previous = *iter;
				if (this->prev_shared == previous) {
					/**
					 * Everything is ok
					 * Added this test because it should happen most of the time and avoid using the
					 * IsVehicleInSharedOrdersList() function that is CPU consumer
					 */
					vehicle_to_remove = previous;
					break;
				}
				if (this->IsVehicleInSharedOrdersList(previous)) {
					/**
					 * This vehicle is in the shared order list.
					 * As it's not this previous one, we have to reorder the chain
					 */
					if (this->prev_shared != NULL) {
						// Remove this vehicle from the list
						this->prev_shared->next_shared = this->next_shared;
						if (this->next_shared != NULL) this->next_shared->prev_shared = this->prev_shared;
						// Insert in the list
						this->prev_shared = previous;
						this->next_shared = previous->next_shared;
						// Update previous and next
						if (previous->next_shared != NULL) previous->next_shared->prev_shared = this;
						previous->next_shared = this;
					}

					/**
					 * Remove the previous vehicle from the shared_order_previous_vehicles
					 */
					vehicle_to_remove = previous;
					break;
				}
			}
			/**
			 * Remove the previous vehicle and add this vehicle to the shared_order_previous_vehicles
			 */
			if (vehicle_to_remove != NULL) st->shared_order_previous_vehicles.remove(vehicle_to_remove);
			st->shared_order_previous_vehicles.push_back(this);

[l_Blue_l]

That was just a simplerfied version of how my system will work. There would be more checks to make sure the vehicle was move to the right position. Also the list would be initialised so each vehicle would be in aproximately the right position.

I have also done a way to initialise an accurate Vehicle list from the Vehicles current order. This is done by sorting the Vehicle from the cur_order_index then sub sorting the vehicles with the same cur_order_index with there current_travel_time.

Also i dont see how this would affects the wait_time as it doesnt even touch any of that code. Also i dont understand your comment about chain order.

[Maedhros]

I'm currently working on a way to set a date when the timetable should start. If a vehicle arrives at the first station before that date it will sit there and wait. This will also make it easy to auto-separate the vehicles by looping through all the shared vehicles, finding the highest timetable start date, and adding (timetable length / number of vehicles) to that starting date.

[l_Blue_l]

This start date thing intresst me. I can't wait to see how this system works.

[MagicBuzz]

Well, I tried to get ride of the list, but the patch don't work as expected, and I can't understand why

I think this function doesn't work.
I rewrote if several times, using different methods, but each time I'm stuck : separation doesn't work anymore.

Code: Select all

/**
 * Returns the actual separation with the vehicle
 * This function must be called ONLY when a vehicle enters a station
 */
int Vehicle::GetSeparationWithVehicle(Vehicle *v)
{
	Order *o;

	/**
	 * Compute the other vehicle current position
	 */
	/// Adds current_order_time plus lateness_counter
	int total = v->current_order_time;

	if (v->cur_order_index == 0 && v->current_order.type != OT_LOADING) {
		/// If it's travelling to the first order, we have to add all times but the travel_time for the first order
		for (o = v->orders->next; o != NULL; o = o->next) total += o->travel_time + o->wait_time;
		total += v->orders->wait_time;
	} else if (first->cur_order_index > 0) {
		/// If it's not at first order, we add the first order wait_time plus all orders times between first and current
		total += v->orders->wait_time;
		/// Then all travel_time and wait_time for orders between first and current
		byte order_index = 0;
		for (o = v->orders->next; ++order_index < v->cur_order_index; o = o->next) total += o->travel_time + o->wait_time;
		/// Adds the current order travel time if it's already loading
		if (v->current_order.type == OT_LOADING) total += v->current_order.travel_time;
	}

	/**
	 * Now, substract this vehicle current position
	 */
	o = this->orders;
	for (int i = 1, cpt = this->cur_order_index; i <= cpt; i++) {
		total -= o->wait_time;
		o = o->next;
		total -= o->travel_time;
	}

	/// If the vehicle is behind this one, we add the timetable total duraction
	if (total < 0) {
		total += this->GetTimetableTotalDuration();
	}

	return total;
}

[l_Blue_l]

Well, I tried to get ride of the list, but the patch don't work as expected, and I can't understand why

I think this function doesn't work.
I rewrote if several times, using different methods, but each time I'm stuck : separation doesn't work anymore.

Code: Select all

/**
 * Returns the actual separation with the vehicle
 * This function must be called ONLY when a vehicle enters a station
 */
int Vehicle::GetSeparationWithVehicle(Vehicle *v)
{
	Order *o;

	/**
	 * Compute the other vehicle current position
	 */
	/// Adds current_order_time plus lateness_counter
	int total = v->current_order_time;

	if (v->cur_order_index == 0 && v->current_order.type != OT_LOADING) {
		/// If it's travelling to the first order, we have to add all times but the travel_time for the first order
		for (o = v->orders->next; o != NULL; o = o->next) total += o->travel_time + o->wait_time;
		total += v->orders->wait_time;
	} else if (first->cur_order_index > 0) {
		/// If it's not at first order, we add the first order wait_time plus all orders times between first and current
		total += v->orders->wait_time;
		/// Then all travel_time and wait_time for orders between first and current
		byte order_index = 0;
		for (o = v->orders->next; ++order_index < v->cur_order_index; o = o->next) total += o->travel_time + o->wait_time;
		/// Adds the current order travel time if it's already loading
		if (v->current_order.type == OT_LOADING) total += v->current_order.travel_time;
	}

	/**
	 * Now, substract this vehicle current position
	 */
	o = this->orders;
	for (int i = 1, cpt = this->cur_order_index; i <= cpt; i++) {
		total -= o->wait_time;
		o = o->next;
		total -= o->travel_time;
	}

	/// If the vehicle is behind this one, we add the timetable total duraction
	if (total < 0) {
		total += this->GetTimetableTotalDuration();
	}

	return total;
}

I dont understand what you are trying to return here. It has no reference to other Vehicles. If you want the seperation from first Vehicle in the shared ordered list maybe this would help

Code: Select all

{
  //adds a timetabled seperation for each vehicle that is infront of this one
  For(All previous shared Vehicles)
  {
     time += timetabledseperation;
  }
 Vehicle u = GetFirstVehicleinChain(this);
 time -= GetVehicleCurrentLoopTime(u);
}
int GetVehicleCurrentLoopTime()
{
  return how long it has been since this vehicle was at the first station;
}

Edit, Ok what i have put here doesnt work but i hope you understand what i mean.

[MagicBuzz]

Finaly I took my previous code, and found a bug, that's why I wasn't able to make it work correctly, as I was confident with my previous (wrong) results...

I updated the patch to r10908, and now I don't store anything anywhere as you suggested. Thus the bug I introduced with the last revision is fixed.

[l_Blue_l]

So GetVehicleSeparation() get the time since that Vehicle should have been at its first order. That is what it does isn't it.

[MagicBuzz]

No, it gives :
"time since the previous vehicle started the route" - "time since the current vehicle started the route".
If the value is negative, then it adds the "timetable total duration" to the result.

So actually it gives the real separation between the vehicles.

There is still a minor bug in it : in some case, the vehicle thinks it's not next the previous vehicle, but after. This seems to occur when the previous vehicle is still loading at the same station.

I'll try to fix it tomorrow.

[l_Blue_l]

aaahhh it all makes sence now thanks.

[MagicBuzz]

Hello,

I spent some nights on a bug, and ended up to fix it.

When two vehicles where loading at the same station, there were a problem with the separation computation. The first vehicle could remain stuck at the station, or a traffic jam would occure, of any other bad thing, like totaly inconsistent separation.

Now this bug is fixed, and according my tests the patch just works fine now.

[CobraA1]

Looks good - hope it makes it into the trunk .

[MagicBuzz]

Before getting in the trunk, I need some feedbacks in order to know if it's bugfree.