From eb52391b9afb6cafcc3fb4616494d622c83f708a Mon Sep 17 00:00:00 2001 From: Duncan Date: Sat, 23 Jan 2021 07:36:46 +0000 Subject: [PATCH] Remove next_in_quadrant (#13754) * Use std::vector of quadrants * Prevent ptr invalidation issues * Remove next_in_quadrant * Make review changes * Rebuild next_in_quadrant for S6Export * Fix formatting * Constexpr where possible * Increment network version and update replays --- CMakeLists.txt | 4 +- openrct2.proj | 4 +- src/openrct2/GameStateSnapshots.cpp | 1 - src/openrct2/network/NetworkBase.cpp | 2 +- src/openrct2/rct2/S6Exporter.cpp | 23 ++++- src/openrct2/rct2/S6Importer.cpp | 1 - src/openrct2/world/Footpath.cpp | 12 ++- src/openrct2/world/Sprite.cpp | 122 ++++++++++----------------- src/openrct2/world/Sprite.h | 67 +++++++++++++-- src/openrct2/world/SpriteBase.h | 1 - test/tests/S6ImportExportTests.cpp | 1 - 11 files changed, 139 insertions(+), 99 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9ecb7bb6cb..c30b3bda1f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -44,8 +44,8 @@ set(TITLE_SEQUENCE_SHA1 "304d13a126c15bf2c86ff13b81a2f2cc1856ac8d") set(OBJECTS_URL "https://github.com/OpenRCT2/objects/releases/download/v1.0.20/objects.zip") set(OBJECTS_SHA1 "151424d24b1d49a167932b58319bedaa6ec368e9") -set(REPLAYS_URL "https://github.com/OpenRCT2/replays/releases/download/v0.0.29/replays.zip") -set(REPLAYS_SHA1 "B64135F28A79758AD9FCB611625ADDB5C89FB40B") +set(REPLAYS_URL "https://github.com/OpenRCT2/replays/releases/download/v0.0.30/replays.zip") +set(REPLAYS_SHA1 "FD0949B81A7A267EA3A8F96CB02C460C8C21B923") option(FORCE32 "Force 32-bit build. It will add `-m32` to compiler flags.") option(WITH_TESTS "Build tests") diff --git a/openrct2.proj b/openrct2.proj index dca3d5d559..ecbd08dcd2 100644 --- a/openrct2.proj +++ b/openrct2.proj @@ -48,8 +48,8 @@ 304d13a126c15bf2c86ff13b81a2f2cc1856ac8d https://github.com/OpenRCT2/objects/releases/download/v1.0.20/objects.zip 151424d24b1d49a167932b58319bedaa6ec368e9 - https://github.com/OpenRCT2/replays/releases/download/v0.0.29/replays.zip - B64135F28A79758AD9FCB611625ADDB5C89FB40B + https://github.com/OpenRCT2/replays/releases/download/v0.0.30/replays.zip + FD0949B81A7A267EA3A8F96CB02C460C8C21B923 diff --git a/src/openrct2/GameStateSnapshots.cpp b/src/openrct2/GameStateSnapshots.cpp index a8c9586917..f9c8f2a5ba 100644 --- a/src/openrct2/GameStateSnapshots.cpp +++ b/src/openrct2/GameStateSnapshots.cpp @@ -200,7 +200,6 @@ struct GameStateSnapshots final : public IGameStateSnapshots const SpriteBase& spriteBase, const SpriteBase& spriteCmp, GameStateSpriteChange_t& changeData) const { COMPARE_FIELD(SpriteBase, sprite_identifier); - COMPARE_FIELD(SpriteBase, next_in_quadrant); COMPARE_FIELD(SpriteBase, linked_list_index); COMPARE_FIELD(SpriteBase, sprite_index); COMPARE_FIELD(SpriteBase, flags); diff --git a/src/openrct2/network/NetworkBase.cpp b/src/openrct2/network/NetworkBase.cpp index 3813e127dc..70fe6f43f3 100644 --- a/src/openrct2/network/NetworkBase.cpp +++ b/src/openrct2/network/NetworkBase.cpp @@ -34,7 +34,7 @@ // This string 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 "14" +#define NETWORK_STREAM_VERSION "15" #define NETWORK_STREAM_ID OPENRCT2_VERSION "-" NETWORK_STREAM_VERSION static Peep* _pickup_peep = nullptr; diff --git a/src/openrct2/rct2/S6Exporter.cpp b/src/openrct2/rct2/S6Exporter.cpp index 04587f41d4..3920f95761 100644 --- a/src/openrct2/rct2/S6Exporter.cpp +++ b/src/openrct2/rct2/S6Exporter.cpp @@ -981,6 +981,27 @@ void S6Exporter::RebuildEntityLinks() } } } + + // Rebuild next_in_quadrant linked list entity indexs + for (auto x = 0; x < 255; ++x) + { + for (auto y = 0; y < 255; ++y) + { + uint16_t previous = SPRITE_INDEX_NULL; + for (auto* entity : EntityTileList(TileCoordsXY{ x, y }.ToCoordsXY())) + { + if (previous != SPRITE_INDEX_NULL) + { + _s6.sprites[previous].unknown.next_in_quadrant = entity->sprite_index; + } + previous = entity->sprite_index; + } + if (previous != SPRITE_INDEX_NULL) + { + _s6.sprites[previous].unknown.next_in_quadrant = SPRITE_INDEX_NULL; + } + } + } } void S6Exporter::ExportSprite(RCT2Sprite* dst, const rct_sprite* src) @@ -1046,8 +1067,8 @@ constexpr RCT12EntityLinkListOffset GetRCT2LinkListOffset(const SpriteBase* src) void S6Exporter::ExportSpriteCommonProperties(RCT12SpriteBase* dst, const SpriteBase* src) { dst->sprite_identifier = src->sprite_identifier; - dst->next_in_quadrant = src->next_in_quadrant; dst->linked_list_type_offset = GetRCT2LinkListOffset(src); + dst->next_in_quadrant = SPRITE_INDEX_NULL; dst->sprite_height_negative = src->sprite_height_negative; dst->sprite_index = src->sprite_index; dst->flags = src->flags; diff --git a/src/openrct2/rct2/S6Importer.cpp b/src/openrct2/rct2/S6Importer.cpp index ce52cfb936..37a14765ea 100644 --- a/src/openrct2/rct2/S6Importer.cpp +++ b/src/openrct2/rct2/S6Importer.cpp @@ -1656,7 +1656,6 @@ public: void ImportSpriteCommonProperties(SpriteBase* dst, const RCT12SpriteBase* src) { dst->sprite_identifier = src->sprite_identifier; - dst->next_in_quadrant = src->next_in_quadrant; dst->linked_list_index = static_cast(EnumValue(src->linked_list_type_offset) >> 1); dst->sprite_height_negative = src->sprite_height_negative; dst->sprite_index = src->sprite_index; diff --git a/src/openrct2/world/Footpath.cpp b/src/openrct2/world/Footpath.cpp index 5dfbefffe5..b2013bcf8e 100644 --- a/src/openrct2/world/Footpath.cpp +++ b/src/openrct2/world/Footpath.cpp @@ -397,16 +397,20 @@ CoordsXY footpath_bridge_get_info_from_pos(const ScreenCoordsXY& screenCoords, i */ void footpath_remove_litter(const CoordsXYZ& footpathPos) { - auto quad = EntityTileList(footpathPos); - for (auto litter : quad) + std::vector removals; + for (auto litter : EntityTileList(footpathPos)) { int32_t distanceZ = abs(litter->z - footpathPos.z); if (distanceZ <= 32) { - litter->Invalidate(); - sprite_remove(litter); + removals.push_back(litter); } } + for (auto* litter : removals) + { + litter->Invalidate(); + sprite_remove(litter); + } } /** diff --git a/src/openrct2/world/Sprite.cpp b/src/openrct2/world/Sprite.cpp index 778f1e3b5f..be98cdf698 100644 --- a/src/openrct2/world/Sprite.cpp +++ b/src/openrct2/world/Sprite.cpp @@ -24,13 +24,14 @@ #include #include #include +#include static rct_sprite _spriteList[MAX_SPRITES]; static std::array, EnumValue(EntityListId::Count)> gEntityLists; static bool _spriteFlashingList[MAX_SPRITES]; -uint16_t gSpriteSpatialIndex[SPATIAL_INDEX_SIZE]; +static std::array, SPATIAL_INDEX_SIZE> gSpriteSpatialIndex; const rct_string_id litterNames[12] = { STR_LITTER_VOMIT, STR_LITTER_VOMIT, @@ -45,7 +46,25 @@ const rct_string_id litterNames[12] = { STR_LITTER_VOMIT, STR_SHOP_ITEM_SINGULAR_EMPTY_JUICE_CUP, STR_SHOP_ITEM_SINGULAR_EMPTY_BOWL_BLUE }; -static size_t GetSpatialIndexOffset(int32_t x, int32_t y); +constexpr size_t GetSpatialIndexOffset(int32_t x, int32_t y) +{ + size_t index = SPATIAL_INDEX_LOCATION_NULL; + if (x != LOCATION_NULL) + { + x = std::clamp(x, 0, 0xFFFF); + y = std::clamp(y, 0, 0xFFFF); + + int16_t flooredX = floor2(x, 32); + uint8_t tileY = y >> 5; + index = (flooredX << 3) | tileY; + } + + if (index >= sizeof(gSpriteSpatialIndex)) + { + return SPATIAL_INDEX_LOCATION_NULL; + } + return index; +} // Required for GetEntity to return a default template<> bool SpriteBase::Is() const @@ -116,7 +135,7 @@ SpriteBase* get_sprite(size_t spriteIndex) return try_get_sprite(spriteIndex); } -uint16_t sprite_get_first_in_quadrant(const CoordsXY& spritePos) +const std::vector& GetEntityTileList(const CoordsXY& spritePos) { return gSpriteSpatialIndex[GetSpatialIndexOffset(spritePos.x, spritePos.y)]; } @@ -242,6 +261,8 @@ void reset_sprite_list() reset_sprite_spatial_index(); } +static void SpriteSpatialInsert(SpriteBase* sprite, const CoordsXY& newLoc); + /** * * rct2: 0x0069EBE4 @@ -250,40 +271,20 @@ void reset_sprite_list() */ void reset_sprite_spatial_index() { - std::fill_n(gSpriteSpatialIndex, std::size(gSpriteSpatialIndex), SPRITE_INDEX_NULL); + for (auto& vec : gSpriteSpatialIndex) + { + vec.clear(); + } for (size_t i = 0; i < MAX_SPRITES; i++) { auto* spr = GetEntity(i); if (spr != nullptr && spr->sprite_identifier != SpriteIdentifier::Null) { - size_t index = GetSpatialIndexOffset(spr->x, spr->y); - uint32_t nextSpriteId = gSpriteSpatialIndex[index]; - gSpriteSpatialIndex[index] = spr->sprite_index; - spr->next_in_quadrant = nextSpriteId; + SpriteSpatialInsert(spr, { spr->x, spr->y }); } } } -static size_t GetSpatialIndexOffset(int32_t x, int32_t y) -{ - size_t index = SPATIAL_INDEX_LOCATION_NULL; - if (x != LOCATION_NULL) - { - x = std::clamp(x, 0, 0xFFFF); - y = std::clamp(y, 0, 0xFFFF); - - int16_t flooredX = floor2(x, 32); - uint8_t tileY = y >> 5; - index = (flooredX << 3) | tileY; - } - - if (index >= sizeof(gSpriteSpatialIndex)) - { - return SPATIAL_INDEX_LOCATION_NULL; - } - return index; -} - #ifndef DISABLE_NETWORK rct_sprite_checksum sprite_checksum() @@ -318,15 +319,6 @@ rct_sprite_checksum sprite_checksum() copy.misc.sprite_left = copy.misc.sprite_right = copy.misc.sprite_top = copy.misc.sprite_bottom = 0; copy.misc.sprite_width = copy.misc.sprite_height_negative = copy.misc.sprite_height_positive = 0; - // Next in quadrant might be a misc sprite, set first non-misc sprite in quadrant. - while (auto* nextSprite = GetEntity(copy.misc.next_in_quadrant)) - { - if (nextSprite->sprite_identifier == SpriteIdentifier::Misc) - copy.misc.next_in_quadrant = nextSprite->next_in_quadrant; - else - break; - } - if (copy.misc.Is()) { // Name is pointer and will not be the same across clients @@ -365,14 +357,12 @@ static void sprite_reset(SpriteBase* sprite) { // Need to retain how the sprite is linked in lists auto llto = sprite->linked_list_index; - uint16_t next_in_quadrant = sprite->next_in_quadrant; uint16_t sprite_index = sprite->sprite_index; _spriteFlashingList[sprite_index] = false; std::memset(sprite, 0, sizeof(rct_sprite)); sprite->linked_list_index = llto; - sprite->next_in_quadrant = next_in_quadrant; sprite->sprite_index = sprite_index; sprite->sprite_identifier = SpriteIdentifier::Null; } @@ -388,19 +378,10 @@ void sprite_clear_all_unused() sprite_reset(sprite); sprite->linked_list_index = EntityListId::Free; - // 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[sprite->sprite_index] = false; } } -static void SpriteSpatialInsert(SpriteBase* sprite, const CoordsXY& newLoc); - static constexpr uint16_t MAX_MISC_SPRITES = 300; static void AddToEntityList(const EntityListId linkedListIndex, SpriteBase* entity) { @@ -605,41 +586,25 @@ void sprite_misc_update_all() static void SpriteSpatialInsert(SpriteBase* sprite, const CoordsXY& newLoc) { size_t newIndex = GetSpatialIndexOffset(newLoc.x, newLoc.y); - - auto* next = &gSpriteSpatialIndex[newIndex]; - while (sprite->sprite_index < *next && *next != SPRITE_INDEX_NULL) - { - auto sprite2 = GetEntity(*next); - next = &sprite2->next_in_quadrant; - } - - sprite->next_in_quadrant = *next; - *next = sprite->sprite_index; + auto& spatialVector = gSpriteSpatialIndex[newIndex]; + auto index = std::lower_bound(std::begin(spatialVector), std::end(spatialVector), sprite->sprite_index); + spatialVector.insert(index, sprite->sprite_index); } static void SpriteSpatialRemove(SpriteBase* sprite) { size_t currentIndex = GetSpatialIndexOffset(sprite->x, sprite->y); - auto* index = &gSpriteSpatialIndex[currentIndex]; - - // This indicates that the spatial index data is incorrect. - if (*index == SPRITE_INDEX_NULL) + auto& spatialVector = gSpriteSpatialIndex[currentIndex]; + auto index = std::lower_bound(std::begin(spatialVector), std::end(spatialVector), sprite->sprite_index); + if (index != std::end(spatialVector) && *index == sprite->sprite_index) + { + spatialVector.erase(index, index + 1); + } + else { log_warning("Bad sprite spatial index. Rebuilding the spatial index..."); reset_sprite_spatial_index(); } - - auto* sprite2 = GetEntity(*index); - while (sprite != sprite2) - { - index = &sprite2->next_in_quadrant; - if (*index == SPRITE_INDEX_NULL) - { - break; - } - sprite2 = GetEntity(*index); - } - *index = sprite->next_in_quadrant; } static void SpriteSpatialMove(SpriteBase* sprite, const CoordsXY& newLoc) @@ -807,17 +772,22 @@ void litter_create(const CoordsXYZD& litterPos, LitterType type) */ void litter_remove_at(const CoordsXYZ& litterPos) { + std::vector removals; for (auto litter : EntityTileList(litterPos)) { if (abs(litter->z - litterPos.z) <= 16) { if (abs(litter->x - litterPos.x) <= 8 && abs(litter->y - litterPos.y) <= 8) { - litter->Invalidate(); - sprite_remove(litter); + removals.push_back(litter); } } } + for (auto* litter : removals) + { + litter->Invalidate(); + sprite_remove(litter); + } } /** diff --git a/src/openrct2/world/Sprite.h b/src/openrct2/world/Sprite.h index 08523bd073..d9e0e18d55 100644 --- a/src/openrct2/world/Sprite.h +++ b/src/openrct2/world/Sprite.h @@ -231,7 +231,6 @@ uint16_t GetEntityListCount(EntityListId list); constexpr const uint32_t SPATIAL_INDEX_SIZE = (MAXIMUM_MAP_SIZE_TECHNICAL * MAXIMUM_MAP_SIZE_TECHNICAL) + 1; constexpr const uint32_t SPATIAL_INDEX_LOCATION_NULL = SPATIAL_INDEX_SIZE - 1; -extern uint16_t gSpriteSpatialIndex[SPATIAL_INDEX_SIZE]; extern const rct_string_id litterNames[12]; @@ -249,7 +248,7 @@ void litter_remove_at(const CoordsXYZ& litterPos); uint16_t remove_floating_sprites(); void sprite_misc_explosion_cloud_create(const CoordsXYZ& cloudPos); void sprite_misc_explosion_flare_create(const CoordsXYZ& flarePos); -uint16_t sprite_get_first_in_quadrant(const CoordsXY& spritePos); +const std::vector& GetEntityTileList(const CoordsXY& spritePos); /////////////////////////////////////////////////////////////// // Balloon @@ -333,25 +332,75 @@ public: using iterator_category = std::forward_iterator_tag; }; +template class EntityTileIterator +{ +private: + std::vector::const_iterator iter; + std::vector::const_iterator end; + T* Entity = nullptr; + +public: + EntityTileIterator(std::vector::const_iterator _iter, std::vector::const_iterator _end) + : iter(_iter) + , end(_end) + { + ++(*this); + } + EntityTileIterator& operator++() + { + Entity = nullptr; + + while (iter != end && Entity == nullptr) + { + Entity = GetEntity(*iter++); + } + return *this; + } + + EntityTileIterator operator++(int) + { + EntityTileIterator retval = *this; + ++(*this); + return *iter; + } + bool operator==(EntityTileIterator other) const + { + return Entity == other.Entity; + } + bool operator!=(EntityTileIterator other) const + { + return !(*this == other); + } + T* operator*() + { + return Entity; + } + // iterator traits + using difference_type = std::ptrdiff_t; + using value_type = T; + using pointer = const T*; + using reference = const T&; + using iterator_category = std::forward_iterator_tag; +}; + template class EntityTileList { private: - uint16_t FirstEntity = SPRITE_INDEX_NULL; - using EntityTileIterator = EntityIterator; + const std::vector& vec; public: EntityTileList(const CoordsXY& loc) - : FirstEntity(sprite_get_first_in_quadrant(loc)) + : vec(GetEntityTileList(loc)) { } - EntityTileIterator begin() + EntityTileIterator begin() { - return EntityTileIterator(FirstEntity); + return EntityTileIterator(std::begin(vec), std::end(vec)); } - EntityTileIterator end() + EntityTileIterator end() { - return EntityTileIterator(SPRITE_INDEX_NULL); + return EntityTileIterator(std::end(vec), std::end(vec)); } }; diff --git a/src/openrct2/world/SpriteBase.h b/src/openrct2/world/SpriteBase.h index 6077fb12c3..419de2effa 100644 --- a/src/openrct2/world/SpriteBase.h +++ b/src/openrct2/world/SpriteBase.h @@ -9,7 +9,6 @@ enum class SpriteIdentifier : uint8_t; struct SpriteBase { SpriteIdentifier sprite_identifier; - uint16_t next_in_quadrant; // Valid values are EntityListId::... EntityListId linked_list_index; // Height from centre of sprite to bottom diff --git a/test/tests/S6ImportExportTests.cpp b/test/tests/S6ImportExportTests.cpp index ef2b99073b..1aa22e6daa 100644 --- a/test/tests/S6ImportExportTests.cpp +++ b/test/tests/S6ImportExportTests.cpp @@ -131,7 +131,6 @@ static void AdvanceGameTicks(uint32_t ticks, std::unique_ptr& context) static void CompareSpriteDataCommon(const SpriteBase& left, const SpriteBase& right) { COMPARE_FIELD(sprite_identifier); - COMPARE_FIELD(next_in_quadrant); COMPARE_FIELD(linked_list_index); COMPARE_FIELD(sprite_index); COMPARE_FIELD(flags);