From 40a7fa70fcd017f9180c07031c8ffd14ad45dce2 Mon Sep 17 00:00:00 2001 From: Peter Froud Date: Mon, 19 Feb 2024 15:58:04 -0800 Subject: [PATCH] Fix empty error messages (#21351) * Add `STR_ERR_CANT_CHANGE_PARK_ENTRANCE_FEE` * Add `STR_ERR_TRACK_ON_THIS_TILE_NEEDS_WATER` * Return existing `GameAction::Result` * Add `STR_ERR_ACTION_INVALID_FOR_THAT_STAFF_TYPE` I am open to suggestions for a different message! Originally this was going to be STR_ERR_WRONG_STAFF_TYPE but I thought that was confusing because the game action arguments do not specify a staff type. We want it to mean "the staff ID you specified is the wrong StaffType for this game action". * Refactor `StaffSetColour()` to return `Result` * Remove unnecessary arguments when `Status` is `Ok` * Refactor `SwapTileElements()` to return `Result` Also add STR_ERR_CANT_SWAP_TILE_ELEMENT_WITH_ITSELF * Format code * Use `STR_ERR_CANT_CHANGE_PARK_ENTRANCE_FEE` in title * Format code using Github web editor * Format indentation --- data/language/en-GB.txt | 4 ++++ .../actions/ParkSetEntranceFeeAction.cpp | 2 +- src/openrct2/actions/StaffSetColourAction.cpp | 8 ++++--- src/openrct2/actions/StaffSetOrdersAction.cpp | 3 ++- src/openrct2/actions/WaterSetHeightAction.cpp | 3 ++- src/openrct2/entity/Staff.cpp | 7 ++++--- src/openrct2/entity/Staff.h | 2 +- src/openrct2/localisation/StringIds.h | 5 ++++- src/openrct2/ride/TrackDesign.cpp | 4 ++-- src/openrct2/world/TileInspector.cpp | 21 ++++++++++++------- 10 files changed, 38 insertions(+), 21 deletions(-) diff --git a/data/language/en-GB.txt b/data/language/en-GB.txt index 2d0ff78ad9..6304cf2f04 100644 --- a/data/language/en-GB.txt +++ b/data/language/en-GB.txt @@ -3683,6 +3683,10 @@ STR_6610 :Path element not found STR_6611 :Wall element not found STR_6612 :Banner element not found STR_6613 :Reload object +STR_6614 :Can’t change park entrance fee +STR_6615 :Track on this tile needs water +STR_6616 :Action invalid for that staff type +STR_6617 :Can’t swap tile element with itself ############# # Scenarios # diff --git a/src/openrct2/actions/ParkSetEntranceFeeAction.cpp b/src/openrct2/actions/ParkSetEntranceFeeAction.cpp index 683361091c..773f607717 100644 --- a/src/openrct2/actions/ParkSetEntranceFeeAction.cpp +++ b/src/openrct2/actions/ParkSetEntranceFeeAction.cpp @@ -46,7 +46,7 @@ GameActions::Result ParkSetEntranceFeeAction::Query() const bool forceFreeEntry = !ParkEntranceFeeUnlocked(); if (noMoney || forceFreeEntry) { - return GameActions::Result(GameActions::Status::Disallowed, STR_NONE, STR_NONE); + return GameActions::Result(GameActions::Status::Disallowed, STR_ERR_CANT_CHANGE_PARK_ENTRANCE_FEE, STR_NONE); } if (_fee < 0.00_GBP || _fee > MAX_ENTRANCE_FEE) { diff --git a/src/openrct2/actions/StaffSetColourAction.cpp b/src/openrct2/actions/StaffSetColourAction.cpp index 3860fccc1c..515ca77f4a 100644 --- a/src/openrct2/actions/StaffSetColourAction.cpp +++ b/src/openrct2/actions/StaffSetColourAction.cpp @@ -47,7 +47,8 @@ GameActions::Result StaffSetColourAction::Query() const auto staffType = static_cast(_staffType); if (staffType != StaffType::Handyman && staffType != StaffType::Mechanic && staffType != StaffType::Security) { - return GameActions::Result(GameActions::Status::InvalidParameters, STR_NONE, STR_NONE); + return GameActions::Result( + GameActions::Status::InvalidParameters, STR_ERR_INVALID_PARAMETER, STR_ERR_ACTION_INVALID_FOR_THAT_STAFF_TYPE); } return GameActions::Result(); } @@ -55,9 +56,10 @@ GameActions::Result StaffSetColourAction::Query() const GameActions::Result StaffSetColourAction::Execute() const { // Update global uniform colour property - if (!StaffSetColour(static_cast(_staffType), _colour)) + auto res = StaffSetColour(static_cast(_staffType), _colour); + if (res.Error != GameActions::Status::Ok) { - return GameActions::Result(GameActions::Status::InvalidParameters, STR_NONE, STR_NONE); + return res; } // Update each staff member's uniform diff --git a/src/openrct2/actions/StaffSetOrdersAction.cpp b/src/openrct2/actions/StaffSetOrdersAction.cpp index a2cdf8909c..995fe6513b 100644 --- a/src/openrct2/actions/StaffSetOrdersAction.cpp +++ b/src/openrct2/actions/StaffSetOrdersAction.cpp @@ -54,7 +54,8 @@ GameActions::Result StaffSetOrdersAction::Query() const || (staff->AssignedStaffType != StaffType::Handyman && staff->AssignedStaffType != StaffType::Mechanic)) { LOG_WARNING("Invalid game command for sprite %u", _spriteIndex); - return GameActions::Result(GameActions::Status::InvalidParameters, STR_NONE, STR_NONE); + return GameActions::Result( + GameActions::Status::InvalidParameters, STR_ERR_INVALID_PARAMETER, STR_ERR_ACTION_INVALID_FOR_THAT_STAFF_TYPE); } return GameActions::Result(); diff --git a/src/openrct2/actions/WaterSetHeightAction.cpp b/src/openrct2/actions/WaterSetHeightAction.cpp index 112f810bb9..619ad00b34 100644 --- a/src/openrct2/actions/WaterSetHeightAction.cpp +++ b/src/openrct2/actions/WaterSetHeightAction.cpp @@ -100,7 +100,8 @@ GameActions::Result WaterSetHeightAction::Query() const } if (surfaceElement->HasTrackThatNeedsWater()) { - return GameActions::Result(GameActions::Status::Disallowed, STR_NONE, STR_NONE); + return GameActions::Result( + GameActions::Status::Disallowed, STR_ERR_INVALID_PARAMETER, STR_ERR_TRACK_ON_THIS_TILE_NEEDS_WATER); } res.Cost = 250; diff --git a/src/openrct2/entity/Staff.cpp b/src/openrct2/entity/Staff.cpp index 56cd43390e..13ce5f1e7f 100644 --- a/src/openrct2/entity/Staff.cpp +++ b/src/openrct2/entity/Staff.cpp @@ -1007,7 +1007,7 @@ colour_t StaffGetColour(StaffType staffType) } } -bool StaffSetColour(StaffType staffType, colour_t value) +GameActions::Result StaffSetColour(StaffType staffType, colour_t value) { auto& gameState = GetGameState(); switch (staffType) @@ -1022,9 +1022,10 @@ bool StaffSetColour(StaffType staffType, colour_t value) gameState.StaffSecurityColour = value; break; default: - return false; + return GameActions::Result( + GameActions::Status::InvalidParameters, STR_ERR_INVALID_PARAMETER, STR_ERR_ACTION_INVALID_FOR_THAT_STAFF_TYPE); } - return true; + return GameActions::Result(); } uint32_t StaffGetAvailableEntertainerCostumes() diff --git a/src/openrct2/entity/Staff.h b/src/openrct2/entity/Staff.h index 2e576dee29..13815ac954 100644 --- a/src/openrct2/entity/Staff.h +++ b/src/openrct2/entity/Staff.h @@ -145,7 +145,7 @@ enum class EntertainerCostume : uint8_t extern const StringId StaffCostumeNames[static_cast(EntertainerCostume::Count)]; colour_t StaffGetColour(StaffType staffType); -bool StaffSetColour(StaffType staffType, colour_t value); +GameActions::Result StaffSetColour(StaffType staffType, colour_t value); uint32_t StaffGetAvailableEntertainerCostumes(); int32_t StaffGetAvailableEntertainerCostumeList(EntertainerCostume* costumeList); diff --git a/src/openrct2/localisation/StringIds.h b/src/openrct2/localisation/StringIds.h index fe80d6e4b3..69514c07ac 100644 --- a/src/openrct2/localisation/StringIds.h +++ b/src/openrct2/localisation/StringIds.h @@ -4023,8 +4023,11 @@ enum : uint16_t STR_ERR_PATH_ELEMENT_NOT_FOUND = 6610, STR_ERR_WALL_ELEMENT_NOT_FOUND = 6611, STR_ERR_BANNER_ELEMENT_NOT_FOUND = 6612, - STR_RELOAD_OBJECT_TIP = 6613, + STR_ERR_CANT_CHANGE_PARK_ENTRANCE_FEE = 6614, + STR_ERR_TRACK_ON_THIS_TILE_NEEDS_WATER = 6615, + STR_ERR_ACTION_INVALID_FOR_THAT_STAFF_TYPE = 6616, + STR_ERR_CANT_SWAP_TILE_ELEMENT_WITH_ITSELF = 6617, // Have to include resource strings (from scenarios and objects) for the time being now that language is partially working /* MAX_STR_COUNT = 32768 */ // MAX_STR_COUNT - upper limit for number of strings, not the current count strings diff --git a/src/openrct2/ride/TrackDesign.cpp b/src/openrct2/ride/TrackDesign.cpp index f6cb90ef6b..c249a720c7 100644 --- a/src/openrct2/ride/TrackDesign.cpp +++ b/src/openrct2/ride/TrackDesign.cpp @@ -1428,7 +1428,7 @@ static std::optional TrackDesignPlaceEntrances( auto res = RideEntranceExitPlaceAction::TrackPlaceQuery(newCoords, false); if (res.Error != GameActions::Status::Ok) { - return GameActions::Result(GameActions::Status::InvalidParameters, STR_NONE, STR_NONE); + return res; } totalCost += res.Cost; @@ -1742,7 +1742,7 @@ static GameActions::Result TrackDesignPlaceRide(TrackDesignState& tds, TrackDesi ride.Delete(); } - auto res = GameActions::Result(GameActions::Status::Ok, STR_NONE, STR_NONE); + auto res = GameActions::Result(); res.Cost = totalCost; return res; diff --git a/src/openrct2/world/TileInspector.cpp b/src/openrct2/world/TileInspector.cpp index 2f84e3d52b..7a76295e1e 100644 --- a/src/openrct2/world/TileInspector.cpp +++ b/src/openrct2/world/TileInspector.cpp @@ -42,7 +42,7 @@ int32_t windowTileInspectorSelectedIndex = -1; using namespace OpenRCT2::TrackMetaData; namespace OpenRCT2::TileInspector { - static bool SwapTileElements(const CoordsXY& loc, int16_t first, int16_t second) + static GameActions::Result SwapTileElements(const CoordsXY& loc, int16_t first, int16_t second) { TileElement* const firstElement = MapGetNthElementAt(loc, first); TileElement* const secondElement = MapGetNthElementAt(loc, second); @@ -50,17 +50,20 @@ namespace OpenRCT2::TileInspector if (firstElement == nullptr) { LOG_ERROR("First element is out of range for the tile"); - return false; + return GameActions::Result( + GameActions::Status::InvalidParameters, STR_ERR_INVALID_PARAMETER, STR_ERR_TILE_ELEMENT_NOT_FOUND); } if (secondElement == nullptr) { LOG_ERROR("Second element is out of range for the tile"); - return false; + return GameActions::Result( + GameActions::Status::InvalidParameters, STR_ERR_INVALID_PARAMETER, STR_ERR_TILE_ELEMENT_NOT_FOUND); } if (firstElement == secondElement) { LOG_ERROR("Can't swap the element with itself"); - return false; + return GameActions::Result( + GameActions::Status::InvalidParameters, STR_ERR_INVALID_PARAMETER, STR_ERR_CANT_SWAP_TILE_ELEMENT_WITH_ITSELF); } // Swap their memory @@ -73,7 +76,7 @@ namespace OpenRCT2::TileInspector secondElement->SetLastForTile(!secondElement->IsLastForTile()); } - return true; + return GameActions::Result(); } static bool IsTileSelected(const CoordsXY& loc) @@ -181,9 +184,10 @@ namespace OpenRCT2::TileInspector { if (isExecuting) { - if (!SwapTileElements(loc, first, second)) + auto res = SwapTileElements(loc, first, second); + if (res.Error != GameActions::Status::Ok) { - return GameActions::Result(GameActions::Status::Unknown, STR_NONE, STR_NONE); + return res; } if (IsTileSelected(loc)) @@ -393,7 +397,8 @@ namespace OpenRCT2::TileInspector || (otherElement->BaseHeight == currentElement->BaseHeight && otherElement->ClearanceHeight > currentElement->ClearanceHeight))) { - if (!SwapTileElements(loc, currentId - 1, currentId)) + auto res = SwapTileElements(loc, currentId - 1, currentId); + if (res.Error != GameActions::Status::Ok) { // don't return error here, we've already ran some actions // and moved things as far as we could, the only sensible