From 8a6d855eddc477d342f4b67a14dd7472e72470da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Wed, 24 Sep 2025 15:42:44 +0300 Subject: [PATCH 1/5] Fix #25201: Unstable sorting of the ride list --- src/openrct2-ui/windows/RideList.cpp | 41 ++++++++++++++++------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/openrct2-ui/windows/RideList.cpp b/src/openrct2-ui/windows/RideList.cpp index a0c20842d0..d88ce4896f 100644 --- a/src/openrct2-ui/windows/RideList.cpp +++ b/src/openrct2-ui/windows/RideList.cpp @@ -983,6 +983,11 @@ namespace OpenRCT2::Ui::Windows void SortList() { + // Maintain stability by first sorting by ride id. + SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { + return thisRide.id.ToUnderlying() < otherRide.id.ToUnderlying(); + }); + switch (listInformationType) { case INFORMATION_TYPE_STATUS: @@ -990,77 +995,77 @@ namespace OpenRCT2::Ui::Windows break; case INFORMATION_TYPE_POPULARITY: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.popularity * 4 <= otherRide.popularity * 4; + return thisRide.popularity < otherRide.popularity; }); break; case INFORMATION_TYPE_SATISFACTION: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.satisfaction * 5 <= otherRide.satisfaction * 5; + return thisRide.satisfaction < otherRide.satisfaction; }); break; case INFORMATION_TYPE_PROFIT: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.profit <= otherRide.profit; + return thisRide.profit < otherRide.profit; }); break; case INFORMATION_TYPE_TOTAL_CUSTOMERS: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.totalCustomers <= otherRide.totalCustomers; + return thisRide.totalCustomers < otherRide.totalCustomers; }); break; case INFORMATION_TYPE_TOTAL_PROFIT: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.totalProfit <= otherRide.totalProfit; + return thisRide.totalProfit < otherRide.totalProfit; }); break; case INFORMATION_TYPE_CUSTOMERS: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return RideCustomersPerHour(thisRide) <= RideCustomersPerHour(otherRide); + return RideCustomersPerHour(thisRide) < RideCustomersPerHour(otherRide); }); break; case INFORMATION_TYPE_AGE: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.buildDate <= otherRide.buildDate; + return thisRide.buildDate < otherRide.buildDate; }); break; case INFORMATION_TYPE_INCOME: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.incomePerHour <= otherRide.incomePerHour; + return thisRide.incomePerHour < otherRide.incomePerHour; }); break; case INFORMATION_TYPE_RUNNING_COST: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.upkeepCost <= otherRide.upkeepCost; + return thisRide.upkeepCost < otherRide.upkeepCost; }); break; case INFORMATION_TYPE_QUEUE_LENGTH: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.getTotalQueueLength() <= otherRide.getTotalQueueLength(); + return thisRide.getTotalQueueLength() < otherRide.getTotalQueueLength(); }); break; case INFORMATION_TYPE_QUEUE_TIME: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.getMaxQueueTime() <= otherRide.getMaxQueueTime(); + return thisRide.getMaxQueueTime() < otherRide.getMaxQueueTime(); }); break; case INFORMATION_TYPE_RELIABILITY: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.reliabilityPercentage <= otherRide.reliabilityPercentage; + return thisRide.reliabilityPercentage < otherRide.reliabilityPercentage; }); break; case INFORMATION_TYPE_DOWN_TIME: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.downtime <= otherRide.downtime; + return thisRide.downtime < otherRide.downtime; }); break; case INFORMATION_TYPE_LAST_INSPECTION: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.lastInspection <= otherRide.lastInspection; + return thisRide.lastInspection < otherRide.lastInspection; }); break; case INFORMATION_TYPE_GUESTS_FAVOURITE: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.guestsFavourite <= otherRide.guestsFavourite; + return thisRide.guestsFavourite < otherRide.guestsFavourite; }); break; case INFORMATION_TYPE_EXCITEMENT: @@ -1068,7 +1073,7 @@ namespace OpenRCT2::Ui::Windows const auto leftValue = thisRide.ratings.isNull() ? RideRating::kUndefined : thisRide.ratings.excitement; const auto rightValue = otherRide.ratings.isNull() ? RideRating::kUndefined : otherRide.ratings.excitement; - return leftValue <= rightValue; + return leftValue < rightValue; }); break; case INFORMATION_TYPE_INTENSITY: @@ -1076,14 +1081,14 @@ namespace OpenRCT2::Ui::Windows const auto leftValue = thisRide.ratings.isNull() ? RideRating::kUndefined : thisRide.ratings.intensity; const auto rightValue = otherRide.ratings.isNull() ? RideRating::kUndefined : otherRide.ratings.intensity; - return leftValue <= rightValue; + return leftValue < rightValue; }); break; case INFORMATION_TYPE_NAUSEA: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { const auto leftValue = thisRide.ratings.isNull() ? RideRating::kUndefined : thisRide.ratings.nausea; const auto rightValue = otherRide.ratings.isNull() ? RideRating::kUndefined : otherRide.ratings.nausea; - return leftValue <= rightValue; + return leftValue < rightValue; }); break; } From 5c693e56d630b11a35ce37dbf10817bcd7a5bccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Wed, 24 Sep 2025 15:45:26 +0300 Subject: [PATCH 2/5] Make unknown popularity go at the bottom when sorted by highest --- src/openrct2-ui/windows/RideList.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2-ui/windows/RideList.cpp b/src/openrct2-ui/windows/RideList.cpp index d88ce4896f..342877d88b 100644 --- a/src/openrct2-ui/windows/RideList.cpp +++ b/src/openrct2-ui/windows/RideList.cpp @@ -995,7 +995,7 @@ namespace OpenRCT2::Ui::Windows break; case INFORMATION_TYPE_POPULARITY: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.popularity < otherRide.popularity; + return static_cast(thisRide.popularity) < static_cast(otherRide.popularity); }); break; case INFORMATION_TYPE_SATISFACTION: From 3bf75a1bc1b6b4ea37d307720b34c6d191da7be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Wed, 24 Sep 2025 15:46:38 +0300 Subject: [PATCH 3/5] Make unknown satisfaction go at the bottom when sorted by highest --- src/openrct2-ui/windows/RideList.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2-ui/windows/RideList.cpp b/src/openrct2-ui/windows/RideList.cpp index 342877d88b..2b440772fe 100644 --- a/src/openrct2-ui/windows/RideList.cpp +++ b/src/openrct2-ui/windows/RideList.cpp @@ -1000,7 +1000,7 @@ namespace OpenRCT2::Ui::Windows break; case INFORMATION_TYPE_SATISFACTION: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.satisfaction < otherRide.satisfaction; + return static_cast(thisRide.satisfaction) < static_cast(otherRide.satisfaction); }); break; case INFORMATION_TYPE_PROFIT: From 43f7d2d91202ec08e9617a779ffbe6d59078c4f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Wed, 24 Sep 2025 16:01:59 +0300 Subject: [PATCH 4/5] Fix logicalCmp not sorting in natural order, refactor the entire thing --- src/openrct2-ui/windows/RideList.cpp | 5 +- src/openrct2/core/String.cpp | 108 ++++++++++++++++++++------- test/tests/StringTest.cpp | 37 +++++++++ 3 files changed, 120 insertions(+), 30 deletions(-) diff --git a/src/openrct2-ui/windows/RideList.cpp b/src/openrct2-ui/windows/RideList.cpp index 2b440772fe..fc0d03232f 100644 --- a/src/openrct2-ui/windows/RideList.cpp +++ b/src/openrct2-ui/windows/RideList.cpp @@ -1004,9 +1004,8 @@ namespace OpenRCT2::Ui::Windows }); break; case INFORMATION_TYPE_PROFIT: - SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { - return thisRide.profit < otherRide.profit; - }); + SortListByPredicate( + [](const Ride& thisRide, const Ride& otherRide) -> bool { return thisRide.profit < otherRide.profit; }); break; case INFORMATION_TYPE_TOTAL_CUSTOMERS: SortListByPredicate([](const Ride& thisRide, const Ride& otherRide) -> bool { diff --git a/src/openrct2/core/String.cpp b/src/openrct2/core/String.cpp index d23dcfcd11..1c3c5ada10 100644 --- a/src/openrct2/core/String.cpp +++ b/src/openrct2/core/String.cpp @@ -722,7 +722,7 @@ namespace OpenRCT2::String return escaped.str(); } - /* Case insensitive logical compare */ + /* Case insensitive logical compare, produces the same output as Notepad++ lexicographical sort */ // Example: // - Guest 10 // - Guest 99 @@ -731,34 +731,88 @@ namespace OpenRCT2::String // - John v2.1 int32_t logicalCmp(const char* s1, const char* s2) { - for (;;) + const auto isDigit = [](char c) { return std::isdigit(static_cast(c)); }; + const auto toUpper = [](char c) { return std::toupper(static_cast(c)); }; + + // Prioritise strings starting with digits + bool s1StartsDigit = isDigit(*s1); + bool s2StartsDigit = isDigit(*s2); + if (s1StartsDigit && !s2StartsDigit) { - if (*s2 == '\0') - return *s1 != '\0'; - if (*s1 == '\0') - return -1; - if (!(isdigit(static_cast(*s1)) && isdigit(static_cast(*s2)))) - { - if (toupper(*s1) != toupper(*s2)) - return toupper(*s1) - toupper(*s2); - - ++s1; - ++s2; - } - else - { - char *lim1, *lim2; - unsigned long n1 = strtoul(s1, &lim1, 10); - unsigned long n2 = strtoul(s2, &lim2, 10); - if (n1 > n2) - return 1; - if (n1 < n2) - return -1; - - s1 = lim1; - s2 = lim2; - } + return -1; // s1 (starts with digit) comes before s2 } + if (!s1StartsDigit && s2StartsDigit) + { + return 1; // s2 (starts with digit) comes before s1 + } + + // If both start with digits, compare lexicographically + if (s1StartsDigit && s2StartsDigit) + { + while (*s1 != '\0' && *s2 != '\0') + { + char c1 = toUpper(*s1); + char c2 = toUpper(*s2); + if (c1 != c2) + { + return c1 - c2; + } + s1++; + s2++; + } + return *s1 == '\0' ? (*s2 == '\0' ? 0 : -1) : 1; + } + + while (*s1 != '\0' && *s2 != '\0') + { + // Check if both characters are digits + if (isDigit(*s1) && isDigit(*s2)) + { + // Skip leading zeros + while (*s1 == '0' && isDigit(*(s1 + 1))) + s1++; + while (*s2 == '0' && isDigit(*(s2 + 1))) + s2++; + + unsigned long long num1 = 0, num2 = 0; + const char* p1 = s1; + const char* p2 = s2; + + while (isDigit(*p1)) + { + num1 = num1 * 10 + (*p1 - '0'); + p1++; + } + while (isDigit(*p2)) + { + num2 = num2 * 10 + (*p2 - '0'); + p2++; + } + + if (num1 != num2) + { + return num1 < num2 ? -1 : 1; + } + + s1 = p1; + s2 = p2; + + continue; + } + + // Compare non-digit characters case-insensitively + char c1 = toUpper(*s1); + char c2 = toUpper(*s2); + if (c1 != c2) + { + return c1 - c2; + } + + s1++; + s2++; + } + + return *s1 == '\0' ? (*s2 == '\0' ? 0 : -1) : 1; } char* safeUtf8Copy(char* destination, const char* source, size_t size) diff --git a/test/tests/StringTest.cpp b/test/tests/StringTest.cpp index 05a72b3f7c..cd71d82f7f 100644 --- a/test/tests/StringTest.cpp +++ b/test/tests/StringTest.cpp @@ -10,6 +10,7 @@ #include "AssertHelpers.hpp" #include "helpers/StringHelpers.hpp" +#include #include #include #include @@ -17,6 +18,7 @@ #include #include #include +#include using namespace OpenRCT2; @@ -250,3 +252,38 @@ TEST_F(CodepointViewTest, CodepointView_iterate) AssertCodepoints("ゲスト", { U'ゲ', U'ス', U'ト' }); AssertCodepoints("<🎢>", { U'<', U'🎢', U'>' }); } + +TEST_F(StringTest, LogicalCompare) +{ + std::vector expected = { + "1001 Troubles", "3D Cinema 1", "Aerial Cycles", "Batflyer", "bpb", + "bpb.sv6", "Drive-by", "foo", "foobar", "Guest 10", + "Guest 99", "Guest 100", "John v2.0", "John v2.1", "River of the Damned", + "Terror-dactyl", + }; + + std::vector inputs = { + "Guest 99", + "Batflyer", + "John v2.1", + "bpb", + "3D Cinema 1", + "Drive-by", + "John v2.0", + "Guest 10", + "Terror-dactyl", + "Aerial Cycles", + "foobar", + "1001 Troubles", + "River of the Damned", + "bpb.sv6", + "Guest 100", + "foo", + }; + + std::sort(inputs.begin(), inputs.end(), [](const auto& a, const auto& b) { + return String::logicalCmp(a.c_str(), b.c_str()) < 0; + }); + + AssertVector(inputs, expected); +} From 15e78dd98eabd6472a5787d38073bad6d216e009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 25 Sep 2025 16:35:08 +0300 Subject: [PATCH 5/5] Update changelog.txt --- distribution/changelog.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/distribution/changelog.txt b/distribution/changelog.txt index 865bea0114..df9d1ebe57 100644 --- a/distribution/changelog.txt +++ b/distribution/changelog.txt @@ -21,6 +21,7 @@ - Fix: [#25163] Some of the Junior Roller Coaster flat to steep track wooden support clearance heights are different to RCT1. - Fix: [#25173] Desync when placing a park entrance in multiplayer. - Fix: [#25179] The LIM Launched Roller Coaster inline twists have incorrect wooden support clearance heights (original bug). +- Fix: [#25201] Ride list sort order can be unstable when sorted in descending order, change the order for unknown popularity and satisfaction to be last not first. - Fix: [#25207] Building a block brake on an LIM coaster does not automatically switch it to powered launch block sectioned mode. - Fix: [#25238] The chance of thunder and lightning effects happening is lower than vanilla.