From f80531070b24b01d68e96f47c910f58f0e8bd097 Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 21 Jan 2021 18:36:34 +0000 Subject: [PATCH] Implement EntityLists (#13853) * Implement EntityLists * Remove dead code * Use alternative name for iterator * Add comments * Increment network version * Update replays * Remove further dead code * Update replays again --- CMakeLists.txt | 4 +- openrct2.proj | 4 +- src/openrct2/GameStateSnapshots.cpp | 2 - src/openrct2/interface/InteractiveConsole.cpp | 2 +- src/openrct2/network/NetworkBase.cpp | 2 +- src/openrct2/rct12/RCT12.h | 49 ++- src/openrct2/rct2/S6Exporter.cpp | 78 +++- src/openrct2/rct2/S6Exporter.h | 1 + src/openrct2/rct2/S6Importer.cpp | 22 +- src/openrct2/world/Sprite.cpp | 333 ++++-------------- src/openrct2/world/Sprite.h | 74 +++- src/openrct2/world/SpriteBase.h | 2 - test/tests/S6ImportExportTests.cpp | 2 - 13 files changed, 234 insertions(+), 341 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f6e21fb8ed..9ecb7bb6cb 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.27/replays.zip") -set(REPLAYS_SHA1 "CC0BE0C9B9829062B67E1CFE14E36BEA1182882D") +set(REPLAYS_URL "https://github.com/OpenRCT2/replays/releases/download/v0.0.29/replays.zip") +set(REPLAYS_SHA1 "B64135F28A79758AD9FCB611625ADDB5C89FB40B") 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 56e9ff602d..dca3d5d559 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.27/replays.zip - CC0BE0C9B9829062B67E1CFE14E36BEA1182882D + https://github.com/OpenRCT2/replays/releases/download/v0.0.29/replays.zip + B64135F28A79758AD9FCB611625ADDB5C89FB40B diff --git a/src/openrct2/GameStateSnapshots.cpp b/src/openrct2/GameStateSnapshots.cpp index 7a97cbee0a..f28d66519b 100644 --- a/src/openrct2/GameStateSnapshots.cpp +++ b/src/openrct2/GameStateSnapshots.cpp @@ -201,8 +201,6 @@ struct GameStateSnapshots final : public IGameStateSnapshots { COMPARE_FIELD(SpriteBase, sprite_identifier); COMPARE_FIELD(SpriteBase, next_in_quadrant); - COMPARE_FIELD(SpriteBase, next); - COMPARE_FIELD(SpriteBase, previous); COMPARE_FIELD(SpriteBase, linked_list_index); COMPARE_FIELD(SpriteBase, sprite_index); COMPARE_FIELD(SpriteBase, flags); diff --git a/src/openrct2/interface/InteractiveConsole.cpp b/src/openrct2/interface/InteractiveConsole.cpp index f7949af113..ac617f7fad 100644 --- a/src/openrct2/interface/InteractiveConsole.cpp +++ b/src/openrct2/interface/InteractiveConsole.cpp @@ -1231,7 +1231,7 @@ static int32_t cc_show_limits(InteractiveConsole& console, [[maybe_unused]] cons int32_t spriteCount = 0; for (int32_t i = 1; i < static_cast(EntityListId::Count); ++i) { - spriteCount += gSpriteListCount[i]; + spriteCount += GetEntityListCount(EntityListId(i)); } int32_t staffCount = 0; diff --git a/src/openrct2/network/NetworkBase.cpp b/src/openrct2/network/NetworkBase.cpp index 630a63eff6..a38a041eda 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 "12" +#define NETWORK_STREAM_VERSION "13" #define NETWORK_STREAM_ID OPENRCT2_VERSION "-" NETWORK_STREAM_VERSION static Peep* _pickup_peep = nullptr; diff --git a/src/openrct2/rct12/RCT12.h b/src/openrct2/rct12/RCT12.h index be39fe9052..48e72a1f10 100644 --- a/src/openrct2/rct12/RCT12.h +++ b/src/openrct2/rct12/RCT12.h @@ -675,27 +675,38 @@ struct RCT12EightCarsCorruptElement15 : RCT12TileElementBase }; assert_struct_size(RCT12EightCarsCorruptElement15, 8); +// Offset into sprite_lists_head and sprite_lists_count +enum class RCT12EntityLinkListOffset : uint8_t +{ + Free = 0, + TrainHead = 1 * sizeof(uint16_t), + Peep = 2 * sizeof(uint16_t), + Misc = 3 * sizeof(uint16_t), + Litter = 4 * sizeof(uint16_t), + Vehicle = 5 * sizeof(uint16_t), +}; + struct RCT12SpriteBase { - SpriteIdentifier sprite_identifier; // 0x00 - uint8_t type; // 0x01 - uint16_t next_in_quadrant; // 0x02 - uint16_t next; // 0x04 - uint16_t previous; // 0x06 - uint8_t linked_list_type_offset; // 0x08 - uint8_t sprite_height_negative; // 0x09 - uint16_t sprite_index; // 0x0A - uint16_t flags; // 0x0C - int16_t x; // 0x0E - int16_t y; // 0x10 - int16_t z; // 0x12 - uint8_t sprite_width; // 0x14 - uint8_t sprite_height_positive; // 0x15 - int16_t sprite_left; // 0x16 - int16_t sprite_top; // 0x18 - int16_t sprite_right; // 0x1A - int16_t sprite_bottom; // 0x1C - uint8_t sprite_direction; // 0x1E + SpriteIdentifier sprite_identifier; // 0x00 + uint8_t type; // 0x01 + uint16_t next_in_quadrant; // 0x02 + uint16_t next; // 0x04 + uint16_t previous; // 0x06 + RCT12EntityLinkListOffset linked_list_type_offset; // 0x08 + uint8_t sprite_height_negative; // 0x09 + uint16_t sprite_index; // 0x0A + uint16_t flags; // 0x0C + int16_t x; // 0x0E + int16_t y; // 0x10 + int16_t z; // 0x12 + uint8_t sprite_width; // 0x14 + uint8_t sprite_height_positive; // 0x15 + int16_t sprite_left; // 0x16 + int16_t sprite_top; // 0x18 + int16_t sprite_right; // 0x1A + int16_t sprite_bottom; // 0x1C + uint8_t sprite_direction; // 0x1E }; assert_struct_size(RCT12SpriteBase, 0x1F); diff --git a/src/openrct2/rct2/S6Exporter.cpp b/src/openrct2/rct2/S6Exporter.cpp index 0b0e1152d8..e75fb3963a 100644 --- a/src/openrct2/rct2/S6Exporter.cpp +++ b/src/openrct2/rct2/S6Exporter.cpp @@ -151,14 +151,6 @@ void S6Exporter::Save(OpenRCT2::IStream* stream, bool isScenario) void S6Exporter::Export() { - int32_t regular_cycle = check_for_sprite_list_cycles(false); - int32_t disjoint_sprites_count = fix_disjoint_sprites(); - 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; { auto temp = utf8_to_rct2(gS6Info.name); @@ -957,8 +949,37 @@ void S6Exporter::ExportSprites() for (int32_t i = 0; i < static_cast(EntityListId::Count); i++) { - _s6.sprite_lists_head[i] = gSpriteListHead[i]; - _s6.sprite_lists_count[i] = gSpriteListCount[i]; + //_s6.sprite_lists_head[i] = gSpriteListHead[i]; + _s6.sprite_lists_count[i] = GetEntityListCount(EntityListId(i)); + } + RebuildEntityLinks(); +} + +void S6Exporter::RebuildEntityLinks() +{ + // Rebuild next/previous linked list entity indexs + for (auto list : + { RCT12EntityLinkListOffset::Free, RCT12EntityLinkListOffset::Litter, RCT12EntityLinkListOffset::Misc, + RCT12EntityLinkListOffset::Peep, RCT12EntityLinkListOffset::TrainHead, RCT12EntityLinkListOffset::Vehicle }) + { + uint16_t previous = SPRITE_INDEX_NULL; + for (auto& entity : _s6.sprites) + { + if (entity.unknown.linked_list_type_offset == list) + { + _s6.sprites[entity.unknown.sprite_index].unknown.previous = previous; + if (previous != SPRITE_INDEX_NULL) + { + _s6.sprites[previous].unknown.next = entity.unknown.sprite_index; + } + else + { + _s6.sprite_lists_head[EnumValue(list)] = entity.unknown.sprite_index; + } + _s6.sprites[entity.unknown.sprite_index].unknown.next = SPRITE_INDEX_NULL; + previous = entity.unknown.sprite_index; + } + } } } @@ -989,13 +1010,44 @@ void S6Exporter::ExportSprite(RCT2Sprite* dst, const rct_sprite* src) } } +constexpr RCT12EntityLinkListOffset GetRCT2LinkListOffset(const SpriteBase* src) +{ + RCT12EntityLinkListOffset output = RCT12EntityLinkListOffset::Free; + switch (src->sprite_identifier) + { + case SpriteIdentifier::Vehicle: + { + auto veh = src->As(); + if (veh && veh->IsHead()) + { + output = RCT12EntityLinkListOffset::TrainHead; + } + else + { + output = RCT12EntityLinkListOffset::Vehicle; + } + } + break; + case SpriteIdentifier::Peep: + output = RCT12EntityLinkListOffset::Peep; + break; + case SpriteIdentifier::Misc: + output = RCT12EntityLinkListOffset::Misc; + break; + case SpriteIdentifier::Litter: + output = RCT12EntityLinkListOffset::Litter; + break; + default: + break; + } + return output; +} + void S6Exporter::ExportSpriteCommonProperties(RCT12SpriteBase* dst, const SpriteBase* src) { dst->sprite_identifier = src->sprite_identifier; dst->next_in_quadrant = src->next_in_quadrant; - dst->next = src->next; - dst->previous = src->previous; - dst->linked_list_type_offset = static_cast(src->linked_list_index) * 2; + dst->linked_list_type_offset = GetRCT2LinkListOffset(src); dst->sprite_height_negative = src->sprite_height_negative; dst->sprite_index = src->sprite_index; dst->flags = src->flags; diff --git a/src/openrct2/rct2/S6Exporter.h b/src/openrct2/rct2/S6Exporter.h index 2a492b98bf..fc1d2e0155 100644 --- a/src/openrct2/rct2/S6Exporter.h +++ b/src/openrct2/rct2/S6Exporter.h @@ -78,4 +78,5 @@ private: std::optional AllocateUserString(std::string_view value); void ExportUserStrings(); + void RebuildEntityLinks(); }; diff --git a/src/openrct2/rct2/S6Importer.cpp b/src/openrct2/rct2/S6Importer.cpp index 6e0bbc6763..ce52cfb936 100644 --- a/src/openrct2/rct2/S6Importer.cpp +++ b/src/openrct2/rct2/S6Importer.cpp @@ -461,15 +461,6 @@ public: auto& park = OpenRCT2::GetContext()->GetGameState()->GetPark(); park.Name = GetUserString(_s6.park_name); - // We try to fix the cycles on import, hence the 'true' parameter - check_for_sprite_list_cycles(true); - int32_t 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); - } - if (String::Equals(_s6.scenario_filename, "Europe - European Cultural Festival.SC6")) { // This scenario breaks pathfinding. Create passages between the worlds. (List is grouped by neighbouring tiles.) @@ -1332,14 +1323,7 @@ public: auto dst = GetEntity(i); ImportSprite(reinterpret_cast(dst), src); } - - for (int32_t i = 0; i < static_cast(EntityListId::Count); i++) - { - gSpriteListHead[i] = _s6.sprite_lists_head[i]; - gSpriteListCount[i] = _s6.sprite_lists_count[i]; - } - // This list contains the number of free slots. Increase it according to our own sprite limit. - gSpriteListCount[static_cast(EntityListId::Free)] += (MAX_SPRITES - RCT2_MAX_SPRITES); + RebuildEntityLists(); } void ImportSprite(rct_sprite* dst, const RCT2Sprite* src) @@ -1673,9 +1657,7 @@ public: { dst->sprite_identifier = src->sprite_identifier; dst->next_in_quadrant = src->next_in_quadrant; - dst->next = src->next; - dst->previous = src->previous; - dst->linked_list_index = static_cast(src->linked_list_type_offset >> 1); + 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; dst->flags = src->flags; diff --git a/src/openrct2/world/Sprite.cpp b/src/openrct2/world/Sprite.cpp index 7b67f35147..dba2cf9b09 100644 --- a/src/openrct2/world/Sprite.cpp +++ b/src/openrct2/world/Sprite.cpp @@ -25,9 +25,8 @@ #include #include -uint16_t gSpriteListHead[static_cast(EntityListId::Count)]; -uint16_t gSpriteListCount[static_cast(EntityListId::Count)]; static rct_sprite _spriteList[MAX_SPRITES]; +static std::array, EnumValue(EntityListId::Count)> gEntityLists; static bool _spriteFlashingList[MAX_SPRITES]; @@ -47,7 +46,6 @@ const rct_string_id litterNames[12] = { STR_LITTER_VOMIT, STR_SHOP_ITEM_SINGULAR_EMPTY_BOWL_BLUE }; static size_t GetSpatialIndexOffset(int32_t x, int32_t y); -static void move_sprite_to_list(SpriteBase* sprite, EntityListId newListIndex); // Required for GetEntity to return a default template<> bool SpriteBase::Is() const @@ -85,7 +83,7 @@ template<> bool SpriteBase::Is() const uint16_t GetEntityListCount(EntityListId list) { - return gSpriteListCount[static_cast(list)]; + return static_cast(gEntityLists[static_cast(list)].size()); } std::string rct_sprite_checksum::ToString() const @@ -174,6 +172,47 @@ void SpriteBase::Invalidate() viewports_invalidate(sprite_left, sprite_top, sprite_right, sprite_bottom, maxZoom); } +constexpr EntityListId EntityIdentifierToListId(const SpriteIdentifier type) +{ + EntityListId linkedListIndex = EntityListId::Free; + switch (type) + { + case SpriteIdentifier::Vehicle: + linkedListIndex = EntityListId::Vehicle; + break; + case SpriteIdentifier::Peep: + linkedListIndex = EntityListId::Peep; + break; + case SpriteIdentifier::Misc: + linkedListIndex = EntityListId::Misc; + break; + case SpriteIdentifier::Litter: + linkedListIndex = EntityListId::Litter; + break; + default: + break; + } + return linkedListIndex; +} + +void RebuildEntityLists() +{ + for (auto& list : gEntityLists) + { + list.clear(); + } + + for (auto& ent : _spriteList) + { + // auto listId = EntityIdentifierToListId(ent.misc.sprite_identifier); + gEntityLists[EnumValue(ent.misc.linked_list_index)].push_back(ent.misc.sprite_index); + } +} + +const std::list& GetEntityList(const EntityListId id) +{ + return gEntityLists[EnumValue(id)]; +} /** * @@ -184,15 +223,6 @@ void reset_sprite_list() gSavedAge = 0; std::memset(static_cast(_spriteList), 0, sizeof(_spriteList)); - for (int32_t i = 0; i < static_cast(EntityListId::Count); i++) - { - gSpriteListHead[i] = SPRITE_INDEX_NULL; - gSpriteListCount[i] = 0; - _spriteFlashingList[i] = false; - } - - SpriteBase* previous_spr = nullptr; - for (int32_t i = 0; i < MAX_SPRITES; ++i) { auto* spr = GetEntity(i); @@ -203,25 +233,12 @@ void reset_sprite_list() spr->sprite_identifier = SpriteIdentifier::Null; spr->sprite_index = i; - spr->next = SPRITE_INDEX_NULL; spr->linked_list_index = EntityListId::Free; - if (previous_spr != nullptr) - { - spr->previous = previous_spr->sprite_index; - previous_spr->next = i; - } - else - { - spr->previous = SPRITE_INDEX_NULL; - gSpriteListHead[static_cast(EntityListId::Free)] = i; - } _spriteFlashingList[i] = false; - previous_spr = spr; } - gSpriteListCount[static_cast(EntityListId::Free)] = MAX_SPRITES; - + RebuildEntityLists(); reset_sprite_spatial_index(); } @@ -348,18 +365,14 @@ 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 = sprite->next; uint16_t next_in_quadrant = sprite->next_in_quadrant; - uint16_t prev = sprite->previous; 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 = next; sprite->next_in_quadrant = next_in_quadrant; - sprite->previous = prev; sprite->sprite_index = sprite_index; sprite->sprite_identifier = SpriteIdentifier::Null; } @@ -389,6 +402,23 @@ void sprite_clear_all_unused() static void SpriteSpatialInsert(SpriteBase* sprite, const CoordsXY& newLoc); static constexpr uint16_t MAX_MISC_SPRITES = 300; +static void AddToEntityList(const EntityListId linkedListIndex, SpriteBase* entity) +{ + auto& list = gEntityLists[EnumValue(linkedListIndex)]; + entity->linked_list_index = linkedListIndex; + // Entity list must be in sprite_index order to prevent desync issues + list.insert(std::lower_bound(std::begin(list), std::end(list), entity->sprite_index), entity->sprite_index); +} + +static void RemoveFromEntityList(SpriteBase* entity) +{ + auto& list = gEntityLists[EnumValue(entity->linked_list_index)]; + auto ptr = std::lower_bound(std::begin(list), std::end(list), entity->sprite_index); + if (ptr != std::end(list) && *ptr == entity->sprite_index) + { + list.erase(ptr); + } +} rct_sprite* create_sprite(SpriteIdentifier spriteIdentifier, EntityListId linkedListIndex) { @@ -410,13 +440,14 @@ rct_sprite* create_sprite(SpriteIdentifier spriteIdentifier, EntityListId linked } } - auto* sprite = GetEntity(gSpriteListHead[static_cast(EntityListId::Free)]); + auto* sprite = GetEntity(gEntityLists[static_cast(EntityListId::Free)].front()); if (sprite == nullptr) { return nullptr; } - move_sprite_to_list(sprite, linkedListIndex); - + gEntityLists[static_cast(EntityListId::Free)].pop_front(); + RemoveFromEntityList(sprite); // remove from Free list + AddToEntityList(linkedListIndex, sprite); // Need to reset all sprite data, as the uninitialised values // may contain garbage and cause a desync later on. sprite_reset(sprite); @@ -459,83 +490,6 @@ rct_sprite* create_sprite(SpriteIdentifier spriteIdentifier) return create_sprite(spriteIdentifier, linkedListIndex); } -/* - * rct2: 0x0069ED0B - * This function moves a sprite to the specified sprite linked list. - * The game uses this list to categorise sprites by type. - */ -static void move_sprite_to_list(SpriteBase* sprite, EntityListId newListIndex) -{ - auto oldListIndex = sprite->linked_list_index; - - // No need to move if the sprite is already in the desired list - if (oldListIndex == newListIndex) - { - return; - } - - // If the sprite is currently the head of the list, the - // sprite following this one becomes the new head of the list. - if (sprite->previous == SPRITE_INDEX_NULL) - { - gSpriteListHead[static_cast(oldListIndex)] = sprite->next; - } - else - { - // Hook up sprite->previous->next to sprite->next, removing the sprite from its old list - auto previous = GetEntity(sprite->previous); - if (previous == nullptr) - { - log_error("Broken previous entity id. Entity list corrupted!"); - } - else - { - previous->next = sprite->next; - } - } - - // Similarly, hook up sprite->next->previous to sprite->previous - if (sprite->next != SPRITE_INDEX_NULL) - { - auto next = GetEntity(sprite->next); - if (next == nullptr) - { - log_error("Broken next entity id. Entity list corrupted!"); - } - else - { - next->previous = sprite->previous; - } - } - - sprite->previous = SPRITE_INDEX_NULL; // We become the new head of the target list, so there's no previous sprite - sprite->linked_list_index = newListIndex; - - sprite->next = gSpriteListHead[static_cast( - newListIndex)]; // This sprite's next sprite is the old head, since we're the new head - gSpriteListHead[static_cast(newListIndex)] = sprite->sprite_index; // Store this sprite's index as head of its - // new list - - if (sprite->next != SPRITE_INDEX_NULL) - { - // Fix the chain by settings sprite->next->previous to sprite_index - auto next = GetEntity(sprite->next); - if (next == nullptr) - { - log_error("Broken next entity id. Entity list corrupted!"); - } - else - { - next->previous = sprite->sprite_index; - } - } - - // These globals are probably counters for each sprite list? - // Decrement old list counter, increment new list counter. - gSpriteListCount[static_cast(oldListIndex)]--; - gSpriteListCount[static_cast(newListIndex)]++; -} - /** * * rct2: 0x00673200 @@ -766,8 +720,9 @@ void sprite_remove(SpriteBase* sprite) } EntityTweener::Get().RemoveEntity(sprite); + RemoveFromEntityList(sprite); // remove from existing list + AddToEntityList(EntityListId::Free, sprite); - move_sprite_to_list(sprite, EntityListId::Free); SpriteSpatialRemove(sprite); sprite_reset(sprite); } @@ -1007,157 +962,3 @@ bool sprite_get_flashing(SpriteBase* sprite) assert(sprite->sprite_index < MAX_SPRITES); return _spriteFlashingList[sprite->sprite_index]; } - -static SpriteBase* find_sprite_list_cycle(uint16_t sprite_idx) -{ - if (sprite_idx == SPRITE_INDEX_NULL) - { - return nullptr; - } - const SpriteBase* fast = GetEntity(sprite_idx); - const SpriteBase* slow = fast; - bool increment_slow = false; - SpriteBase* cycle_start = nullptr; - while (fast->sprite_index != SPRITE_INDEX_NULL) - { - // increment fast every time, unless reached the end - if (fast->next == SPRITE_INDEX_NULL) - { - break; - } - else - { - fast = GetEntity(fast->next); - } - // increment slow only every second iteration - if (increment_slow) - { - slow = GetEntity(slow->next); - } - increment_slow = !increment_slow; - if (fast == slow) - { - cycle_start = GetEntity(slow->sprite_index); - break; - } - } - return cycle_start; -} - -static bool index_is_in_list(uint16_t index, EntityListId sl) -{ - for (auto entity : EntityList(sl)) - { - if (entity->sprite_index == index) - { - return true; - } - } - return false; -} - -int32_t check_for_sprite_list_cycles(bool fix) -{ - for (int32_t i = 0; i < static_cast(EntityListId::Count); i++) - { - auto* cycle_start = find_sprite_list_cycle(gSpriteListHead[i]); - if (cycle_start != nullptr) - { - if (fix) - { - // Fix head list, but only in reverse order - // This is likely not needed, but just in case - auto head = GetEntity(gSpriteListHead[i]); - if (head == nullptr) - { - log_error("SpriteListHead is corrupted!"); - return -1; - } - head->previous = SPRITE_INDEX_NULL; - - // Store the leftover part of cycle to be fixed - uint16_t cycle_next = cycle_start->next; - - // Break the cycle - cycle_start->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, static_cast(i))) - { - auto* spr = GetEntity(cycle_next); - if (spr == nullptr) - { - log_error("EntityList is corrupted!"); - return -1; - } - cycle_start->next = cycle_next; - spr->previous = cycle_start->sprite_index; - cycle_next = spr->next; - spr->next = SPRITE_INDEX_NULL; - cycle_start = spr; - } - } - return i; - } - } - return -1; -} - -/** - * Finds and fixes null sprites that are not reachable via EntityListId::Free list. - * - * @return count of disjoint sprites found - */ -int32_t fix_disjoint_sprites() -{ - // Find reachable sprites - bool reachable[MAX_SPRITES] = { false }; - - SpriteBase* null_list_tail = nullptr; - for (uint16_t sprite_idx = gSpriteListHead[static_cast(EntityListId::Free)]; 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 = GetEntity(sprite_idx); - if (null_list_tail == nullptr) - { - log_error("Broken Entity list"); - sprite_idx = SPRITE_INDEX_NULL; - return 0; - } - sprite_idx = null_list_tail->next; - } - - int32_t count = 0; - - // Find all null sprites - for (uint16_t sprite_idx = 0; sprite_idx < MAX_SPRITES; sprite_idx++) - { - auto* spr = GetEntity(sprite_idx); - if (spr != nullptr && spr->sprite_identifier == SpriteIdentifier::Null) - { - openrct2_assert(null_list_tail != nullptr, "Null list is empty, yet found null sprites"); - spr->sprite_index = sprite_idx; - if (!reachable[sprite_idx]) - { - // Add the sprite directly to the list - if (null_list_tail == nullptr) - { - gSpriteListHead[static_cast(EntityListId::Free)] = sprite_idx; - spr->previous = SPRITE_INDEX_NULL; - } - else - { - null_list_tail->next = sprite_idx; - spr->previous = null_list_tail->sprite_index; - } - spr->next = SPRITE_INDEX_NULL; - null_list_tail = spr; - count++; - reachable[sprite_idx] = true; - } - } - } - return count; -} diff --git a/src/openrct2/world/Sprite.h b/src/openrct2/world/Sprite.h index a90787080d..08523bd073 100644 --- a/src/openrct2/world/Sprite.h +++ b/src/openrct2/world/Sprite.h @@ -16,6 +16,8 @@ #include "Fountain.h" #include "SpriteBase.h" +#include + #define SPRITE_INDEX_NULL 0xFFFF #define MAX_SPRITES 10000 @@ -226,8 +228,6 @@ template T* TryGetEntity(size_t sprite_idx) } uint16_t GetEntityListCount(EntityListId list); -extern uint16_t gSpriteListHead[static_cast(EntityListId::Count)]; -extern uint16_t gSpriteListCount[static_cast(EntityListId::Count)]; 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; @@ -237,6 +237,7 @@ extern const rct_string_id litterNames[12]; rct_sprite* create_sprite(SpriteIdentifier spriteIdentifier); rct_sprite* create_sprite(SpriteIdentifier spriteIdentifier, EntityListId linkedListIndex); +void RebuildEntityLists(); void reset_sprite_list(); void reset_sprite_spatial_index(); void sprite_clear_all_unused(); @@ -273,8 +274,8 @@ rct_sprite_checksum sprite_checksum(); void sprite_set_flashing(SpriteBase* sprite, bool flashing); bool sprite_get_flashing(SpriteBase* sprite); -int32_t check_for_sprite_list_cycles(bool fix); -int32_t fix_disjoint_sprites(); + +const std::list& GetEntityList(const EntityListId id); template class EntityIterator { @@ -354,25 +355,76 @@ public: } }; +template class EntityListIterator +{ +private: + std::list::const_iterator iter; + std::list::const_iterator end; + T* Entity = nullptr; + +public: + EntityListIterator(std::list::const_iterator _iter, std::list::const_iterator _end) + : iter(_iter) + , end(_end) + { + ++(*this); + } + EntityListIterator& operator++() + { + Entity = nullptr; + + while (iter != end && Entity == nullptr) + { + Entity = GetEntity(*iter++); + } + return *this; + } + + EntityListIterator operator++(int) + { + EntityListIterator retval = *this; + ++(*this); + return *iter; + } + bool operator==(EntityListIterator other) const + { + return Entity == other.Entity; + } + bool operator!=(EntityListIterator 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 EntityList { private: - uint16_t FirstEntity = SPRITE_INDEX_NULL; - using EntityListIterator = EntityIterator; + using EntityListIterator_t = EntityListIterator; + const std::list& vec; public: EntityList(EntityListId type) - : FirstEntity(gSpriteListHead[static_cast(type)]) + : vec(GetEntityList(type)) { } - EntityListIterator begin() + EntityListIterator_t begin() { - return EntityListIterator(FirstEntity); + return EntityListIterator_t(std::cbegin(vec), std::cend(vec)); } - EntityListIterator end() + EntityListIterator_t end() { - return EntityListIterator(SPRITE_INDEX_NULL); + return EntityListIterator_t(std::cend(vec), std::cend(vec)); } }; diff --git a/src/openrct2/world/SpriteBase.h b/src/openrct2/world/SpriteBase.h index fe006ad493..6077fb12c3 100644 --- a/src/openrct2/world/SpriteBase.h +++ b/src/openrct2/world/SpriteBase.h @@ -10,8 +10,6 @@ struct SpriteBase { SpriteIdentifier sprite_identifier; uint16_t next_in_quadrant; - uint16_t next; - uint16_t previous; // 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 a37a71f409..ef2b99073b 100644 --- a/test/tests/S6ImportExportTests.cpp +++ b/test/tests/S6ImportExportTests.cpp @@ -132,8 +132,6 @@ static void CompareSpriteDataCommon(const SpriteBase& left, const SpriteBase& ri { COMPARE_FIELD(sprite_identifier); COMPARE_FIELD(next_in_quadrant); - COMPARE_FIELD(next); - COMPARE_FIELD(previous); COMPARE_FIELD(linked_list_index); COMPARE_FIELD(sprite_index); COMPARE_FIELD(flags);