(svn r22392) [1.1] -Backport from trunk:

- Fix: Crash when clicking a removed company in the vehicle list dropdowns [FS#4592] (r22373)
- Fix: Make sure saving has completely and utterly finished before starting a new one. Otherwise you could start a save, which would be marked as done by the previous save stopping and then yet another save could be started... and that could create a deadlock [FS#4596] (r22371)
- Fix: When inserting automatic orders, do not create consecutive duplicate orders (r22333, r22332, r22331, r22330, r22329, r22328, r22327)
- Fix: Destinations of conditional orders were update incorrectly when deleting orders in front of the conditional orders, if the target order was the order just before of the conditional order (r22326)

src/ground_vehicle.hpp

@@ -50,8 +50,9 @@ struct GroundVehicleCache {
 
 /** Ground vehicle flags. */
 enum GroundVehicleFlags {
-	GVF_GOINGUP_BIT       = 0,
+	GVF_GOINGUP_BIT               = 0,  ///< Vehicle is currently going uphill. (Cached track information for acceleration)
-	GVF_GOINGDOWN_BIT     = 1,
+	GVF_GOINGDOWN_BIT             = 1,  ///< Vehicle is currently going downhill. (Cached track information for acceleration)
+	GVF_SUPPRESS_AUTOMATIC_ORDERS = 2,  ///< Disable insertion and removal of automatic orders until the vehicle completes the real order.
 };
 
 /**

src/order_cmd.cpp

@@ -830,6 +830,13 @@ void InsertOrder(Vehicle *v, Order *new_o, VehicleOrderID sel_ord)
 				u->cur_real_order_index = cur;
 			}
 		}
+		if (sel_ord == u->cur_auto_order_index && u->IsGroundVehicle()) {
+			/* We are inserting an order just before the current automatic order.
+			 * We do not know whether we will reach current automatic or the newly inserted order first.
+			 * So, disable creation of automatic orders until we are on track again. */
+			uint16 &gv_flags = u->GetGroundVehicleFlags();
+			SetBit(gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS);
+		}
 		if (sel_ord <= u->cur_auto_order_index) {
 			uint cur = u->cur_auto_order_index + 1;
 			/* Check if we don't go out of bound */
@@ -969,11 +976,12 @@ void DeleteOrder(Vehicle *v, VehicleOrderID sel_ord)
 		if (order->IsType(OT_CONDITIONAL)) {
 			VehicleOrderID order_id = order->GetConditionSkipToOrder();
 			if (order_id >= sel_ord) {
-				order->SetConditionSkipToOrder(max(order_id - 1, 0));
+				order_id = max(order_id - 1, 0);
 			}
 			if (order_id == cur_order_id) {
-				order->SetConditionSkipToOrder((order_id + 1) % v->GetNumOrders());
+				order_id = (order_id + 1) % v->GetNumOrders();
 			}
+			order->SetConditionSkipToOrder(order_id);
 		}
 		cur_order_id++;
 	}
@@ -1863,6 +1871,13 @@ bool UpdateOrderDest(Vehicle *v, const Order *order, int conditional_depth)
 				v->cur_auto_order_index = v->cur_real_order_index = next_order;
 				v->UpdateRealOrderIndex();
 				v->current_order_time += v->GetOrder(v->cur_real_order_index)->travel_time;
+
+				/* Disable creation of automatic orders.
+				 * When inserting them we do not know that we would have to make the conditional orders point to them. */
+				if (v->IsGroundVehicle()) {
+					uint16 &gv_flags = v->GetGroundVehicleFlags();
+					SetBit(gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS);
+				}
 			} else {
 				UpdateVehicleTimetable(v, true);
 				v->IncrementRealOrderIndex();

src/roadveh_cmd.cpp

@@ -1580,6 +1580,7 @@ static void CheckIfRoadVehNeedsService(RoadVehicle *v)
 		return;
 	}
 
+	SetBit(v->gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS);
 	v->current_order.MakeGoToDepot(depot, ODTFB_SERVICE);
 	v->dest_tile = rfdd.tile;
 	SetWindowWidgetDirty(WC_VEHICLE_VIEW, v->index, VVW_WIDGET_START_STOP_VEH);

src/saveload/saveload.cpp

@@ -2419,6 +2419,9 @@ void WaitTillSaved()
 	_save_thread->Join();
 	delete _save_thread;
 	_save_thread = NULL;
+
+	/* Make sure every other state is handled properly as well. */
+	ProcessAsyncSaveFinish();
 }
 
 /**

src/train_cmd.cpp

@@ -2287,19 +2287,21 @@ static bool TryReserveSafeTrack(const Train *v, TileIndex tile, Trackdir td, boo
 class VehicleOrderSaver
 {
 private:
-	Vehicle        *v;
+	Train          *v;
 	Order          old_order;
 	TileIndex      old_dest_tile;
 	StationID      old_last_station_visited;
 	VehicleOrderID index;
+	bool           suppress_automatic_orders;
 
 public:
-	VehicleOrderSaver(Vehicle *_v) :
+	VehicleOrderSaver(Train *_v) :
 		v(_v),
 		old_order(_v->current_order),
 		old_dest_tile(_v->dest_tile),
 		old_last_station_visited(_v->last_station_visited),
-		index(_v->cur_real_order_index)
+		index(_v->cur_real_order_index),
+		suppress_automatic_orders(HasBit(_v->gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS))
 	{
 	}
 
@@ -2308,6 +2310,7 @@ public:
 		this->v->current_order = this->old_order;
 		this->v->dest_tile = this->old_dest_tile;
 		this->v->last_station_visited = this->old_last_station_visited;
+		SB(this->v->gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS, 1, suppress_automatic_orders ? 1: 0);
 	}
 
 	/**
@@ -3767,6 +3770,7 @@ static void CheckIfTrainNeedsService(Train *v)
 		return;
 	}
 
+	SetBit(v->gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS);
 	v->current_order.MakeGoToDepot(depot, ODTFB_SERVICE);
 	v->dest_tile = tfdd.tile;
 	SetWindowWidgetDirty(WC_VEHICLE_VIEW, v->index, VVW_WIDGET_START_STOP_VEH);

src/vehicle.cpp

@@ -1797,6 +1797,17 @@ uint GetVehicleCapacity(const Vehicle *v, uint16 *mail_capacity)
  */
 void Vehicle::DeleteUnreachedAutoOrders()
 {
+	if (this->IsGroundVehicle()) {
+		uint16 &gv_flags = this->GetGroundVehicleFlags();
+		if (HasBit(gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS)) {
+			/* Do not delete orders, only skip them */
+			ClrBit(gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS);
+			this->cur_auto_order_index = this->cur_real_order_index;
+			InvalidateVehicleOrder(this, 0);
+			return;
+		}
+	}
+
 	const Order *order = this->GetOrder(this->cur_auto_order_index);
 	while (order != NULL) {
 		if (this->cur_auto_order_index == this->cur_real_order_index) break;
@@ -1843,18 +1854,77 @@ void Vehicle::BeginLoading()
 		this->current_order.SetNonStopType(ONSF_NO_STOP_AT_ANY_STATION);
 
 	} else {
+		assert(this->IsGroundVehicle());
+		bool suppress_automatic_orders = HasBit(this->GetGroundVehicleFlags(), GVF_SUPPRESS_AUTOMATIC_ORDERS);
+
 		/* We weren't scheduled to stop here. Insert an automatic order
 		 * to show that we are stopping here, but only do that if the order
 		 * list isn't empty. */
 		Order *in_list = this->GetOrder(this->cur_auto_order_index);
-		if (in_list != NULL && this->orders.list->GetNumOrders() < MAX_VEH_ORDER_ID &&
+		if (in_list != NULL &&
 				(!in_list->IsType(OT_AUTOMATIC) ||
-				in_list->GetDestination() != this->last_station_visited) &&
+				in_list->GetDestination() != this->last_station_visited)) {
-				Order::CanAllocateItem()) {
+			/* Do not create consecutive duplicates of automatic orders */
+			Order *prev_order = this->cur_auto_order_index > 0 ? this->GetOrder(this->cur_auto_order_index - 1) : NULL;
+			if (prev_order == NULL ||
+					(!prev_order->IsType(OT_AUTOMATIC) && !prev_order->IsType(OT_GOTO_STATION)) ||
+					prev_order->GetDestination() != this->last_station_visited) {
+
+				/* Prefer deleting automatic orders instead of inserting new ones,
+				 * so test whether the right order follows later */
+				int target_index = this->cur_auto_order_index;
+				bool found = false;
+				while (target_index != this->cur_real_order_index) {
+					const Order *order = this->GetOrder(target_index);
+					if (order->IsType(OT_AUTOMATIC) && order->GetDestination() == this->last_station_visited) {
+						found = true;
+						break;
+					}
+					target_index++;
+					if (target_index >= this->orders.list->GetNumOrders()) target_index = 0;
+					assert(target_index != this->cur_auto_order_index); // infinite loop?
+				}
+
+				if (found) {
+					if (suppress_automatic_orders) {
+						/* Skip to the found order */
+						this->cur_auto_order_index = target_index;
+						InvalidateVehicleOrder(this, 0);
+					} else {
+						/* Delete all automatic orders up to the station we just reached */
+						const Order *order = this->GetOrder(this->cur_auto_order_index);
+						while (!order->IsType(OT_AUTOMATIC) || order->GetDestination() != this->last_station_visited) {
+							if (order->IsType(OT_AUTOMATIC)) {
+								/* Delete order effectively deletes order, so get the next before deleting it. */
+								order = order->next;
+								DeleteOrder(this, this->cur_auto_order_index);
+							} else {
+								/* Skip non-automatic orders, e.g. service-orders */
+								order = order->next;
+								this->cur_auto_order_index++;
+							}
+
+							/* Wrap around */
+							if (order == NULL) {
+								order = this->GetOrder(0);
+								this->cur_auto_order_index = 0;
+							}
+							assert(order != NULL);
+						}
+					}
+				} else if (!suppress_automatic_orders && this->orders.list->GetNumOrders() < MAX_VEH_ORDER_ID && Order::CanAllocateItem()) {
+					/* Insert new automatic order */
 					Order *auto_order = new Order();
 					auto_order->MakeAutomatic(this->last_station_visited);
 					InsertOrder(this, auto_order, this->cur_auto_order_index);
 					if (this->cur_auto_order_index > 0) --this->cur_auto_order_index;
+
+					/* InsertOrder disabled creation of automatic orders for all vehicles with the same automatic order.
+					 * Reenable it for this vehicle */
+					uint16 &gv_flags = this->GetGroundVehicleFlags();
+					ClrBit(gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS);
+				}
+			}
 		}
 		this->current_order.MakeLoading(false);
 	}
@@ -1971,6 +2041,11 @@ CommandCost Vehicle::SendToDepot(DoCommandFlag flags, DepotCommand command)
 			 * then skip to the next order; effectively cancelling this forced service */
 			if (this->current_order.GetDepotOrderType() & ODTFB_PART_OF_ORDERS) this->IncrementRealOrderIndex();
 
+			if (this->IsGroundVehicle()) {
+				uint16 &gv_flags = this->GetGroundVehicleFlags();
+				SetBit(gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS);
+			}
+
 			this->current_order.MakeDummy();
 			SetWindowWidgetDirty(WC_VEHICLE_VIEW, this->index, VVW_WIDGET_START_STOP_VEH);
 		}
