From 1229ce6b228c7d47f2a110fbb4d07ff69c976a0e Mon Sep 17 00:00:00 2001 From: Daniel Karandikar Date: Mon, 15 Mar 2021 19:55:36 +0000 Subject: [PATCH 1/6] Refactor Banner to use new Window framework Also tidy up a couple of bits of duplicated code Pulled out into CreateViewport method and tileElement member --- src/openrct2-ui/windows/Banner.cpp | 444 ++++++++++------------- src/openrct2/interface/Window.cpp | 2 + src/openrct2/interface/Window_internal.h | 3 + 3 files changed, 204 insertions(+), 245 deletions(-) diff --git a/src/openrct2-ui/windows/Banner.cpp b/src/openrct2-ui/windows/Banner.cpp index eef24e875c..4044756a2f 100644 --- a/src/openrct2-ui/windows/Banner.cpp +++ b/src/openrct2-ui/windows/Banner.cpp @@ -69,264 +69,218 @@ static rct_widget window_banner_widgets[] = { { WIDGETS_END }, }; -static void window_banner_mouseup(rct_window *w, rct_widgetindex widgetIndex); -static void window_banner_mousedown(rct_window *w, rct_widgetindex widgetIndex, rct_widget* widget); -static void window_banner_dropdown(rct_window *w, rct_widgetindex widgetIndex, int32_t dropdownIndex); -static void window_banner_textinput(rct_window *w, rct_widgetindex widgetIndex, char *text); -static void window_banner_viewport_rotate(rct_window *w); -static void window_banner_invalidate(rct_window *w); -static void window_banner_paint(rct_window *w, rct_drawpixelinfo *dpi); - -static rct_window_event_list window_banner_events([](auto& events) -{ - events.mouse_up = &window_banner_mouseup; - events.mouse_down = &window_banner_mousedown; - events.dropdown = &window_banner_dropdown; - events.text_input = &window_banner_textinput; - events.viewport_rotate = &window_banner_viewport_rotate; - events.invalidate = &window_banner_invalidate; - events.paint = &window_banner_paint; -}); // clang-format on +class BannerWindow final : public Window +{ +private: + Banner* banner; + CoordsXYZ bannerViewPos; + TileElement* tileElement = nullptr; + + void CreateViewport() + { + rct_widget* viewportWidget = &window_banner_widgets[WIDX_VIEWPORT]; + viewport_create( + this, windowPos + ScreenCoordsXY{ viewportWidget->left + 1, viewportWidget->top + 1 }, + (viewportWidget->width()) - 1, (viewportWidget->height()) - 1, 0, bannerViewPos, 0, SPRITE_INDEX_NULL); + + if (viewport != nullptr) + viewport->flags = gConfigGeneral.always_show_gridlines ? VIEWPORT_FLAG_GRIDLINES : 0; + Invalidate(); + } + + void InitTileElement() + { + TileElement* tile_element = map_get_first_element_at(banner->position.ToCoordsXY().ToTileCentre()); + if (tile_element == nullptr) + { + tileElement = nullptr; + return; + } + + while (1) + { + if ((tile_element->GetType() == TILE_ELEMENT_TYPE_BANNER) && (tile_element->AsBanner()->GetIndex() == number)) + break; + tile_element++; + } + tileElement = tile_element; + } + +public: + void OnOpen() override + { + widgets = window_banner_widgets; + enabled_widgets = (1 << WIDX_CLOSE) | (1 << WIDX_BANNER_TEXT) | (1 << WIDX_BANNER_NO_ENTRY) + | (1 << WIDX_BANNER_DEMOLISH) | (1 << WIDX_MAIN_COLOUR) | (1 << WIDX_TEXT_COLOUR_DROPDOWN) + | (1 << WIDX_TEXT_COLOUR_DROPDOWN_BUTTON); + WindowInitScrollWidgets(this); + } + + BannerWindow* Initialise(rct_windownumber _number) + { + number = _number; + banner = GetBanner(number); + + InitTileElement(); + if (tileElement == nullptr) + return nullptr; + + frame_no = tileElement->GetBaseZ(); + bannerViewPos = CoordsXYZ{ banner->position.ToCoordsXY().ToTileCentre(), frame_no }; + CreateViewport(); + + return this; + } + + void OnMouseDown(rct_widgetindex widgetIndex) override + { + rct_widget* widget = &widgets[widgetIndex]; + + switch (widgetIndex) + { + case WIDX_MAIN_COLOUR: + WindowDropdownShowColour(this, widget, TRANSLUCENT(colours[1]), banner->colour); + break; + case WIDX_TEXT_COLOUR_DROPDOWN_BUTTON: + + for (int32_t i = 0; i < 13; ++i) + { + gDropdownItemsFormat[i] = STR_DROPDOWN_MENU_LABEL; + gDropdownItemsArgs[i] = BannerColouredTextFormats[i + 1]; + } + + // Switch to the dropdown box widget. + widget--; + + WindowDropdownShowTextCustomWidth( + { 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); + break; + } + } + + void OnMouseUp(rct_widgetindex widgetIndex) override + { + switch (widgetIndex) + { + case WIDX_CLOSE: + Close(); + break; + case WIDX_BANNER_DEMOLISH: + { + auto bannerRemoveAction = BannerRemoveAction( + { banner->position.ToCoordsXY(), tileElement->GetBaseZ(), tileElement->AsBanner()->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); + break; + case WIDX_BANNER_NO_ENTRY: + { + textinput_cancel(); + auto bannerSetStyle = BannerSetStyleAction( + BannerSetStyleType::NoEntry, number, banner->flags ^ BANNER_FLAG_NO_ENTRY); + GameActions::Execute(&bannerSetStyle); + break; + } + } + } + + void OnDropdown(rct_widgetindex widgetIndex, int32_t dropdownIndex) override + { + switch (widgetIndex) + { + case WIDX_MAIN_COLOUR: + { + if (dropdownIndex == -1) + break; + + auto bannerSetStyle = BannerSetStyleAction(BannerSetStyleType::PrimaryColour, number, dropdownIndex); + GameActions::Execute(&bannerSetStyle); + break; + } + case WIDX_TEXT_COLOUR_DROPDOWN_BUTTON: + { + if (dropdownIndex == -1) + break; + auto bannerSetStyle = BannerSetStyleAction(BannerSetStyleType::TextColour, number, dropdownIndex + 1); + GameActions::Execute(&bannerSetStyle); + break; + } + } + } + + void OnTextInput(rct_widgetindex widgetIndex, std::string_view text) override + { + if (widgetIndex == WIDX_BANNER_TEXT) + { + auto bannerSetNameAction = BannerSetNameAction(number, std::string(text)); + GameActions::Execute(&bannerSetNameAction); + } + } + + void OnViewportRotate() override + { + RemoveViewport(); + CreateViewport(); + } + + void OnDraw(rct_drawpixelinfo& dpi) override + { + DrawWidgets(dpi); + + if (viewport != nullptr) + { + window_draw_viewport(&dpi, this); + } + } + + void OnPrepareDraw() override + { + rct_widget* colour_btn = &window_banner_widgets[WIDX_MAIN_COLOUR]; + colour_btn->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)) + { + colour_btn->type = WindowWidgetType::ColourBtn; + } + 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) + { + pressed_widgets |= (1ULL << WIDX_BANNER_NO_ENTRY); + disabled_widgets |= (1ULL << WIDX_BANNER_TEXT) | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN) + | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN_BUTTON); + } + colour_btn->image = SPRITE_ID_PALETTE_COLOUR_1(banner->colour) | IMAGE_TYPE_TRANSPARENT | SPR_PALETTE_BTN; + rct_widget* drop_down_widget = &window_banner_widgets[WIDX_TEXT_COLOUR_DROPDOWN]; + drop_down_widget->text = BannerColouredTextFormats[banner->text_colour]; + } +}; + /** * * rct2: 0x006BA305 */ rct_window* window_banner_open(rct_windownumber number) { - rct_window* w; - rct_widget* viewportWidget; + auto w = static_cast(window_bring_to_front_by_number(WC_BANNER, number)); - // Check if window is already open - w = window_bring_to_front_by_number(WC_BANNER, number); if (w != nullptr) return w; - w = WindowCreateAutoPos(WW, WH, &window_banner_events, WC_BANNER, WF_NO_SCROLLING); - w->widgets = window_banner_widgets; - w->enabled_widgets = (1 << WIDX_CLOSE) | (1 << WIDX_BANNER_TEXT) | (1 << WIDX_BANNER_NO_ENTRY) | (1 << WIDX_BANNER_DEMOLISH) - | (1 << WIDX_MAIN_COLOUR) | (1 << WIDX_TEXT_COLOUR_DROPDOWN) | (1 << WIDX_TEXT_COLOUR_DROPDOWN_BUTTON); + w = WindowCreate(WC_BANNER, WW, WH, 0); - w->number = number; - WindowInitScrollWidgets(w); - - auto banner = GetBanner(w->number); - auto bannerViewPos = banner->position.ToCoordsXY().ToTileCentre(); - - TileElement* tile_element = map_get_first_element_at(bannerViewPos); - if (tile_element == nullptr) - return nullptr; - while (1) - { - if ((tile_element->GetType() == TILE_ELEMENT_TYPE_BANNER) && (tile_element->AsBanner()->GetIndex() == w->number)) - { - break; - } - - tile_element++; - } - - int32_t view_z = tile_element->GetBaseZ(); - w->frame_no = view_z; - - // Create viewport - viewportWidget = &window_banner_widgets[WIDX_VIEWPORT]; - viewport_create( - w, w->windowPos + ScreenCoordsXY{ viewportWidget->left + 1, viewportWidget->top + 1 }, viewportWidget->width() - 2, - viewportWidget->height() - 2, 0, { bannerViewPos, view_z }, 0, SPRITE_INDEX_NULL); - - w->viewport->flags = gConfigGeneral.always_show_gridlines ? VIEWPORT_FLAG_GRIDLINES : 0; - w->Invalidate(); + if (w != nullptr) + w = w->Initialise(number); return w; } - -/** - * - * rct2: 0x6ba4d6 - */ -static void window_banner_mouseup(rct_window* w, rct_widgetindex widgetIndex) -{ - auto banner = GetBanner(w->number); - auto bannerPos = banner->position.ToCoordsXY(); - - TileElement* tile_element = map_get_first_element_at(bannerPos); - if (tile_element == nullptr) - return; - - while (1) - { - if ((tile_element->GetType() == TILE_ELEMENT_TYPE_BANNER) && (tile_element->AsBanner()->GetIndex() == w->number)) - break; - tile_element++; - } - - switch (widgetIndex) - { - case WIDX_CLOSE: - window_close(w); - break; - case WIDX_BANNER_DEMOLISH: - { - auto bannerRemoveAction = BannerRemoveAction( - { bannerPos, tile_element->GetBaseZ(), tile_element->AsBanner()->GetPosition() }); - GameActions::Execute(&bannerRemoveAction); - break; - } - case WIDX_BANNER_TEXT: - window_text_input_raw_open( - w, 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, w->number, banner->flags ^ BANNER_FLAG_NO_ENTRY); - GameActions::Execute(&bannerSetStyle); - break; - } - } -} - -/** - * - * rct2: 0x6ba4ff - */ -static void window_banner_mousedown(rct_window* w, rct_widgetindex widgetIndex, rct_widget* widget) -{ - auto banner = GetBanner(w->number); - - switch (widgetIndex) - { - case WIDX_MAIN_COLOUR: - WindowDropdownShowColour(w, widget, TRANSLUCENT(w->colours[1]), banner->colour); - break; - case WIDX_TEXT_COLOUR_DROPDOWN_BUTTON: - - for (int32_t i = 0; i < 13; ++i) - { - gDropdownItemsFormat[i] = STR_DROPDOWN_MENU_LABEL; - gDropdownItemsArgs[i] = BannerColouredTextFormats[i + 1]; - } - - // Switch to the dropdown box widget. - widget--; - - WindowDropdownShowTextCustomWidth( - { widget->left + w->windowPos.x, widget->top + w->windowPos.y }, widget->height() + 1, w->colours[1], 0, - Dropdown::Flag::StayOpen, 13, widget->width() - 3); - - Dropdown::SetChecked(banner->text_colour - 1, true); - break; - } -} - -/** - * - * rct2: 0x6ba517 - */ -static void window_banner_dropdown(rct_window* w, rct_widgetindex widgetIndex, int32_t dropdownIndex) -{ - switch (widgetIndex) - { - case WIDX_MAIN_COLOUR: - { - if (dropdownIndex == -1) - break; - - auto bannerSetStyle = BannerSetStyleAction(BannerSetStyleType::PrimaryColour, w->number, dropdownIndex); - GameActions::Execute(&bannerSetStyle); - break; - } - case WIDX_TEXT_COLOUR_DROPDOWN_BUTTON: - { - if (dropdownIndex == -1) - break; - auto bannerSetStyle = BannerSetStyleAction(BannerSetStyleType::TextColour, w->number, dropdownIndex + 1); - GameActions::Execute(&bannerSetStyle); - break; - } - } -} - -/** - * - * rct2: 0x6ba50c - */ -static void window_banner_textinput(rct_window* w, rct_widgetindex widgetIndex, char* text) -{ - if (widgetIndex == WIDX_BANNER_TEXT && text != nullptr) - { - auto bannerSetNameAction = BannerSetNameAction(w->number, text); - GameActions::Execute(&bannerSetNameAction); - } -} - -/** - * - * rct2: 0x006BA44D - */ -static void window_banner_invalidate(rct_window* w) -{ - auto banner = GetBanner(w->number); - rct_widget* colour_btn = &window_banner_widgets[WIDX_MAIN_COLOUR]; - colour_btn->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)) - { - colour_btn->type = WindowWidgetType::ColourBtn; - } - - w->pressed_widgets &= ~(1ULL << WIDX_BANNER_NO_ENTRY); - w->disabled_widgets &= ~( - (1ULL << WIDX_BANNER_TEXT) | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN) | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN_BUTTON)); - - if (banner->flags & BANNER_FLAG_NO_ENTRY) - { - w->pressed_widgets |= (1ULL << WIDX_BANNER_NO_ENTRY); - w->disabled_widgets |= (1ULL << WIDX_BANNER_TEXT) | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN) - | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN_BUTTON); - } - - colour_btn->image = SPRITE_ID_PALETTE_COLOUR_1(banner->colour) | IMAGE_TYPE_TRANSPARENT | SPR_PALETTE_BTN; - - rct_widget* drop_down_widget = &window_banner_widgets[WIDX_TEXT_COLOUR_DROPDOWN]; - drop_down_widget->text = BannerColouredTextFormats[banner->text_colour]; -} - -/* rct2: 0x006BA4C5 */ -static void window_banner_paint(rct_window* w, rct_drawpixelinfo* dpi) -{ - WindowDrawWidgets(w, dpi); - - // Draw viewport - if (w->viewport != nullptr) - { - window_draw_viewport(dpi, w); - } -} - -/** - * - * rct2: 0x6BA7B5 - */ -static void window_banner_viewport_rotate(rct_window* w) -{ - w->RemoveViewport(); - - auto banner = GetBanner(w->number); - auto bannerViewPos = CoordsXYZ{ banner->position.ToCoordsXY().ToTileCentre(), w->frame_no }; - - // Create viewport - rct_widget* viewportWidget = &window_banner_widgets[WIDX_VIEWPORT]; - viewport_create( - w, w->windowPos + ScreenCoordsXY{ viewportWidget->left + 1, viewportWidget->top + 1 }, (viewportWidget->width()) - 1, - (viewportWidget->height()) - 1, 0, bannerViewPos, 0, SPRITE_INDEX_NULL); - - if (w->viewport != nullptr) - w->viewport->flags = gConfigGeneral.always_show_gridlines ? VIEWPORT_FLAG_GRIDLINES : 0; - w->Invalidate(); -} diff --git a/src/openrct2/interface/Window.cpp b/src/openrct2/interface/Window.cpp index 1256e392d1..a79aaeee0d 100644 --- a/src/openrct2/interface/Window.cpp +++ b/src/openrct2/interface/Window.cpp @@ -1562,6 +1562,8 @@ void window_event_textinput_call(rct_window* w, rct_widgetindex widgetIndex, cha void window_event_viewport_rotate_call(rct_window* w) { + if (w->event_handlers == nullptr) + w->OnViewportRotate(); if (w->event_handlers != nullptr) if (w->event_handlers->viewport_rotate != nullptr) w->event_handlers->viewport_rotate(w); diff --git a/src/openrct2/interface/Window_internal.h b/src/openrct2/interface/Window_internal.h index 8272f45d95..9fd1784815 100644 --- a/src/openrct2/interface/Window_internal.h +++ b/src/openrct2/interface/Window_internal.h @@ -179,6 +179,9 @@ struct rct_window virtual void OnToolAbort(rct_widgetindex widgetIndex) { } + virtual void OnViewportRotate() + { + } }; #ifdef __WARN_SUGGEST_FINAL_METHODS__ From 248d6a068366174dac454bd4f0ef943fe2cf6281 Mon Sep 17 00:00:00 2001 From: Daniel Karandikar Date: Mon, 22 Mar 2021 18:56:24 +0000 Subject: [PATCH 2/6] Prepend _ to private member fields --- src/openrct2-ui/windows/Banner.cpp | 40 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/openrct2-ui/windows/Banner.cpp b/src/openrct2-ui/windows/Banner.cpp index 4044756a2f..a710eff2c8 100644 --- a/src/openrct2-ui/windows/Banner.cpp +++ b/src/openrct2-ui/windows/Banner.cpp @@ -74,16 +74,16 @@ static rct_widget window_banner_widgets[] = { class BannerWindow final : public Window { private: - Banner* banner; - CoordsXYZ bannerViewPos; - TileElement* tileElement = nullptr; + Banner* _banner; + CoordsXYZ _bannerViewPos; + TileElement* _tileElement = nullptr; void CreateViewport() { rct_widget* viewportWidget = &window_banner_widgets[WIDX_VIEWPORT]; viewport_create( this, windowPos + ScreenCoordsXY{ viewportWidget->left + 1, viewportWidget->top + 1 }, - (viewportWidget->width()) - 1, (viewportWidget->height()) - 1, 0, bannerViewPos, 0, SPRITE_INDEX_NULL); + (viewportWidget->width()) - 1, (viewportWidget->height()) - 1, 0, _bannerViewPos, 0, SPRITE_INDEX_NULL); if (viewport != nullptr) viewport->flags = gConfigGeneral.always_show_gridlines ? VIEWPORT_FLAG_GRIDLINES : 0; @@ -92,10 +92,10 @@ private: void InitTileElement() { - TileElement* tile_element = map_get_first_element_at(banner->position.ToCoordsXY().ToTileCentre()); + TileElement* tile_element = map_get_first_element_at(_banner->position.ToCoordsXY().ToTileCentre()); if (tile_element == nullptr) { - tileElement = nullptr; + _tileElement = nullptr; return; } @@ -105,7 +105,7 @@ private: break; tile_element++; } - tileElement = tile_element; + _tileElement = tile_element; } public: @@ -121,14 +121,14 @@ public: BannerWindow* Initialise(rct_windownumber _number) { number = _number; - banner = GetBanner(number); + _banner = GetBanner(number); InitTileElement(); - if (tileElement == nullptr) + if (_tileElement == nullptr) return nullptr; - frame_no = tileElement->GetBaseZ(); - bannerViewPos = CoordsXYZ{ banner->position.ToCoordsXY().ToTileCentre(), frame_no }; + frame_no = _tileElement->GetBaseZ(); + _bannerViewPos = CoordsXYZ{ _banner->position.ToCoordsXY().ToTileCentre(), frame_no }; CreateViewport(); return this; @@ -141,7 +141,7 @@ public: 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: @@ -158,7 +158,7 @@ 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; } } @@ -173,19 +173,19 @@ public: case WIDX_BANNER_DEMOLISH: { auto bannerRemoveAction = BannerRemoveAction( - { banner->position.ToCoordsXY(), tileElement->GetBaseZ(), tileElement->AsBanner()->GetPosition() }); + { _banner->position.ToCoordsXY(), _tileElement->GetBaseZ(), _tileElement->AsBanner()->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; } @@ -246,7 +246,7 @@ public: rct_widget* colour_btn = &window_banner_widgets[WIDX_MAIN_COLOUR]; colour_btn->type = WindowWidgetType::Empty; // Scenery item not sure why we use this instead of banner? - rct_scenery_entry* sceneryEntry = get_banner_entry(banner->type); + rct_scenery_entry* sceneryEntry = get_banner_entry(_banner->type); if (sceneryEntry != nullptr && (sceneryEntry->banner.flags & BANNER_ENTRY_FLAG_HAS_PRIMARY_COLOUR)) { colour_btn->type = WindowWidgetType::ColourBtn; @@ -254,15 +254,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); } - colour_btn->image = SPRITE_ID_PALETTE_COLOUR_1(banner->colour) | IMAGE_TYPE_TRANSPARENT | SPR_PALETTE_BTN; + colour_btn->image = SPRITE_ID_PALETTE_COLOUR_1(_banner->colour) | IMAGE_TYPE_TRANSPARENT | SPR_PALETTE_BTN; rct_widget* drop_down_widget = &window_banner_widgets[WIDX_TEXT_COLOUR_DROPDOWN]; - drop_down_widget->text = BannerColouredTextFormats[banner->text_colour]; + drop_down_widget->text = BannerColouredTextFormats[_banner->text_colour]; } }; From 1551389bf5e55405650383e2b5ac4d24b1142c66 Mon Sep 17 00:00:00 2001 From: Daniel Karandikar Date: Mon, 22 Mar 2021 18:58:43 +0000 Subject: [PATCH 3/6] camelCase for local vars --- src/openrct2-ui/windows/Banner.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/openrct2-ui/windows/Banner.cpp b/src/openrct2-ui/windows/Banner.cpp index a710eff2c8..eebf8ce9f9 100644 --- a/src/openrct2-ui/windows/Banner.cpp +++ b/src/openrct2-ui/windows/Banner.cpp @@ -92,8 +92,8 @@ private: void InitTileElement() { - TileElement* tile_element = map_get_first_element_at(_banner->position.ToCoordsXY().ToTileCentre()); - if (tile_element == nullptr) + TileElement* tileElement = map_get_first_element_at(_banner->position.ToCoordsXY().ToTileCentre()); + if (tileElement == nullptr) { _tileElement = nullptr; return; @@ -101,11 +101,11 @@ private: while (1) { - if ((tile_element->GetType() == TILE_ELEMENT_TYPE_BANNER) && (tile_element->AsBanner()->GetIndex() == number)) + if ((tileElement->GetType() == TILE_ELEMENT_TYPE_BANNER) && (tileElement->AsBanner()->GetIndex() == number)) break; - tile_element++; + tileElement++; } - _tileElement = tile_element; + _tileElement = tileElement; } public: @@ -243,13 +243,13 @@ public: void OnPrepareDraw() override { - rct_widget* colour_btn = &window_banner_widgets[WIDX_MAIN_COLOUR]; - colour_btn->type = WindowWidgetType::Empty; + 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)) { - colour_btn->type = WindowWidgetType::ColourBtn; + colourBtn->type = WindowWidgetType::ColourBtn; } pressed_widgets &= ~(1ULL << WIDX_BANNER_NO_ENTRY); disabled_widgets &= ~( @@ -260,9 +260,9 @@ public: disabled_widgets |= (1ULL << WIDX_BANNER_TEXT) | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN) | (1ULL << WIDX_TEXT_COLOUR_DROPDOWN_BUTTON); } - colour_btn->image = SPRITE_ID_PALETTE_COLOUR_1(_banner->colour) | IMAGE_TYPE_TRANSPARENT | SPR_PALETTE_BTN; - rct_widget* drop_down_widget = &window_banner_widgets[WIDX_TEXT_COLOUR_DROPDOWN]; - drop_down_widget->text = BannerColouredTextFormats[_banner->text_colour]; + 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]; } }; From 4c1d35478c2d9f9a9335835ea81b677a6f85e37f Mon Sep 17 00:00:00 2001 From: Daniel Karandikar Date: Mon, 22 Mar 2021 18:59:54 +0000 Subject: [PATCH 4/6] Tidy Window.cpp --- src/openrct2/interface/Window.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2/interface/Window.cpp b/src/openrct2/interface/Window.cpp index a79aaeee0d..5ec6b896e4 100644 --- a/src/openrct2/interface/Window.cpp +++ b/src/openrct2/interface/Window.cpp @@ -1564,7 +1564,7 @@ void window_event_viewport_rotate_call(rct_window* w) { if (w->event_handlers == nullptr) w->OnViewportRotate(); - if (w->event_handlers != nullptr) + else if (w->event_handlers != nullptr) if (w->event_handlers->viewport_rotate != nullptr) w->event_handlers->viewport_rotate(w); } From 5ef750a879b33265025c4484e323f9d557bd6503 Mon Sep 17 00:00:00 2001 From: Daniel Karandikar Date: Mon, 22 Mar 2021 19:03:22 +0000 Subject: [PATCH 5/6] Tidy up BannerWindow::Initialise --- src/openrct2-ui/windows/Banner.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/openrct2-ui/windows/Banner.cpp b/src/openrct2-ui/windows/Banner.cpp index eebf8ce9f9..a2225df03d 100644 --- a/src/openrct2-ui/windows/Banner.cpp +++ b/src/openrct2-ui/windows/Banner.cpp @@ -118,20 +118,18 @@ public: WindowInitScrollWidgets(this); } - BannerWindow* Initialise(rct_windownumber _number) + void Initialise(rct_windownumber _number) { number = _number; _banner = GetBanner(number); InitTileElement(); if (_tileElement == nullptr) - return nullptr; + return; frame_no = _tileElement->GetBaseZ(); _bannerViewPos = CoordsXYZ{ _banner->position.ToCoordsXY().ToTileCentre(), frame_no }; CreateViewport(); - - return this; } void OnMouseDown(rct_widgetindex widgetIndex) override @@ -280,7 +278,7 @@ rct_window* window_banner_open(rct_windownumber number) w = WindowCreate(WC_BANNER, WW, WH, 0); if (w != nullptr) - w = w->Initialise(number); + w->Initialise(number); return w; } From d4efbdf53170e6df0110d0ed211b0abf5336ec74 Mon Sep 17 00:00:00 2001 From: Daniel Karandikar Date: Mon, 22 Mar 2021 19:22:27 +0000 Subject: [PATCH 6/6] Refactor InitTileElement Break loop when going to go outside of Tile --- src/openrct2-ui/windows/Banner.cpp | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/openrct2-ui/windows/Banner.cpp b/src/openrct2-ui/windows/Banner.cpp index a2225df03d..f0442ce136 100644 --- a/src/openrct2-ui/windows/Banner.cpp +++ b/src/openrct2-ui/windows/Banner.cpp @@ -93,19 +93,21 @@ private: void InitTileElement() { TileElement* tileElement = map_get_first_element_at(_banner->position.ToCoordsXY().ToTileCentre()); - if (tileElement == nullptr) + if (tileElement != nullptr) { - _tileElement = nullptr; - return; + while (1) + { + if ((tileElement->GetType() == TILE_ELEMENT_TYPE_BANNER) && (tileElement->AsBanner()->GetIndex() == number)) + { + _tileElement = tileElement; + return; + } + if (tileElement->IsLastForTile()) + break; + tileElement++; + } } - - while (1) - { - if ((tileElement->GetType() == TILE_ELEMENT_TYPE_BANNER) && (tileElement->AsBanner()->GetIndex() == number)) - break; - tileElement++; - } - _tileElement = tileElement; + _tileElement = nullptr; } public: