Replace sprite_left,t,r,b with SpriteRect (#15408)

* Replace sprite_left,t,r,b with SpriteRect

This is required for the NSF. I had to add a silly constructor to rct_sprite for now. Will eventually be removed one day...

* Fix compilation and memory leak

* Add some UB to prevent some other UB

* Make review change

src/openrct2-ui/interface/ViewportInteraction.cpp

@@ -711,11 +711,11 @@ PeepDistance GetClosestPeep(const ScreenCoordsXY& viewportCoords, const int32_t
 {
     for (auto peep : EntityList<T>())
     {
-        if (peep->sprite_left == LOCATION_NULL)
+        if (peep->x == LOCATION_NULL)
             continue;
 
-        auto distance = abs(((peep->sprite_left + peep->sprite_right) / 2) - viewportCoords.x)
+        auto distance = abs(((peep->SpriteRect.GetLeft() + peep->SpriteRect.GetRight()) / 2) - viewportCoords.x)
-            + abs(((peep->sprite_top + peep->sprite_bottom) / 2) - viewportCoords.y);
+            + abs(((peep->SpriteRect.GetTop() + peep->SpriteRect.GetBottom()) / 2) - viewportCoords.y);
         if (distance > maxDistance)
             continue;
 

src/openrct2/paint/sprite/Paint.Sprite.cpp

@@ -92,8 +92,8 @@ void sprite_paint_setup(paint_session* session, const uint16_t x, const uint16_t
 
         dpi = &session->DPI;
 
-        if (dpi->y + dpi->height <= spr->sprite_top || spr->sprite_bottom <= dpi->y || dpi->x + dpi->width <= spr->sprite_left
+        if (dpi->y + dpi->height <= spr->SpriteRect.GetTop() || spr->SpriteRect.GetBottom() <= dpi->y
-            || spr->sprite_right <= dpi->x)
+            || dpi->x + dpi->width <= spr->SpriteRect.GetLeft() || spr->SpriteRect.GetRight() <= dpi->x)
         {
             continue;
         }

src/openrct2/peep/Peep.cpp

@@ -1177,15 +1177,15 @@ void peep_update_crowd_noise()
 
     for (auto peep : EntityList<Guest>())
     {
-        if (peep->sprite_left == LOCATION_NULL)
+        if (peep->x == LOCATION_NULL)
             continue;
-        if (viewport->viewPos.x > peep->sprite_right)
+        if (viewport->viewPos.x > peep->SpriteRect.GetRight())
             continue;
-        if (viewport->viewPos.x + viewport->view_width < peep->sprite_left)
+        if (viewport->viewPos.x + viewport->view_width < peep->SpriteRect.GetLeft())
             continue;
-        if (viewport->viewPos.y > peep->sprite_bottom)
+        if (viewport->viewPos.y > peep->SpriteRect.GetBottom())
             continue;
-        if (viewport->viewPos.y + viewport->view_height < peep->sprite_top)
+        if (viewport->viewPos.y + viewport->view_height < peep->SpriteRect.GetTop())
             continue;
 
         visiblePeeps += peep->State == PeepState::Queuing ? 1 : 2;

src/openrct2/rct1/S4Importer.cpp

@@ -2688,10 +2688,7 @@ namespace RCT1
         dst->sprite_height_positive = src->sprite_height_positive;
         dst->sprite_direction = src->sprite_direction;
 
-        dst->sprite_left = src->sprite_left;
+        dst->SpriteRect = ScreenRect(src->sprite_left, src->sprite_top, src->sprite_right, src->sprite_bottom);
-        dst->sprite_top = src->sprite_top;
-        dst->sprite_right = src->sprite_right;
-        dst->sprite_bottom = src->sprite_bottom;
 
         dst->mass = src->mass;
         dst->num_seats = src->num_seats;

src/openrct2/rct2/S6Exporter.cpp

@@ -1227,10 +1227,10 @@ void S6Exporter::ExportEntityCommonProperties(RCT12SpriteBase* dst, const Entity
     dst->z = src->z;
     dst->sprite_width = src->sprite_width;
     dst->sprite_height_positive = src->sprite_height_positive;
-    dst->sprite_left = src->sprite_left;
+    dst->sprite_left = src->SpriteRect.GetLeft();
-    dst->sprite_top = src->sprite_top;
+    dst->sprite_top = src->SpriteRect.GetTop();
-    dst->sprite_right = src->sprite_right;
+    dst->sprite_right = src->SpriteRect.GetRight();
-    dst->sprite_bottom = src->sprite_bottom;
+    dst->sprite_bottom = src->SpriteRect.GetBottom();
     dst->sprite_direction = src->sprite_direction;
 }
 

src/openrct2/rct2/S6Importer.cpp

@@ -1546,10 +1546,7 @@ public:
         dst->z = src->z;
         dst->sprite_width = src->sprite_width;
         dst->sprite_height_positive = src->sprite_height_positive;
-        dst->sprite_left = src->sprite_left;
+        dst->SpriteRect = ScreenRect(src->sprite_left, src->sprite_top, src->sprite_right, src->sprite_bottom);
-        dst->sprite_top = src->sprite_top;
-        dst->sprite_right = src->sprite_right;
-        dst->sprite_bottom = src->sprite_bottom;
         dst->sprite_direction = src->sprite_direction;
     }
 

src/openrct2/ride/Vehicle.cpp

@@ -967,7 +967,7 @@ bool Vehicle::SoundCanPlay() const
     if (sound1_id == OpenRCT2::Audio::SoundId::Null && sound2_id == OpenRCT2::Audio::SoundId::Null)
         return false;
 
-    if (sprite_left == LOCATION_NULL)
+    if (x == LOCATION_NULL)
         return false;
 
     if (g_music_tracking_viewport == nullptr)
@@ -984,7 +984,7 @@ bool Vehicle::SoundCanPlay() const
         bottom -= quarter_h;
     }
 
-    if (left >= sprite_right || bottom >= sprite_bottom)
+    if (left >= SpriteRect.GetRight() || bottom >= SpriteRect.GetBottom())
         return false;
 
     int16_t right = g_music_tracking_viewport->view_width + left;
@@ -996,7 +996,7 @@ bool Vehicle::SoundCanPlay() const
         top += quarter_h + quarter_h;
     }
 
-    if (right < sprite_left || top < sprite_top)
+    if (right < SpriteRect.GetRight() || top < SpriteRect.GetTop())
         return false;
 
     return true;
@@ -1026,7 +1026,7 @@ OpenRCT2::Audio::VehicleSoundParams Vehicle::CreateSoundParam(uint16_t priority)
 {
     OpenRCT2::Audio::VehicleSoundParams param;
     param.priority = priority;
-    int32_t panX = (sprite_left / 2) + (sprite_right / 2) - g_music_tracking_viewport->viewPos.x;
+    int32_t panX = (SpriteRect.GetLeft() / 2) + (SpriteRect.GetRight() / 2) - g_music_tracking_viewport->viewPos.x;
     panX = panX / g_music_tracking_viewport->zoom;
     panX += g_music_tracking_viewport->pos.x;
 
@@ -1037,7 +1037,7 @@ OpenRCT2::Audio::VehicleSoundParams Vehicle::CreateSoundParam(uint16_t priority)
     }
     param.pan_x = ((((panX * 65536) / screenWidth) - 0x8000) >> 4);
 
-    int32_t panY = (sprite_top / 2) + (sprite_bottom / 2) - g_music_tracking_viewport->viewPos.y;
+    int32_t panY = (SpriteRect.GetTop() / 2) + (SpriteRect.GetBottom() / 2) - g_music_tracking_viewport->viewPos.y;
     panY = panY / g_music_tracking_viewport->zoom;
     panY += g_music_tracking_viewport->pos.y;
 

src/openrct2/world/EntityBase.h

@@ -1,8 +1,7 @@
 #pragma once
 
 #include "../common.h"
-
+#include "../world/Location.hpp"
-struct CoordsXYZ;
 
 enum class EntityType : uint8_t
 {
@@ -37,10 +36,7 @@ struct EntityBase
     // Height from centre of sprite to top
     uint8_t sprite_height_positive;
     // Screen Coordinates of sprite
-    int16_t sprite_left;
+    ScreenRect SpriteRect;
-    int16_t sprite_top;
-    int16_t sprite_right;
-    int16_t sprite_bottom;
 
     uint8_t sprite_direction;
 

src/openrct2/world/Sprite.cpp

@@ -163,7 +163,7 @@ const std::vector<uint16_t>& GetEntityTileList(const CoordsXY& spritePos)
 
 void EntityBase::Invalidate()
 {
-    if (sprite_left == LOCATION_NULL)
+    if (x == LOCATION_NULL)
         return;
 
     int32_t maxZoom = 0;
@@ -196,7 +196,7 @@ void EntityBase::Invalidate()
             break;
     }
 
-    viewports_invalidate(sprite_left, sprite_top, sprite_right, sprite_bottom, maxZoom);
+    viewports_invalidate(SpriteRect.GetLeft(), SpriteRect.GetTop(), SpriteRect.GetRight(), SpriteRect.GetBottom(), maxZoom);
 }
 
 static void ResetEntityLists()
@@ -228,7 +228,19 @@ const std::list<uint16_t>& GetEntityList(const EntityType id)
 void reset_sprite_list()
 {
     gSavedAge = 0;
-    std::memset(static_cast<void*>(_spriteList), 0, sizeof(_spriteList));
+
+    // Free all associated Entity pointers prior to zeroing memory
+    for (int32_t i = 0; i < MAX_ENTITIES; ++i)
+    {
+        auto* spr = GetEntity(i);
+        if (spr == nullptr)
+        {
+            continue;
+        }
+        FreeEntity(*spr);
+    }
+
+    std::fill(std::begin(_spriteList), std::end(_spriteList), rct_sprite());
     OpenRCT2::RideUse::GetHistory().Clear();
     OpenRCT2::RideUse::GetTypeHistory().Clear();
     for (int32_t i = 0; i < MAX_ENTITIES; ++i)
@@ -238,7 +250,6 @@ void reset_sprite_list()
         {
             continue;
         }
-        FreeEntity(*spr);
         spr->Type = EntityType::Null;
         spr->sprite_index = i;
 
@@ -313,7 +324,8 @@ static void sprite_reset(EntityBase* sprite)
     uint16_t sprite_index = sprite->sprite_index;
     _spriteFlashingList[sprite_index] = false;
 
-    std::memset(sprite, 0, sizeof(rct_sprite));
+    rct_sprite* spr = reinterpret_cast<rct_sprite*>(sprite);
+    *spr = rct_sprite();
 
     sprite->sprite_index = sprite_index;
     sprite->Type = EntityType::Null;
@@ -370,7 +382,7 @@ static void PrepareNewEntity(EntityBase* base, const EntityType type)
     base->sprite_width = 0x10;
     base->sprite_height_negative = 0x14;
     base->sprite_height_positive = 0x8;
-    base->sprite_left = LOCATION_NULL;
+    base->SpriteRect = {};
 
     SpriteSpatialInsert(base, { LOCATION_NULL, 0 });
 }
@@ -505,7 +517,6 @@ void EntityBase::MoveTo(const CoordsXYZ& newLocation)
 
     if (loc.x == LOCATION_NULL)
     {
-        sprite_left = LOCATION_NULL;
         x = loc.x;
         y = loc.y;
         z = loc.z;
@@ -533,10 +544,9 @@ void sprite_set_coordinates(const CoordsXYZ& spritePos, EntityBase* sprite)
 {
     auto screenCoords = translate_3d_to_2d_with_z(get_current_rotation(), spritePos);
 
-    sprite->sprite_left = screenCoords.x - sprite->sprite_width;
+    sprite->SpriteRect = ScreenRect(
-    sprite->sprite_right = screenCoords.x + sprite->sprite_width;
+        screenCoords - ScreenCoordsXY{ sprite->sprite_width, sprite->sprite_height_negative },
-    sprite->sprite_top = screenCoords.y - sprite->sprite_height_negative;
+        screenCoords + ScreenCoordsXY{ sprite->sprite_width, sprite->sprite_height_positive });
-    sprite->sprite_bottom = screenCoords.y + sprite->sprite_height_positive;
     sprite->x = spritePos.x;
     sprite->y = spritePos.y;
     sprite->z = spritePos.z;

src/openrct2/world/Sprite.h

@@ -23,6 +23,11 @@ union rct_sprite
 {
     uint8_t pad_00[0x200];
     EntityBase base;
+    // Provide a constructor as EntityBase is not trivialy constructable
+    rct_sprite()
+        : pad_00()
+    {
+    }
 };
 assert_struct_size(rct_sprite, 0x200);