From 7e84e0ef3f13b438e537fb83ecc859d98d25be52 Mon Sep 17 00:00:00 2001 From: Duncan Date: Sun, 13 Jun 2021 12:34:54 +0100 Subject: [PATCH] Fix #14893: Crash in MapCheckCapacityAndReorganise() --- src/openrct2/actions/BannerPlaceAction.cpp | 10 +++++----- .../actions/LargeSceneryPlaceAction.cpp | 19 ++++++------------- src/openrct2/actions/MazePlaceTrackAction.cpp | 19 ++++++------------- src/openrct2/actions/MazeSetTrackAction.cpp | 19 ++++++------------- .../actions/PlaceParkEntranceAction.cpp | 12 ++++++------ .../actions/RideEntranceExitPlaceAction.cpp | 16 ++++++++-------- .../actions/SmallSceneryPlaceAction.cpp | 10 +++++----- src/openrct2/actions/WallPlaceAction.cpp | 5 ----- 8 files changed, 42 insertions(+), 68 deletions(-) diff --git a/src/openrct2/actions/BannerPlaceAction.cpp b/src/openrct2/actions/BannerPlaceAction.cpp index 3f982a71de..c235d5891d 100644 --- a/src/openrct2/actions/BannerPlaceAction.cpp +++ b/src/openrct2/actions/BannerPlaceAction.cpp @@ -55,17 +55,17 @@ GameActions::Result::Ptr BannerPlaceAction::Query() const res->Expenditure = ExpenditureType::Landscaping; res->ErrorTitle = STR_CANT_POSITION_THIS_HERE; + if (!LocationValid(_loc)) + { + return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_POSITION_THIS_HERE); + } + if (!MapCheckCapacityAndReorganise(_loc)) { log_error("No free map elements."); return MakeResult(GameActions::Status::NoFreeElements, STR_CANT_POSITION_THIS_HERE); } - if (!LocationValid(_loc)) - { - return MakeResult(GameActions::Status::InvalidParameters, STR_CANT_POSITION_THIS_HERE); - } - auto pathElement = GetValidPathElement(); if (pathElement == nullptr) diff --git a/src/openrct2/actions/LargeSceneryPlaceAction.cpp b/src/openrct2/actions/LargeSceneryPlaceAction.cpp index dd637c86d8..51fe9f8760 100644 --- a/src/openrct2/actions/LargeSceneryPlaceAction.cpp +++ b/src/openrct2/actions/LargeSceneryPlaceAction.cpp @@ -142,12 +142,6 @@ GameActions::Result::Ptr LargeSceneryPlaceAction::Query() const } } - if (!CheckMapCapacity(sceneryEntry->tiles, totalNumTiles)) - { - log_error("No free map elements available"); - return std::make_unique(GameActions::Status::NoFreeElements); - } - uint8_t tileNum = 0; for (rct_large_scenery_tile* tile = sceneryEntry->tiles; tile->x_offset != -1; tile++, tileNum++) { @@ -200,6 +194,12 @@ GameActions::Result::Ptr LargeSceneryPlaceAction::Query() const } } + if (!CheckMapCapacity(sceneryEntry->tiles, totalNumTiles)) + { + log_error("No free map elements available"); + return std::make_unique(GameActions::Status::NoFreeElements); + } + // Force ride construction to recheck area _currentTrackSelectionFlags |= TRACK_SELECTION_FLAG_RECHECK; @@ -234,7 +234,6 @@ GameActions::Result::Ptr LargeSceneryPlaceAction::Execute() const return std::make_unique(GameActions::Status::InvalidParameters); } - uint32_t totalNumTiles = GetTotalNumTiles(sceneryEntry->tiles); int16_t maxHeight = GetMaxSurfaceHeight(sceneryEntry->tiles); if (_loc.z != 0) @@ -244,12 +243,6 @@ GameActions::Result::Ptr LargeSceneryPlaceAction::Execute() const res->Position.z = maxHeight; - if (!CheckMapCapacity(sceneryEntry->tiles, totalNumTiles)) - { - log_error("No free map elements available"); - return std::make_unique(GameActions::Status::NoFreeElements); - } - uint8_t tileNum = 0; for (rct_large_scenery_tile* tile = sceneryEntry->tiles; tile->x_offset != -1; tile++, tileNum++) { diff --git a/src/openrct2/actions/MazePlaceTrackAction.cpp b/src/openrct2/actions/MazePlaceTrackAction.cpp index dc19183047..aa394ea0ea 100644 --- a/src/openrct2/actions/MazePlaceTrackAction.cpp +++ b/src/openrct2/actions/MazePlaceTrackAction.cpp @@ -39,12 +39,6 @@ GameActions::Result::Ptr MazePlaceTrackAction::Query() const res->Position = _loc + CoordsXYZ{ 8, 8, 0 }; res->Expenditure = ExpenditureType::RideConstruction; res->ErrorTitle = STR_RIDE_CONSTRUCTION_CANT_CONSTRUCT_THIS_HERE; - if (!MapCheckCapacityAndReorganise(_loc)) - { - res->Error = GameActions::Status::NoFreeElements; - res->ErrorMessage = STR_TILE_ELEMENT_LIMIT_REACHED; - return res; - } if ((_loc.z & 0xF) != 0) { res->Error = GameActions::Status::Unknown; @@ -59,6 +53,12 @@ GameActions::Result::Ptr MazePlaceTrackAction::Query() const return res; } + if (!MapCheckCapacityAndReorganise(_loc)) + { + res->Error = GameActions::Status::NoFreeElements; + res->ErrorMessage = STR_TILE_ELEMENT_LIMIT_REACHED; + return res; + } auto surfaceElement = map_get_surface_element_at(_loc); if (surfaceElement == nullptr) { @@ -135,13 +135,6 @@ GameActions::Result::Ptr MazePlaceTrackAction::Execute() const return res; } - if (!MapCheckCapacityAndReorganise(_loc)) - { - res->Error = GameActions::Status::NoFreeElements; - res->ErrorMessage = STR_NONE; - return res; - } - uint32_t flags = GetFlags(); if (!(flags & GAME_COMMAND_FLAG_GHOST)) { diff --git a/src/openrct2/actions/MazeSetTrackAction.cpp b/src/openrct2/actions/MazeSetTrackAction.cpp index 322811c418..d2755adb1c 100644 --- a/src/openrct2/actions/MazeSetTrackAction.cpp +++ b/src/openrct2/actions/MazeSetTrackAction.cpp @@ -51,12 +51,6 @@ GameActions::Result::Ptr MazeSetTrackAction::Query() const res->Position = _loc + CoordsXYZ{ 8, 8, 0 }; res->Expenditure = ExpenditureType::RideConstruction; res->ErrorTitle = STR_RIDE_CONSTRUCTION_CANT_CONSTRUCT_THIS_HERE; - if (!MapCheckCapacityAndReorganise(_loc)) - { - res->Error = GameActions::Status::NoFreeElements; - res->ErrorMessage = STR_TILE_ELEMENT_LIMIT_REACHED; - return res; - } if ((_loc.z & 0xF) != 0 && _mode == GC_SET_MAZE_TRACK_BUILD) { res->Error = GameActions::Status::Unknown; @@ -71,6 +65,12 @@ GameActions::Result::Ptr MazeSetTrackAction::Query() const return res; } + if (!MapCheckCapacityAndReorganise(_loc)) + { + res->Error = GameActions::Status::NoFreeElements; + res->ErrorMessage = STR_TILE_ELEMENT_LIMIT_REACHED; + return res; + } auto surfaceElement = map_get_surface_element_at(_loc); if (surfaceElement == nullptr) { @@ -159,13 +159,6 @@ GameActions::Result::Ptr MazeSetTrackAction::Execute() const return res; } - if (!MapCheckCapacityAndReorganise(_loc)) - { - res->Error = GameActions::Status::NoFreeElements; - res->ErrorMessage = STR_NONE; - return res; - } - uint32_t flags = GetFlags(); if (!(flags & GAME_COMMAND_FLAG_GHOST)) { diff --git a/src/openrct2/actions/PlaceParkEntranceAction.cpp b/src/openrct2/actions/PlaceParkEntranceAction.cpp index 19ae8ead8f..cccd9e92f9 100644 --- a/src/openrct2/actions/PlaceParkEntranceAction.cpp +++ b/src/openrct2/actions/PlaceParkEntranceAction.cpp @@ -51,12 +51,6 @@ GameActions::Result::Ptr PlaceParkEntranceAction::Query() const res->Expenditure = ExpenditureType::LandPurchase; res->Position = { _loc.x, _loc.y, _loc.z }; - if (!CheckMapCapacity(3)) - { - return std::make_unique( - GameActions::Status::NoFreeElements, STR_CANT_BUILD_PARK_ENTRANCE_HERE, STR_NONE); - } - if (!LocationValid(_loc) || _loc.x <= 32 || _loc.y <= 32 || _loc.x >= (gMapSizeUnits - 32) || _loc.y >= (gMapSizeUnits - 32)) { @@ -64,6 +58,12 @@ GameActions::Result::Ptr PlaceParkEntranceAction::Query() const GameActions::Status::InvalidParameters, STR_CANT_BUILD_PARK_ENTRANCE_HERE, STR_TOO_CLOSE_TO_EDGE_OF_MAP); } + if (!CheckMapCapacity(3)) + { + return std::make_unique( + GameActions::Status::NoFreeElements, STR_CANT_BUILD_PARK_ENTRANCE_HERE, STR_NONE); + } + if (gParkEntrances.size() >= MAX_PARK_ENTRANCES) { return std::make_unique( diff --git a/src/openrct2/actions/RideEntranceExitPlaceAction.cpp b/src/openrct2/actions/RideEntranceExitPlaceAction.cpp index f60574d0c5..df2b87bf2c 100644 --- a/src/openrct2/actions/RideEntranceExitPlaceAction.cpp +++ b/src/openrct2/actions/RideEntranceExitPlaceAction.cpp @@ -50,10 +50,6 @@ GameActions::Result::Ptr RideEntranceExitPlaceAction::Query() const { auto errorTitle = _isExit ? STR_CANT_BUILD_MOVE_EXIT_FOR_THIS_RIDE_ATTRACTION : STR_CANT_BUILD_MOVE_ENTRANCE_FOR_THIS_RIDE_ATTRACTION; - if (!MapCheckCapacityAndReorganise(_loc)) - { - return MakeResult(GameActions::Status::NoFreeElements, errorTitle); - } auto ride = get_ride(_rideIndex); if (ride == nullptr) @@ -98,6 +94,10 @@ GameActions::Result::Ptr RideEntranceExitPlaceAction::Query() const return MakeResult(GameActions::Status::NotOwned, errorTitle); } + if (!MapCheckCapacityAndReorganise(_loc)) + { + return MakeResult(GameActions::Status::NoFreeElements, errorTitle); + } auto clear_z = z + (_isExit ? RideExitHeight : RideEntranceHeight); auto canBuild = MapCanConstructWithClearAt( { _loc, z, clear_z }, &map_place_non_scenery_clear_func, { 0b1111, 0 }, GetFlags()); @@ -217,16 +217,16 @@ GameActions::Result::Ptr RideEntranceExitPlaceAction::TrackPlaceQuery(const Coor { auto errorTitle = isExit ? STR_CANT_BUILD_MOVE_EXIT_FOR_THIS_RIDE_ATTRACTION : STR_CANT_BUILD_MOVE_ENTRANCE_FOR_THIS_RIDE_ATTRACTION; - if (!MapCheckCapacityAndReorganise(loc)) - { - return MakeResult(GameActions::Status::NoFreeElements, errorTitle); - } if (!gCheatsSandboxMode && !map_is_location_owned(loc)) { return MakeResult(GameActions::Status::NotOwned, errorTitle); } + if (!MapCheckCapacityAndReorganise(loc)) + { + return MakeResult(GameActions::Status::NoFreeElements, errorTitle); + } int16_t baseZ = loc.z; int16_t clearZ = baseZ + (isExit ? RideExitHeight : RideEntranceHeight); auto canBuild = MapCanConstructWithClearAt({ loc, baseZ, clearZ }, &map_place_non_scenery_clear_func, { 0b1111, 0 }, 0); diff --git a/src/openrct2/actions/SmallSceneryPlaceAction.cpp b/src/openrct2/actions/SmallSceneryPlaceAction.cpp index adb2c8743c..5aaeabc114 100644 --- a/src/openrct2/actions/SmallSceneryPlaceAction.cpp +++ b/src/openrct2/actions/SmallSceneryPlaceAction.cpp @@ -111,16 +111,16 @@ GameActions::Result::Ptr SmallSceneryPlaceAction::Query() const res->Position.z = surfaceHeight; } - if (!MapCheckCapacityAndReorganise(_loc)) - { - return std::make_unique(GameActions::Status::NoFreeElements); - } - if (!LocationValid(_loc)) { return MakeResult(GameActions::Status::InvalidParameters); } + if (!MapCheckCapacityAndReorganise(_loc)) + { + return std::make_unique(GameActions::Status::NoFreeElements); + } + if (!byte_9D8150 && (_loc.x > gMapSizeMaxXY || _loc.y > gMapSizeMaxXY)) { return std::make_unique(GameActions::Status::InvalidParameters); diff --git a/src/openrct2/actions/WallPlaceAction.cpp b/src/openrct2/actions/WallPlaceAction.cpp index 86331606b2..604c58f532 100644 --- a/src/openrct2/actions/WallPlaceAction.cpp +++ b/src/openrct2/actions/WallPlaceAction.cpp @@ -361,11 +361,6 @@ GameActions::Result::Ptr WallPlaceAction::Execute() const } } - if (!MapCheckCapacityAndReorganise(_loc)) - { - return MakeResult(GameActions::Status::NoFreeElements, STR_TILE_ELEMENT_LIMIT_REACHED); - } - if (wallEntry->scrolling_mode != SCROLLING_MODE_NONE) { if (_bannerId == BANNER_INDEX_NULL)