From 67f1359936bdd70a5153304160d125d2144e3835 Mon Sep 17 00:00:00 2001
From: James Warwood <warwood.aj@gmail.com>
Date: Thu, 30 May 2019 09:33:13 +0100
Subject: [PATCH] Fix: Unable to change colours of scrolling wall banner sign
 via sign dialog (#9303)

1. Check in SignSetStlyeAction query was accidentally inverting value of
   wallFound boolean
2. Code in action execution was not looping through the tile elements to find
   the correct wall piece like the query did (which meant just fixing 1. would
   cause a segfault)

Moved code to find relevant wall tile element into new helper function
banner_get_scrolling_wall_tile_element() in Banner; use in both query and action
to avoid any duplication of search logic.
---
 src/openrct2/actions/SignSetStyleAction.hpp | 27 ++++++---------------
 src/openrct2/network/Network.cpp            |  2 +-
 src/openrct2/world/Banner.cpp               | 26 ++++++++++++++++++++
 src/openrct2/world/Banner.h                 |  1 +
 4 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/src/openrct2/actions/SignSetStyleAction.hpp b/src/openrct2/actions/SignSetStyleAction.hpp
index dae643cf23..7493d0fc24 100644
--- a/src/openrct2/actions/SignSetStyleAction.hpp
+++ b/src/openrct2/actions/SignSetStyleAction.hpp
@@ -77,23 +77,9 @@ public:
         }
         else
         {
-            TileElement* tileElement = map_get_first_element_at(coords.x / 32, coords.y / 32);
-            bool wallFound = false;
-            do
-            {
-                if (tileElement->GetType() != TILE_ELEMENT_TYPE_WALL)
-                    continue;
+            WallElement* wallElement = banner_get_scrolling_wall_tile_element(static_cast<BannerIndex>(_bannerIndex));
 
-                rct_scenery_entry* scenery_entry = tileElement->AsWall()->GetEntry();
-                if (scenery_entry->wall.scrolling_mode == SCROLLING_MODE_NONE)
-                    continue;
-                if (tileElement->AsWall()->GetBannerIndex() != (BannerIndex)_bannerIndex)
-                    continue;
-                wallFound = true;
-                break;
-            } while (!(tileElement++)->IsLastForTile());
-
-            if (!wallFound == false)
+            if (!wallElement)
             {
                 log_warning("Invalid game command for setting sign style, banner id '%d' not found", _bannerIndex);
                 return MakeResult(GA_ERROR::INVALID_PARAMETERS, STR_NONE);
@@ -121,10 +107,11 @@ public:
         }
         else
         {
-            TileElement* tileElement = map_get_first_element_at(coords.x / 32, coords.y / 32);
-            tileElement->AsWall()->SetPrimaryColour(_mainColour);
-            tileElement->AsWall()->SetSecondaryColour(_textColour);
-            map_invalidate_tile(coords.x, coords.y, tileElement->base_height * 8, tileElement->clearance_height * 8);
+            WallElement* wallElement = banner_get_scrolling_wall_tile_element(static_cast<BannerIndex>(_bannerIndex));
+
+            wallElement->SetPrimaryColour(_mainColour);
+            wallElement->SetSecondaryColour(_textColour);
+            map_invalidate_tile(coords.x, coords.y, wallElement->base_height * 8, wallElement->clearance_height * 8);
         }
 
         auto intent = Intent(INTENT_ACTION_UPDATE_BANNER);
diff --git a/src/openrct2/network/Network.cpp b/src/openrct2/network/Network.cpp
index 3429d978fb..4bc9502f69 100644
--- a/src/openrct2/network/Network.cpp
+++ b/src/openrct2/network/Network.cpp
@@ -33,7 +33,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 "35"
+#define NETWORK_STREAM_VERSION "36"
 #define NETWORK_STREAM_ID OPENRCT2_VERSION "-" NETWORK_STREAM_VERSION
 
 static Peep* _pickup_peep = nullptr;
diff --git a/src/openrct2/world/Banner.cpp b/src/openrct2/world/Banner.cpp
index f0f3aa0ce6..4ec4bd87a2 100644
--- a/src/openrct2/world/Banner.cpp
+++ b/src/openrct2/world/Banner.cpp
@@ -128,6 +128,32 @@ TileElement* banner_get_tile_element(BannerIndex bannerIndex)
     return nullptr;
 }
 
+WallElement* banner_get_scrolling_wall_tile_element(BannerIndex bannerIndex)
+{
+    rct_banner* banner = &gBanners[bannerIndex];
+    TileElement* tileElement = map_get_first_element_at(banner->x, banner->y);
+
+    if (tileElement == nullptr)
+        return nullptr;
+
+    do
+    {
+        auto wallElement = tileElement->AsWall();
+
+        if (wallElement == nullptr)
+            continue;
+
+        rct_scenery_entry* scenery_entry = wallElement->GetEntry();
+        if (scenery_entry->wall.scrolling_mode == SCROLLING_MODE_NONE)
+            continue;
+        if (wallElement->GetBannerIndex() != bannerIndex)
+            continue;
+        return wallElement;
+    } while (!(tileElement++)->IsLastForTile());
+
+    return nullptr;
+}
+
 /**
  *
  *  rct2: 0x006B7D86
diff --git a/src/openrct2/world/Banner.h b/src/openrct2/world/Banner.h
index 962bfb119f..b504d66a0f 100644
--- a/src/openrct2/world/Banner.h
+++ b/src/openrct2/world/Banner.h
@@ -49,6 +49,7 @@ extern rct_banner gBanners[MAX_BANNERS];
 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);
 uint8_t banner_get_closest_ride_index(int32_t x, int32_t y, int32_t z);
 void banner_reset_broken_index();
 void fix_duplicated_banners();