Fix excessive recursion in link graph refresher with conditional order loops

src/linkgraph/refresh.cpp

@@ -65,26 +65,6 @@
 	}
 }
 
-/**
- * Comparison operator to allow hops to be used in a std::set.
- * @param other Other hop to be compared with.
- * @return If this hop is "smaller" than the other (defined by from, to and cargo in this order).
- */
-bool LinkRefresher::Hop::operator<(const Hop &other) const
-{
-	if (this->from < other.from) {
-		return true;
-	} else if (this->from > other.from) {
-		return false;
-	}
-	if (this->to < other.to) {
-		return true;
-	} else if (this->to > other.to) {
-		return false;
-	}
-	return this->cargo < other.cargo;
-}
-
 /**
  * Constructor for link refreshing algorithm.
  * @param vehicle Vehicle to refresh links for.
@@ -215,10 +195,17 @@ const Order *LinkRefresher::PredictNextOrder(const Order *cur, const Order *next
 			if (skip_to != nullptr && num_hops < std::min<uint>(64, this->vehicle->orders->GetNumOrders()) && skip_to != next) {
 				/* Make copies of capacity tracking lists. There is potential
 				 * for optimization here: If the vehicle never refits we don't
-				 * need to copy anything. Also, if we've seen the branched link
-				 * before we don't need to branch at all. */
-				LinkRefresher branch(*this);
-				branch.RefreshLinks(cur, skip_to, flags, num_hops + 1);
+				 * need to copy anything. */
+
+				/* Record the branch before executing it,
+				 * to avoid recursively executing it again. */
+				Hop hop(cur->index, skip_to->index, this->cargo, flags);
+				auto iter = this->seen_hops->lower_bound(hop);
+				if (iter == this->seen_hops->end() || *iter != hop) {
+					this->seen_hops->insert(iter, hop);
+					LinkRefresher branch(*this);
+					branch.RefreshLinks(cur, skip_to, flags, num_hops + 1);
+				}
 			}
 		}
 
@@ -331,10 +318,11 @@ void LinkRefresher::RefreshLinks(const Order *cur, const Order *next, uint8 flag
 		next = this->PredictNextOrder(cur, next, flags, num_hops);
 		if (next == nullptr) break;
 		Hop hop(cur->index, next->index, this->cargo);
-		if (this->seen_hops->find(hop) != this->seen_hops->end()) {
+		auto iter = this->seen_hops->lower_bound(hop);
+		if (iter != this->seen_hops->end() && *iter == hop) {
 			break;
 		} else {
-			this->seen_hops->insert(hop);
+			this->seen_hops->insert(iter, hop);
 		}
 
 		/* Don't use the same order again, but choose a new one in the next round. */

src/linkgraph/refresh.h

@@ -61,6 +61,7 @@ protected:
 		OrderID from;  ///< Last order where vehicle could interact with cargo or absolute first order.
 		OrderID to;    ///< Next order to be processed.
 		CargoID cargo; ///< Cargo the consist is probably carrying or CT_INVALID if unknown.
+		uint8 flags;   ///< Flags, for branches
 
 		/**
 		 * Default constructor should not be called but has to be visible for
@@ -74,8 +75,10 @@ protected:
 		 * @param to Second order of the hop.
 		 * @param cargo Cargo the consist is probably carrying when passing the hop.
 		 */
-		Hop(OrderID from, OrderID to, CargoID cargo) : from(from), to(to), cargo(cargo) {}
-		bool operator<(const Hop &other) const;
+		Hop(OrderID from, OrderID to, CargoID cargo, uint8 flags = 0) : from(from), to(to), cargo(cargo), flags(flags) {}
+		bool operator<(const Hop &other) const { return std::tie(this->from, this->to, this->cargo, this->flags) < std::tie(other.from, other.to, other.cargo, other.flags); }
+		bool operator==(const Hop &other) const { return std::tie(this->from, this->to, this->cargo, this->flags) == std::tie(other.from, other.to, other.cargo, other.flags); }
+		bool operator!=(const Hop &other) const { return !(*this == other); }
 	};
 
 	typedef std::vector<RefitDesc> RefitList;