From 07028086928eb831c9d1af90ae09f411bde34cda Mon Sep 17 00:00:00 2001 From: Duncan Date: Sun, 19 Jan 2020 13:14:37 +0000 Subject: [PATCH] Use a game action result for clear at function (#10576) * Use a game action result for clear at function * Clean up callers of set_error_text * Make it compile * Make suggested changes * fix compile * Fix nullptr deref --- src/openrct2/actions/LandSetHeightAction.hpp | 15 +-- src/openrct2/actions/WallPlaceAction.hpp | 48 ++++----- src/openrct2/localisation/Localisation.h | 6 ++ src/openrct2/world/Map.cpp | 102 ++++++++++++------- src/openrct2/world/Map.h | 3 +- 5 files changed, 105 insertions(+), 69 deletions(-) diff --git a/src/openrct2/actions/LandSetHeightAction.hpp b/src/openrct2/actions/LandSetHeightAction.hpp index 4be41db2b4..87582a68a8 100644 --- a/src/openrct2/actions/LandSetHeightAction.hpp +++ b/src/openrct2/actions/LandSetHeightAction.hpp @@ -84,8 +84,9 @@ public: TileElement* tileElement = CheckTreeObstructions(); if (tileElement != nullptr) { - map_obstruction_set_error_text(tileElement); - return std::make_unique(GA_ERROR::DISALLOWED, gGameCommandErrorText); + auto res = MakeResult(GA_ERROR::DISALLOWED, STR_NONE); + map_obstruction_set_error_text(tileElement, *res); + return res; } } sceneryRemovalCost = GetSmallSceneryRemovalCost(); @@ -117,8 +118,9 @@ public: TileElement* tileElement = CheckFloatingStructures(reinterpret_cast(surfaceElement), _height); if (tileElement != nullptr) { - map_obstruction_set_error_text(tileElement); - return std::make_unique(GA_ERROR::DISALLOWED, gGameCommandErrorText); + auto res = MakeResult(GA_ERROR::DISALLOWED, STR_NONE); + map_obstruction_set_error_text(tileElement, *res); + return res; } if (!gCheatsDisableClearanceChecks) @@ -143,8 +145,9 @@ public: tileElement = CheckUnremovableObstructions(reinterpret_cast(surfaceElement), zCorner); if (tileElement != nullptr) { - map_obstruction_set_error_text(tileElement); - return std::make_unique(GA_ERROR::DISALLOWED, gGameCommandErrorText); + auto res = MakeResult(GA_ERROR::DISALLOWED, STR_NONE); + map_obstruction_set_error_text(tileElement, *res); + return res; } } auto res = std::make_unique(); diff --git a/src/openrct2/actions/WallPlaceAction.hpp b/src/openrct2/actions/WallPlaceAction.hpp index 13b0d4b8d4..4380eebef3 100644 --- a/src/openrct2/actions/WallPlaceAction.hpp +++ b/src/openrct2/actions/WallPlaceAction.hpp @@ -279,16 +279,16 @@ public: bool wallAcrossTrack = false; if (!(GetFlags() & GAME_COMMAND_FLAG_PATH_SCENERY) && !gCheatsDisableClearanceChecks) { - if (!WallCheckObstruction(wallEntry, targetHeight / 8, clearanceHeight, &wallAcrossTrack)) + auto result = WallCheckObstruction(wallEntry, targetHeight / 8, clearanceHeight, &wallAcrossTrack); + if (result->Error != GA_ERROR::OK) { - return std::make_unique( - GA_ERROR::NO_CLEARANCE, gGameCommandErrorText, gCommonFormatArgs); + return result; } } if (!map_check_free_elements_and_reorganise(1)) { - return std::make_unique(GA_ERROR::NO_FREE_ELEMENTS, gGameCommandErrorText); + return MakeResult(GA_ERROR::NO_FREE_ELEMENTS, STR_TILE_ELEMENT_LIMIT_REACHED); } res->Cost = wallEntry->wall.price; @@ -380,16 +380,16 @@ public: bool wallAcrossTrack = false; if (!(GetFlags() & GAME_COMMAND_FLAG_PATH_SCENERY) && !gCheatsDisableClearanceChecks) { - if (!WallCheckObstruction(wallEntry, targetHeight / 8, clearanceHeight, &wallAcrossTrack)) + auto result = WallCheckObstruction(wallEntry, targetHeight / 8, clearanceHeight, &wallAcrossTrack); + if (result->Error != GA_ERROR::OK) { - return std::make_unique( - GA_ERROR::NO_CLEARANCE, gGameCommandErrorText, gCommonFormatArgs); + return result; } } if (!map_check_free_elements_and_reorganise(1)) { - return std::make_unique(GA_ERROR::NO_FREE_ELEMENTS, gGameCommandErrorText); + return MakeResult(GA_ERROR::NO_FREE_ELEMENTS, STR_TILE_ELEMENT_LIMIT_REACHED); } TileElement* tileElement = tile_element_insert({ _loc.x / 32, _loc.y / 32, targetHeight / 8 }, 0b0000); @@ -603,7 +603,7 @@ private: * * rct2: 0x006E5C1A */ - bool WallCheckObstruction(rct_scenery_entry * wall, int32_t z0, int32_t z1, bool* wallAcrossTrack) const + GameActionResult::Ptr WallCheckObstruction(rct_scenery_entry * wall, int32_t z0, int32_t z1, bool* wallAcrossTrack) const { int32_t entryType, sequence; rct_scenery_entry* entry; @@ -613,8 +613,7 @@ private: gMapGroundFlags = ELEMENT_IS_ABOVE_GROUND; if (map_is_location_at_edge(_loc)) { - gGameCommandErrorText = STR_OFF_EDGE_OF_MAP; - return false; + return MakeResult(GA_ERROR::INVALID_PARAMETERS, STR_OFF_EDGE_OF_MAP); } TileElement* tileElement = map_get_first_element_at(_loc); @@ -636,24 +635,25 @@ private: int32_t direction = tileElement->GetDirection(); if (_edge == direction) { - map_obstruction_set_error_text(tileElement); - return false; + auto res = MakeResult(GA_ERROR::NO_CLEARANCE, STR_NONE); + map_obstruction_set_error_text(tileElement, *res); + return res; } continue; } if (tileElement->GetOccupiedQuadrants() == 0) continue; - + auto res = MakeResult(GA_ERROR::NO_CLEARANCE, STR_NONE); switch (elementType) { case TILE_ELEMENT_TYPE_ENTRANCE: - map_obstruction_set_error_text(tileElement); - return false; + map_obstruction_set_error_text(tileElement, *res); + return res; case TILE_ELEMENT_TYPE_PATH: if (tileElement->AsPath()->GetEdges() & (1 << _edge)) { - map_obstruction_set_error_text(tileElement); - return false; + map_obstruction_set_error_text(tileElement, *res); + return res; } break; case TILE_ELEMENT_TYPE_LARGE_SCENERY: @@ -665,8 +665,8 @@ private: int32_t direction = ((_edge - tileElement->GetDirection()) & TILE_ELEMENT_DIRECTION_MASK) + 8; if (!(tile->flags & (1 << direction))) { - map_obstruction_set_error_text(tileElement); - return false; + map_obstruction_set_error_text(tileElement, *res); + return res; } } break; @@ -674,20 +674,20 @@ private: entry = tileElement->AsSmallScenery()->GetEntry(); if (scenery_small_entry_has_flag(entry, SMALL_SCENERY_FLAG_NO_WALLS)) { - map_obstruction_set_error_text(tileElement); - return false; + map_obstruction_set_error_text(tileElement, *res); + return res; } break; case TILE_ELEMENT_TYPE_TRACK: if (!WallCheckObstructionWithTrack(wall, z0, tileElement->AsTrack(), wallAcrossTrack)) { - return false; + return res; } break; } } while (!(tileElement++)->IsLastForTile()); - return true; + return MakeResult(); } /** diff --git a/src/openrct2/localisation/Localisation.h b/src/openrct2/localisation/Localisation.h index f129ea246b..b34e387bb0 100644 --- a/src/openrct2/localisation/Localisation.h +++ b/src/openrct2/localisation/Localisation.h @@ -85,6 +85,12 @@ extern const rct_string_id DateGameShortMonthNames[MONTH_COUNT]; std::memcpy(args + offset, &value, size); } +template void constexpr set_format_arg(uint8_t* args, size_t offset, uintptr_t value) +{ + static_assert(sizeof(T) <= sizeof(uintptr_t), "Type too large"); + set_format_arg_body(args, offset, value, sizeof(T)); +} + #define set_format_arg(offset, type, value) \ do \ { \ diff --git a/src/openrct2/world/Map.cpp b/src/openrct2/world/Map.cpp index 92429c7479..c1e0a52166 100644 --- a/src/openrct2/world/Map.cpp +++ b/src/openrct2/world/Map.cpp @@ -1191,65 +1191,68 @@ TileElement* tile_element_insert(const TileCoordsXYZ& loc, int32_t occupiedQuadr return insertedElement; } +class ConstructClearResult final : public GameActionResult +{ +public: + uint8_t GroundFlags{ 0 }; +}; + /** * * rct2: 0x0068BB18 */ -void map_obstruction_set_error_text(TileElement* tileElement) +void map_obstruction_set_error_text(TileElement* tileElement, GameActionResult& res) { - rct_string_id errorStringId; Ride* ride; rct_scenery_entry* sceneryEntry; - errorStringId = STR_OBJECT_IN_THE_WAY; + res.ErrorMessage = STR_OBJECT_IN_THE_WAY; switch (tileElement->GetType()) { case TILE_ELEMENT_TYPE_SURFACE: - errorStringId = STR_RAISE_OR_LOWER_LAND_FIRST; + res.ErrorMessage = STR_RAISE_OR_LOWER_LAND_FIRST; break; case TILE_ELEMENT_TYPE_PATH: - errorStringId = STR_FOOTPATH_IN_THE_WAY; + res.ErrorMessage = STR_FOOTPATH_IN_THE_WAY; break; case TILE_ELEMENT_TYPE_TRACK: ride = get_ride(tileElement->AsTrack()->GetRideIndex()); if (ride != nullptr) { - errorStringId = STR_X_IN_THE_WAY; - ride->FormatNameTo(gCommonFormatArgs); + res.ErrorMessage = STR_X_IN_THE_WAY; + ride->FormatNameTo(res.ErrorMessageArgs.data()); } break; case TILE_ELEMENT_TYPE_SMALL_SCENERY: sceneryEntry = tileElement->AsSmallScenery()->GetEntry(); - errorStringId = STR_X_IN_THE_WAY; - set_format_arg(0, rct_string_id, sceneryEntry->name); + res.ErrorMessage = STR_X_IN_THE_WAY; + set_format_arg(res.ErrorMessageArgs.data(), 0, sceneryEntry->name); break; case TILE_ELEMENT_TYPE_ENTRANCE: switch (tileElement->AsEntrance()->GetEntranceType()) { case ENTRANCE_TYPE_RIDE_ENTRANCE: - errorStringId = STR_RIDE_ENTRANCE_IN_THE_WAY; + res.ErrorMessage = STR_RIDE_ENTRANCE_IN_THE_WAY; break; case ENTRANCE_TYPE_RIDE_EXIT: - errorStringId = STR_RIDE_EXIT_IN_THE_WAY; + res.ErrorMessage = STR_RIDE_EXIT_IN_THE_WAY; break; case ENTRANCE_TYPE_PARK_ENTRANCE: - errorStringId = STR_PARK_ENTRANCE_IN_THE_WAY; + res.ErrorMessage = STR_PARK_ENTRANCE_IN_THE_WAY; break; } break; case TILE_ELEMENT_TYPE_WALL: sceneryEntry = tileElement->AsWall()->GetEntry(); - errorStringId = STR_X_IN_THE_WAY; - set_format_arg(0, rct_string_id, sceneryEntry->name); + res.ErrorMessage = STR_X_IN_THE_WAY; + set_format_arg(res.ErrorMessageArgs.data(), 0, sceneryEntry->name); break; case TILE_ELEMENT_TYPE_LARGE_SCENERY: sceneryEntry = tileElement->AsLargeScenery()->GetEntry(); - errorStringId = STR_X_IN_THE_WAY; - set_format_arg(0, rct_string_id, sceneryEntry->name); + res.ErrorMessage = STR_X_IN_THE_WAY; + set_format_arg(res.ErrorMessageArgs.data(), 0, sceneryEntry->name); break; } - - gGameCommandErrorText = errorStringId; } /** @@ -1262,30 +1265,35 @@ void map_obstruction_set_error_text(TileElement* tileElement) * ebp = clearFunc * bl = bl */ -bool map_can_construct_with_clear_at( - const CoordsXYRangedZ& pos, CLEAR_FUNC clearFunc, QuarterTile quarterTile, uint8_t flags, money32* price, - uint8_t crossingMode) +static GameActionResult::Ptr map_can_construct_with_clear_at( + const CoordsXYRangedZ& pos, CLEAR_FUNC clearFunc, QuarterTile quarterTile, uint8_t flags, uint8_t crossingMode) { int32_t northZ, eastZ, baseHeight, southZ, westZ, water_height; northZ = eastZ = baseHeight = southZ = westZ = water_height = 0; + auto res = std::make_unique(); uint8_t slope = 0; - gMapGroundFlags = ELEMENT_IS_ABOVE_GROUND; + res->GroundFlags = ELEMENT_IS_ABOVE_GROUND; bool canBuildCrossing = false; if (pos.x >= gMapSizeUnits || pos.y >= gMapSizeUnits || pos.x < 32 || pos.y < 32) { - gGameCommandErrorText = STR_OFF_EDGE_OF_MAP; - return false; + res->Error = GA_ERROR::INVALID_PARAMETERS; + res->ErrorMessage = STR_OFF_EDGE_OF_MAP; + return res; } if (gCheatsDisableClearanceChecks) { - return true; + return res; } TileElement* tileElement = map_get_first_element_at(pos); if (tileElement == nullptr) - return false; + { + res->Error = GA_ERROR::UNKNOWN; + res->ErrorMessage = 0; + return res; + } do { if (tileElement->GetType() != TILE_ELEMENT_TYPE_SURFACE) @@ -1303,7 +1311,7 @@ bool map_can_construct_with_clear_at( water_height = tileElement->AsSurface()->GetWaterHeight() * 2 * 8; if (water_height && water_height > pos.baseZ && tileElement->GetBaseZ() < pos.clearanceZ) { - gMapGroundFlags |= ELEMENT_IS_UNDERWATER; + res->GroundFlags |= ELEMENT_IS_UNDERWATER; if (water_height < pos.clearanceZ) { goto loc_68BAE6; @@ -1313,10 +1321,12 @@ bool map_can_construct_with_clear_at( if (gParkFlags & PARK_FLAGS_FORBID_HIGH_CONSTRUCTION) { auto heightFromGround = pos.clearanceZ - tileElement->GetBaseZ(); + if (heightFromGround > (18 * COORDS_Z_STEP)) { - gGameCommandErrorText = STR_LOCAL_AUTHORITY_WONT_ALLOW_CONSTRUCTION_ABOVE_TREE_HEIGHT; - return false; + res->Error = GA_ERROR::DISALLOWED; + res->ErrorMessage = STR_LOCAL_AUTHORITY_WONT_ALLOW_CONSTRUCTION_ABOVE_TREE_HEIGHT; + return res; } } @@ -1332,8 +1342,8 @@ bool map_can_construct_with_clear_at( if (tileElement->GetBaseZ() >= pos.clearanceZ) { // loc_68BA81 - gMapGroundFlags |= ELEMENT_IS_UNDERGROUND; - gMapGroundFlags &= ~ELEMENT_IS_ABOVE_GROUND; + res->GroundFlags |= ELEMENT_IS_UNDERGROUND; + res->GroundFlags &= ~ELEMENT_IS_ABOVE_GROUND; } else { @@ -1381,7 +1391,7 @@ bool map_can_construct_with_clear_at( loc_68BABC: if (clearFunc != nullptr) { - if (!clearFunc(&tileElement, pos, flags, price)) + if (!clearFunc(&tileElement, pos, flags, &res->Cost)) { continue; } @@ -1408,27 +1418,43 @@ bool map_can_construct_with_clear_at( if (tileElement != nullptr) { - map_obstruction_set_error_text(tileElement); + map_obstruction_set_error_text(tileElement, *res); } - return false; + return res; loc_68BAE6: if (clearFunc != nullptr) { - if (!clearFunc(&tileElement, pos, flags, price)) + if (!clearFunc(&tileElement, pos, flags, &res->Cost)) { goto loc_68B9B7; } } if (tileElement != nullptr) { - gGameCommandErrorText = STR_CANNOT_BUILD_PARTLY_ABOVE_AND_PARTLY_BELOW_WATER; + res->Error = GA_ERROR::NO_CLEARANCE; + res->ErrorMessage = STR_CANNOT_BUILD_PARTLY_ABOVE_AND_PARTLY_BELOW_WATER; } - return false; + return res; } } } while (!(tileElement++)->IsLastForTile()); - return true; + return res; +} + +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); + gGameCommandErrorText = res->ErrorMessage; + std::copy(res->ErrorMessageArgs.begin(), res->ErrorMessageArgs.end(), gCommonFormatArgs); + if (price != nullptr) + { + *price = res->Cost; + } + gMapGroundFlags = dynamic_cast(res.get())->GroundFlags; + return res->Error == GA_ERROR::OK; } /** diff --git a/src/openrct2/world/Map.h b/src/openrct2/world/Map.h index 1d03f321db..83239fc88a 100644 --- a/src/openrct2/world/Map.h +++ b/src/openrct2/world/Map.h @@ -251,7 +251,8 @@ 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); -void map_obstruction_set_error_text(TileElement* tileElement); +class GameActionResult; +void map_obstruction_set_error_text(TileElement* tileElement, GameActionResult& res); uint16_t check_max_allowable_land_rights_for_tile(const CoordsXYZ& tileMapPos);