From bb34213b93dc6f4b750597d64ae96964a8ee154f Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 17 Jun 2020 22:35:13 +0100 Subject: [PATCH] MapCanConstructAt refactor (#11977) * Start using MapCanConstructAt * Fix #11675. Pass the full error message arguments on peep pickup This was causing a crash as the error message arguments were being cleared which would mean eventually a nullptr dereference would happen. --- src/openrct2/actions/GameAction.h | 6 +++++ src/openrct2/actions/MazeSetTrackAction.hpp | 11 +++++---- src/openrct2/actions/PeepPickupAction.hpp | 8 +++---- .../actions/PlaceParkEntranceAction.hpp | 5 ++-- src/openrct2/actions/WaterSetHeightAction.hpp | 5 ++-- src/openrct2/peep/Peep.cpp | 24 ++++++++++--------- src/openrct2/peep/Peep.h | 3 ++- src/openrct2/world/Map.cpp | 22 +++++++---------- src/openrct2/world/Map.h | 7 +++++- 9 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/openrct2/actions/GameAction.h b/src/openrct2/actions/GameAction.h index 12f8548041..eee65266ec 100644 --- a/src/openrct2/actions/GameAction.h +++ b/src/openrct2/actions/GameAction.h @@ -140,6 +140,12 @@ public: std::string GetErrorMessage() const; }; +class ConstructClearResult final : public GameActionResult +{ +public: + uint8_t GroundFlags{ 0 }; +}; + /** * */ diff --git a/src/openrct2/actions/MazeSetTrackAction.hpp b/src/openrct2/actions/MazeSetTrackAction.hpp index 4a48437c09..1cb1f564ab 100644 --- a/src/openrct2/actions/MazeSetTrackAction.hpp +++ b/src/openrct2/actions/MazeSetTrackAction.hpp @@ -141,21 +141,22 @@ public: res->ErrorMessage = STR_INVALID_SELECTION_OF_OBJECTS; return res; } - - if (!map_can_construct_at({ _loc.ToTileStart(), baseHeight, clearanceHeight }, { 0b1111, 0 })) + auto constructResult = MapCanConstructAt({ _loc.ToTileStart(), baseHeight, clearanceHeight }, { 0b1111, 0 }); + if (constructResult->Error != GA_ERROR::OK) { return MakeResult( - GA_ERROR::NO_CLEARANCE, res->ErrorTitle.GetStringId(), gGameCommandErrorText, gCommonFormatArgs); + GA_ERROR::NO_CLEARANCE, res->ErrorTitle.GetStringId(), constructResult->ErrorMessage.GetStringId(), + constructResult->ErrorMessageArgs.data()); } - if (gMapGroundFlags & ELEMENT_IS_UNDERWATER) + if (constructResult->GroundFlags & ELEMENT_IS_UNDERWATER) { res->Error = GA_ERROR::NO_CLEARANCE; res->ErrorMessage = STR_RIDE_CANT_BUILD_THIS_UNDERWATER; return res; } - if (gMapGroundFlags & ELEMENT_IS_UNDERGROUND) + if (constructResult->GroundFlags & ELEMENT_IS_UNDERGROUND) { res->Error = GA_ERROR::NO_CLEARANCE; res->ErrorMessage = STR_CAN_ONLY_BUILD_THIS_ABOVE_GROUND; diff --git a/src/openrct2/actions/PeepPickupAction.hpp b/src/openrct2/actions/PeepPickupAction.hpp index 474e52e367..2f8fc8309e 100644 --- a/src/openrct2/actions/PeepPickupAction.hpp +++ b/src/openrct2/actions/PeepPickupAction.hpp @@ -109,9 +109,9 @@ public: return MakeResult(GA_ERROR::UNKNOWN, STR_ERR_CANT_PLACE_PERSON_HERE); } - if (!peep->Place(TileCoordsXYZ(_loc), false)) + if (auto res2 = peep->Place(TileCoordsXYZ(_loc), false); res2->Error != GA_ERROR::OK) { - return MakeResult(GA_ERROR::UNKNOWN, STR_ERR_CANT_PLACE_PERSON_HERE, gGameCommandErrorText); + return res2; } break; default: @@ -179,9 +179,9 @@ public: break; case PeepPickupType::Place: res->Position = _loc; - if (!peep->Place(TileCoordsXYZ(_loc), true)) + if (auto res2 = peep->Place(TileCoordsXYZ(_loc), true); res2->Error != GA_ERROR::OK) { - return MakeResult(GA_ERROR::UNKNOWN, STR_ERR_CANT_PLACE_PERSON_HERE, gGameCommandErrorText); + return res2; } CancelConcurrentPickups(peep); break; diff --git a/src/openrct2/actions/PlaceParkEntranceAction.hpp b/src/openrct2/actions/PlaceParkEntranceAction.hpp index f9a766fa24..47be7cb972 100644 --- a/src/openrct2/actions/PlaceParkEntranceAction.hpp +++ b/src/openrct2/actions/PlaceParkEntranceAction.hpp @@ -93,10 +93,11 @@ public: entranceLoc.y += CoordsDirectionDelta[(_loc.direction + 1) & 0x3].y * 2; } - if (!map_can_construct_at({ entranceLoc, zLow, zHigh }, { 0b1111, 0 })) + if (auto res2 = MapCanConstructAt({ entranceLoc, zLow, zHigh }, { 0b1111, 0 }); res2->Error != GA_ERROR::OK) { return std::make_unique( - GA_ERROR::NO_CLEARANCE, STR_CANT_BUILD_PARK_ENTRANCE_HERE, gGameCommandErrorText, gCommonFormatArgs); + GA_ERROR::NO_CLEARANCE, STR_CANT_BUILD_PARK_ENTRANCE_HERE, res2->ErrorMessage.GetStringId(), + res2->ErrorMessageArgs.data()); } // Check that entrance element does not already exist at this location diff --git a/src/openrct2/actions/WaterSetHeightAction.hpp b/src/openrct2/actions/WaterSetHeightAction.hpp index 85d00605b1..bb3a0f3dcb 100644 --- a/src/openrct2/actions/WaterSetHeightAction.hpp +++ b/src/openrct2/actions/WaterSetHeightAction.hpp @@ -94,9 +94,10 @@ public: zLow = temp; } - if (!map_can_construct_at({ _coords, zLow, zHigh }, { 0b1111, 0b1111 })) + if (auto res2 = MapCanConstructAt({ _coords, zLow, zHigh }, { 0b1111, 0b1111 }); res2->Error != GA_ERROR::OK) { - return MakeResult(GA_ERROR::NO_CLEARANCE, STR_NONE, gGameCommandErrorText, gCommonFormatArgs); + return MakeResult( + GA_ERROR::NO_CLEARANCE, STR_NONE, res2->ErrorMessage.GetStringId(), res2->ErrorMessageArgs.data()); } if (surfaceElement->HasTrackThatNeedsWater()) { diff --git a/src/openrct2/peep/Peep.cpp b/src/openrct2/peep/Peep.cpp index 3f0a0fbb93..4b9eb8b489 100644 --- a/src/openrct2/peep/Peep.cpp +++ b/src/openrct2/peep/Peep.cpp @@ -14,6 +14,7 @@ #include "../Game.h" #include "../Input.h" #include "../OpenRCT2.h" +#include "../actions/GameAction.h" #include "../audio/AudioMixer.h" #include "../audio/audio.h" #include "../config/Config.h" @@ -743,8 +744,8 @@ void Peep::PickupAbort(int32_t old_x) gPickupPeepImage = UINT32_MAX; } -// Returns true when a peep can be dropped at the given location. When apply is set to true the peep gets dropped. -bool Peep::Place(const TileCoordsXYZ& location, bool apply) +// Returns GA_ERROR::OK when a peep can be dropped at the given location. When apply is set to true the peep gets dropped. +std::unique_ptr Peep::Place(const TileCoordsXYZ& location, bool apply) { auto* pathElement = map_get_path_element_at(location); TileElement* tileElement = reinterpret_cast(pathElement); @@ -754,7 +755,7 @@ bool Peep::Place(const TileCoordsXYZ& location, bool apply) } if (!tileElement) - return false; + return std::make_unique(GA_ERROR::INVALID_PARAMETERS, STR_ERR_CANT_PLACE_PERSON_HERE); // Set the coordinate of destination to be exactly // in the middle of a tile. @@ -762,18 +763,19 @@ bool Peep::Place(const TileCoordsXYZ& location, bool apply) if (!map_is_location_owned(destination)) { - gGameCommandErrorTitle = STR_ERR_CANT_PLACE_PERSON_HERE; - return false; + return std::make_unique(GA_ERROR::NOT_OWNED, STR_ERR_CANT_PLACE_PERSON_HERE); } - if (!map_can_construct_at({ destination, destination.z, destination.z + (1 * 8) }, { 0b1111, 0 })) + if (auto res = MapCanConstructAt({ destination, destination.z, destination.z + (1 * 8) }, { 0b1111, 0 }); + res->Error != GA_ERROR::OK) { - if (gGameCommandErrorText != STR_RAISE_OR_LOWER_LAND_FIRST) + if (res->ErrorMessage.GetStringId() != STR_RAISE_OR_LOWER_LAND_FIRST) { - if (gGameCommandErrorText != STR_FOOTPATH_IN_THE_WAY) + if (res->ErrorMessage.GetStringId() != STR_FOOTPATH_IN_THE_WAY) { - gGameCommandErrorTitle = STR_ERR_CANT_PLACE_PERSON_HERE; - return false; + return std::make_unique( + GA_ERROR::NO_CLEARANCE, STR_ERR_CANT_PLACE_PERSON_HERE, res->ErrorMessage.GetStringId(), + res->ErrorMessageArgs.data()); } } } @@ -797,7 +799,7 @@ bool Peep::Place(const TileCoordsXYZ& location, bool apply) } } - return true; + return std::make_unique(); } /** diff --git a/src/openrct2/peep/Peep.h b/src/openrct2/peep/Peep.h index a472867101..d39e3b9fc7 100644 --- a/src/openrct2/peep/Peep.h +++ b/src/openrct2/peep/Peep.h @@ -48,6 +48,7 @@ constexpr auto PEEP_CLEARANCE_HEIGHT = 4 * COORDS_Z_STEP; class Formatter; struct TileElement; struct Ride; +class GameActionResult; enum PeepType : uint8_t { @@ -769,7 +770,7 @@ public: // Peep void SetNextFlags(uint8_t next_direction, bool is_sloped, bool is_surface); void Pickup(); void PickupAbort(int32_t old_x); - bool Place(const TileCoordsXYZ& location, bool apply); + std::unique_ptr Place(const TileCoordsXYZ& location, bool apply); static Peep* Generate(const CoordsXYZ& coords); void RemoveFromQueue(); void RemoveFromRide(); diff --git a/src/openrct2/world/Map.cpp b/src/openrct2/world/Map.cpp index 587b87f6e1..513ff8aec3 100644 --- a/src/openrct2/world/Map.cpp +++ b/src/openrct2/world/Map.cpp @@ -1218,12 +1218,6 @@ TileElement* tile_element_insert(const CoordsXYZ& loc, int32_t occupiedQuadrants return insertedElement; } -class ConstructClearResult final : public GameActionResult -{ -public: - uint8_t GroundFlags{ 0 }; -}; - /** * * rct2: 0x0068BB18 @@ -1301,7 +1295,7 @@ void map_obstruction_set_error_text(TileElement* tileElement, GameActionResult& * ebp = clearFunc * bl = bl */ -static GameActionResult::Ptr map_can_construct_with_clear_at( +std::unique_ptr MapCanConstructWithClearAt( const CoordsXYRangedZ& pos, CLEAR_FUNC clearFunc, QuarterTile quarterTile, uint8_t flags, uint8_t crossingMode) { int32_t northZ, eastZ, baseHeight, southZ, westZ, water_height; @@ -1483,7 +1477,7 @@ bool map_can_construct_with_clear_at( const CoordsXYRangedZ& pos, CLEAR_FUNC clearFunc, QuarterTile quarterTile, uint8_t flags, money32* price, uint8_t crossingMode) { - GameActionResult::Ptr res = map_can_construct_with_clear_at(pos, clearFunc, quarterTile, flags, crossingMode); + auto res = MapCanConstructWithClearAt(pos, clearFunc, quarterTile, flags, crossingMode); if (auto message = res->ErrorMessage.AsStringId()) gGameCommandErrorText = *message; else @@ -1493,12 +1487,8 @@ bool map_can_construct_with_clear_at( { *price += res->Cost; } - auto ccr = dynamic_cast(res.get()); - if (ccr == nullptr) - { - return false; - } - gMapGroundFlags = ccr->GroundFlags; + + gMapGroundFlags = res->GroundFlags; return res->Error == GA_ERROR::OK; } @@ -1511,6 +1501,10 @@ int32_t map_can_construct_at(const CoordsXYRangedZ& pos, QuarterTile bl) return map_can_construct_with_clear_at(pos, nullptr, bl, 0, nullptr, CREATE_CROSSING_MODE_NONE); } +std::unique_ptr MapCanConstructAt(const CoordsXYRangedZ& pos, QuarterTile bl) +{ + return MapCanConstructWithClearAt(pos, nullptr, bl, 0, CREATE_CROSSING_MODE_NONE); +} /** * Updates grass length, scenery age and jumping fountains. * diff --git a/src/openrct2/world/Map.h b/src/openrct2/world/Map.h index 4cfeaea5e1..8a731c5c08 100644 --- a/src/openrct2/world/Map.h +++ b/src/openrct2/world/Map.h @@ -187,6 +187,9 @@ void map_reorganise_elements(); bool map_check_free_elements_and_reorganise(int32_t num_elements); TileElement* tile_element_insert(const CoordsXYZ& loc, int32_t occupiedQuadrants); +class GameActionResult; +class ConstructClearResult; + using CLEAR_FUNC = int32_t (*)(TileElement** tile_element, const CoordsXY& coords, uint8_t flags, money32* price); int32_t map_place_non_scenery_clear_func(TileElement** tile_element, const CoordsXY& coords, uint8_t flags, money32* price); @@ -194,6 +197,9 @@ int32_t map_place_scenery_clear_func(TileElement** tile_element, const CoordsXY& bool map_can_construct_with_clear_at( const CoordsXYRangedZ& pos, CLEAR_FUNC clearFunc, QuarterTile quarterTile, uint8_t flags, money32* price, uint8_t crossingMode); +std::unique_ptr MapCanConstructWithClearAt( + const CoordsXYRangedZ& pos, CLEAR_FUNC clearFunc, QuarterTile quarterTile, uint8_t flags, uint8_t crossingMode); +std::unique_ptr MapCanConstructAt(const CoordsXYRangedZ& pos, QuarterTile bl); int32_t map_can_construct_at(const CoordsXYRangedZ& pos, QuarterTile bl); struct tile_element_iterator @@ -253,7 +259,6 @@ TileElement* map_get_track_element_at_from_ride(const CoordsXYZ& trackPos, ride_ TileElement* map_get_track_element_at_with_direction_from_ride(const CoordsXYZD& trackPos, ride_id_t rideIndex); bool map_is_location_at_edge(const CoordsXY& loc); -class GameActionResult; void map_obstruction_set_error_text(TileElement* tileElement, GameActionResult& res); uint16_t check_max_allowable_land_rights_for_tile(const CoordsXYZ& tileMapPos);