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

@@ -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 #

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)
     {

src/openrct2/actions/StaffSetColourAction.cpp

@@ -47,7 +47,8 @@ GameActions::Result StaffSetColourAction::Query() const
     auto staffType = static_cast<StaffType>(_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>(_staffType), _colour))
+    auto res = StaffSetColour(static_cast<StaffType>(_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

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();

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;

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()

src/openrct2/entity/Staff.h

@@ -145,7 +145,7 @@ enum class EntertainerCostume : uint8_t
 extern const StringId StaffCostumeNames[static_cast<uint8_t>(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);
 

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

src/openrct2/ride/TrackDesign.cpp

@@ -1428,7 +1428,7 @@ static std::optional<GameActions::Result> 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;

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