@@ -1986,6 +2061,11 @@ CommandCost Vehicle::SendToDepot(DoCommandFlag flags, DepotCommand command)
 	if (flags & DC_EXEC) {
 		if (this->current_order.IsType(OT_LOADING)) this->LeaveStation();
 
+		if (this->IsGroundVehicle()) {
+			uint16 &gv_flags = this->GetGroundVehicleFlags();
+			SetBit(gv_flags, GVF_SUPPRESS_AUTOMATIC_ORDERS);
+		}
+
 		this->dest_tile = location;
 		this->current_order.MakeGoToDepot(destination, ODTF_MANUAL);
 		if (!(command & DEPOT_SERVICE)) this->current_order.SetDepotActionType(ODATFB_HALT);
@@ -2391,6 +2471,36 @@ const GroundVehicleCache *Vehicle::GetGroundVehicleCache() const
 	}
 }
 
+/**
+ * Access the ground vehicle flags of the vehicle.
+ * @pre The vehicle is a #GroundVehicle.
+ * @return #GroundVehicleFlags of the vehicle.
+ */
+uint16 &Vehicle::GetGroundVehicleFlags()
+{
+	assert(this->IsGroundVehicle());
+	if (this->type == VEH_TRAIN) {
+		return Train::From(this)->gv_flags;
+	} else {
+		return RoadVehicle::From(this)->gv_flags;
+	}
+}
+
+/**
+ * Access the ground vehicle flags of the vehicle.
+ * @pre The vehicle is a #GroundVehicle.
+ * @return #GroundVehicleFlags of the vehicle.
+ */
+const uint16 &Vehicle::GetGroundVehicleFlags() const
+{
+	assert(this->IsGroundVehicle());
+	if (this->type == VEH_TRAIN) {
+		return Train::From(this)->gv_flags;
+	} else {
+		return RoadVehicle::From(this)->gv_flags;
+	}
+}
+
 /**
  * Calculates the set of vehicles that will be affected by a given selection.
  * @param set [inout] Set of affected vehicles.

src/vehicle_base.h

@@ -249,6 +249,9 @@ public:
 	GroundVehicleCache *GetGroundVehicleCache();
 	const GroundVehicleCache *GetGroundVehicleCache() const;
 
+	uint16 &GetGroundVehicleFlags();
+	const uint16 &GetGroundVehicleFlags() const;
+
 	void DeleteUnreachedAutoOrders();
 
 	void HandleLoading(bool mode = false);

src/vehicle_gui.cpp

@@ -1635,16 +1635,7 @@ static WindowDesc _vehicle_list_desc(
 
 static void ShowVehicleListWindowLocal(CompanyID company, VehicleListType vlt, VehicleType vehicle_type, uint16 unique_number)
 {
-	if (!Company::IsValidID(company)) {
-		company = _local_company;
-		/* This can happen when opening the vehicle list as a spectator.
-		 * While it would be cleaner to check this somewhere else, having
-		 * it here reduces code duplication */
 	if (!Company::IsValidID(company)) return;
-		_vehicle_list_desc.flags |= WDF_CONSTRUCTION;
-	} else {
-		_vehicle_list_desc.flags &= ~WDF_CONSTRUCTION;
-	}
 
 	_vehicle_list_desc.cls = GetWindowClassForVehicleType(vehicle_type);
 	AllocateWindowDescFront<VehicleListWindow>(&_vehicle_list_desc, VehicleListIdentifier(vlt, vehicle_type, company, unique_number).Pack());
@@ -1671,6 +1662,15 @@ void ShowVehicleListWindow(const Vehicle *v)
 
 void ShowVehicleListWindow(CompanyID company, VehicleType vehicle_type, StationID station)
 {
+	if (!Company::IsValidID(company)) {
+		company = _local_company;
+		/* This can happen when opening the vehicle list as a spectator. */
+		if (!Company::IsValidID(company)) return;
+		_vehicle_list_desc.flags |= WDF_CONSTRUCTION;
+	} else {
+		_vehicle_list_desc.flags &= ~WDF_CONSTRUCTION;
+	}
+
 	ShowVehicleListWindowLocal(company, VL_STATION_LIST, vehicle_type, station);
 }