From 7f085e2752d07da71dceb603daa5f0bf98d3319a Mon Sep 17 00:00:00 2001
From: Duncan <duncans_pumpkin@hotmail.co.uk>
Date: Fri, 4 Nov 2022 20:17:31 +0000
Subject: [PATCH] Update rating skipping unused ride ids (#14425)

* Skip empty ride ids when evaluating ride ratings

This isn't much of a problem atm but when we increase the limit it may start to become an issue

* Increment network version

* Update changelog
---
 distribution/changelog.txt           |  1 +
 src/openrct2/network/NetworkBase.cpp |  2 +-
 src/openrct2/ride/Ride.cpp           |  5 +++++
 src/openrct2/ride/Ride.h             |  1 +
 src/openrct2/ride/RideRatings.cpp    | 33 +++++++++++++++++++++++-----
 5 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/distribution/changelog.txt b/distribution/changelog.txt
index 3c77c20a25..024514b957 100644
--- a/distribution/changelog.txt
+++ b/distribution/changelog.txt
@@ -8,6 +8,7 @@
 - Change: [#18230] Make the large flat to steep pieces available on the corkscrew roller coaster without cheats.
 - Change: [#18381] Convert custom invisible paths to the built-in ones.
 - Fix: [#14312] Research ride type message incorrect.
+- Fix: [#14425] Ride ratings do not skip unallocated ride ids.
 - Fix: [#15969] Guests heading for ride use vanilla behaviour
 - Fix: [#17316] Sides of River Rapids’ corners overlay other parts of the track.
 - Fix: [#17657] When switching from buying land rights to buying construction rights, grid disables and won't re-enable afterwards.
diff --git a/src/openrct2/network/NetworkBase.cpp b/src/openrct2/network/NetworkBase.cpp
index d494f49270..db114e35c8 100644
--- a/src/openrct2/network/NetworkBase.cpp
+++ b/src/openrct2/network/NetworkBase.cpp
@@ -43,7 +43,7 @@
 // It is used for making sure only compatible builds get connected, even within
 // single OpenRCT2 version.
 
-#define NETWORK_STREAM_VERSION "7"
+#define NETWORK_STREAM_VERSION "8"
 
 #define NETWORK_STREAM_ID OPENRCT2_VERSION "-" NETWORK_STREAM_VERSION
 
diff --git a/src/openrct2/ride/Ride.cpp b/src/openrct2/ride/Ride.cpp
index 9edb0875a2..ee812506ad 100644
--- a/src/openrct2/ride/Ride.cpp
+++ b/src/openrct2/ride/Ride.cpp
@@ -145,6 +145,11 @@ RideManager::Iterator RideManager::end()
     return RideManager::Iterator(*this, endIndex, endIndex);
 }
 
+RideManager::Iterator RideManager::get(RideId rideId)
+{
+    return RideManager::Iterator(*this, rideId.ToUnderlying(), _rides.size());
+}
+
 RideId GetNextFreeRideId()
 {
     auto result = static_cast<RideId::UnderlyingType>(_rides.size());
diff --git a/src/openrct2/ride/Ride.h b/src/openrct2/ride/Ride.h
index f8915405f0..439038e070 100644
--- a/src/openrct2/ride/Ride.h
+++ b/src/openrct2/ride/Ride.h
@@ -979,6 +979,7 @@ struct RideManager
     size_t size() const;
     Iterator begin();
     Iterator end();
+    Iterator get(RideId rideId);
     Iterator begin() const
     {
         return (const_cast<RideManager*>(this))->begin();
diff --git a/src/openrct2/ride/RideRatings.cpp b/src/openrct2/ride/RideRatings.cpp
index 8f4ba1989c..8164318822 100644
--- a/src/openrct2/ride/RideRatings.cpp
+++ b/src/openrct2/ride/RideRatings.cpp
@@ -154,24 +154,45 @@ static void ride_ratings_update_state(RideRatingUpdateState& state)
     }
 }
 
+static RideId GetNextRideToUpdate(RideId currentRide)
+{
+    auto rm = GetRideManager();
+    if (rm.size() == 0)
+    {
+        return RideId::GetNull();
+    }
+    // Skip all empty ride ids
+    auto nextRide = std::next(rm.get(currentRide));
+    // If at end, loop around
+    if (nextRide == rm.end())
+    {
+        nextRide = rm.begin();
+    }
+    return (*nextRide).id;
+}
+
 /**
  *
  *  rct2: 0x006B5A5C
  */
 static void ride_ratings_update_state_0(RideRatingUpdateState& state)
 {
-    auto nextRide = RideId::FromUnderlying(state.CurrentRide.ToUnderlying() + 1);
-    if (nextRide.ToUnderlying() >= OpenRCT2::Limits::MaxRidesInPark)
+    // It is possible that the current ride being calculated has
+    // been removed or due to import invalid. For both, reset
+    // ratings and start check at the start
+    if (get_ride(state.CurrentRide) == nullptr)
     {
-        nextRide = {};
+        state.CurrentRide = {};
     }
 
-    auto ride = get_ride(nextRide);
-    if (ride != nullptr && ride->status != RideStatus::Closed && !(ride->lifecycle_flags & RIDE_LIFECYCLE_FIXED_RATINGS))
+    auto nextRideId = GetNextRideToUpdate(state.CurrentRide);
+    auto nextRide = get_ride(nextRideId);
+    if (nextRide != nullptr && nextRide->status != RideStatus::Closed
+        && !(nextRide->lifecycle_flags & RIDE_LIFECYCLE_FIXED_RATINGS))
     {
         state.State = RIDE_RATINGS_STATE_INITIALISE;
     }
-    state.CurrentRide = nextRide;
+    state.CurrentRide = nextRideId;
 }
 
 /**