From 70fc620ebee6363430ed4fa1c077bfd04e6cd09f Mon Sep 17 00:00:00 2001 From: Hielke Morsink Date: Thu, 11 Apr 2019 22:51:08 +0200 Subject: [PATCH 1/3] Fix #9083: Wrong sign index is set for clients This issue was caused by the ghost for banners, which already create an entry on the banner. The game command is then received while the ghost still exists, giving the placed banner another ID. This fix is basically a copy of the fix that is also `game_do_command_p` that tackles this same issue. --- src/openrct2/network/Network.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/openrct2/network/Network.cpp b/src/openrct2/network/Network.cpp index 32a355c775..94e737183d 100644 --- a/src/openrct2/network/Network.cpp +++ b/src/openrct2/network/Network.cpp @@ -1976,6 +1976,17 @@ void Network::ProcessGameCommands() if (gc.action != nullptr) { + // Remove ghost scenery so it doesn't interfere with incoming network command + switch (gc.action->GetType()) + { + case GAME_COMMAND_PLACE_WALL: + case GAME_COMMAND_PLACE_LARGE_SCENERY: + case GAME_COMMAND_PLACE_BANNER: + case GAME_COMMAND_PLACE_SCENERY: + scenery_remove_ghost_tool_placement(); + break; + } + GameAction* action = gc.action.get(); action->SetFlags(action->GetFlags() | GAME_COMMAND_FLAG_NETWORKED); From 8035127acd3c1da5e07498bf3c7a88f2e3795b8c Mon Sep 17 00:00:00 2001 From: duncanspumpkin Date: Wed, 10 Apr 2019 18:45:54 +0100 Subject: [PATCH 2/3] Allocate banner index in the constructor. Note this does not fix the issue as ghosts still break placement --- .../actions/LargeSceneryPlaceAction.hpp | 39 ++++++++++---- src/openrct2/actions/WallPlaceAction.hpp | 51 ++++++++++++++----- 2 files changed, 66 insertions(+), 24 deletions(-) diff --git a/src/openrct2/actions/LargeSceneryPlaceAction.hpp b/src/openrct2/actions/LargeSceneryPlaceAction.hpp index 80111a85cf..60796a24ca 100644 --- a/src/openrct2/actions/LargeSceneryPlaceAction.hpp +++ b/src/openrct2/actions/LargeSceneryPlaceAction.hpp @@ -26,6 +26,7 @@ private: uint8_t _sceneryType{ std::numeric_limits::max() }; uint8_t _primaryColour; uint8_t _secondaryColour; + BannerIndex _bannerId{ BANNER_INDEX_NULL }; public: LargeSceneryPlaceAction() = default; @@ -36,6 +37,14 @@ public: , _primaryColour(primaryColour) , _secondaryColour(secondaryColour) { + rct_scenery_entry* sceneryEntry = get_large_scenery_entry(_sceneryType); + if (sceneryEntry != nullptr) + { + if (sceneryEntry->large_scenery.scrolling_mode != SCROLLING_MODE_NONE) + { + _bannerId = create_new_banner(0); + } + } } uint16_t GetActionFlags() const override @@ -47,7 +56,8 @@ public: { GameAction::Serialise(stream); - stream << DS_TAG(_loc) << DS_TAG(_sceneryType) << DS_TAG(_primaryColour) << DS_TAG(_secondaryColour); + stream << DS_TAG(_loc) << DS_TAG(_sceneryType) << DS_TAG(_primaryColour) << DS_TAG(_secondaryColour) + << DS_TAG(_bannerId); } GameActionResult::Ptr Query() const override @@ -61,7 +71,6 @@ public: res->Position.z = surfaceHeight; gSceneryGroundFlags = 0; - BannerIndex bannerId = BANNER_INDEX_NULL; money32 supportsCost = 0; if (_primaryColour > TILE_ELEMENT_COLOUR_MASK || _secondaryColour > TILE_ELEMENT_COLOUR_MASK) @@ -97,9 +106,13 @@ public: if (sceneryEntry->large_scenery.scrolling_mode != SCROLLING_MODE_NONE) { - bannerId = create_new_banner(0); + if (_bannerId == BANNER_INDEX_NULL) + { + log_error("Banner Index not specified."); + return MakeResult(GA_ERROR::INVALID_PARAMETERS, STR_TOO_MANY_BANNERS_IN_GAME, STR_CANT_POSITION_THIS_HERE); + } - if (bannerId == BANNER_INDEX_NULL) + if (gBanners[_bannerId].type != BANNER_NULL) { log_error("No free banners available"); return MakeResult(GA_ERROR::NO_FREE_ELEMENTS, STR_CANT_POSITION_THIS_HERE); @@ -180,7 +193,6 @@ public: res->Position.z = surfaceHeight; gSceneryGroundFlags = 0; - BannerIndex bannerId = BANNER_INDEX_NULL; money32 supportsCost = 0; rct_scenery_entry* sceneryEntry = get_large_scenery_entry(_sceneryType); @@ -208,16 +220,23 @@ public: if (sceneryEntry->large_scenery.scrolling_mode != SCROLLING_MODE_NONE) { - bannerId = create_new_banner(GAME_COMMAND_FLAG_APPLY); + if (_bannerId == BANNER_INDEX_NULL) + { + log_error("No free banners available"); + return MakeResult(GA_ERROR::NO_FREE_ELEMENTS, STR_TOO_MANY_BANNERS_IN_GAME, STR_CANT_POSITION_THIS_HERE); + } - if (bannerId == BANNER_INDEX_NULL) + if (gBanners[_bannerId].type != BANNER_NULL) { log_error("No free banners available"); return MakeResult(GA_ERROR::NO_FREE_ELEMENTS, STR_CANT_POSITION_THIS_HERE); } - rct_banner* banner = &gBanners[bannerId]; - banner->flags |= BANNER_FLAG_IS_LARGE_SCENERY; + rct_banner* banner = &gBanners[_bannerId]; + banner->string_idx = STR_DEFAULT_SIGN; + banner->colour = 2; + banner->text_colour = 2; + banner->flags = BANNER_FLAG_IS_LARGE_SCENERY; banner->type = 0; banner->x = _loc.x / 32; banner->y = _loc.y / 32; @@ -278,7 +297,7 @@ public: newTileElement->clearance_height = zHigh; auto newSceneryElement = newTileElement->AsLargeScenery(); - SetNewLargeSceneryElement(*newSceneryElement, tileNum, bannerId); + SetNewLargeSceneryElement(*newSceneryElement, tileNum, _bannerId); if (tileNum == 0) { diff --git a/src/openrct2/actions/WallPlaceAction.hpp b/src/openrct2/actions/WallPlaceAction.hpp index e2c55ee387..12fd45d70d 100644 --- a/src/openrct2/actions/WallPlaceAction.hpp +++ b/src/openrct2/actions/WallPlaceAction.hpp @@ -32,6 +32,7 @@ private: int32_t _primaryColour; int32_t _secondaryColour; int32_t _tertiaryColour; + BannerIndex _bannerId{ BANNER_INDEX_NULL }; public: WallPlaceAction() @@ -47,6 +48,14 @@ public: , _secondaryColour(secondaryColour) , _tertiaryColour(tertiaryColour) { + rct_scenery_entry* sceneryEntry = get_wall_entry(_wallType); + if (sceneryEntry != nullptr) + { + if (sceneryEntry->wall.scrolling_mode != SCROLLING_MODE_NONE) + { + _bannerId = create_new_banner(0); + } + } } uint16_t GetActionFlags() const override @@ -59,7 +68,7 @@ public: GameAction::Serialise(stream); stream << DS_TAG(_wallType) << DS_TAG(_loc) << DS_TAG(_edge) << DS_TAG(_primaryColour) << DS_TAG(_secondaryColour) - << DS_TAG(_tertiaryColour); + << DS_TAG(_tertiaryColour) << DS_TAG(_bannerId); } GameActionResult::Ptr Query() const override @@ -222,11 +231,17 @@ public: if (wallEntry->wall.scrolling_mode != SCROLLING_MODE_NONE) { - auto bannerIndex = create_new_banner(0); - - if (bannerIndex == 0xFF) + if (_bannerId == BANNER_INDEX_NULL) { - return MakeResult(GA_ERROR::NO_FREE_ELEMENTS, STR_CANT_BUILD_PARK_ENTRANCE_HERE, STR_TOO_MANY_BANNERS_IN_GAME); + log_error("Banner Index not specified."); + return MakeResult( + GA_ERROR::INVALID_PARAMETERS, STR_TOO_MANY_BANNERS_IN_GAME, STR_CANT_BUILD_PARK_ENTRANCE_HERE); + } + + if (gBanners[_bannerId].type != BANNER_NULL) + { + log_error("No free banners available"); + return MakeResult(GA_ERROR::NO_FREE_ELEMENTS, STR_CANT_BUILD_PARK_ENTRANCE_HERE); } } @@ -297,7 +312,6 @@ public: } } - BannerIndex bannerIndex = BANNER_INDEX_NULL; rct_scenery_entry* wallEntry = get_wall_entry(_wallType); if (wallEntry == nullptr) @@ -308,15 +322,24 @@ public: if (wallEntry->wall.scrolling_mode != SCROLLING_MODE_NONE) { - bannerIndex = create_new_banner(GAME_COMMAND_FLAG_APPLY); - - if (bannerIndex == 0xFF) + if (_bannerId == BANNER_INDEX_NULL) { - return MakeResult(GA_ERROR::NO_FREE_ELEMENTS, STR_CANT_BUILD_PARK_ENTRANCE_HERE, STR_TOO_MANY_BANNERS_IN_GAME); + log_error("Banner Index not specified."); + return MakeResult( + GA_ERROR::INVALID_PARAMETERS, STR_TOO_MANY_BANNERS_IN_GAME, STR_CANT_BUILD_PARK_ENTRANCE_HERE); } - rct_banner* banner = &gBanners[bannerIndex]; - banner->flags |= BANNER_FLAG_IS_WALL; + if (gBanners[_bannerId].type != BANNER_NULL) + { + log_error("No free banners available"); + return MakeResult(GA_ERROR::NO_FREE_ELEMENTS, STR_CANT_BUILD_PARK_ENTRANCE_HERE); + } + + rct_banner* banner = &gBanners[_bannerId]; + banner->string_idx = STR_DEFAULT_SIGN; + banner->colour = 2; + banner->text_colour = 2; + banner->flags = BANNER_FLAG_IS_WALL; banner->type = 0; banner->x = _loc.x / 32; banner->y = _loc.y / 32; @@ -372,9 +395,9 @@ public: } wallElement->SetEntryIndex(_wallType); - if (bannerIndex != 0xFF) + if (_bannerId != BANNER_INDEX_NULL) { - wallElement->SetBannerIndex(bannerIndex); + wallElement->SetBannerIndex(_bannerId); } if (wallEntry->wall.flags & WALL_SCENERY_HAS_TERNARY_COLOUR) From 71af88fc5c85a9d5278c0b55f2373b094b983f0f Mon Sep 17 00:00:00 2001 From: Hielke Morsink Date: Sat, 20 Apr 2019 14:47:45 +0200 Subject: [PATCH 3/3] Bump network version --- src/openrct2/network/Network.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2/network/Network.cpp b/src/openrct2/network/Network.cpp index 94e737183d..074e8c8d2d 100644 --- a/src/openrct2/network/Network.cpp +++ b/src/openrct2/network/Network.cpp @@ -31,7 +31,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 "18" +#define NETWORK_STREAM_VERSION "19" #define NETWORK_STREAM_ID OPENRCT2_VERSION "-" NETWORK_STREAM_VERSION static Peep* _pickup_peep = nullptr;