diff --git a/distribution/changelog.txt b/distribution/changelog.txt index c2df57cefb..3a9871a497 100644 --- a/distribution/changelog.txt +++ b/distribution/changelog.txt @@ -23,6 +23,7 @@ - Change: [#19091] [Plugin] Add game action information to callback arguments of custom actions. - Change: [#19233] Reduce lift speed minimum and maximum values for “Classic Wooden Coaster”. - Fix: [#474] Mini golf window shows more players than there actually are (original bug). +- Fix: [#18260] Crash opening parks that have multiple tiles referencing the same banner entry. - Fix: [#18467] “Selected only” Object Selection filter is active in Track Designs Manager, and cannot be toggled. - Fix: [#18905] Ride Construction window theme is not applied correctly. - Fix: [#18911] Mini Golf station does not draw correctly from all angles. diff --git a/src/openrct2/Game.cpp b/src/openrct2/Game.cpp index 41aec39566..5c771f004c 100644 --- a/src/openrct2/Game.cpp +++ b/src/openrct2/Game.cpp @@ -440,11 +440,8 @@ void GameFixSaveVars() ResearchFix(); - // Fix banner list pointing to NULL map elements - BannerResetBrokenIndex(); - // Fix banners which share their index - BannerFixDuplicates(); + BannerApplyFixes(); // Fix invalid vehicle sprite sizes, thus preventing visual corruption of sprites FixInvalidVehicleSpriteSizes(); diff --git a/src/openrct2/world/Banner.cpp b/src/openrct2/world/Banner.cpp index 5dd64a1d62..58d3a5d0ab 100644 --- a/src/openrct2/world/Banner.cpp +++ b/src/openrct2/world/Banner.cpp @@ -238,15 +238,45 @@ RideId BannerGetClosestRideIndex(const CoordsXYZ& mapPos) return rideIndex; } -void BannerResetBrokenIndex() +struct BannerElementWithPos +{ + BannerElement* Element; + TileCoordsXY Pos; +}; + +// Returns a list of BannerElement's with the tile position. +static std::vector GetAllBannerElementsOnMap() +{ + std::vector banners; + for (int y = 0; y < gMapSize.y; y++) + { + for (int x = 0; x < gMapSize.x; x++) + { + const auto tilePos = TileCoordsXY{ x, y }; + for (auto* bannerElement : OpenRCT2::TileElementsView(tilePos.ToCoordsXY())) + { + auto bannerIndex = bannerElement->GetIndex(); + if (bannerIndex == BannerIndex::GetNull()) + continue; + + banners.push_back({ bannerElement, tilePos }); + } + } + } + return banners; +} + +// Iterates all banners and checks if the tile specified by the position actually +// has a tile with the banner index, if no tile is found then the banner element will be released. +static void BannerDeallocateUnlinked() { for (BannerIndex::UnderlyingType index = 0; index < _banners.size(); index++) { const auto bannerId = BannerIndex::FromUnderlying(index); - auto tileElement = BannerGetTileElement(bannerId); + auto* tileElement = BannerGetTileElement(bannerId); if (tileElement == nullptr) { - auto banner = GetBanner(bannerId); + auto* banner = GetBanner(bannerId); if (banner != nullptr) { banner->type = BANNER_NULL; @@ -255,58 +285,81 @@ void BannerResetBrokenIndex() } } -void BannerFixDuplicates() +// BannerElement tiles should not share a banner entry, this iterates +// over all banner elements that shares the index and creates a new entry also +// copying the data from the current assigned banner entry. +static void BannerFixDuplicates(std::vector& bannerElements) { - // For each banner in the map, check if the banner index is in use already, and if so, create a new entry for it - std::vector activeBanners; - activeBanners.resize(MAX_BANNERS); + // Sort the banners by index + std::sort(bannerElements.begin(), bannerElements.end(), [](const BannerElementWithPos& a, const BannerElementWithPos& b) { + return a.Element->GetIndex() < b.Element->GetIndex(); + }); - for (int y = 0; y < gMapSize.y; y++) + // Create a list of all banners with duplicate indices. + std::vector duplicates; + for (size_t i = 1; i < bannerElements.size(); i++) { - for (int x = 0; x < gMapSize.x; x++) + if (bannerElements[i - 1].Element->GetIndex() == bannerElements[i].Element->GetIndex()) { - const auto bannerPos = TileCoordsXY{ x, y }.ToCoordsXY(); - for (auto* bannerElement : OpenRCT2::TileElementsView(bannerPos)) - { - auto bannerIndex = bannerElement->GetIndex(); - if (bannerIndex == BannerIndex::GetNull()) - continue; - - const auto index = bannerIndex.ToUnderlying(); - if (activeBanners[index]) - { - LOG_INFO( - "Duplicated banner with index %d found at x = %d, y = %d and z = %d.", index, x, y, - bannerElement->BaseHeight); - - // Banner index is already in use by another banner, so duplicate it - auto newBanner = CreateBanner(); - if (newBanner == nullptr) - { - LOG_ERROR("Failed to create new banner."); - continue; - } - Guard::Assert(!activeBanners[newBanner->id.ToUnderlying()]); - - // Copy over the original banner, but update the location - const auto* oldBanner = GetBanner(bannerIndex); - if (oldBanner != nullptr) - { - auto newBannerId = newBanner->id; - - *newBanner = *oldBanner; - newBanner->id = newBannerId; - newBanner->position = { x, y }; - } - - bannerElement->SetIndex(newBanner->id); - } - - // Mark banner index as in-use - activeBanners[index] = true; - } + duplicates.push_back(bannerElements[i]); } } + + // For each duplicate, create a new banner and copy the old data + for (const auto& duplicate : duplicates) + { + const auto oldIndex = duplicate.Element->GetIndex(); + const auto* oldBanner = GetBanner(oldIndex); + if (oldBanner == nullptr) + { + LOG_ERROR("Unable to get old banner for index %u.", oldIndex.ToUnderlying()); + continue; + } + + auto* newBanner = CreateBanner(); + if (newBanner == nullptr) + { + LOG_ERROR("Failed to create new banner."); + continue; + } + + const auto newBannerId = newBanner->id; + + // Copy the old data to the new banner. + *newBanner = *oldBanner; + newBanner->id = newBannerId; + + // Assign the new banner index to the tile element. + duplicate.Element->SetIndex(newBannerId); + } +} + +// Ensures that all banner entries have the correct position based on the element +// that references the banner entry. +static void BannerFixPositions(std::vector& bannerElements) +{ + for (const auto& entry : bannerElements) + { + const auto index = entry.Element->GetIndex(); + auto* banner = GetBanner(index); + if (banner == nullptr) + { + LOG_ERROR("Unable to get banner for index %u.", index.ToUnderlying()); + continue; + } + banner->position = entry.Pos; + } +} + +void BannerApplyFixes() +{ + auto bannerElements = GetAllBannerElementsOnMap(); + + BannerFixDuplicates(bannerElements); + + BannerFixPositions(bannerElements); + + BannerDeallocateUnlinked(); } Banner* BannerElement::GetBanner() const diff --git a/src/openrct2/world/Banner.h b/src/openrct2/world/Banner.h index 9a7f513d91..0c9eeed0fc 100644 --- a/src/openrct2/world/Banner.h +++ b/src/openrct2/world/Banner.h @@ -58,8 +58,7 @@ void BannerInit(); TileElement* BannerGetTileElement(BannerIndex bannerIndex); WallElement* BannerGetScrollingWallTileElement(BannerIndex bannerIndex); RideId BannerGetClosestRideIndex(const CoordsXYZ& mapPos); -void BannerResetBrokenIndex(); -void BannerFixDuplicates(); +void BannerApplyFixes(); void UnlinkAllRideBanners(); void UnlinkAllBannersForRide(RideId rideId); Banner* GetBanner(BannerIndex id);