Fix #5096: Crash on park with out of bounds sprite coordinates

distribution/changelog.txt

@@ -18,6 +18,7 @@
 - Fix: [#4968] Completing a scenario does not save the name that is entered.
 - Fix: [#4996] Objects unloaded after loading landscape.
 - Fix: [#5003] Able to remove entrance/exit of unedittable rides (such as in Volcania).
+- Fix: [#5096] Failure to open parks with out of bounds sprite coordinates.
 - Fix: [#5114] Some entertainer costumes never select-able.
 
 0.0.5 (2016-12-27)

src/openrct2/world/sprite.c

@@ -14,7 +14,6 @@
  *****************************************************************************/
 #pragma endregion
 
-#include "../rct2/addresses.h"
 #include "../audio/audio.h"
 #include "../cheats.h"
 #include "../game.h"
@@ -22,6 +21,7 @@
 #include "../localisation/date.h"
 #include "../localisation/localisation.h"
 #include "../OpenRCT2.h"
+#include "../rct2/addresses.h"
 #include "../scenario/scenario.h"
 #include "fountain.h"
 #include "sprite.h"
@@ -36,11 +36,15 @@ uint16 *gSpriteListCount = RCT2_ADDRESS(RCT2_ADDRESS_SPRITE_LISTS_COUNT, uint16)
 static rct_sprite *_spriteList = RCT2_ADDRESS(RCT2_ADDRESS_SPRITE_LIST, rct_sprite);
 #endif
 
+#define SPATIAL_INDEX_LOCATION_NULL 0x10000
+
 uint16 gSpriteSpatialIndex[0x10001];
 
 static rct_xyz16 _spritelocations1[MAX_SPRITES];
 static rct_xyz16 _spritelocations2[MAX_SPRITES];
 
+static size_t GetSpatialIndexOffset(sint32 x, sint32 y);
+
 rct_sprite *get_sprite(size_t sprite_idx)
 {
 	openrct2_assert(sprite_idx < MAX_SPRITES, "Tried getting sprite %u", sprite_idx);
@@ -148,18 +152,11 @@ void reset_sprite_list()
  */
 void reset_sprite_spatial_index()
 {
-	memset(gSpriteSpatialIndex, -1, 0x10001 * sizeof(uint16));
+	memset(gSpriteSpatialIndex, -1, sizeof(gSpriteSpatialIndex));
 	for (size_t i = 0; i < MAX_SPRITES; i++) {
 		rct_sprite *spr = get_sprite(i);
 		if (spr->unknown.sprite_identifier != SPRITE_IDENTIFIER_NULL) {
-			uint32 index;
+			size_t index = GetSpatialIndexOffset(spr->unknown.x, spr->unknown.y);
-			if (spr->unknown.x == SPRITE_LOCATION_NULL) {
-				index = 0x10000;
-			} else {
-				sint16 x = floor2(spr->unknown.x, 32);
-				uint8 tileY = spr->unknown.y >> 5;
-				index = (x << 3) | tileY;
-			}
 			uint16 nextSpriteId = gSpriteSpatialIndex[index];
 			gSpriteSpatialIndex[index] = spr->unknown.sprite_index;
 			spr->unknown.next_in_quadrant = nextSpriteId;
@@ -167,6 +164,22 @@ void reset_sprite_spatial_index()
 	}
 }
 
+static size_t GetSpatialIndexOffset(sint32 x, sint32 y)
+{
+	size_t index = SPATIAL_INDEX_LOCATION_NULL;
+	if (x != SPRITE_LOCATION_NULL) {
+		x = clamp(0, x, 0xFFFF);
+		y = clamp(0, y, 0xFFFF);
+
+		sint16 flooredX = floor2(x, 32);
+		uint8 tileY = y >> 5;
+		index = (flooredX << 3) | tileY;
+	}
+
+	openrct2_assert(index < sizeof(gSpriteSpatialIndex), "GetSpatialIndexOffset out of range");
+	return index;
+}
+
 #ifndef DISABLE_NETWORK
 
 static uint8 _spriteChecksum[EVP_MAX_MD_SIZE + 1];
@@ -289,8 +302,8 @@ rct_sprite *create_sprite(uint8 bl)
 	sprite->flags = 0;
 	sprite->sprite_left = SPRITE_LOCATION_NULL;
 
-	sprite->next_in_quadrant = gSpriteSpatialIndex[0x10000];
+	sprite->next_in_quadrant = gSpriteSpatialIndex[SPATIAL_INDEX_LOCATION_NULL];
-	gSpriteSpatialIndex[0x10000] = sprite->sprite_index;
+	gSpriteSpatialIndex[SPATIAL_INDEX_LOCATION_NULL] = sprite->sprite_index;
 
 	return (rct_sprite*)sprite;
 }
@@ -495,46 +508,36 @@ void sprite_misc_update_all()
  * @param z (dx)
  * @param sprite (esi)
  */
-void sprite_move(sint16 x, sint16 y, sint16 z, rct_sprite* sprite){
+void sprite_move(sint16 x, sint16 y, sint16 z, rct_sprite *sprite)
-	if (x < 0 || y < 0 || x > 0x1FFF || y > 0x1FFF)
+{
+	if (x < 0 || y < 0 || x > 0x1FFF || y > 0x1FFF) {
 		x = SPRITE_LOCATION_NULL;
-
-	sint32 new_position = x;
-	if (x == SPRITE_LOCATION_NULL)new_position = 0x10000;
-	else{
-		new_position &= 0x1FE0;
-		new_position = (y >> 5) | (new_position << 3);
 	}
 
-	sint32 current_position = sprite->unknown.x;
+	size_t newIndex = GetSpatialIndexOffset(x, y);
-	if (sprite->unknown.x == SPRITE_LOCATION_NULL)current_position = 0x10000;
+	size_t currentIndex = GetSpatialIndexOffset(sprite->unknown.x, sprite->unknown.y);
-	else{
+	if (newIndex != currentIndex) {
-		current_position &= 0x1FE0;
+		uint16 *spriteIndex = &gSpriteSpatialIndex[currentIndex];
-		current_position = (sprite->unknown.y >> 5) | (current_position << 3);
+		rct_sprite *sprite2 = get_sprite(*spriteIndex);
+		while (sprite != sprite2) {
+			spriteIndex = &sprite2->unknown.next_in_quadrant;
+			sprite2 = get_sprite(*spriteIndex);
+		}
+		*spriteIndex = sprite->unknown.next_in_quadrant;
+
+		sint32 tempSpriteIndex = gSpriteSpatialIndex[newIndex];
+		gSpriteSpatialIndex[newIndex] = sprite->unknown.sprite_index;
+		sprite->unknown.next_in_quadrant = tempSpriteIndex;
 	}
 
-	if (new_position != current_position){
+	if (x == SPRITE_LOCATION_NULL) {
-		uint16* sprite_idx = &gSpriteSpatialIndex[current_position];
-		rct_sprite* sprite2 = get_sprite(*sprite_idx);
-		while (sprite != sprite2){
-			sprite_idx = &sprite2->unknown.next_in_quadrant;
-			sprite2 = get_sprite(*sprite_idx);
-		}
-		*sprite_idx = sprite->unknown.next_in_quadrant;
-
-		sint32 temp_sprite_idx = gSpriteSpatialIndex[new_position];
-		gSpriteSpatialIndex[new_position] = sprite->unknown.sprite_index;
-		sprite->unknown.next_in_quadrant = temp_sprite_idx;
-	}
-
-	if (x == SPRITE_LOCATION_NULL){
 		sprite->unknown.sprite_left = SPRITE_LOCATION_NULL;
 		sprite->unknown.x = x;
 		sprite->unknown.y = y;
 		sprite->unknown.z = z;
-		return;
+	} else {
-	}
 		sprite_set_coordinates(x, y, z, sprite);
+	}
 }
 
 void sprite_set_coordinates(sint16 x, sint16 y, sint16 z, rct_sprite *sprite){
@@ -577,13 +580,7 @@ void sprite_remove(rct_sprite *sprite)
 	user_string_free(sprite->unknown.name_string_idx);
 	sprite->unknown.sprite_identifier = SPRITE_IDENTIFIER_NULL;
 
-	uint32 quadrantIndex = sprite->unknown.x;
+	size_t quadrantIndex = GetSpatialIndexOffset(sprite->unknown.x, sprite->unknown.y);
-	if (sprite->unknown.x == SPRITE_LOCATION_NULL) {
-		quadrantIndex = 0x10000;
-	} else {
-		quadrantIndex = (floor2(sprite->unknown.x, 32) << 3) | (sprite->unknown.y >> 5);
-	}
-
 	uint16 *spriteIndex = &gSpriteSpatialIndex[quadrantIndex];
 	rct_sprite *quadrantSprite;
 	while ((quadrantSprite = get_sprite(*spriteIndex)) != sprite) {