Improve handling of conditional order waiting loops

Do not leave station/depot/waypoint at all if conditional order
loop would result in re-starting waiting/loading

Only actually leave and update timetable at end of loop

Rate-limit loop checks regardless of timetabled wait time

src/order_cmd.cpp

@@ -2783,14 +2783,18 @@ static StationID GetNextRealStation(const Vehicle *v, const Order *order, int co
 	return GetNextRealStation(v, (order->next != nullptr) ? order->next : v->GetFirstOrder(), ++conditional_depth);
 }
 
+static std::vector<TraceRestrictSlotID> _pco_deferred_slot_acquires;
+static std::vector<TraceRestrictSlotID> _pco_deferred_slot_releases;
+static btree::btree_map<Order *, int8> _pco_deferred_original_percent_cond;
+
 /**
  * Process a conditional order and determine the next order.
  * @param order the order the vehicle currently has
  * @param v the vehicle to update
- * @param dry_run whether this is a dry-run, so do not execute side-effects
+ * @param mode whether this is a dry-run so do not execute side-effects, or if side-effects are deferred
  * @return index of next order to jump to, or INVALID_VEH_ORDER_ID to use the next order
  */
-VehicleOrderID ProcessConditionalOrder(const Order *order, const Vehicle *v, bool dry_run)
+VehicleOrderID ProcessConditionalOrder(const Order *order, const Vehicle *v, ProcessConditionalOrderMode mode)
 {
 	if (order->GetType() != OT_CONDITIONAL) return INVALID_VEH_ORDER_ID;
 
@@ -2830,18 +2834,44 @@ VehicleOrderID ProcessConditionalOrder(const Order *order, const Vehicle *v, boo
 			break;
 		}
 		case OCV_SLOT_OCCUPANCY: {
-			const TraceRestrictSlot* slot = TraceRestrictSlot::GetIfValid(order->GetXData());
+			TraceRestrictSlotID slot_id = order->GetXData();
-			if (slot != nullptr) skip_order = OrderConditionCompare(occ, slot->occupants.size() >= slot->max_occupancy, value);
+			TraceRestrictSlot* slot = TraceRestrictSlot::GetIfValid(slot_id);
+			if (slot != nullptr) {
+				size_t count = slot->occupants.size();
+				if (mode == PCO_DEFERRED) {
+					if (find_index(_pco_deferred_slot_releases, slot_id) >= 0 && slot->IsOccupant(v->index)) {
+						count--;
+					} else if (find_index(_pco_deferred_slot_acquires, slot_id) >= 0 && !slot->IsOccupant(v->index)) {
+						count++;
+					}
+				}
+				skip_order = OrderConditionCompare(occ, count >= slot->max_occupancy, value);
+			}
 			break;
 		}
 		case OCV_VEH_IN_SLOT: {
-			TraceRestrictSlot* slot = TraceRestrictSlot::GetIfValid(order->GetXData());
+			TraceRestrictSlotID slot_id = order->GetXData();
+			TraceRestrictSlot* slot = TraceRestrictSlot::GetIfValid(slot_id);
 			if (slot != nullptr) {
 				bool occupant = slot->IsOccupant(v->index);
+				if (mode == PCO_DEFERRED) {
+					if (occupant && find_index(_pco_deferred_slot_releases, slot_id) >= 0) {
+						occupant = false;
+					} else if (!occupant && find_index(_pco_deferred_slot_acquires, slot_id) >= 0) {
+						occupant = true;
+					}
+				}
 				if (occ == OCC_EQUALS || occ == OCC_NOT_EQUALS) {
-					if (!occupant && !dry_run) {
+					if (!occupant && mode == PCO_EXEC) {
 						occupant = slot->Occupy(v->index);
 					}
+					if (!occupant && mode == PCO_DEFERRED) {
+						occupant = slot->OccupyDryRun(v->index);
+						if (occupant) {
+							include(_pco_deferred_slot_acquires, slot_id);
+							container_unordered_remove(_pco_deferred_slot_releases, slot_id);
+						}
+					}
 					occ = (occ == OCC_EQUALS) ? OCC_IS_TRUE : OCC_IS_FALSE;
 				}
 				skip_order = OrderConditionCompare(occ, occupant, value);
@@ -2856,7 +2886,10 @@ VehicleOrderID ProcessConditionalOrder(const Order *order, const Vehicle *v, boo
 		case OCV_PERCENT: {
 			/* get a non-const reference to the current order */
 			Order *ord = const_cast<Order *>(order);
-			skip_order = ord->UpdateJumpCounter((byte)value, dry_run);
+			if (mode == PCO_DEFERRED) {
+				_pco_deferred_original_percent_cond.insert({ ord, ord->GetJumpCounter() });
+			}
+			skip_order = ord->UpdateJumpCounter((byte)value, mode == PCO_DRY_RUN);
 			break;
 		}
 		case OCV_REMAINING_LIFETIME: skip_order = OrderConditionCompare(occ, std::max(v->max_age - v->age + DAYS_IN_LEAP_YEAR - 1, 0) / DAYS_IN_LEAP_YEAR, value); break;
@@ -2892,6 +2925,73 @@ VehicleOrderID ProcessConditionalOrder(const Order *order, const Vehicle *v, boo
 	return skip_order ? order->GetConditionSkipToOrder() : (VehicleOrderID)INVALID_VEH_ORDER_ID;
 }
 
+/* FlushAdvanceOrderIndexDeferred must be called after calling this */
+VehicleOrderID AdvanceOrderIndexDeferred(const Vehicle *v, VehicleOrderID index)
+{
+	int depth = 0;
+	++index;
+
+	do {
+		/* Wrap around. */
+		if (index >= v->GetNumOrders()) index = 0;
+
+		Order *order = v->GetOrder(index);
+		assert(order != nullptr);
+
+		switch (order->GetType()) {
+			case OT_RELEASE_SLOT:
+				if (TraceRestrictSlot::IsValidID(order->GetDestination())) {
+					include(_pco_deferred_slot_releases, order->GetDestination());
+					container_unordered_remove(_pco_deferred_slot_acquires, order->GetDestination());
+				}
+				break;
+
+			case OT_CONDITIONAL: {
+				VehicleOrderID next = ProcessConditionalOrder(order, v, PCO_DEFERRED);
+				if (next != INVALID_VEH_ORDER_ID) {
+					depth++;
+					index = next;
+					/* Don't increment next, so no break here. */
+					continue;
+				}
+				break;
+			}
+
+			default:
+				return index;
+		}
+		/* Don't increment inside the while because otherwise conditional
+		 * orders can lead to an infinite loop. */
+		++index;
+		depth++;
+	} while (depth < v->GetNumOrders());
+
+	/* Wrap around. */
+	if (index >= v->GetNumOrders()) index = 0;
+
+	return index;
+}
+
+void FlushAdvanceOrderIndexDeferred(const Vehicle *v, bool apply)
+{
+	if (apply) {
+		for (TraceRestrictSlotID slot : _pco_deferred_slot_acquires) {
+			TraceRestrictSlot::Get(slot)->Occupy(v->index);
+		}
+		for (TraceRestrictSlotID slot : _pco_deferred_slot_releases) {
+			TraceRestrictSlot::Get(slot)->Vacate(v->index);
+		}
+	} else {
+		for (auto item : _pco_deferred_original_percent_cond) {
+			item.first->SetJumpCounter(item.second);
+		}
+	}
+
+	_pco_deferred_slot_acquires.clear();
+	_pco_deferred_slot_releases.clear();
+	_pco_deferred_original_percent_cond.clear();
+}
+
 /**
  * Update the vehicle's destination tile from an order.
  * @param order the order the vehicle currently has
@@ -3071,6 +3171,8 @@ bool ProcessOrders(Vehicle *v)
 	 */
 	bool may_reverse = v->current_order.IsType(OT_NOTHING);
 
+	ClrBit(v->vehicle_flags, VF_COND_ORDER_WAIT);
+
 	/* Check if we've reached a 'via' destination. */
 	if (((v->current_order.IsType(OT_GOTO_STATION) && (v->current_order.GetNonStopType() & ONSF_NO_STOP_AT_DESTINATION_STATION)) ||
 			(v->current_order.IsType(OT_GOTO_WAYPOINT) && !v->current_order.IsWaitTimetabled())) &&

src/order_func.h

@@ -21,7 +21,16 @@ void CheckOrders(const Vehicle*);
 void DeleteVehicleOrders(Vehicle *v, bool keep_orderlist = false, bool reset_order_indices = true);
 bool ProcessOrders(Vehicle *v);
 bool UpdateOrderDest(Vehicle *v, const Order *order, int conditional_depth = 0, bool pbs_look_ahead = false);
-VehicleOrderID ProcessConditionalOrder(const Order *order, const Vehicle *v, bool dry_run = false);
+
+enum ProcessConditionalOrderMode {
+	PCO_EXEC,
+	PCO_DRY_RUN,
+	PCO_DEFERRED,
+};
+
+VehicleOrderID ProcessConditionalOrder(const Order *order, const Vehicle *v, ProcessConditionalOrderMode mode = PCO_EXEC);
+VehicleOrderID AdvanceOrderIndexDeferred(const Vehicle *v, VehicleOrderID index);
+void FlushAdvanceOrderIndexDeferred(const Vehicle *v, bool apply);
 uint GetOrderDistance(const Order *prev, const Order *cur, const Vehicle *v, int conditional_depth = 0);
 
 void DrawOrderString(const Vehicle *v, const Order *order, int order_index, int y, bool selected, bool timetable, int left, int middle, int right);

src/train_cmd.cpp

@@ -698,7 +698,7 @@ void AdvanceOrderIndex(const Vehicle *v, VehicleOrderID &index)
 			case OT_GOTO_WAYPOINT:
 				return;
 			case OT_CONDITIONAL: {
-				VehicleOrderID next = ProcessConditionalOrder(order, v, true);
+				VehicleOrderID next = ProcessConditionalOrder(order, v, PCO_DRY_RUN);
 				if (next != INVALID_VEH_ORDER_ID) {
 					depth++;
 					index = next;
@@ -3880,7 +3880,7 @@ public:
 					this->v->current_order = *order;
 					return UpdateOrderDest(this->v, order, 0, true);
 				case OT_CONDITIONAL: {
-					VehicleOrderID next = ProcessConditionalOrder(order, this->v, true);
+					VehicleOrderID next = ProcessConditionalOrder(order, this->v, PCO_DRY_RUN);
 					if (next != INVALID_VEH_ORDER_ID) {
 						depth++;
 						this->v->cur_real_order_index = next;
@@ -6368,6 +6368,7 @@ static bool TrainLocoHandler(Train *v, bool mode)
 	if (CheckTrainStayInDepot(v)) return true;
 
 	if (v->current_order.IsType(OT_WAITING) && v->reverse_distance == 0) {
+		if (mode) return true;
 		v->HandleWaiting(false, true);
 		if (v->current_order.IsType(OT_WAITING)) return true;
 		if (IsRailWaypointTile(v->tile)) {

src/vehicle.cpp

@@ -3334,6 +3334,8 @@ void Vehicle::LeaveStation()
 	delete this->cargo_payment;
 	assert(this->cargo_payment == nullptr); // cleared by ~CargoPayment
 
+	ClrBit(this->vehicle_flags, VF_COND_ORDER_WAIT);
+
 	TileIndex station_tile = INVALID_TILE;
 
 	if (this->type == VEH_TRAIN) {
@@ -3497,6 +3499,20 @@ void Vehicle::ResetRefitCaps()
 	for (Vehicle *v = this; v != nullptr; v = v->Next()) v->refit_cap = v->cargo_cap;
 }
 
+static bool ShouldVehicleContinueWaiting(Vehicle *v)
+{
+	if (v->GetNumOrders() < 1) return false;
+
+	/* Rate-limit re-checking of conditional order loop */
+	if (HasBit(v->vehicle_flags, VF_COND_ORDER_WAIT) && v->tick_counter % 32 != 0) return true;
+
+	/* If conditional orders lead back to this order, just keep waiting without leaving the order */
+	bool loop = AdvanceOrderIndexDeferred(v, v->cur_implicit_order_index) == v->cur_implicit_order_index;
+	FlushAdvanceOrderIndexDeferred(v, loop);
+	if (loop) SetBit(v->vehicle_flags, VF_COND_ORDER_WAIT);
+	return loop;
+}
+
 /**
  * Handle the loading of the vehicle; when not it skips through dummy
  * orders and does nothing in all other cases.
@@ -3517,7 +3533,7 @@ void Vehicle::HandleLoading(bool mode)
 			if (!mode && this->type != VEH_TRAIN) PayStationSharingFee(this, Station::Get(this->last_station_visited));
 
 			/* Not the first call for this tick, or still loading */
-			if (mode || !HasBit(this->vehicle_flags, VF_LOADING_FINISHED) || (this->current_order_time < wait_time && this->current_order.GetLeaveType() != OLT_LEAVE_EARLY)) {
+			if (mode || !HasBit(this->vehicle_flags, VF_LOADING_FINISHED) || (this->current_order_time < wait_time && this->current_order.GetLeaveType() != OLT_LEAVE_EARLY) || ShouldVehicleContinueWaiting(this)) {
 				if (!mode && this->type == VEH_TRAIN && HasBit(Train::From(this)->flags, VRF_ADVANCE_IN_PLATFORM)) this->AdvanceLoadingInStation();
 				return;
 			}
@@ -3559,8 +3575,12 @@ void Vehicle::HandleWaiting(bool stop_waiting, bool process_orders)
 			if (!stop_waiting && this->current_order_time < wait_time && this->current_order.GetLeaveType() != OLT_LEAVE_EARLY) {
 				return;
 			}
+			if (!stop_waiting && process_orders && ShouldVehicleContinueWaiting(this)) {
+				return;
+			}
 
 			/* When wait_time is expired, we move on. */
+			ClrBit(this->vehicle_flags, VF_COND_ORDER_WAIT);
 			UpdateVehicleTimetable(this, false);
 			this->IncrementImplicitOrderIndex();
 			this->current_order.MakeDummy();
@@ -3668,6 +3688,7 @@ CommandCost Vehicle::SendToDepot(DoCommandFlag flags, DepotCommand command, Tile
 
 	if (flags & DC_EXEC) {
 		if (this->current_order.IsAnyLoadingType()) this->LeaveStation();
+		if (this->current_order.IsType(OT_WAITING)) this->HandleWaiting(true);
 
 		if (this->type == VEH_TRAIN) {
 			for (Train *v = Train::From(this); v != nullptr; v = v->Next()) {
@@ -4123,6 +4144,7 @@ void DumpVehicleFlagsGeneric(const Vehicle *v, T dump, U dump_header)
 	dump('s', "VF_TIMETABLE_SEPARATION",    HasBit(v->vehicle_flags, VF_TIMETABLE_SEPARATION));
 	dump('a', "VF_AUTOMATE_TIMETABLE",      HasBit(v->vehicle_flags, VF_AUTOMATE_TIMETABLE));
 	dump('Q', "VF_HAVE_SLOT",               HasBit(v->vehicle_flags, VF_HAVE_SLOT));
+	dump('W', "VF_COND_ORDER_WAIT",         HasBit(v->vehicle_flags, VF_COND_ORDER_WAIT));
 	dump_header("vcf:", "cached_veh_flags:");
 	dump('l', "VCF_LAST_VISUAL_EFFECT",     HasBit(v->vcache.cached_veh_flags, VCF_LAST_VISUAL_EFFECT));
 	dump('z', "VCF_GV_ZERO_SLOPE_RESIST",   HasBit(v->vcache.cached_veh_flags, VCF_GV_ZERO_SLOPE_RESIST));

src/vehicle_base.h

@@ -63,6 +63,7 @@ enum VehicleFlags {
 	VF_TIMETABLE_SEPARATION     = 14, ///< Whether timetable auto-separation is enabled
 	VF_AUTOMATE_TIMETABLE       = 15, ///< Whether the vehicle should manage the timetable automatically.
 	VF_HAVE_SLOT                = 16, ///< Vehicle has 1 or more slots
+	VF_COND_ORDER_WAIT          = 17, ///< Vehicle is waiting due to conditional order loop
 };
 
 /** Bit numbers used to indicate which of the #NewGRFCache values are valid. */