Fix #7561: Fix power/running-cost vehicle sorter

1 files changed, 28 insertions, 14 deletions

diff --git a/src/build_vehicle_gui.cpp b/src/build_vehicle_gui.cpp
index 2778ccd55..7e8eea4c5 100644
--- a/src/build_vehicle_gui.cpp
+++ b/src/build_vehicle_gui.cpp
@@ -269,20 +269,34 @@ static bool EnginePowerVsRunningCostSorter(const EngineID &a, const EngineID &b)
 {
 	const Engine *e_a = Engine::Get(a);
 	const Engine *e_b = Engine::Get(b);
-
-	/* Here we are using a few tricks to get the right sort.
-	 * We want power/running cost, but since we usually got higher running cost than power and we store the result in an int,
-	 * we will actually calculate cunning cost/power (to make it more than 1).
-	 * Because of this, the return value have to be reversed as well and we return b - a instead of a - b.
-	 * Another thing is that both power and running costs should be doubled for multiheaded engines.
-	 * Since it would be multiplying with 2 in both numerator and denominator, it will even themselves out and we skip checking for multiheaded. */
-	Money va = (e_a->GetRunningCost()) / max(1U, (uint)e_a->GetPower());
-	Money vb = (e_b->GetRunningCost()) / max(1U, (uint)e_b->GetPower());
-	int r = ClampToI32(vb - va);
-
-	/* Use EngineID to sort instead since we want consistent sorting */
-	if (r == 0) return EngineNumberSorter(a, b);
-	return _engine_sort_direction ? r > 0 : r < 0;
+	uint p_a = e_a->GetPower();
+	uint p_b = e_b->GetPower();
+	Money r_a = e_a->GetRunningCost();
+	Money r_b = e_b->GetRunningCost();
+	/* Check if running cost is zero in one or both engines.
+	 * If only one of them is zero then that one has higher value,
+	 * else if both have zero cost then compare powers. */
+	if (r_a == 0) {
+		if (r_b == 0) {
+			/* If it is ambiguous which to return go with their ID */
+			if (p_a == p_b) return EngineNumberSorter(a, b);
+			return _engine_sort_direction != (p_a < p_b);
+		}
+		return !_engine_sort_direction;
+	}
+	if (r_b == 0) return _engine_sort_direction;
+	/* Using double for more precision when comparing close values.
+	 * This shouldn't have any major effects in performance nor in keeping
+	 * the game in sync between players since it's used in GUI only in client side */
+	double v_a = (double)p_a / (double)r_a;
+	double v_b = (double)p_b / (double)r_b;
+	/* Use EngineID to sort if both have same power/running cost,
+	 * since we want consistent sorting.
+	 * Also if both have no power then sort with reverse of running cost to simulate
+	 * previous sorting behaviour for wagons. */
+	if (v_a == 0 && v_b == 0) return !EngineRunningCostSorter(a, b);
+	if (v_a == v_b)  return EngineNumberSorter(a, b);
+	return _engine_sort_direction != (v_a < v_b);
 }
 
 /* Train sorting functions */