From 8f59fe91cb6540bce2b2e0f9eaee69449e2b02d0 Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 3 Jun 2021 21:43:30 +0100 Subject: [PATCH] BannerSceneryEntry Refactor (#14810) * Refactor BannerSceneryEntry to enforce type * Remove old comment * Fix incorrect renameing --- .../interface/ViewportInteraction.cpp | 4 ++-- src/openrct2-ui/windows/Banner.cpp | 6 ++--- src/openrct2-ui/windows/Scenery.cpp | 22 ++++++++++--------- src/openrct2-ui/windows/TopToolbar.cpp | 4 ++-- src/openrct2/actions/BannerPlaceAction.cpp | 8 +++---- src/openrct2/actions/BannerRemoveAction.cpp | 8 +++---- src/openrct2/object/BannerObject.cpp | 16 +++++++------- src/openrct2/object/BannerObject.h | 2 +- src/openrct2/object/ObjectManager.cpp | 4 ++-- .../paint/tile_element/Paint.Banner.cpp | 8 +++---- src/openrct2/world/Banner.cpp | 2 +- src/openrct2/world/Scenery.cpp | 6 ++--- src/openrct2/world/Scenery.h | 19 ++++++++-------- src/openrct2/world/TileElement.h | 3 ++- 14 files changed, 57 insertions(+), 55 deletions(-) diff --git a/src/openrct2-ui/interface/ViewportInteraction.cpp b/src/openrct2-ui/interface/ViewportInteraction.cpp index 7e8619ba5c..c5b4c89a63 100644 --- a/src/openrct2-ui/interface/ViewportInteraction.cpp +++ b/src/openrct2-ui/interface/ViewportInteraction.cpp @@ -422,13 +422,13 @@ InteractionInfo ViewportInteractionGetItemRight(const ScreenCoordsXY& screenCoor case ViewportInteractionItem::Banner: { auto banner = tileElement->AsBanner()->GetBanner(); - sceneryEntry = get_banner_entry(banner->type); + 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(sceneryEntry->name); + ft.Add(bannerEntry->name); SetMapTooltip(ft); return info; } diff --git a/src/openrct2-ui/windows/Banner.cpp b/src/openrct2-ui/windows/Banner.cpp index 542f7cf879..323db0bb33 100644 --- a/src/openrct2-ui/windows/Banner.cpp +++ b/src/openrct2-ui/windows/Banner.cpp @@ -248,9 +248,9 @@ public: { rct_widget* colourBtn = &window_banner_widgets[WIDX_MAIN_COLOUR]; colourBtn->type = WindowWidgetType::Empty; - // Scenery item not sure why we use this instead of banner? - rct_scenery_entry* sceneryEntry = get_banner_entry(_banner->type); - if (sceneryEntry != nullptr && (sceneryEntry->banner.flags & BANNER_ENTRY_FLAG_HAS_PRIMARY_COLOUR)) + + auto* bannerEntry = get_banner_entry(_banner->type); + if (bannerEntry != nullptr && (bannerEntry->flags & BANNER_ENTRY_FLAG_HAS_PRIMARY_COLOUR)) { colourBtn->type = WindowWidgetType::ColourBtn; } diff --git a/src/openrct2-ui/windows/Scenery.cpp b/src/openrct2-ui/windows/Scenery.cpp index 2d0779fa64..8a4b6e6d65 100644 --- a/src/openrct2-ui/windows/Scenery.cpp +++ b/src/openrct2-ui/windows/Scenery.cpp @@ -296,11 +296,11 @@ void window_scenery_init() // banners for (ObjectEntryIndex sceneryId = 0; sceneryId < MAX_BANNER_OBJECTS; sceneryId++) { - rct_scenery_entry* sceneryEntry = get_banner_entry(sceneryId); - if (sceneryEntry == nullptr) + auto* bannerEntry = get_banner_entry(sceneryId); + if (bannerEntry == nullptr) continue; - init_scenery_entry({ SCENERY_TYPE_BANNER, sceneryId }, sceneryEntry->banner.scenery_tab_id); + init_scenery_entry({ SCENERY_TYPE_BANNER, sceneryId }, bannerEntry->scenery_tab_id); } // path bits @@ -1025,8 +1025,8 @@ void window_scenery_invalidate(rct_window* w) if (tabSelectedScenery.SceneryType == SCENERY_TYPE_BANNER) { - sceneryEntry = get_banner_entry(tabSelectedScenery.EntryIndex); - if (sceneryEntry->banner.flags & BANNER_ENTRY_FLAG_HAS_PRIMARY_COLOUR) + auto* bannerEntry = get_banner_entry(tabSelectedScenery.EntryIndex); + if (bannerEntry->flags & BANNER_ENTRY_FLAG_HAS_PRIMARY_COLOUR) { window_scenery_widgets[WIDX_SCENERY_PRIMARY_COLOUR_BUTTON].type = WindowWidgetType::ColourBtn; } @@ -1162,10 +1162,12 @@ void window_scenery_paint(rct_window* w, rct_drawpixelinfo* dpi) name = sceneryEntry->name; break; case SCENERY_TYPE_BANNER: - sceneryEntry = get_banner_entry(selectedSceneryEntry.EntryIndex); - price = sceneryEntry->banner.price; - name = sceneryEntry->name; + { + auto* bannerEntry = get_banner_entry(selectedSceneryEntry.EntryIndex); + price = bannerEntry->price; + name = bannerEntry->name; break; + } } if (w->scenery.SelectedScenery.IsUndefined() && gSceneryPlaceCost != MONEY32_UNDEFINED) @@ -1241,8 +1243,8 @@ void window_scenery_scrollpaint(rct_window* w, rct_drawpixelinfo* dpi, int32_t s { if (currentSceneryGlobal.SceneryType == SCENERY_TYPE_BANNER) { - sceneryEntry = get_banner_entry(currentSceneryGlobal.EntryIndex); - uint32_t imageId = sceneryEntry->image + gWindowSceneryRotation * 2; + auto* bannerEntry = get_banner_entry(currentSceneryGlobal.EntryIndex); + uint32_t imageId = bannerEntry->image + gWindowSceneryRotation * 2; imageId |= (gWindowSceneryPrimaryColour << 19) | IMAGE_TYPE_REMAP; gfx_draw_sprite(&clipdpi, imageId, { 0x21, 0x28 }, w->colours[1]); diff --git a/src/openrct2-ui/windows/TopToolbar.cpp b/src/openrct2-ui/windows/TopToolbar.cpp index b3d67b6d98..73b2907eb9 100644 --- a/src/openrct2-ui/windows/TopToolbar.cpp +++ b/src/openrct2-ui/windows/TopToolbar.cpp @@ -1045,8 +1045,8 @@ static void repaint_scenery_tool_down(const ScreenCoordsXY& windowPos, rct_widge auto banner = info.Element->AsBanner()->GetBanner(); if (banner != nullptr) { - auto scenery_entry = get_banner_entry(banner->type); - if (scenery_entry->banner.flags & BANNER_ENTRY_FLAG_HAS_PRIMARY_COLOUR) + auto* bannerEntry = get_banner_entry(banner->type); + if (bannerEntry->flags & BANNER_ENTRY_FLAG_HAS_PRIMARY_COLOUR) { auto repaintScenery = BannerSetColourAction( { info.Loc, info.Element->GetBaseZ(), info.Element->AsBanner()->GetPosition() }, diff --git a/src/openrct2/actions/BannerPlaceAction.cpp b/src/openrct2/actions/BannerPlaceAction.cpp index 4991d0f847..92b7a1e872 100644 --- a/src/openrct2/actions/BannerPlaceAction.cpp +++ b/src/openrct2/actions/BannerPlaceAction.cpp @@ -99,13 +99,13 @@ GameActions::Result::Ptr BannerPlaceAction::Query() const return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_POSITION_THIS_HERE); } - rct_scenery_entry* bannerEntry = get_banner_entry(_bannerType); + auto* bannerEntry = get_banner_entry(_bannerType); if (bannerEntry == nullptr) { log_error("Invalid banner object type. bannerType = ", _bannerType); return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_POSITION_THIS_HERE); } - res->Cost = bannerEntry->banner.price; + res->Cost = bannerEntry->price; return res; } @@ -130,7 +130,7 @@ GameActions::Result::Ptr BannerPlaceAction::Execute() const return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_POSITION_THIS_HERE); } - rct_scenery_entry* bannerEntry = get_banner_entry(_bannerType); + auto* bannerEntry = get_banner_entry(_bannerType); if (bannerEntry == nullptr) { log_error("Invalid banner object type. bannerType = ", _bannerType); @@ -162,7 +162,7 @@ GameActions::Result::Ptr BannerPlaceAction::Execute() const map_invalidate_tile_full(_loc); map_animation_create(MAP_ANIMATION_TYPE_BANNER, CoordsXYZ{ _loc, bannerElement->GetBaseZ() }); - res->Cost = bannerEntry->banner.price; + res->Cost = bannerEntry->price; return res; } diff --git a/src/openrct2/actions/BannerRemoveAction.cpp b/src/openrct2/actions/BannerRemoveAction.cpp index 0d0f6ce5e5..184e6dd362 100644 --- a/src/openrct2/actions/BannerRemoveAction.cpp +++ b/src/openrct2/actions/BannerRemoveAction.cpp @@ -74,10 +74,10 @@ GameActions::Result::Ptr BannerRemoveAction::Query() const return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_REMOVE_THIS); } - rct_scenery_entry* bannerEntry = get_banner_entry(banner->type); + auto* bannerEntry = get_banner_entry(banner->type); if (bannerEntry != nullptr) { - res->Cost = -((bannerEntry->banner.price * 3) / 4); + res->Cost = -((bannerEntry->price * 3) / 4); } return res; @@ -112,10 +112,10 @@ GameActions::Result::Ptr BannerRemoveAction::Execute() const return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_REMOVE_THIS); } - rct_scenery_entry* bannerEntry = get_banner_entry(banner->type); + auto* bannerEntry = get_banner_entry(banner->type); if (bannerEntry != nullptr) { - res->Cost = -((bannerEntry->banner.price * 3) / 4); + res->Cost = -((bannerEntry->price * 3) / 4); } reinterpret_cast(bannerElement)->RemoveBannerEntry(); diff --git a/src/openrct2/object/BannerObject.cpp b/src/openrct2/object/BannerObject.cpp index 01e434b32a..9238f70ef5 100644 --- a/src/openrct2/object/BannerObject.cpp +++ b/src/openrct2/object/BannerObject.cpp @@ -20,10 +20,10 @@ void BannerObject::ReadLegacy(IReadObjectContext* context, OpenRCT2::IStream* stream) { stream->Seek(6, OpenRCT2::STREAM_SEEK_CURRENT); - _legacyType.banner.scrolling_mode = stream->ReadValue(); - _legacyType.banner.flags = stream->ReadValue(); - _legacyType.banner.price = stream->ReadValue(); - _legacyType.banner.scenery_tab_id = OBJECT_ENTRY_INDEX_NULL; + _legacyType.scrolling_mode = stream->ReadValue(); + _legacyType.flags = stream->ReadValue(); + _legacyType.price = stream->ReadValue(); + _legacyType.scenery_tab_id = OBJECT_ENTRY_INDEX_NULL; stream->Seek(2, OpenRCT2::STREAM_SEEK_CURRENT); GetStringTable().Read(context, stream, ObjectStringID::NAME); @@ -34,7 +34,7 @@ void BannerObject::ReadLegacy(IReadObjectContext* context, OpenRCT2::IStream* st GetImageTable().Read(context, stream); // Validate properties - if (_legacyType.large_scenery.price <= 0) + if (_legacyType.price <= 0) { context->LogError(ObjectError::InvalidProperty, "Price can not be free or negative."); } @@ -89,9 +89,9 @@ void BannerObject::ReadJson(IReadObjectContext* context, json_t& root) if (properties.is_object()) { - _legacyType.banner.scrolling_mode = Json::GetNumber(properties["scrollingMode"]); - _legacyType.banner.price = Json::GetNumber(properties["price"]); - _legacyType.banner.flags = Json::GetFlags( + _legacyType.scrolling_mode = Json::GetNumber(properties["scrollingMode"]); + _legacyType.price = Json::GetNumber(properties["price"]); + _legacyType.flags = Json::GetFlags( properties, { { "hasPrimaryColour", BANNER_ENTRY_FLAG_HAS_PRIMARY_COLOUR }, diff --git a/src/openrct2/object/BannerObject.h b/src/openrct2/object/BannerObject.h index 1877ce6d25..4ee48e5bb5 100644 --- a/src/openrct2/object/BannerObject.h +++ b/src/openrct2/object/BannerObject.h @@ -16,7 +16,7 @@ class BannerObject final : public SceneryObject { private: - rct_scenery_entry _legacyType = {}; + BannerSceneryEntry _legacyType = {}; public: explicit BannerObject(const rct_object_entry& entry) diff --git a/src/openrct2/object/ObjectManager.cpp b/src/openrct2/object/ObjectManager.cpp index 1cee523deb..58290cad00 100644 --- a/src/openrct2/object/ObjectManager.cpp +++ b/src/openrct2/object/ObjectManager.cpp @@ -503,8 +503,8 @@ private: } case ObjectType::Banners: { - sceneryEntry = static_cast(loadedObject->GetLegacyData()); - sceneryEntry->banner.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject.get()); + auto* bannerEntry = static_cast(loadedObject->GetLegacyData()); + bannerEntry->scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject.get()); break; } case ObjectType::PathBits: diff --git a/src/openrct2/paint/tile_element/Paint.Banner.cpp b/src/openrct2/paint/tile_element/Paint.Banner.cpp index 6a388f068c..0b3f492ba6 100644 --- a/src/openrct2/paint/tile_element/Paint.Banner.cpp +++ b/src/openrct2/paint/tile_element/Paint.Banner.cpp @@ -55,8 +55,8 @@ void banner_paint(paint_session* session, uint8_t direction, int32_t height, con return; } - auto banner_scenery = get_banner_entry(banner->type); - if (banner_scenery == nullptr) + auto* bannerEntry = get_banner_entry(banner->type); + if (bannerEntry == nullptr) { return; } @@ -66,7 +66,7 @@ void banner_paint(paint_session* session, uint8_t direction, int32_t height, con CoordsXYZ boundBoxOffset = CoordsXYZ(BannerBoundBoxes[direction][0], height + 2); - uint32_t base_id = (direction << 1) + banner_scenery->image; + uint32_t base_id = (direction << 1) + bannerEntry->image; uint32_t image_id = base_id; if (tile_element->IsGhost()) // if being placed @@ -93,7 +93,7 @@ void banner_paint(paint_session* session, uint8_t direction, int32_t height, con if (direction >= 2 || (tile_element->IsGhost())) return; - uint16_t scrollingMode = banner_scenery->banner.scrolling_mode; + uint16_t scrollingMode = bannerEntry->scrolling_mode; if (scrollingMode >= MAX_SCROLLING_TEXT_MODES) { return; diff --git a/src/openrct2/world/Banner.cpp b/src/openrct2/world/Banner.cpp index b7a40e6781..de001a8541 100644 --- a/src/openrct2/world/Banner.cpp +++ b/src/openrct2/world/Banner.cpp @@ -333,7 +333,7 @@ Banner* BannerElement::GetBanner() const return ::GetBanner(GetIndex()); } -rct_scenery_entry* BannerElement::GetEntry() const +BannerSceneryEntry* BannerElement::GetEntry() const { auto banner = GetBanner(); if (banner != nullptr) diff --git a/src/openrct2/world/Scenery.cpp b/src/openrct2/world/Scenery.cpp index 97eb623b69..47c3e0366b 100644 --- a/src/openrct2/world/Scenery.cpp +++ b/src/openrct2/world/Scenery.cpp @@ -241,14 +241,14 @@ WallSceneryEntry* get_wall_entry(ObjectEntryIndex entryIndex) return result; } -rct_scenery_entry* get_banner_entry(ObjectEntryIndex entryIndex) +BannerSceneryEntry* get_banner_entry(ObjectEntryIndex entryIndex) { - rct_scenery_entry* result = nullptr; + BannerSceneryEntry* result = nullptr; auto& objMgr = OpenRCT2::GetContext()->GetObjectManager(); auto obj = objMgr.GetLoadedObject(ObjectType::Banners, entryIndex); if (obj != nullptr) { - result = static_cast(obj->GetLegacyData()); + result = static_cast(obj->GetLegacyData()); } return result; } diff --git a/src/openrct2/world/Scenery.h b/src/openrct2/world/Scenery.h index 47c7fa70ef..3cd0fc2d3f 100644 --- a/src/openrct2/world/Scenery.h +++ b/src/openrct2/world/Scenery.h @@ -121,14 +121,6 @@ enum WALL_SCENERY_2_FLAGS WALL_SCENERY_2_ANIMATED = (1 << 4), // 0x10 }; -struct rct_banner_scenery_entry -{ - uint8_t scrolling_mode; // 0x06 - uint8_t flags; // 0x07 - int16_t price; // 0x08 - ObjectEntryIndex scenery_tab_id; // 0x0A -}; - struct SceneryEntryBase { rct_string_id name; @@ -143,7 +135,6 @@ struct rct_scenery_entry { rct_small_scenery_entry small_scenery; rct_large_scenery_entry large_scenery; - rct_banner_scenery_entry banner; }; }; @@ -167,6 +158,14 @@ struct PathBitEntry : SceneryEntryBase ObjectEntryIndex scenery_tab_id; }; +struct BannerSceneryEntry : SceneryEntryBase +{ + uint8_t scrolling_mode; + uint8_t flags; + int16_t price; + ObjectEntryIndex scenery_tab_id; +}; + #pragma pack(pop) struct rct_scenery_group_entry @@ -262,7 +261,7 @@ void scenery_set_default_placement_configuration(); void scenery_remove_ghost_tool_placement(); WallSceneryEntry* get_wall_entry(ObjectEntryIndex entryIndex); -rct_scenery_entry* get_banner_entry(ObjectEntryIndex entryIndex); +BannerSceneryEntry* get_banner_entry(ObjectEntryIndex entryIndex); PathBitEntry* get_footpath_item_entry(ObjectEntryIndex entryIndex); rct_scenery_group_entry* get_scenery_group_entry(ObjectEntryIndex entryIndex); diff --git a/src/openrct2/world/TileElement.h b/src/openrct2/world/TileElement.h index aafa8282d1..db6036f37b 100644 --- a/src/openrct2/world/TileElement.h +++ b/src/openrct2/world/TileElement.h @@ -20,6 +20,7 @@ struct CoordsXY; struct rct_scenery_entry; struct WallSceneryEntry; struct PathBitEntry; +struct BannerSceneryEntry; struct rct_footpath_entry; class LargeSceneryObject; class TerrainSurfaceObject; @@ -606,7 +607,7 @@ private: #pragma clang diagnostic pop public: Banner* GetBanner() const; - rct_scenery_entry* GetEntry() const; + BannerSceneryEntry* GetEntry() const; BannerIndex GetIndex() const; void SetIndex(BannerIndex newIndex);