Corrupt ride window when trains have more than 144 cars

[Broxzier]

This is likely the source of many crash reports that have to do with the ride window's events. When there are more than 144 cars, WindowRideVehicleScrollpaint writes out of bounds, overwriting other static fields (most likely the others in Ride.cpp though).

The static array:

OpenRCT2/src/openrct2-ui/windows/Ride.cpp

Line 2906 in 85baa6f

static VehicleDrawInfo _sprites_to_draw[144];

Used in this loop, which may loop more than 144 times for cheated trains:

OpenRCT2/src/openrct2-ui/windows/Ride.cpp

Lines 2932 to 2934 in 85baa6f

	for (int32_t i = 0; i < ride->num_vehicles; i++)
	{
	VehicleDrawInfo* nextSpriteToDraw = _sprites_to_draw;

Writing out of bounds here:

OpenRCT2/src/openrct2-ui/windows/Ride.cpp

Lines 2971 to 2974 in 85baa6f

	nextSpriteToDraw->x = x;
	nextSpriteToDraw->y = y;
	nextSpriteToDraw->imageId = imageId;
	nextSpriteToDraw++;

Making sure the array is longer than the maximum number of vehicles may fix this, though since trains can have invisible cars, I'm not entirely sure the actual number can still be higher than 255.

[Gymnasiast]

Do we have any idea where the 144 comes from?

[Broxzier]

It was added 8 years ago 8355bf6, likely taken from the original game. I can't think of any vehicles from the original games that support that many cars per train. Chris Sawyer may simply have picked this size to be sure it would be enough.

[Gymnasiast]

I can't think of any vehicles from the original games that support that many cars per train

The number of cars per train seems irrelevant here, it loops over the trains, not the individual cars per train.

[Broxzier]

Exactly the opposite, the number of trains is irrelevant, the number of cars is not. It loops over the cars for each vehicle to draw the full train in the scroll view widget. At the beginning of the vehicle loop, it stores a pointer to this list in nextSpriteToDraw, then in the loop inside the vehicle loop, it uses the value and increments the pointer with each car.

[Broxzier]

Then why does it loop over ride->num_vehicles, which contains the number of trains?

Because it draws the cars for every train, and reuses the array for each train.

At the beginning of the vehicle loop, it stores a pointer to this list in nextSpriteToDraw, then in the loop inside the vehicle loop, it uses the value and increments the pointer with each car.

OpenRCT2/src/openrct2-ui/windows/Ride.cpp

Line 2939 in 85baa6f

for (int32_t j = 0; j < ride->num_cars_per_train; j++)

FWIW I've already got a PR ready that fixes the crashes: #17074