summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authordorobouNeko <gonzalob1302@protonmail.com>2019-05-05 17:02:06 -0400
committerCharles Pigott <charlespigott@googlemail.com>2019-10-21 22:31:25 +0100
commitcbefc1d99491def454fe25d1700c6d41925c81d6 (patch)
tree30ec736b7a1c4ab9e438d745939dfd8d4916a095
parent81272000f8504f9f9d0c71c644c8a5493a91e807 (diff)
downloadopenttd-cbefc1d99491def454fe25d1700c6d41925c81d6.tar.xz
Fix #7561: Fix power/running-cost vehicle sorter
-rw-r--r--src/build_vehicle_gui.cpp42
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 */