From 247d72b1aa1d7d68e4c775f98f63e64e024b7bab Mon Sep 17 00:00:00 2001 From: rwjuk Date: Wed, 5 Jul 2017 19:36:35 +0100 Subject: [PATCH 01/13] Implement sprite cycle checking --- src/openrct2/rct2/S6Exporter.cpp | 2 +- src/openrct2/rct2/S6Importer.cpp | 4 ++ src/openrct2/world/sprite.c | 106 +++++++++++++++++++++++++++++++ src/openrct2/world/sprite.h | 3 +- 4 files changed, 113 insertions(+), 2 deletions(-) diff --git a/src/openrct2/rct2/S6Exporter.cpp b/src/openrct2/rct2/S6Exporter.cpp index 7a62e5626b..086456d20f 100644 --- a/src/openrct2/rct2/S6Exporter.cpp +++ b/src/openrct2/rct2/S6Exporter.cpp @@ -152,6 +152,7 @@ void S6Exporter::Save(IStream * stream, bool isScenario) void S6Exporter::Export() { + openrct2_assert(!(check_for_spatial_index_cycles(false) || check_for_sprite_list_cycles(false)), "A sprite cycle exists."); _s6.info = gS6Info; uint32 researchedTrackPiecesA[128]; uint32 researchedTrackPiecesB[128]; @@ -449,7 +450,6 @@ extern "C" map_reorganise_elements(); sprite_clear_all_unused(); - viewport_set_saved_view(); bool result = false; diff --git a/src/openrct2/rct2/S6Importer.cpp b/src/openrct2/rct2/S6Importer.cpp index 53ac9a2971..d12ea1071e 100644 --- a/src/openrct2/rct2/S6Importer.cpp +++ b/src/openrct2/rct2/S6Importer.cpp @@ -413,6 +413,10 @@ public: map_update_tile_pointers(); game_convert_strings_to_utf8(); map_count_remaining_land_rights(); + + // We try to fix the cycles on import, hence the 'true' parameter + check_for_sprite_list_cycles(true); + check_for_spatial_index_cycles(true); } void Initialise() diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index e313a71106..7f4cf210a4 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -267,6 +267,7 @@ void sprite_clear_all_unused() sprite->previous = previousSpriteIndex; sprite->linked_list_type_offset = SPRITE_LIST_NULL * 2; sprite->sprite_index = spriteIndex; + sprite->next_in_quadrant = (sprite->next_in_quadrant == 0) ? SPRITE_INDEX_NULL : sprite->next_in_quadrant; _spriteFlashingList[spriteIndex] = false; spriteIndex = nextSpriteIndex; } @@ -277,6 +278,7 @@ static void sprite_reset(rct_unk_sprite *sprite) // Need to retain how the sprite is linked in lists uint8 llto = sprite->linked_list_type_offset; uint16 next = sprite->next; + uint16 next_in_quadrant = sprite->next_in_quadrant; uint16 prev = sprite->previous; uint16 sprite_index = sprite->sprite_index; _spriteFlashingList[sprite_index] = false; @@ -285,6 +287,7 @@ static void sprite_reset(rct_unk_sprite *sprite) sprite->linked_list_type_offset = llto; sprite->next = next; + sprite->next_in_quadrant = next_in_quadrant; sprite->previous = prev; sprite->sprite_index = sprite_index; } @@ -823,3 +826,106 @@ bool sprite_get_flashing(rct_sprite *sprite) assert(sprite->unknown.sprite_index < MAX_SPRITES); return _spriteFlashingList[sprite->unknown.sprite_index]; } + +static bool sprite_is_in_cycle(uint16 sprite_idx, bool fix) +{ + if (sprite_idx == SPRITE_INDEX_NULL) + { + return false; + } + const rct_sprite * fast = get_sprite(sprite_idx); + const rct_sprite * slow = fast; + bool increment_slow = false; + bool cycled = false; + while (fast->unknown.sprite_index != SPRITE_INDEX_NULL) + { + // increment fast every time, unless reached the end + if (fast->unknown.next == SPRITE_INDEX_NULL) + { + break; + } + else { + fast = get_sprite(fast->unknown.next); + } + // increment slow only every second iteration + if (increment_slow) + { + slow = get_sprite(slow->unknown.next); + } + increment_slow = !increment_slow; + if (fast == slow) + { + cycled = true; + if (fix) + { + rct_sprite * spr = get_sprite(sprite_idx); + // There may be a better solution than simply setting this to 0xFFFF + spr->unknown.next = SPRITE_INDEX_NULL; + } + break; + } + } + return cycled; +} + +static bool sprite_is_in_cycle_quadrant(uint16 sprite_idx, bool fix) +{ + if (sprite_idx == SPRITE_INDEX_NULL) + { + return false; + } + const rct_sprite * fast = get_sprite(sprite_idx); + const rct_sprite * slow = fast; + bool increment_slow = false; + bool cycled = false; + while (fast->unknown.sprite_index != SPRITE_INDEX_NULL) + { + // increment fast every time, unless reached the end + if (fast->unknown.next_in_quadrant == SPRITE_INDEX_NULL) + { + break; + } + else { + fast = get_sprite(fast->unknown.next_in_quadrant); + } + // increment slow only every second iteration + if (increment_slow) + { + slow = get_sprite(slow->unknown.next_in_quadrant); + } + increment_slow = !increment_slow; + if (fast == slow) + { + cycled = true; + if (fix) + { + rct_sprite * spr = get_sprite(sprite_idx); + + // There may be a better solution than simply setting this to 0xFFFF + spr->unknown.next_in_quadrant = SPRITE_INDEX_NULL; + } + break; + } + } + return cycled; +} + +bool check_for_sprite_list_cycles(bool fix) +{ + for (sint32 i = 0; i < 6; i++) { + if (sprite_is_in_cycle(gSpriteListHead[i], fix)) { + return true; + } + } + return false; +} + +bool check_for_spatial_index_cycles(bool fix) +{ + for (size_t i = 0; i < 0x10000; i++) { + if (sprite_is_in_cycle_quadrant(gSpriteSpatialIndex[i], fix)) { + return true; + } + } + return false; +} \ No newline at end of file diff --git a/src/openrct2/world/sprite.h b/src/openrct2/world/sprite.h index 55dde661bb..1cbe3415d0 100644 --- a/src/openrct2/world/sprite.h +++ b/src/openrct2/world/sprite.h @@ -478,6 +478,7 @@ const char *sprite_checksum(); void sprite_set_flashing(rct_sprite *sprite, bool flashing); bool sprite_get_flashing(rct_sprite *sprite); - +bool check_for_sprite_list_cycles(bool fix); +bool check_for_spatial_index_cycles(bool fix); #endif From 372781cc74d5bd78a6ed22e1b0c0d22a3016ac9b Mon Sep 17 00:00:00 2001 From: rwjuk Date: Wed, 5 Jul 2017 22:33:59 +0100 Subject: [PATCH 02/13] Code quality improvements --- src/openrct2/world/sprite.c | 54 ++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index 7f4cf210a4..086d8b8306 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -259,15 +259,13 @@ void sprite_clear_all_unused() spriteIndex = gSpriteListHead[SPRITE_LIST_NULL]; while (spriteIndex != SPRITE_INDEX_NULL) { sprite = &get_sprite(spriteIndex)->unknown; - nextSpriteIndex = sprite->next; - previousSpriteIndex = sprite->previous; - memset(sprite, 0, sizeof(rct_sprite)); - sprite->sprite_identifier = SPRITE_IDENTIFIER_NULL; - sprite->next = nextSpriteIndex; - sprite->previous = previousSpriteIndex; + sprite_reset(sprite); sprite->linked_list_type_offset = SPRITE_LIST_NULL * 2; - sprite->sprite_index = spriteIndex; - sprite->next_in_quadrant = (sprite->next_in_quadrant == 0) ? SPRITE_INDEX_NULL : sprite->next_in_quadrant; + + // sprite->next_in_quadrant will only end up as zero owing to corruption + // most likely due to previous builds not preserving it when resetting sprites + // We reset it to SPRITE_INDEX_NULL to prevent cycles in the sprite lists + if (sprite->next_in_quadrant == 0) { sprite->next_in_quadrant = SPRITE_INDEX_NULL; } _spriteFlashingList[spriteIndex] = false; spriteIndex = nextSpriteIndex; } @@ -290,6 +288,7 @@ static void sprite_reset(rct_unk_sprite *sprite) sprite->next_in_quadrant = next_in_quadrant; sprite->previous = prev; sprite->sprite_index = sprite_index; + sprite->sprite_identifier = SPRITE_IDENTIFIER_NULL; } // Resets all sprites in SPRITE_LIST_NULL list @@ -827,7 +826,7 @@ bool sprite_get_flashing(rct_sprite *sprite) return _spriteFlashingList[sprite->unknown.sprite_index]; } -static bool sprite_is_in_cycle(uint16 sprite_idx, bool fix) +static bool sprite_is_in_cycle(uint16 sprite_idx) { if (sprite_idx == SPRITE_INDEX_NULL) { @@ -856,19 +855,13 @@ static bool sprite_is_in_cycle(uint16 sprite_idx, bool fix) if (fast == slow) { cycled = true; - if (fix) - { - rct_sprite * spr = get_sprite(sprite_idx); - // There may be a better solution than simply setting this to 0xFFFF - spr->unknown.next = SPRITE_INDEX_NULL; - } break; } } return cycled; } -static bool sprite_is_in_cycle_quadrant(uint16 sprite_idx, bool fix) +static bool sprite_is_in_quadrant_cycle(uint16 sprite_idx) { if (sprite_idx == SPRITE_INDEX_NULL) { @@ -897,13 +890,6 @@ static bool sprite_is_in_cycle_quadrant(uint16 sprite_idx, bool fix) if (fast == slow) { cycled = true; - if (fix) - { - rct_sprite * spr = get_sprite(sprite_idx); - - // There may be a better solution than simply setting this to 0xFFFF - spr->unknown.next_in_quadrant = SPRITE_INDEX_NULL; - } break; } } @@ -912,8 +898,15 @@ static bool sprite_is_in_cycle_quadrant(uint16 sprite_idx, bool fix) bool check_for_sprite_list_cycles(bool fix) { - for (sint32 i = 0; i < 6; i++) { + for (sint32 i = 0; i < NUM_SPRITE_LISTS; i++) { if (sprite_is_in_cycle(gSpriteListHead[i], fix)) { + if (fix) + { + rct_sprite * spr = get_sprite(gSpriteListHead[i]); + + // There may be a better solution than simply setting this to 0xFFFF + spr->unknown.next = SPRITE_INDEX_NULL; + } return true; } } @@ -922,10 +915,17 @@ bool check_for_sprite_list_cycles(bool fix) bool check_for_spatial_index_cycles(bool fix) { - for (size_t i = 0; i < 0x10000; i++) { - if (sprite_is_in_cycle_quadrant(gSpriteSpatialIndex[i], fix)) { + for (sint32 i = 0; i < SPATIAL_INDEX_LOCATION_NULL; i++) { + if (sprite_is_in_quadrant_cycle(gSpriteSpatialIndex[i])) { + if (fix) + { + rct_sprite * spr = get_sprite(gSpriteSpatialIndex[i]); + + // There may be a better solution than simply setting this to 0xFFFF + spr->unknown.next_in_quadrant = SPRITE_INDEX_NULL; + } return true; } } return false; -} \ No newline at end of file +} From 28789c37673a9d3a531b27203ac94d0389074b13 Mon Sep 17 00:00:00 2001 From: rwjuk Date: Wed, 5 Jul 2017 22:42:45 +0100 Subject: [PATCH 03/13] Restore resetting of sprite index --- src/openrct2/world/sprite.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index 086d8b8306..2da130a022 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -262,6 +262,10 @@ void sprite_clear_all_unused() sprite_reset(sprite); sprite->linked_list_type_offset = SPRITE_LIST_NULL * 2; + // This shouldn't be necessary, as sprite_reset() preserves the index + // but it has been left in as a safety net in case the index isn't set correctly + sprite->sprite_index = spriteIndex; + // sprite->next_in_quadrant will only end up as zero owing to corruption // most likely due to previous builds not preserving it when resetting sprites // We reset it to SPRITE_INDEX_NULL to prevent cycles in the sprite lists From 72a1ad89a33edd10731d30d5caf8937e575e68ff Mon Sep 17 00:00:00 2001 From: rwjuk Date: Wed, 5 Jul 2017 22:44:05 +0100 Subject: [PATCH 04/13] Remove superfluous parameter --- src/openrct2/world/sprite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index 2da130a022..fb81396841 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -903,7 +903,7 @@ static bool sprite_is_in_quadrant_cycle(uint16 sprite_idx) bool check_for_sprite_list_cycles(bool fix) { for (sint32 i = 0; i < NUM_SPRITE_LISTS; i++) { - if (sprite_is_in_cycle(gSpriteListHead[i], fix)) { + if (sprite_is_in_cycle(gSpriteListHead[i])) { if (fix) { rct_sprite * spr = get_sprite(gSpriteListHead[i]); From 67cbe2a2a18c26f1dcfab266afaf06e47b19984a Mon Sep 17 00:00:00 2001 From: rwjuk Date: Wed, 5 Jul 2017 22:55:46 +0100 Subject: [PATCH 05/13] Fix compilation issues and restore line that went astray --- src/openrct2/world/sprite.c | 57 +++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index fb81396841..0efa585d52 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -247,34 +247,6 @@ const char * sprite_checksum() #endif // DISABLE_NETWORK -/** - * Clears all the unused sprite memory to zero. Probably so that it can be compressed better when saving. - * rct2: 0x0069EBA4 - */ -void sprite_clear_all_unused() -{ - rct_unk_sprite *sprite; - uint16 spriteIndex, nextSpriteIndex, previousSpriteIndex; - - spriteIndex = gSpriteListHead[SPRITE_LIST_NULL]; - while (spriteIndex != SPRITE_INDEX_NULL) { - sprite = &get_sprite(spriteIndex)->unknown; - sprite_reset(sprite); - sprite->linked_list_type_offset = SPRITE_LIST_NULL * 2; - - // This shouldn't be necessary, as sprite_reset() preserves the index - // but it has been left in as a safety net in case the index isn't set correctly - sprite->sprite_index = spriteIndex; - - // sprite->next_in_quadrant will only end up as zero owing to corruption - // most likely due to previous builds not preserving it when resetting sprites - // We reset it to SPRITE_INDEX_NULL to prevent cycles in the sprite lists - if (sprite->next_in_quadrant == 0) { sprite->next_in_quadrant = SPRITE_INDEX_NULL; } - _spriteFlashingList[spriteIndex] = false; - spriteIndex = nextSpriteIndex; - } -} - static void sprite_reset(rct_unk_sprite *sprite) { // Need to retain how the sprite is linked in lists @@ -295,6 +267,35 @@ static void sprite_reset(rct_unk_sprite *sprite) sprite->sprite_identifier = SPRITE_IDENTIFIER_NULL; } +/** +* Clears all the unused sprite memory to zero. Probably so that it can be compressed better when saving. +* rct2: 0x0069EBA4 +*/ +void sprite_clear_all_unused() +{ + rct_unk_sprite *sprite; + uint16 spriteIndex, nextSpriteIndex; + + spriteIndex = gSpriteListHead[SPRITE_LIST_NULL]; + while (spriteIndex != SPRITE_INDEX_NULL) { + sprite = &get_sprite(spriteIndex)->unknown; + nextSpriteIndex = sprite->next; + sprite_reset(sprite); + sprite->linked_list_type_offset = SPRITE_LIST_NULL * 2; + + // This shouldn't be necessary, as sprite_reset() preserves the index + // but it has been left in as a safety net in case the index isn't set correctly + sprite->sprite_index = spriteIndex; + + // sprite->next_in_quadrant will only end up as zero owing to corruption + // most likely due to previous builds not preserving it when resetting sprites + // We reset it to SPRITE_INDEX_NULL to prevent cycles in the sprite lists + if (sprite->next_in_quadrant == 0) { sprite->next_in_quadrant = SPRITE_INDEX_NULL; } + _spriteFlashingList[spriteIndex] = false; + spriteIndex = nextSpriteIndex; + } +} + // Resets all sprites in SPRITE_LIST_NULL list void reset_empty_sprites() { From 00848ca629fde254cf793cba87c5b7013aa0081f Mon Sep 17 00:00:00 2001 From: rwjuk Date: Wed, 5 Jul 2017 23:28:14 +0100 Subject: [PATCH 06/13] Correct sprite cycle fixing algorithm --- src/openrct2/world/sprite.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index 0efa585d52..8f3b835a95 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -831,7 +831,7 @@ bool sprite_get_flashing(rct_sprite *sprite) return _spriteFlashingList[sprite->unknown.sprite_index]; } -static bool sprite_is_in_cycle(uint16 sprite_idx) +static rct_sprite * find_sprite_list_cycle(uint16 sprite_idx) { if (sprite_idx == SPRITE_INDEX_NULL) { @@ -840,7 +840,7 @@ static bool sprite_is_in_cycle(uint16 sprite_idx) const rct_sprite * fast = get_sprite(sprite_idx); const rct_sprite * slow = fast; bool increment_slow = false; - bool cycled = false; + rct_sprite * cycle_start = NULL; while (fast->unknown.sprite_index != SPRITE_INDEX_NULL) { // increment fast every time, unless reached the end @@ -859,14 +859,14 @@ static bool sprite_is_in_cycle(uint16 sprite_idx) increment_slow = !increment_slow; if (fast == slow) { - cycled = true; + cycle_start = get_sprite(slow->unknown.sprite_index); break; } } - return cycled; + return cycle_start; } -static bool sprite_is_in_quadrant_cycle(uint16 sprite_idx) +static rct_sprite * find_sprite_quadrant_cycle(uint16 sprite_idx) { if (sprite_idx == SPRITE_INDEX_NULL) { @@ -875,7 +875,7 @@ static bool sprite_is_in_quadrant_cycle(uint16 sprite_idx) const rct_sprite * fast = get_sprite(sprite_idx); const rct_sprite * slow = fast; bool increment_slow = false; - bool cycled = false; + rct_sprite * cycle_start = NULL; while (fast->unknown.sprite_index != SPRITE_INDEX_NULL) { // increment fast every time, unless reached the end @@ -894,23 +894,23 @@ static bool sprite_is_in_quadrant_cycle(uint16 sprite_idx) increment_slow = !increment_slow; if (fast == slow) { - cycled = true; + cycle_start = get_sprite(slow->unknown.sprite_index); break; } } - return cycled; + return cycle_start; } bool check_for_sprite_list_cycles(bool fix) { for (sint32 i = 0; i < NUM_SPRITE_LISTS; i++) { - if (sprite_is_in_cycle(gSpriteListHead[i])) { + rct_sprite * cycle_start = find_sprite_list_cycle(gSpriteListHead[i]); + if (cycle_start != NULL) + { if (fix) { - rct_sprite * spr = get_sprite(gSpriteListHead[i]); - // There may be a better solution than simply setting this to 0xFFFF - spr->unknown.next = SPRITE_INDEX_NULL; + cycle_start->unknown.next = SPRITE_INDEX_NULL; } return true; } @@ -921,13 +921,13 @@ bool check_for_sprite_list_cycles(bool fix) bool check_for_spatial_index_cycles(bool fix) { for (sint32 i = 0; i < SPATIAL_INDEX_LOCATION_NULL; i++) { - if (sprite_is_in_quadrant_cycle(gSpriteSpatialIndex[i])) { + rct_sprite * cycle_start = find_sprite_quadrant_cycle(gSpriteSpatialIndex[i]); + if (cycle_start != NULL) + { if (fix) { - rct_sprite * spr = get_sprite(gSpriteSpatialIndex[i]); - // There may be a better solution than simply setting this to 0xFFFF - spr->unknown.next_in_quadrant = SPRITE_INDEX_NULL; + cycle_start->unknown.next_in_quadrant = SPRITE_INDEX_NULL; } return true; } From 07faba7aa538062311b82563b38c6827a283eafd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Thu, 6 Jul 2017 22:17:02 +0200 Subject: [PATCH 07/13] Re-add sprites that got disconnected in a looped list --- src/openrct2/world/sprite.c | 70 +++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 3 deletions(-) diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index 8f3b835a95..107dab6c5e 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -901,16 +901,65 @@ static rct_sprite * find_sprite_quadrant_cycle(uint16 sprite_idx) return cycle_start; } +static bool index_is_in_list(uint16 index, enum SPRITE_LIST sl) +{ + uint16 sprite_index = gSpriteListHead[sl]; + while (sprite_index != SPRITE_INDEX_NULL) + { + if (sprite_index == index) + { + return true; + } + sprite_index = get_sprite(sprite_index)->unknown.next; + } + return false; +} + +static bool index_is_in_spatial_list(uint16 index, sint32 quadrant) +{ + uint16 sprite_index = gSpriteSpatialIndex[quadrant]; + while (sprite_index != SPRITE_INDEX_NULL) + { + if (sprite_index == index) + { + return true; + } + sprite_index = get_sprite(sprite_index)->unknown.next_in_quadrant; + } + return false; +} + bool check_for_sprite_list_cycles(bool fix) { for (sint32 i = 0; i < NUM_SPRITE_LISTS; i++) { rct_sprite * cycle_start = find_sprite_list_cycle(gSpriteListHead[i]); - if (cycle_start != NULL) + if (cycle_start != NULL) { if (fix) { - // There may be a better solution than simply setting this to 0xFFFF + // Fix head list, but only in reverse order + // This is likely not needed, but just in case + get_sprite(gSpriteListHead[i])->unknown.previous = SPRITE_INDEX_NULL; + + // Store the leftover part of cycle to be fixed + uint16 cycle_next = cycle_start->unknown.next; + + // Break the cycle cycle_start->unknown.next = SPRITE_INDEX_NULL; + + // Now re-add remainder of the cycle back to list, safely. + // Add each sprite to the list until we encounter one that is already part of the list. + while (!index_is_in_list(cycle_next, i)) + { + rct_sprite * spr = get_sprite(cycle_next); + + cycle_start->unknown.next = cycle_next; + spr->unknown.previous = cycle_start->unknown.sprite_index; + cycle_next = spr->unknown.next; + spr->unknown.next = SPRITE_INDEX_NULL; + cycle_start = spr; + } + } return true; } @@ -926,8 +975,23 @@ bool check_for_spatial_index_cycles(bool fix) { if (fix) { - // There may be a better solution than simply setting this to 0xFFFF + // Store the leftover part of cycle to be fixed + uint16 cycle_next = cycle_start->unknown.next_in_quadrant; + + // Break the cycle cycle_start->unknown.next_in_quadrant = SPRITE_INDEX_NULL; + + // Now re-add remainder of the cycle back to list, safely. + // Add each sprite to the list until we encounter one that is already part of the list. + while (!index_is_in_list(cycle_next, i)) + { + rct_sprite * spr = get_sprite(cycle_next); + + cycle_start->unknown.next_in_quadrant = cycle_next; + cycle_next = spr->unknown.next_in_quadrant; + spr->unknown.next_in_quadrant = SPRITE_INDEX_NULL; + cycle_start = spr; + } } return true; } From bed085d329e9e706e7e906b233fb1689861ba066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Thu, 6 Jul 2017 22:18:05 +0200 Subject: [PATCH 08/13] Remove unneeded sprite reset This is done in all exports now --- src/openrct2/rct2/S6Exporter.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/openrct2/rct2/S6Exporter.cpp b/src/openrct2/rct2/S6Exporter.cpp index 086456d20f..6e2951c61b 100644 --- a/src/openrct2/rct2/S6Exporter.cpp +++ b/src/openrct2/rct2/S6Exporter.cpp @@ -449,7 +449,6 @@ extern "C" } map_reorganise_elements(); - sprite_clear_all_unused(); viewport_set_saved_view(); bool result = false; From 029aea0fc252995c521abe5bb2b35fa841929d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Thu, 6 Jul 2017 22:20:07 +0200 Subject: [PATCH 09/13] Remove reset_empty_sprites Leave in sprite_clear_all_unused as the safer version --- src/openrct2/rct2/S6Exporter.cpp | 4 +--- src/openrct2/world/sprite.c | 16 ---------------- src/openrct2/world/sprite.h | 1 - 3 files changed, 1 insertion(+), 20 deletions(-) diff --git a/src/openrct2/rct2/S6Exporter.cpp b/src/openrct2/rct2/S6Exporter.cpp index 6e2951c61b..45c8dda18d 100644 --- a/src/openrct2/rct2/S6Exporter.cpp +++ b/src/openrct2/rct2/S6Exporter.cpp @@ -184,9 +184,7 @@ void S6Exporter::Export() // Might as well reset them in here to zero out the space and improve // compression ratios. Especially useful for multiplayer servers that // use zlib on the sent stream. - { - reset_empty_sprites(); - } + sprite_clear_all_unused(); for (sint32 i = 0; i < MAX_SPRITES; i++) { memcpy(&_s6.sprites[i], get_sprite(i), sizeof(rct_sprite)); diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index 107dab6c5e..f153d3ef6d 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -296,22 +296,6 @@ void sprite_clear_all_unused() } } -// Resets all sprites in SPRITE_LIST_NULL list -void reset_empty_sprites() -{ - uint16 spriteIndex; - spriteIndex = gSpriteListHead[SPRITE_LIST_NULL]; - while (spriteIndex != SPRITE_INDEX_NULL) - { - rct_unk_sprite *sprite = &(get_sprite(spriteIndex))->unknown; - spriteIndex = sprite->next; - if (sprite->sprite_identifier == SPRITE_IDENTIFIER_NULL) - { - sprite_reset(sprite); - } - } -} - /* * rct2: 0x0069EC6B * bl: if bl & 2 > 0, the sprite ends up in the MISC linked list. diff --git a/src/openrct2/world/sprite.h b/src/openrct2/world/sprite.h index 1cbe3415d0..5f0f4c7848 100644 --- a/src/openrct2/world/sprite.h +++ b/src/openrct2/world/sprite.h @@ -419,7 +419,6 @@ extern uint16 *gSpriteListCount; extern uint16 gSpriteSpatialIndex[0x10001]; rct_sprite *create_sprite(uint8 bl); -void reset_empty_sprites(); void reset_sprite_list(); void reset_sprite_spatial_index(); void sprite_clear_all_unused(); From ab95988c661e388c84066c4c805fbe602412201a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Thu, 6 Jul 2017 22:28:10 +0200 Subject: [PATCH 10/13] Improve error reporting for detected cycles --- src/openrct2/rct2/S6Exporter.cpp | 5 ++++- src/openrct2/world/sprite.c | 12 ++++++------ src/openrct2/world/sprite.h | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/openrct2/rct2/S6Exporter.cpp b/src/openrct2/rct2/S6Exporter.cpp index 45c8dda18d..91d4018af1 100644 --- a/src/openrct2/rct2/S6Exporter.cpp +++ b/src/openrct2/rct2/S6Exporter.cpp @@ -152,7 +152,10 @@ void S6Exporter::Save(IStream * stream, bool isScenario) void S6Exporter::Export() { - openrct2_assert(!(check_for_spatial_index_cycles(false) || check_for_sprite_list_cycles(false)), "A sprite cycle exists."); + sint32 spatial_cycle = check_for_spatial_index_cycles(false); + sint32 regular_cycle = check_for_sprite_list_cycles(false); + openrct2_assert(spatial_cycle == -1, "Sprite cycle exists in spatial list %d", spatial_cycle); + openrct2_assert(regular_cycle == -1, "Sprite cycle exists in regular list %d", regular_cycle); _s6.info = gS6Info; uint32 researchedTrackPiecesA[128]; uint32 researchedTrackPiecesB[128]; diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index f153d3ef6d..98f80778c9 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -913,7 +913,7 @@ static bool index_is_in_spatial_list(uint16 index, sint32 quadrant) return false; } -bool check_for_sprite_list_cycles(bool fix) +sint32 check_for_sprite_list_cycles(bool fix) { for (sint32 i = 0; i < NUM_SPRITE_LISTS; i++) { rct_sprite * cycle_start = find_sprite_list_cycle(gSpriteListHead[i]); @@ -945,13 +945,13 @@ bool check_for_sprite_list_cycles(bool fix) } } - return true; + return i; } } - return false; + return -1; } -bool check_for_spatial_index_cycles(bool fix) +sint32 check_for_spatial_index_cycles(bool fix) { for (sint32 i = 0; i < SPATIAL_INDEX_LOCATION_NULL; i++) { rct_sprite * cycle_start = find_sprite_quadrant_cycle(gSpriteSpatialIndex[i]); @@ -977,8 +977,8 @@ bool check_for_spatial_index_cycles(bool fix) cycle_start = spr; } } - return true; + return i; } } - return false; + return -1; } diff --git a/src/openrct2/world/sprite.h b/src/openrct2/world/sprite.h index 5f0f4c7848..ba132ff8f7 100644 --- a/src/openrct2/world/sprite.h +++ b/src/openrct2/world/sprite.h @@ -477,7 +477,7 @@ const char *sprite_checksum(); void sprite_set_flashing(rct_sprite *sprite, bool flashing); bool sprite_get_flashing(rct_sprite *sprite); -bool check_for_sprite_list_cycles(bool fix); -bool check_for_spatial_index_cycles(bool fix); +sint32 check_for_sprite_list_cycles(bool fix); +sint32 check_for_spatial_index_cycles(bool fix); #endif From 12500dd802caf45aebcc08546c25e0817f21f3d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Fri, 7 Jul 2017 00:12:54 +0200 Subject: [PATCH 11/13] Fixup the disjoint null sprites --- src/openrct2/rct2/S6Exporter.cpp | 6 +++++ src/openrct2/rct2/S6Importer.cpp | 6 +++++ src/openrct2/world/sprite.c | 44 ++++++++++++++++++++++++++++++++ src/openrct2/world/sprite.h | 1 + 4 files changed, 57 insertions(+) diff --git a/src/openrct2/rct2/S6Exporter.cpp b/src/openrct2/rct2/S6Exporter.cpp index 91d4018af1..bbf977651f 100644 --- a/src/openrct2/rct2/S6Exporter.cpp +++ b/src/openrct2/rct2/S6Exporter.cpp @@ -154,8 +154,14 @@ void S6Exporter::Export() { sint32 spatial_cycle = check_for_spatial_index_cycles(false); sint32 regular_cycle = check_for_sprite_list_cycles(false); + sint32 disjoint_sprites_count = fix_disjoint_sprites(); openrct2_assert(spatial_cycle == -1, "Sprite cycle exists in spatial list %d", spatial_cycle); openrct2_assert(regular_cycle == -1, "Sprite cycle exists in regular list %d", regular_cycle); + // This one is less harmful, no need to assert for it ~janisozaur + if (disjoint_sprites_count > 0) + { + log_error("Found %d disjoint null sprites", disjoint_sprites_count); + } _s6.info = gS6Info; uint32 researchedTrackPiecesA[128]; uint32 researchedTrackPiecesB[128]; diff --git a/src/openrct2/rct2/S6Importer.cpp b/src/openrct2/rct2/S6Importer.cpp index d12ea1071e..3f71867f5c 100644 --- a/src/openrct2/rct2/S6Importer.cpp +++ b/src/openrct2/rct2/S6Importer.cpp @@ -417,6 +417,12 @@ public: // We try to fix the cycles on import, hence the 'true' parameter check_for_sprite_list_cycles(true); check_for_spatial_index_cycles(true); + sint32 disjoint_sprites_count = fix_disjoint_sprites(); + // This one is less harmful, no need to assert for it ~janisozaur + if (disjoint_sprites_count > 0) + { + log_error("Found %d disjoint null sprites", disjoint_sprites_count); + } } void Initialise() diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index 98f80778c9..67058f0cb0 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -951,6 +951,50 @@ sint32 check_for_sprite_list_cycles(bool fix) return -1; } +/** + * Finds and fixes null sprites that are not reachable via SPRITE_LIST_NULL list. + * + * @return count of disjoint sprites found + */ +sint32 fix_disjoint_sprites() +{ + // Find reachable sprites + bool reachable[MAX_SPRITES] = {}; + uint16 sprite_idx = gSpriteListHead[SPRITE_LIST_NULL]; + rct_sprite * null_list_tail = NULL; + while (sprite_idx != SPRITE_INDEX_NULL) + { + reachable[sprite_idx] = true; + // cache the tail, so we don't have to walk the list twice + null_list_tail = get_sprite(sprite_idx); + sprite_idx = null_list_tail->unknown.next; + } + + sint32 count = 0; + + // Find all null sprites + for (sprite_idx = 0; sprite_idx < MAX_SPRITES; sprite_idx++) + { + rct_sprite * spr = get_sprite(sprite_idx); + if (spr->unknown.sprite_identifier == SPRITE_IDENTIFIER_NULL) + { + openrct2_assert(null_list_tail != NULL, "Null list is empty, yet found null sprites"); + spr->unknown.sprite_index = sprite_idx; + if (!reachable[sprite_idx]) + { + // Add the sprite directly to the list + null_list_tail->unknown.next = sprite_idx; + spr->unknown.next = SPRITE_INDEX_NULL; + spr->unknown.previous = null_list_tail->unknown.sprite_index; + null_list_tail = spr; + count++; + reachable[sprite_idx] = true; + } + } + } + return count; +} + sint32 check_for_spatial_index_cycles(bool fix) { for (sint32 i = 0; i < SPATIAL_INDEX_LOCATION_NULL; i++) { diff --git a/src/openrct2/world/sprite.h b/src/openrct2/world/sprite.h index ba132ff8f7..44929c96ca 100644 --- a/src/openrct2/world/sprite.h +++ b/src/openrct2/world/sprite.h @@ -479,5 +479,6 @@ void sprite_set_flashing(rct_sprite *sprite, bool flashing); bool sprite_get_flashing(rct_sprite *sprite); sint32 check_for_sprite_list_cycles(bool fix); sint32 check_for_spatial_index_cycles(bool fix); +sint32 fix_disjoint_sprites(); #endif From a3414502a6ec67580139d162a1620b8f83f5bd2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Fri, 7 Jul 2017 00:17:36 +0200 Subject: [PATCH 12/13] Update network version --- src/openrct2/network/network.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2/network/network.h b/src/openrct2/network/network.h index 7404efc04d..3efb600404 100644 --- a/src/openrct2/network/network.h +++ b/src/openrct2/network/network.h @@ -55,7 +55,7 @@ extern "C" { // This define specifies which version of network stream current build uses. // It is used for making sure only compatible builds get connected, even within // single OpenRCT2 version. -#define NETWORK_STREAM_VERSION "29" +#define NETWORK_STREAM_VERSION "30" #define NETWORK_STREAM_ID OPENRCT2_VERSION "-" NETWORK_STREAM_VERSION #ifdef __cplusplus From 8d02ce37b56fda04c25ac9b2fdd0d82918cf3f8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Fri, 7 Jul 2017 07:56:45 +0200 Subject: [PATCH 13/13] Fix compilation on MSVC --- src/openrct2/world/sprite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2/world/sprite.c b/src/openrct2/world/sprite.c index 67058f0cb0..53b88d3ec0 100644 --- a/src/openrct2/world/sprite.c +++ b/src/openrct2/world/sprite.c @@ -959,7 +959,7 @@ sint32 check_for_sprite_list_cycles(bool fix) sint32 fix_disjoint_sprites() { // Find reachable sprites - bool reachable[MAX_SPRITES] = {}; + bool reachable[MAX_SPRITES] = { false }; uint16 sprite_idx = gSpriteListHead[SPRITE_LIST_NULL]; rct_sprite * null_list_tail = NULL; while (sprite_idx != SPRITE_INDEX_NULL)