From e9eb1b8304fdc461b6f7386e7b2fa9137012070d Mon Sep 17 00:00:00 2001 From: Duncan Date: Tue, 27 Jul 2021 10:25:58 +0100 Subject: [PATCH] Banner Refactor from NSF (#14788) * Banner refactor * Fix mistake in fix_duplicated_banners * Reduce limit back * Fix export * Free banner on delete * Increment network version * Fix potential banner pointer crashes Co-authored-by: Ted John --- .../interface/ViewportInteraction.cpp | 20 +- src/openrct2-ui/windows/Banner.cpp | 48 +++-- src/openrct2-ui/windows/Sign.cpp | 27 ++- src/openrct2-ui/windows/TileInspector.cpp | 6 +- src/openrct2-ui/windows/TopToolbar.cpp | 20 +- src/openrct2/Editor.cpp | 11 +- src/openrct2/actions/BannerPlaceAction.cpp | 57 +++--- src/openrct2/actions/BannerPlaceAction.h | 16 +- src/openrct2/actions/BannerRemoveAction.cpp | 14 +- .../actions/BannerSetColourAction.cpp | 6 +- src/openrct2/actions/BannerSetNameAction.cpp | 11 +- src/openrct2/actions/BannerSetStyleAction.cpp | 15 +- .../actions/LargeSceneryPlaceAction.cpp | 99 ++++------ .../actions/LargeSceneryPlaceAction.h | 2 +- src/openrct2/actions/RideDemolishAction.cpp | 10 +- src/openrct2/actions/SignSetNameAction.cpp | 8 +- src/openrct2/actions/SignSetStyleAction.cpp | 10 +- src/openrct2/actions/WallPlaceAction.cpp | 50 ++--- src/openrct2/actions/WallPlaceAction.h | 3 +- src/openrct2/interface/InteractiveConsole.cpp | 10 +- src/openrct2/network/NetworkBase.cpp | 2 +- .../paint/tile_element/Paint.Wall.cpp | 2 +- src/openrct2/rct1/S4Importer.cpp | 22 ++- src/openrct2/rct2/S6Exporter.cpp | 9 +- src/openrct2/rct2/S6Importer.cpp | 39 +++- src/openrct2/world/Banner.cpp | 177 +++++++++++++----- src/openrct2/world/Banner.h | 10 +- src/openrct2/world/TileElement.cpp | 2 +- src/openrct2/world/TileInspector.cpp | 14 +- 29 files changed, 415 insertions(+), 305 deletions(-) diff --git a/src/openrct2-ui/interface/ViewportInteraction.cpp b/src/openrct2-ui/interface/ViewportInteraction.cpp index 5bd98c98fc..eb1628903d 100644 --- a/src/openrct2-ui/interface/ViewportInteraction.cpp +++ b/src/openrct2-ui/interface/ViewportInteraction.cpp @@ -423,15 +423,19 @@ InteractionInfo ViewportInteractionGetItemRight(const ScreenCoordsXY& screenCoor case ViewportInteractionItem::Banner: { auto banner = tileElement->AsBanner()->GetBanner(); - auto* bannerEntry = get_banner_entry(banner->type); + if (banner != nullptr) + { + auto* bannerEntry = get_banner_entry(banner->type); - auto ft = Formatter(); - ft.Add(STR_MAP_TOOLTIP_BANNER_STRINGID_STRINGID); - banner->FormatTextTo(ft, /*addColour*/ true); - ft.Add(STR_MAP_TOOLTIP_STRINGID_CLICK_TO_MODIFY); - ft.Add(bannerEntry->name); - SetMapTooltip(ft); - return info; + auto ft = Formatter(); + ft.Add(STR_MAP_TOOLTIP_BANNER_STRINGID_STRINGID); + banner->FormatTextTo(ft, /*addColour*/ true); + ft.Add(STR_MAP_TOOLTIP_STRINGID_CLICK_TO_MODIFY); + ft.Add(bannerEntry->name); + SetMapTooltip(ft); + return info; + } + break; } default: break; diff --git a/src/openrct2-ui/windows/Banner.cpp b/src/openrct2-ui/windows/Banner.cpp index 8a7f372593..eae4fac74f 100644 --- a/src/openrct2-ui/windows/Banner.cpp +++ b/src/openrct2-ui/windows/Banner.cpp @@ -74,7 +74,6 @@ static rct_widget window_banner_widgets[] = { class BannerWindow final : public Window { private: - Banner* _banner; CoordsXYZ _bannerViewPos; void CreateViewport() @@ -91,12 +90,13 @@ private: BannerElement* GetBannerElement() { - if (_banner == nullptr) + auto* banner = GetBanner(number); + if (banner == nullptr) { return nullptr; } - TileElement* tileElement = map_get_first_element_at(_banner->position.ToCoordsXY().ToTileCentre()); + TileElement* tileElement = map_get_first_element_at(banner->position.ToCoordsXY().ToTileCentre()); if (tileElement == nullptr) { return nullptr; @@ -131,24 +131,29 @@ public: void Initialise(rct_windownumber _number) { number = _number; - _banner = GetBanner(number); + auto* banner = GetBanner(number); auto* bannerElement = GetBannerElement(); if (bannerElement == nullptr) return; - _bannerViewPos = CoordsXYZ{ _banner->position.ToCoordsXY().ToTileCentre(), bannerElement->GetBaseZ() }; + _bannerViewPos = CoordsXYZ{ banner->position.ToCoordsXY().ToTileCentre(), bannerElement->GetBaseZ() }; CreateViewport(); } void OnMouseDown(rct_widgetindex widgetIndex) override { rct_widget* widget = &widgets[widgetIndex]; - + auto* banner = GetBanner(number); + if (banner == nullptr) + { + Close(); + return; + } switch (widgetIndex) { case WIDX_MAIN_COLOUR: - WindowDropdownShowColour(this, widget, TRANSLUCENT(colours[1]), _banner->colour); + WindowDropdownShowColour(this, widget, TRANSLUCENT(colours[1]), banner->colour); break; case WIDX_TEXT_COLOUR_DROPDOWN_BUTTON: @@ -165,13 +170,19 @@ public: { widget->left + windowPos.x, widget->top + windowPos.y }, widget->height() + 1, colours[1], 0, Dropdown::Flag::StayOpen, 13, widget->width() - 3); - Dropdown::SetChecked(_banner->text_colour - 1, true); + Dropdown::SetChecked(banner->text_colour - 1, true); break; } } void OnMouseUp(rct_widgetindex widgetIndex) override { + auto* banner = GetBanner(number); + if (banner == nullptr) + { + Close(); + return; + } switch (widgetIndex) { case WIDX_CLOSE: @@ -180,23 +191,23 @@ public: case WIDX_BANNER_DEMOLISH: { auto* bannerElement = GetBannerElement(); - if (_banner == nullptr || bannerElement == nullptr) + if (bannerElement == nullptr) break; auto bannerRemoveAction = BannerRemoveAction( - { _banner->position.ToCoordsXY(), bannerElement->GetBaseZ(), bannerElement->GetPosition() }); + { banner->position.ToCoordsXY(), bannerElement->GetBaseZ(), bannerElement->GetPosition() }); GameActions::Execute(&bannerRemoveAction); break; } case WIDX_BANNER_TEXT: window_text_input_raw_open( - this, WIDX_BANNER_TEXT, STR_BANNER_TEXT, STR_ENTER_BANNER_TEXT, _banner->GetText().c_str(), 32); + this, WIDX_BANNER_TEXT, STR_BANNER_TEXT, STR_ENTER_BANNER_TEXT, banner->GetText().c_str(), 32); break; case WIDX_BANNER_NO_ENTRY: { textinput_cancel(); auto bannerSetStyle = BannerSetStyleAction( - BannerSetStyleType::NoEntry, number, _banner->flags ^ BANNER_FLAG_NO_ENTRY); + BannerSetStyleType::NoEntry, number, banner->flags ^ BANNER_FLAG_NO_ENTRY); GameActions::Execute(&bannerSetStyle); break; } @@ -254,10 +265,15 @@ public: void OnPrepareDraw() override { + auto* banner = GetBanner(number); + if (banner == nullptr) + { + return; + } rct_widget* colourBtn = &window_banner_widgets[WIDX_MAIN_COLOUR]; colourBtn->type = WindowWidgetType::Empty; - auto* bannerEntry = get_banner_entry(_banner->type); + auto* bannerEntry = get_banner_entry(banner->type); if (bannerEntry != nullptr && (bannerEntry->flags & BANNER_ENTRY_FLAG_HAS_PRIMARY_COLOUR)) { colourBtn->type = WindowWidgetType::ColourBtn; @@ -265,15 +281,15 @@ public: pressed_widgets &= ~(1ULL << WIDX_BANNER_NO_ENTRY); disabled_widgets &= ~( (1ULL << WIDX_BANNER_TEXT) | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN) | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN_BUTTON)); - if (_banner->flags & BANNER_FLAG_NO_ENTRY) + if (banner->flags & BANNER_FLAG_NO_ENTRY) { pressed_widgets |= (1ULL << WIDX_BANNER_NO_ENTRY); disabled_widgets |= (1ULL << WIDX_BANNER_TEXT) | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN) | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN_BUTTON); } - colourBtn->image = SPRITE_ID_PALETTE_COLOUR_1(_banner->colour) | IMAGE_TYPE_TRANSPARENT | SPR_PALETTE_BTN; + colourBtn->image = SPRITE_ID_PALETTE_COLOUR_1(banner->colour) | IMAGE_TYPE_TRANSPARENT | SPR_PALETTE_BTN; rct_widget* dropDownWidget = &window_banner_widgets[WIDX_TEXT_COLOUR_DROPDOWN]; - dropDownWidget->text = BannerColouredTextFormats[_banner->text_colour]; + dropDownWidget->text = BannerColouredTextFormats[banner->text_colour]; } }; diff --git a/src/openrct2-ui/windows/Sign.cpp b/src/openrct2-ui/windows/Sign.cpp index fd4f8176eb..28836b9741 100644 --- a/src/openrct2-ui/windows/Sign.cpp +++ b/src/openrct2-ui/windows/Sign.cpp @@ -58,13 +58,13 @@ class SignWindow final : public Window { private: bool _isSmall = false; - Banner* _banner = nullptr; void ShowTextInput() { - if (_banner != nullptr) + auto* banner = GetBanner(number); + if (banner != nullptr) { - auto bannerText = _banner->GetText(); + auto bannerText = banner->GetText(); window_text_input_raw_open(this, WIDX_SIGN_TEXT, STR_SIGN_TEXT_TITLE, STR_SIGN_TEXT_PROMPT, bannerText.c_str(), 32); } } @@ -87,12 +87,13 @@ public: { number = windowNumber; _isSmall = isSmall; - - _banner = GetBanner(number); - if (_banner == nullptr) + auto* banner = GetBanner(number); + if (banner == nullptr) + { return false; + } - auto signViewPosition = _banner->position.ToCoordsXY().ToTileCentre(); + auto signViewPosition = banner->position.ToCoordsXY().ToTileCentre(); auto* tileElement = banner_get_tile_element(number); if (tileElement == nullptr) return false; @@ -138,6 +139,12 @@ public: void OnMouseUp(rct_widgetindex widgetIndex) override { + auto* banner = GetBanner(number); + if (banner == nullptr) + { + Close(); + return; + } switch (widgetIndex) { case WIDX_CLOSE: @@ -150,7 +157,7 @@ public: { Close(); } - auto bannerCoords = _banner->position.ToCoordsXY(); + auto bannerCoords = banner->position.ToCoordsXY(); if (_isSmall) { @@ -282,6 +289,10 @@ public: RemoveViewport(); auto banner = GetBanner(number); + if (banner == nullptr) + { + return; + } auto signViewPos = CoordsXYZ{ banner->position.ToCoordsXY().ToTileCentre(), frame_no }; diff --git a/src/openrct2-ui/windows/TileInspector.cpp b/src/openrct2-ui/windows/TileInspector.cpp index e0ad6f530e..c0c16e4ab7 100644 --- a/src/openrct2-ui/windows/TileInspector.cpp +++ b/src/openrct2-ui/windows/TileInspector.cpp @@ -2095,7 +2095,7 @@ static void window_tile_inspector_paint(rct_window* w, rct_drawpixelinfo* dpi) // Banner info auto banner = tileElement->AsWall()->GetBanner(); - if (banner != nullptr && !banner->IsNull()) + if (banner != nullptr) { Formatter ft; banner->FormatTextTo(ft); @@ -2160,7 +2160,7 @@ static void window_tile_inspector_paint(rct_window* w, rct_drawpixelinfo* dpi) if (largeSceneryEntry != nullptr && largeSceneryEntry->scrolling_mode != SCROLLING_MODE_NONE) { auto banner = sceneryElement->GetBanner(); - if (banner != nullptr && !banner->IsNull()) + if (banner != nullptr) { Formatter ft; banner->FormatTextTo(ft); @@ -2193,7 +2193,7 @@ static void window_tile_inspector_paint(rct_window* w, rct_drawpixelinfo* dpi) // Details // Banner info auto banner = tileElement->AsBanner()->GetBanner(); - if (banner != nullptr && !banner->IsNull()) + if (banner != nullptr) { Formatter ft; banner->FormatTextTo(ft); diff --git a/src/openrct2-ui/windows/TopToolbar.cpp b/src/openrct2-ui/windows/TopToolbar.cpp index 5c0d53c84b..b540eece80 100644 --- a/src/openrct2-ui/windows/TopToolbar.cpp +++ b/src/openrct2-ui/windows/TopToolbar.cpp @@ -1974,18 +1974,12 @@ static void window_top_toolbar_scenery_tool_down(const ScreenCoordsXY& windowPos CoordsXYZD loc{ gridPos, z, direction }; auto primaryColour = gWindowSceneryPrimaryColour; - auto bannerIndex = create_new_banner(0); - if (bannerIndex == BANNER_INDEX_NULL) - { - context_show_error(STR_CANT_POSITION_THIS_HERE, STR_TOO_MANY_BANNERS_IN_GAME, {}); - break; - } - auto bannerPlaceAction = BannerPlaceAction(loc, selectedScenery, bannerIndex, primaryColour); - bannerPlaceAction.SetCallback([=](const GameAction* ga, const GameActions::Result* result) { + auto bannerPlaceAction = BannerPlaceAction(loc, selectedScenery, primaryColour); + bannerPlaceAction.SetCallback([=](const GameAction* ga, const BannerPlaceActionResult* result) { if (result->Error == GameActions::Status::Ok) { OpenRCT2::Audio::Play3D(OpenRCT2::Audio::SoundId::PlaceItem, result->Position); - context_open_detail_window(WD_BANNER, bannerIndex); + context_open_detail_window(WD_BANNER, result->bannerId); } }); GameActions::Execute(&bannerPlaceAction); @@ -2571,13 +2565,7 @@ static money32 try_place_ghost_banner(CoordsXYZD loc, ObjectEntryIndex entryInde // 6e2612 auto primaryColour = gWindowSceneryPrimaryColour; - auto bannerIndex = create_new_banner(0); - if (bannerIndex == BANNER_INDEX_NULL) - { - // Silently fail as this is just for the ghost - return 0; - } - auto bannerPlaceAction = BannerPlaceAction(loc, entryIndex, bannerIndex, primaryColour); + auto bannerPlaceAction = BannerPlaceAction(loc, entryIndex, primaryColour); bannerPlaceAction.SetFlags(GAME_COMMAND_FLAG_GHOST | GAME_COMMAND_FLAG_ALLOW_DURING_PAUSED | GAME_COMMAND_FLAG_NO_SPEND); auto res = GameActions::Execute(&bannerPlaceAction); if (res->Error != GameActions::Status::Ok) diff --git a/src/openrct2/Editor.cpp b/src/openrct2/Editor.cpp index fb778c1453..18e3262bbf 100644 --- a/src/openrct2/Editor.cpp +++ b/src/openrct2/Editor.cpp @@ -297,16 +297,7 @@ namespace Editor static void ClearMapForEditing(bool fromSave) { map_remove_all_rides(); - - // - for (BannerIndex i = 0; i < MAX_BANNERS; i++) - { - auto banner = GetBanner(i); - if (banner->IsNull()) - { - banner->flags &= ~BANNER_FLAG_LINKED_TO_RIDE; - } - } + UnlinkAllRideBanners(); ride_init_all(); diff --git a/src/openrct2/actions/BannerPlaceAction.cpp b/src/openrct2/actions/BannerPlaceAction.cpp index c235d5891d..28a029cc40 100644 --- a/src/openrct2/actions/BannerPlaceAction.cpp +++ b/src/openrct2/actions/BannerPlaceAction.cpp @@ -18,10 +18,29 @@ using namespace OpenRCT2; -BannerPlaceAction::BannerPlaceAction(const CoordsXYZD& loc, uint8_t bannerType, BannerIndex bannerIndex, uint8_t primaryColour) +BannerPlaceActionResult::BannerPlaceActionResult() + : GameActions::Result(GameActions::Status::Ok, STR_CANT_POSITION_THIS_HERE) +{ +} + +BannerPlaceActionResult::BannerPlaceActionResult(GameActions::Status err) + : GameActions::Result(err, STR_CANT_POSITION_THIS_HERE) +{ +} + +BannerPlaceActionResult::BannerPlaceActionResult(GameActions::Status err, rct_string_id msg) + : GameActions::Result(err, STR_CANT_POSITION_THIS_HERE, msg) +{ +} + +BannerPlaceActionResult::BannerPlaceActionResult(GameActions::Status err, rct_string_id title, rct_string_id message) + : GameActions::Result(err, title, message) +{ +} + +BannerPlaceAction::BannerPlaceAction(const CoordsXYZD& loc, uint8_t bannerType, uint8_t primaryColour) : _loc(loc) , _bannerType(bannerType) - , _bannerIndex(bannerIndex) , _primaryColour(primaryColour) { } @@ -31,7 +50,6 @@ void BannerPlaceAction::AcceptParameters(GameActionParameterVisitor& visitor) visitor.Visit(_loc); visitor.Visit("object", _bannerType); visitor.Visit("primaryColour", _primaryColour); - _bannerIndex = create_new_banner(0); } uint16_t BannerPlaceAction::GetActionFlags() const @@ -43,7 +61,7 @@ void BannerPlaceAction::Serialise(DataSerialiser& stream) { GameAction::Serialise(stream); - stream << DS_TAG(_loc) << DS_TAG(_bannerType) << DS_TAG(_bannerIndex) << DS_TAG(_primaryColour); + stream << DS_TAG(_loc) << DS_TAG(_bannerType) << DS_TAG(_primaryColour); } GameActions::Result::Ptr BannerPlaceAction::Query() const @@ -86,17 +104,10 @@ GameActions::Result::Ptr BannerPlaceAction::Query() const return MakeResult(GameActions::Status::ItemAlreadyPlaced, STR_CANT_POSITION_THIS_HERE, STR_BANNER_SIGN_IN_THE_WAY); } - if (_bannerIndex == BANNER_INDEX_NULL || _bannerIndex >= MAX_BANNERS) + if (HasReachedBannerLimit()) { - log_error("Invalid banner index, bannerIndex = %u", _bannerIndex); - return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_POSITION_THIS_HERE); - } - - auto banner = GetBanner(_bannerIndex); - if (!banner->IsNull()) - { - log_error("Banner index in use, bannerIndex = %u", _bannerIndex); - return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_POSITION_THIS_HERE); + log_error("No free banners available"); + return MakeResult(GameActions::Status::InvalidParameters, STR_TOO_MANY_BANNERS_IN_GAME); } auto* bannerEntry = get_banner_entry(_bannerType); @@ -124,12 +135,6 @@ GameActions::Result::Ptr BannerPlaceAction::Execute() const return MakeResult(GameActions::Status::NoFreeElements, STR_CANT_POSITION_THIS_HERE); } - if (_bannerIndex == BANNER_INDEX_NULL || _bannerIndex >= MAX_BANNERS) - { - log_error("Invalid banner index, bannerIndex = %u", _bannerIndex); - return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_POSITION_THIS_HERE); - } - auto* bannerEntry = get_banner_entry(_bannerType); if (bannerEntry == nullptr) { @@ -137,11 +142,11 @@ GameActions::Result::Ptr BannerPlaceAction::Execute() const return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_POSITION_THIS_HERE); } - auto banner = GetBanner(_bannerIndex); - if (!banner->IsNull()) + auto banner = CreateBanner(); + if (banner == nullptr) { - log_error("Banner index in use, bannerIndex = %u", _bannerIndex); - return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_POSITION_THIS_HERE); + log_error("No free banners available"); + return MakeResult(GameActions::Status::InvalidParameters, STR_TOO_MANY_BANNERS_IN_GAME); } banner->flags = 0; banner->text = {}; @@ -150,13 +155,15 @@ GameActions::Result::Ptr BannerPlaceAction::Execute() const banner->colour = _primaryColour; banner->position = TileCoordsXY(_loc); + res->bannerId = banner->id; + auto* bannerElement = TileElementInsert({ _loc, _loc.z + (2 * COORDS_Z_STEP) }, 0b0000); Guard::Assert(bannerElement != nullptr); bannerElement->SetClearanceZ(_loc.z + PATH_CLEARANCE); bannerElement->SetPosition(_loc.direction); bannerElement->ResetAllowedEdges(); - bannerElement->SetIndex(_bannerIndex); + bannerElement->SetIndex(banner->id); bannerElement->SetGhost(GetFlags() & GAME_COMMAND_FLAG_GHOST); map_invalidate_tile_full(_loc); diff --git a/src/openrct2/actions/BannerPlaceAction.h b/src/openrct2/actions/BannerPlaceAction.h index d937f44b5f..94d9e7feb3 100644 --- a/src/openrct2/actions/BannerPlaceAction.h +++ b/src/openrct2/actions/BannerPlaceAction.h @@ -11,17 +11,27 @@ #include "GameAction.h" -DEFINE_GAME_ACTION(BannerPlaceAction, GameCommand::PlaceBanner, GameActions::Result) +class BannerPlaceActionResult final : public GameActions::Result +{ +public: + BannerPlaceActionResult(); + BannerPlaceActionResult(GameActions::Status err); + BannerPlaceActionResult(GameActions::Status err, rct_string_id msg); + BannerPlaceActionResult(GameActions::Status err, rct_string_id title, rct_string_id message); + + BannerIndex bannerId = BANNER_INDEX_NULL; +}; + +DEFINE_GAME_ACTION(BannerPlaceAction, GameCommand::PlaceBanner, BannerPlaceActionResult) { private: CoordsXYZD _loc; ObjectEntryIndex _bannerType{ BANNER_NULL }; - BannerIndex _bannerIndex{ BANNER_INDEX_NULL }; uint8_t _primaryColour{}; public: BannerPlaceAction() = default; - BannerPlaceAction(const CoordsXYZD& loc, uint8_t bannerType, BannerIndex bannerIndex, uint8_t primaryColour); + BannerPlaceAction(const CoordsXYZD& loc, uint8_t bannerType, uint8_t primaryColour); void AcceptParameters(GameActionParameterVisitor & visitor) override; diff --git a/src/openrct2/actions/BannerRemoveAction.cpp b/src/openrct2/actions/BannerRemoveAction.cpp index 184e6dd362..10be36f15a 100644 --- a/src/openrct2/actions/BannerRemoveAction.cpp +++ b/src/openrct2/actions/BannerRemoveAction.cpp @@ -61,16 +61,17 @@ GameActions::Result::Ptr BannerRemoveAction::Query() const return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_REMOVE_THIS); } - if (bannerElement->GetIndex() >= MAX_BANNERS || bannerElement->GetIndex() == BANNER_INDEX_NULL) + auto bannerIndex = bannerElement->GetIndex(); + if (bannerIndex == BANNER_INDEX_NULL) { - log_error("Invalid banner index. index = ", bannerElement->GetIndex()); + log_error("Invalid banner index. index = ", bannerIndex); return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_REMOVE_THIS); } auto banner = bannerElement->GetBanner(); if (banner == nullptr) { - log_error("Invalid banner index. index = ", bannerElement->GetIndex()); + log_error("Invalid banner index. index = ", bannerIndex); return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_REMOVE_THIS); } @@ -99,16 +100,17 @@ GameActions::Result::Ptr BannerRemoveAction::Execute() const return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_REMOVE_THIS); } - if (bannerElement->GetIndex() >= MAX_BANNERS || bannerElement->GetIndex() == BANNER_INDEX_NULL) + auto bannerIndex = bannerElement->GetIndex(); + if (bannerIndex == BANNER_INDEX_NULL) { - log_error("Invalid banner index. index = ", bannerElement->GetIndex()); + log_error("Invalid banner index. index = ", bannerIndex); return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_REMOVE_THIS); } auto banner = bannerElement->GetBanner(); if (banner == nullptr) { - log_error("Invalid banner index. index = ", bannerElement->GetIndex()); + log_error("Invalid banner index. index = ", bannerIndex); return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_REMOVE_THIS); } diff --git a/src/openrct2/actions/BannerSetColourAction.cpp b/src/openrct2/actions/BannerSetColourAction.cpp index 108f584224..0243cf5fd6 100644 --- a/src/openrct2/actions/BannerSetColourAction.cpp +++ b/src/openrct2/actions/BannerSetColourAction.cpp @@ -84,10 +84,11 @@ GameActions::Result::Ptr BannerSetColourAction::QueryExecute(bool isExecuting) c } auto index = bannerElement->GetIndex(); - if (index >= MAX_BANNERS || index == BANNER_INDEX_NULL) + auto banner = GetBanner(index); + if (banner == nullptr) { log_error("Invalid banner index: index = %u", index); - return MakeResult(GameActions::Status::Unknown, STR_CANT_REPAINT_THIS); + return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_REPAINT_THIS); } if (isExecuting) @@ -96,7 +97,6 @@ GameActions::Result::Ptr BannerSetColourAction::QueryExecute(bool isExecuting) c intent.putExtra(INTENT_EXTRA_BANNER_INDEX, index); context_broadcast_intent(&intent); - auto banner = GetBanner(index); banner->colour = _primaryColour; map_invalidate_tile_zoom1({ _loc, _loc.z, _loc.z + 32 }); } diff --git a/src/openrct2/actions/BannerSetNameAction.cpp b/src/openrct2/actions/BannerSetNameAction.cpp index 8d4c088267..765acecb42 100644 --- a/src/openrct2/actions/BannerSetNameAction.cpp +++ b/src/openrct2/actions/BannerSetNameAction.cpp @@ -44,9 +44,10 @@ void BannerSetNameAction::Serialise(DataSerialiser& stream) GameActions::Result::Ptr BannerSetNameAction::Query() const { - if (_bannerIndex >= MAX_BANNERS) + auto banner = GetBanner(_bannerIndex); + if (banner == nullptr) { - log_warning("Invalid game command for setting banner name, banner id = %d", _bannerIndex); + log_warning("Invalid banner id, banner id = %d", _bannerIndex); return MakeResult(GameActions::Status::InvalidParameters, STR_NONE); } return MakeResult(); @@ -55,6 +56,12 @@ GameActions::Result::Ptr BannerSetNameAction::Query() const GameActions::Result::Ptr BannerSetNameAction::Execute() const { auto banner = GetBanner(_bannerIndex); + if (banner == nullptr) + { + log_warning("Invalid banner id, banner id = %d", _bannerIndex); + return MakeResult(GameActions::Status::InvalidParameters, STR_NONE); + } + banner->text = _name; auto intent = Intent(INTENT_ACTION_UPDATE_BANNER); diff --git a/src/openrct2/actions/BannerSetStyleAction.cpp b/src/openrct2/actions/BannerSetStyleAction.cpp index a231bd64bd..035b2bb3e7 100644 --- a/src/openrct2/actions/BannerSetStyleAction.cpp +++ b/src/openrct2/actions/BannerSetStyleAction.cpp @@ -46,12 +46,12 @@ GameActions::Result::Ptr BannerSetStyleAction::Query() const { auto res = MakeResult(); - if (_bannerIndex >= MAX_BANNERS || _bannerIndex == BANNER_INDEX_NULL) - { - return MakeResult(GameActions::Status::InvalidParameters, STR_INVALID_SELECTION_OF_OBJECTS); - } - auto banner = GetBanner(_bannerIndex); + if (banner == nullptr) + { + log_error("Invalid banner index: index = %u", _bannerIndex); + return MakeResult(GameActions::Status::InvalidParameters, STR_NONE); + } res->Expenditure = ExpenditureType::Landscaping; auto location = banner->position.ToCoordsXY().ToTileCentre(); @@ -101,6 +101,11 @@ GameActions::Result::Ptr BannerSetStyleAction::Execute() const auto res = MakeResult(); auto banner = GetBanner(_bannerIndex); + if (banner == nullptr) + { + log_error("Invalid banner index: index = %u", _bannerIndex); + return MakeResult(GameActions::Status::InvalidParameters, STR_NONE); + } res->Expenditure = ExpenditureType::Landscaping; auto location = banner->position.ToCoordsXY().ToTileCentre(); diff --git a/src/openrct2/actions/LargeSceneryPlaceAction.cpp b/src/openrct2/actions/LargeSceneryPlaceAction.cpp index 84d7fb8de2..ac7eabb46e 100644 --- a/src/openrct2/actions/LargeSceneryPlaceAction.cpp +++ b/src/openrct2/actions/LargeSceneryPlaceAction.cpp @@ -44,14 +44,6 @@ LargeSceneryPlaceAction::LargeSceneryPlaceAction( , _primaryColour(primaryColour) , _secondaryColour(secondaryColour) { - auto* sceneryEntry = get_large_scenery_entry(_sceneryType); - if (sceneryEntry != nullptr) - { - if (sceneryEntry->scrolling_mode != SCROLLING_MODE_NONE) - { - _bannerId = create_new_banner(0); - } - } } void LargeSceneryPlaceAction::AcceptParameters(GameActionParameterVisitor& visitor) @@ -60,14 +52,6 @@ void LargeSceneryPlaceAction::AcceptParameters(GameActionParameterVisitor& visit visitor.Visit("object", _sceneryType); visitor.Visit("primaryColour", _primaryColour); visitor.Visit("secondaryColour", _secondaryColour); - auto* sceneryEntry = get_large_scenery_entry(_sceneryType); - if (sceneryEntry != nullptr) - { - if (sceneryEntry->scrolling_mode != SCROLLING_MODE_NONE) - { - _bannerId = create_new_banner(0); - } - } } uint16_t LargeSceneryPlaceAction::GetActionFlags() const @@ -79,7 +63,7 @@ void LargeSceneryPlaceAction::Serialise(DataSerialiser& stream) { GameAction::Serialise(stream); - stream << DS_TAG(_loc) << DS_TAG(_sceneryType) << DS_TAG(_primaryColour) << DS_TAG(_secondaryColour) << DS_TAG(_bannerId); + stream << DS_TAG(_loc) << DS_TAG(_sceneryType) << DS_TAG(_primaryColour) << DS_TAG(_secondaryColour); } GameActions::Result::Ptr LargeSceneryPlaceAction::Query() const @@ -128,17 +112,10 @@ GameActions::Result::Ptr LargeSceneryPlaceAction::Query() const if (sceneryEntry->scrolling_mode != SCROLLING_MODE_NONE) { - if (_bannerId == BANNER_INDEX_NULL) - { - log_error("Banner Index not specified."); - return MakeResult(GameActions::Status::InvalidParameters, STR_TOO_MANY_BANNERS_IN_GAME); - } - - auto banner = GetBanner(_bannerId); - if (!banner->IsNull()) + if (HasReachedBannerLimit()) { log_error("No free banners available"); - return std::make_unique(GameActions::Status::NoFreeElements); + return MakeResult(GameActions::Status::InvalidParameters, STR_TOO_MANY_BANNERS_IN_GAME); } } @@ -243,6 +220,34 @@ GameActions::Result::Ptr LargeSceneryPlaceAction::Execute() const res->Position.z = maxHeight; + // Allocate banner + Banner* banner = nullptr; + if (sceneryEntry->scrolling_mode != SCROLLING_MODE_NONE) + { + banner = CreateBanner(); + if (banner == nullptr) + { + log_error("No free banners available"); + return MakeResult(GameActions::Status::InvalidParameters, STR_TOO_MANY_BANNERS_IN_GAME); + } + + banner->text = {}; + banner->colour = 2; + banner->text_colour = 2; + banner->flags = BANNER_FLAG_IS_LARGE_SCENERY; + banner->type = 0; + banner->position = TileCoordsXY(_loc); + + ride_id_t rideIndex = banner_get_closest_ride_index({ _loc, maxHeight }); + if (rideIndex != RIDE_ID_NULL) + { + banner->ride_index = rideIndex; + banner->flags |= BANNER_FLAG_LINKED_TO_RIDE; + } + + res->bannerId = banner->id; + } + uint8_t tileNum = 0; for (rct_large_scenery_tile* tile = sceneryEntry->tiles; tile->x_offset != -1; tile++, tileNum++) { @@ -261,6 +266,7 @@ GameActions::Result::Ptr LargeSceneryPlaceAction::Execute() const isTree); if (canBuild->Error != GameActions::Status::Ok) { + DeleteBanner(banner->id); canBuild->ErrorTitle = STR_CANT_POSITION_THIS_HERE; return canBuild; } @@ -283,6 +289,11 @@ GameActions::Result::Ptr LargeSceneryPlaceAction::Execute() const newSceneryElement->SetClearanceZ(zHigh); SetNewLargeSceneryElement(*newSceneryElement, tileNum); + if (banner != nullptr) + { + newSceneryElement->SetBannerIndex(banner->id); + } + map_animation_create(MAP_ANIMATION_TYPE_LARGE_SCENERY, { curTile, zLow }); map_invalidate_tile_full(curTile); @@ -292,37 +303,6 @@ GameActions::Result::Ptr LargeSceneryPlaceAction::Execute() const } } - // Allocate banner after all tiles to ensure banner id doesn't need to be freed. - if (sceneryEntry->scrolling_mode != SCROLLING_MODE_NONE) - { - if (_bannerId == BANNER_INDEX_NULL) - { - log_error("No free banners available"); - return MakeResult(GameActions::Status::NoFreeElements, STR_TOO_MANY_BANNERS_IN_GAME); - } - - auto banner = GetBanner(_bannerId); - if (!banner->IsNull()) - { - log_error("No free banners available"); - return std::make_unique(GameActions::Status::NoFreeElements); - } - - banner->text = {}; - banner->colour = 2; - banner->text_colour = 2; - banner->flags = BANNER_FLAG_IS_LARGE_SCENERY; - banner->type = 0; - banner->position = TileCoordsXY(_loc); - - ride_id_t rideIndex = banner_get_closest_ride_index({ _loc, maxHeight }); - if (rideIndex != RIDE_ID_NULL) - { - banner->ride_index = rideIndex; - banner->flags |= BANNER_FLAG_LINKED_TO_RIDE; - } - } - // Force ride construction to recheck area _currentTrackSelectionFlags |= TRACK_SELECTION_FLAG_RECHECK; @@ -403,11 +383,6 @@ void LargeSceneryPlaceAction::SetNewLargeSceneryElement(LargeSceneryElement& sce sceneryElement.SetPrimaryColour(_primaryColour); sceneryElement.SetSecondaryColour(_secondaryColour); - if (_bannerId != BANNER_INDEX_NULL) - { - sceneryElement.SetBannerIndex(_bannerId); - } - if (GetFlags() & GAME_COMMAND_FLAG_GHOST) { sceneryElement.SetGhost(true); diff --git a/src/openrct2/actions/LargeSceneryPlaceAction.h b/src/openrct2/actions/LargeSceneryPlaceAction.h index 85b95cf197..dfba4eb416 100644 --- a/src/openrct2/actions/LargeSceneryPlaceAction.h +++ b/src/openrct2/actions/LargeSceneryPlaceAction.h @@ -23,6 +23,7 @@ public: uint8_t GroundFlags{ 0 }; int32_t firstTileHeight{ 0 }; + BannerIndex bannerId = BANNER_INDEX_NULL; }; DEFINE_GAME_ACTION(LargeSceneryPlaceAction, GameCommand::PlaceLargeScenery, LargeSceneryPlaceActionResult) @@ -32,7 +33,6 @@ private: ObjectEntryIndex _sceneryType{ OBJECT_ENTRY_INDEX_NULL }; uint8_t _primaryColour{}; uint8_t _secondaryColour{}; - BannerIndex _bannerId{ BANNER_INDEX_NULL }; public: LargeSceneryPlaceAction() = default; diff --git a/src/openrct2/actions/RideDemolishAction.cpp b/src/openrct2/actions/RideDemolishAction.cpp index a63dbc7ab2..7e5e0b89ab 100644 --- a/src/openrct2/actions/RideDemolishAction.cpp +++ b/src/openrct2/actions/RideDemolishAction.cpp @@ -132,15 +132,7 @@ GameActions::Result::Ptr RideDemolishAction::DemolishRide(Ride* ride) const ride_clear_leftover_entrances(ride); News::DisableNewsItems(News::ItemType::Ride, _rideIndex); - for (BannerIndex i = 0; i < MAX_BANNERS; i++) - { - auto banner = GetBanner(i); - if (!banner->IsNull() && banner->flags & BANNER_FLAG_LINKED_TO_RIDE && banner->ride_index == _rideIndex) - { - banner->flags &= ~BANNER_FLAG_LINKED_TO_RIDE; - banner->text = {}; - } - } + UnlinkAllBannersForRide(_rideIndex); for (auto peep : EntityList()) { diff --git a/src/openrct2/actions/SignSetNameAction.cpp b/src/openrct2/actions/SignSetNameAction.cpp index e58b2ffc52..d13941edd7 100644 --- a/src/openrct2/actions/SignSetNameAction.cpp +++ b/src/openrct2/actions/SignSetNameAction.cpp @@ -45,7 +45,8 @@ void SignSetNameAction::Serialise(DataSerialiser& stream) GameActions::Result::Ptr SignSetNameAction::Query() const { - if (_bannerIndex >= MAX_BANNERS) + auto banner = GetBanner(_bannerIndex); + if (banner == nullptr) { log_warning("Invalid game command for setting sign name, banner id = %d", _bannerIndex); return MakeResult(GameActions::Status::InvalidParameters, STR_NONE); @@ -56,6 +57,11 @@ GameActions::Result::Ptr SignSetNameAction::Query() const GameActions::Result::Ptr SignSetNameAction::Execute() const { auto banner = GetBanner(_bannerIndex); + if (banner == nullptr) + { + log_warning("Invalid game command for setting sign name, banner id = %d", _bannerIndex); + return MakeResult(GameActions::Status::InvalidParameters, STR_NONE); + } if (!_name.empty()) { diff --git a/src/openrct2/actions/SignSetStyleAction.cpp b/src/openrct2/actions/SignSetStyleAction.cpp index eb2e0525c7..de79708f6b 100644 --- a/src/openrct2/actions/SignSetStyleAction.cpp +++ b/src/openrct2/actions/SignSetStyleAction.cpp @@ -39,9 +39,10 @@ void SignSetStyleAction::Serialise(DataSerialiser& stream) GameActions::Result::Ptr SignSetStyleAction::Query() const { - if (_bannerIndex >= MAX_BANNERS) + auto banner = GetBanner(_bannerIndex); + if (banner == nullptr) { - log_warning("Invalid game command for setting sign style, banner id '%d' out of range", _bannerIndex); + log_error("Invalid banner id. id = ", _bannerIndex); return MakeResult(GameActions::Status::InvalidParameters, STR_NONE); } @@ -76,6 +77,11 @@ GameActions::Result::Ptr SignSetStyleAction::Query() const GameActions::Result::Ptr SignSetStyleAction::Execute() const { auto banner = GetBanner(_bannerIndex); + if (banner == nullptr) + { + log_error("Invalid banner id. id = ", _bannerIndex); + return MakeResult(GameActions::Status::InvalidParameters, STR_NONE); + } CoordsXY coords = banner->position.ToCoordsXY(); diff --git a/src/openrct2/actions/WallPlaceAction.cpp b/src/openrct2/actions/WallPlaceAction.cpp index 583db43616..a02479b4ed 100644 --- a/src/openrct2/actions/WallPlaceAction.cpp +++ b/src/openrct2/actions/WallPlaceAction.cpp @@ -50,14 +50,6 @@ WallPlaceAction::WallPlaceAction( , _secondaryColour(secondaryColour) , _tertiaryColour(tertiaryColour) { - auto* sceneryEntry = get_wall_entry(_wallType); - if (sceneryEntry != nullptr) - { - if (sceneryEntry->scrolling_mode != SCROLLING_MODE_NONE) - { - _bannerId = create_new_banner(0); - } - } } void WallPlaceAction::AcceptParameters(GameActionParameterVisitor& visitor) @@ -68,14 +60,6 @@ void WallPlaceAction::AcceptParameters(GameActionParameterVisitor& visitor) visitor.Visit("primaryColour", _primaryColour); visitor.Visit("secondaryColour", _secondaryColour); visitor.Visit("tertiaryColour", _tertiaryColour); - auto* sceneryEntry = get_wall_entry(_wallType); - if (sceneryEntry != nullptr) - { - if (sceneryEntry->scrolling_mode != SCROLLING_MODE_NONE) - { - _bannerId = create_new_banner(0); - } - } } uint16_t WallPlaceAction::GetActionFlags() const @@ -88,7 +72,7 @@ void WallPlaceAction::Serialise(DataSerialiser& stream) GameAction::Serialise(stream); stream << DS_TAG(_wallType) << DS_TAG(_loc) << DS_TAG(_edge) << DS_TAG(_primaryColour) << DS_TAG(_secondaryColour) - << DS_TAG(_tertiaryColour) << DS_TAG(_bannerId); + << DS_TAG(_tertiaryColour); } GameActions::Result::Ptr WallPlaceAction::Query() const @@ -253,18 +237,10 @@ GameActions::Result::Ptr WallPlaceAction::Query() const if (wallEntry->scrolling_mode != SCROLLING_MODE_NONE) { - if (_bannerId == BANNER_INDEX_NULL) - { - log_error("Banner Index not specified."); - return std::make_unique( - GameActions::Status::InvalidParameters, STR_TOO_MANY_BANNERS_IN_GAME); - } - - auto banner = GetBanner(_bannerId); - if (!banner->IsNull()) + if (HasReachedBannerLimit()) { log_error("No free banners available"); - return std::make_unique(GameActions::Status::NoFreeElements); + return MakeResult(GameActions::Status::InvalidParameters, STR_TOO_MANY_BANNERS_IN_GAME); } } @@ -361,20 +337,14 @@ GameActions::Result::Ptr WallPlaceAction::Execute() const } } + Banner* banner = nullptr; if (wallEntry->scrolling_mode != SCROLLING_MODE_NONE) { - if (_bannerId == BANNER_INDEX_NULL) - { - log_error("Banner Index not specified."); - return std::make_unique( - GameActions::Status::InvalidParameters, STR_TOO_MANY_BANNERS_IN_GAME); - } - - auto banner = GetBanner(_bannerId); - if (!banner->IsNull()) + banner = CreateBanner(); + if (banner == nullptr) { log_error("No free banners available"); - return std::make_unique(GameActions::Status::NoFreeElements); + return MakeResult(GameActions::Status::InvalidParameters, STR_TOO_MANY_BANNERS_IN_GAME); } banner->text = {}; @@ -390,6 +360,8 @@ GameActions::Result::Ptr WallPlaceAction::Execute() const banner->ride_index = rideIndex; banner->flags |= BANNER_FLAG_LINKED_TO_RIDE; } + + res->bannerId = banner->id; } auto* wallElement = TileElementInsert(targetLoc, 0b0000); @@ -404,9 +376,9 @@ GameActions::Result::Ptr WallPlaceAction::Execute() const wallElement->SetAcrossTrack(wallAcrossTrack); wallElement->SetEntryIndex(_wallType); - if (_bannerId != BANNER_INDEX_NULL) + if (banner != nullptr) { - wallElement->SetBannerIndex(_bannerId); + wallElement->SetBannerIndex(banner->id); } if (wallEntry->flags & WALL_SCENERY_HAS_TERNARY_COLOUR) diff --git a/src/openrct2/actions/WallPlaceAction.h b/src/openrct2/actions/WallPlaceAction.h index 8a1776395a..7608a0ce7d 100644 --- a/src/openrct2/actions/WallPlaceAction.h +++ b/src/openrct2/actions/WallPlaceAction.h @@ -11,6 +11,7 @@ #include "../ride/RideData.h" #include "../ride/TrackData.h" +#include "../world/Banner.h" #include "../world/Scenery.h" #include "GameAction.h" @@ -23,6 +24,7 @@ public: WallPlaceActionResult(GameActions::Status error, rct_string_id msg, uint8_t* args); TileElement* tileElement = nullptr; + BannerIndex bannerId = BANNER_INDEX_NULL; }; DEFINE_GAME_ACTION(WallPlaceAction, GameCommand::PlaceWall, WallPlaceActionResult) @@ -34,7 +36,6 @@ private: int32_t _primaryColour{ COLOUR_BLACK }; int32_t _secondaryColour{ COLOUR_BLACK }; int32_t _tertiaryColour{ COLOUR_BLACK }; - BannerIndex _bannerId{ BANNER_INDEX_NULL }; public: WallPlaceAction() = default; diff --git a/src/openrct2/interface/InteractiveConsole.cpp b/src/openrct2/interface/InteractiveConsole.cpp index cc5741263f..7545d42a93 100644 --- a/src/openrct2/interface/InteractiveConsole.cpp +++ b/src/openrct2/interface/InteractiveConsole.cpp @@ -1259,15 +1259,7 @@ static int32_t cc_show_limits(InteractiveConsole& console, [[maybe_unused]] cons int32_t staffCount = GetEntityListCount(EntityType::Staff); - int32_t bannerCount = 0; - for (BannerIndex i = 0; i < MAX_BANNERS; ++i) - { - auto banner = GetBanner(i); - if (!banner->IsNull()) - { - bannerCount++; - } - } + auto bannerCount = GetNumBanners(); console.WriteFormatLine("Sprites: %d/%d", spriteCount, MAX_ENTITIES); console.WriteFormatLine("Map Elements: %zu/%d", tileElementCount, MAX_TILE_ELEMENTS); diff --git a/src/openrct2/network/NetworkBase.cpp b/src/openrct2/network/NetworkBase.cpp index 05782288d9..053339fafc 100644 --- a/src/openrct2/network/NetworkBase.cpp +++ b/src/openrct2/network/NetworkBase.cpp @@ -37,7 +37,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 "0" +#define NETWORK_STREAM_VERSION "1" #define NETWORK_STREAM_ID OPENRCT2_VERSION "-" NETWORK_STREAM_VERSION static Peep* _pickup_peep = nullptr; diff --git a/src/openrct2/paint/tile_element/Paint.Wall.cpp b/src/openrct2/paint/tile_element/Paint.Wall.cpp index fc66fae338..ffe208e5df 100644 --- a/src/openrct2/paint/tile_element/Paint.Wall.cpp +++ b/src/openrct2/paint/tile_element/Paint.Wall.cpp @@ -432,7 +432,7 @@ void fence_paint(paint_session* session, uint8_t direction, int32_t height, cons } auto banner = tile_element->AsWall()->GetBanner(); - if (banner != nullptr && !banner->IsNull()) + if (banner != nullptr) { auto ft = Formatter(); banner->FormatTextTo(ft); diff --git a/src/openrct2/rct1/S4Importer.cpp b/src/openrct2/rct1/S4Importer.cpp index c60234fa38..6522fea81d 100644 --- a/src/openrct2/rct1/S4Importer.cpp +++ b/src/openrct2/rct1/S4Importer.cpp @@ -1806,19 +1806,27 @@ private: auto dst2 = dst->AsBanner(); auto src2 = src->AsBanner(); - auto index = src2->GetIndex(); - if (index != RCT12_BANNER_INDEX_NULL) - dst2->SetIndex(index); - else - dst2->SetIndex(BANNER_INDEX_NULL); dst2->SetPosition(src2->GetPosition()); dst2->SetAllowedEdges(src2->GetAllowedEdges()); + auto index = src2->GetIndex(); if (index < std::size(_s4.banners)) { auto srcBanner = &_s4.banners[index]; - auto dstBanner = GetBanner(index); - ImportBanner(dstBanner, srcBanner); + auto dstBanner = GetOrCreateBanner(index); + if (dstBanner == nullptr) + { + dst2->SetIndex(BANNER_INDEX_NULL); + } + else + { + ImportBanner(dstBanner, srcBanner); + dst2->SetIndex(index); + } + } + else + { + dst2->SetIndex(BANNER_INDEX_NULL); } return 1; } diff --git a/src/openrct2/rct2/S6Exporter.cpp b/src/openrct2/rct2/S6Exporter.cpp index a612e6af35..9388d6e41b 100644 --- a/src/openrct2/rct2/S6Exporter.cpp +++ b/src/openrct2/rct2/S6Exporter.cpp @@ -1518,7 +1518,14 @@ void S6Exporter::ExportBanners() { auto src = GetBanner(i); auto dst = &_s6.banners[i]; - ExportBanner(*dst, *src); + if (src != nullptr) + { + ExportBanner(*dst, *src); + } + else + { + ExportBanner(*dst, {}); + } } } diff --git a/src/openrct2/rct2/S6Importer.cpp b/src/openrct2/rct2/S6Importer.cpp index d4120c213b..f4763855b4 100644 --- a/src/openrct2/rct2/S6Importer.cpp +++ b/src/openrct2/rct2/S6Importer.cpp @@ -1246,9 +1246,16 @@ public: if (bannerIndex < std::size(_s6.banners)) { auto srcBanner = &_s6.banners[bannerIndex]; - auto dstBanner = GetBanner(bannerIndex); - ImportBanner(dstBanner, srcBanner); - dst2->SetBannerIndex(src2->GetBannerIndex()); + auto dstBanner = GetOrCreateBanner(bannerIndex); + if (dstBanner == nullptr) + { + dst2->SetBannerIndex(BANNER_INDEX_NULL); + } + else + { + ImportBanner(dstBanner, srcBanner); + dst2->SetBannerIndex(src2->GetBannerIndex()); + } } } break; @@ -1272,9 +1279,16 @@ public: if (bannerIndex < std::size(_s6.banners)) { auto srcBanner = &_s6.banners[bannerIndex]; - auto dstBanner = GetBanner(bannerIndex); - ImportBanner(dstBanner, srcBanner); - dst2->SetBannerIndex(src2->GetBannerIndex()); + auto dstBanner = GetOrCreateBanner(bannerIndex); + if (dstBanner == nullptr) + { + dst2->SetBannerIndex(BANNER_INDEX_NULL); + } + else + { + ImportBanner(dstBanner, srcBanner); + dst2->SetBannerIndex(src2->GetBannerIndex()); + } } } break; @@ -1284,7 +1298,6 @@ public: auto dst2 = dst->AsBanner(); auto src2 = src->AsBanner(); - dst2->SetIndex(src2->GetIndex()); dst2->SetPosition(src2->GetPosition()); dst2->SetAllowedEdges(src2->GetAllowedEdges()); @@ -1292,8 +1305,16 @@ public: if (bannerIndex < std::size(_s6.banners)) { auto srcBanner = &_s6.banners[bannerIndex]; - auto dstBanner = GetBanner(bannerIndex); - ImportBanner(dstBanner, srcBanner); + auto dstBanner = GetOrCreateBanner(bannerIndex); + if (dstBanner == nullptr) + { + dst2->SetIndex(BANNER_INDEX_NULL); + } + else + { + ImportBanner(dstBanner, srcBanner); + dst2->SetIndex(bannerIndex); + } } else { diff --git a/src/openrct2/world/Banner.cpp b/src/openrct2/world/Banner.cpp index de001a8541..28a6bac8b4 100644 --- a/src/openrct2/world/Banner.cpp +++ b/src/openrct2/world/Banner.cpp @@ -31,7 +31,7 @@ #include #include -static Banner _banners[MAX_BANNERS]; +static std::vector _banners; std::string Banner::GetText() const { @@ -114,9 +114,17 @@ static BannerIndex BannerGetNewIndex() { for (BannerIndex bannerIndex = 0; bannerIndex < MAX_BANNERS; bannerIndex++) { - if (_banners[bannerIndex].IsNull()) + if (bannerIndex < _banners.size()) { - return bannerIndex; + if (_banners[bannerIndex].IsNull()) + { + return bannerIndex; + } + } + else + { + _banners.emplace_back(); + return static_cast(_banners.size() - 1); } } return BANNER_INDEX_NULL; @@ -128,41 +136,7 @@ static BannerIndex BannerGetNewIndex() */ void banner_init() { - for (auto& banner : _banners) - { - banner = {}; - } -} - -/** - * Creates a new banner and returns the index of the banner - * If the flag GAME_COMMAND_FLAG_APPLY is NOT set then returns - * the first unused index but does NOT mark the banner as created. - * returns 0xFF on failure. - * - * rct2: 0x006BA278 - */ -BannerIndex create_new_banner(uint8_t flags) -{ - BannerIndex bannerIndex = BannerGetNewIndex(); - - if (bannerIndex == BANNER_INDEX_NULL) - { - gGameCommandErrorText = STR_TOO_MANY_BANNERS_IN_GAME; - return bannerIndex; - } - - if (flags & GAME_COMMAND_FLAG_APPLY) - { - auto banner = &_banners[bannerIndex]; - - banner->flags = 0; - banner->type = 0; - banner->text = {}; - banner->colour = 2; - banner->text_colour = 2; - } - return bannerIndex; + _banners.clear(); } TileElement* banner_get_tile_element(BannerIndex bannerIndex) @@ -261,12 +235,16 @@ ride_id_t banner_get_closest_ride_index(const CoordsXYZ& mapPos) void banner_reset_broken_index() { - for (BannerIndex bannerIndex = 0; bannerIndex < MAX_BANNERS; bannerIndex++) + for (BannerIndex bannerIndex = 0; bannerIndex < _banners.size(); bannerIndex++) { auto tileElement = banner_get_tile_element(bannerIndex); if (tileElement == nullptr) { - _banners[bannerIndex].type = BANNER_NULL; + auto banner = GetBanner(bannerIndex); + if (banner != nullptr) + { + banner->type = BANNER_NULL; + } } } } @@ -274,7 +252,7 @@ void banner_reset_broken_index() void fix_duplicated_banners() { // For each banner in the map, check if the banner index is in use already, and if so, create a new entry for it - bool activeBanners[std::size(_banners)]{}; + std::vector activeBanners; for (int y = 0; y < MAXIMUM_MAP_SIZE_TECHNICAL; y++) { for (int x = 0; x < MAXIMUM_MAP_SIZE_TECHNICAL; x++) @@ -292,6 +270,10 @@ void fix_duplicated_banners() if (bannerIndex == BANNER_INDEX_NULL) continue; + if (activeBanners.size() <= bannerIndex) + { + activeBanners.resize(bannerIndex + 1); + } if (activeBanners[bannerIndex]) { log_info( @@ -299,24 +281,23 @@ void fix_duplicated_banners() tileElement->base_height); // Banner index is already in use by another banner, so duplicate it - auto newBannerIndex = create_new_banner(GAME_COMMAND_FLAG_APPLY); - if (newBannerIndex == BANNER_INDEX_NULL) + auto newBanner = CreateBanner(); + if (newBanner == nullptr) { log_error("Failed to create new banner."); continue; } - Guard::Assert(!activeBanners[newBannerIndex]); + Guard::Assert(!activeBanners[newBanner->id]); // Copy over the original banner, but update the location - auto newBanner = GetBanner(newBannerIndex); auto oldBanner = GetBanner(bannerIndex); - if (oldBanner != nullptr && newBanner != nullptr) + if (oldBanner != nullptr) { *newBanner = *oldBanner; newBanner->position = { x, y }; } - tileElement->AsBanner()->SetIndex(newBannerIndex); + tileElement->AsBanner()->SetIndex(newBanner->id); } // Mark banner index as in-use @@ -379,11 +360,111 @@ void BannerElement::ResetAllowedEdges() AllowedEdges |= 0b00001111; } +void UnlinkAllRideBanners() +{ + for (auto& banner : _banners) + { + if (!banner.IsNull()) + { + banner.flags &= ~BANNER_FLAG_LINKED_TO_RIDE; + banner.ride_index = RIDE_ID_NULL; + } + } +} + +void UnlinkAllBannersForRide(ride_id_t rideId) +{ + for (auto& banner : _banners) + { + if (!banner.IsNull() && (banner.flags & BANNER_FLAG_LINKED_TO_RIDE) && banner.ride_index == rideId) + { + banner.flags &= ~BANNER_FLAG_LINKED_TO_RIDE; + banner.ride_index = RIDE_ID_NULL; + banner.text = {}; + } + } +} + Banner* GetBanner(BannerIndex id) { - if (id < std::size(_banners)) + if (id < _banners.size()) { + auto banner = &_banners[id]; + if (banner != nullptr && !banner->IsNull()) + { + return banner; + } + } + return nullptr; +} + +Banner* GetOrCreateBanner(BannerIndex id) +{ + if (id < MAX_BANNERS) + { + if (id >= _banners.size()) + { + _banners.resize(id + 1); + } return &_banners[id]; } return nullptr; } + +Banner* CreateBanner() +{ + auto bannerIndex = BannerGetNewIndex(); + auto banner = GetOrCreateBanner(bannerIndex); + if (banner != nullptr) + { + banner->id = bannerIndex; + banner->flags = 0; + banner->type = 0; + banner->text = {}; + banner->colour = 2; + banner->text_colour = 2; + } + return banner; +} + +void DeleteBanner(BannerIndex id) +{ + auto* const banner = GetBanner(id); + if (banner != nullptr) + { + *banner = {}; + } +} + +void TrimBanners() +{ + if (_banners.size() > 0) + { + auto lastBannerId = _banners.size() - 1; + while (lastBannerId != std::numeric_limits::max() && _banners[lastBannerId].IsNull()) + { + lastBannerId--; + } + _banners.resize(lastBannerId + 1); + _banners.shrink_to_fit(); + } +} + +size_t GetNumBanners() +{ + size_t count = 0; + for (const auto& banner : _banners) + { + if (!banner.IsNull()) + { + count++; + } + } + return count; +} + +bool HasReachedBannerLimit() +{ + auto numBanners = GetNumBanners(); + return numBanners >= MAX_BANNERS; +} diff --git a/src/openrct2/world/Banner.h b/src/openrct2/world/Banner.h index e586b0a3d5..1ec15ec3c2 100644 --- a/src/openrct2/world/Banner.h +++ b/src/openrct2/world/Banner.h @@ -29,6 +29,7 @@ constexpr uint8_t SCROLLING_MODE_NONE = 255; struct Banner { + BannerIndex id = BANNER_INDEX_NULL; ObjectEntryIndex type = BANNER_NULL; uint8_t flags{}; std::string text; @@ -56,10 +57,17 @@ enum BANNER_FLAGS }; void banner_init(); -BannerIndex create_new_banner(uint8_t flags); TileElement* banner_get_tile_element(BannerIndex bannerIndex); WallElement* banner_get_scrolling_wall_tile_element(BannerIndex bannerIndex); ride_id_t banner_get_closest_ride_index(const CoordsXYZ& mapPos); void banner_reset_broken_index(); void fix_duplicated_banners(); +void UnlinkAllRideBanners(); +void UnlinkAllBannersForRide(ride_id_t rideId); Banner* GetBanner(BannerIndex id); +Banner* GetOrCreateBanner(BannerIndex id); +Banner* CreateBanner(); +void DeleteBanner(BannerIndex id); +void TrimBanners(); +size_t GetNumBanners(); +bool HasReachedBannerLimit(); diff --git a/src/openrct2/world/TileElement.cpp b/src/openrct2/world/TileElement.cpp index 0dbc860b71..68eac33494 100644 --- a/src/openrct2/world/TileElement.cpp +++ b/src/openrct2/world/TileElement.cpp @@ -82,7 +82,7 @@ void TileElement::RemoveBannerEntry() if (banner != nullptr) { window_close_by_number(WC_BANNER, bannerIndex); - *banner = {}; + DeleteBanner(banner->id); } } diff --git a/src/openrct2/world/TileInspector.cpp b/src/openrct2/world/TileInspector.cpp index 7dbb1dec4f..ac95bbfe60 100644 --- a/src/openrct2/world/TileInspector.cpp +++ b/src/openrct2/world/TileInspector.cpp @@ -373,17 +373,17 @@ namespace OpenRCT2::TileInspector if (bannerIndex != BANNER_INDEX_NULL) { // The element to be pasted refers to a banner index - make a copy of it - auto newBannerIndex = create_new_banner(GAME_COMMAND_FLAG_APPLY); - if (newBannerIndex == BANNER_INDEX_NULL) + auto newBanner = CreateBanner(); + if (newBanner == nullptr) { - return std::make_unique(GameActions::Status::Unknown, STR_NONE); + log_error("No free banners available"); + return std::make_unique(GameActions::Status::Unknown, STR_TOO_MANY_BANNERS_IN_GAME); } - auto& newBanner = *GetBanner(newBannerIndex); - newBanner = *GetBanner(bannerIndex); - newBanner.position = tileLoc; + + newBanner->position = tileLoc; // Use the new banner index - element.SetBannerIndex(newBannerIndex); + element.SetBannerIndex(newBanner->id); } // The occupiedQuadrants will be automatically set when the element is copied over, so it's not necessary to set