Fix: park loan is clamped to a 32-bit integer (#19025)

data/language/en-GB.txt

@@ -3646,6 +3646,7 @@ STR_6540    :{WINDOW_COLOUR_2}Special thanks to the following companies for allo
 STR_6541    :{WINDOW_COLOUR_2}Rocky Mountain Construction Group, Josef Wiegand GmbH & Co. KG
 STR_6542    :Contributors
 STR_6543    :Contributors…
+STR_6544    :Loan cannot be negative!
 
 #############
 # Scenarios #

distribution/changelog.txt

@@ -18,6 +18,7 @@
 - Fix: [#18905] Ride Construction window theme is not applied correctly.
 - Fix: [#18911] Mini Golf station does not draw correctly from all angles.
 - Fix: [#18971] New Game does not prompt for save before quitting.
+- Fix: [#19025] Park loan behaves inconsistently with non-round and out-of-bounds values.
 - Fix: [#19026] Park loan is clamped to a 32-bit integer.
 - Fix: [#19091] [Plugin] Remote plugins in multiplayer servers do not unload properly.
 - Fix: [#19112] Clearing the last character in the Object Selection filter does not properly reset it.

src/openrct2-ui/windows/Finances.cpp

@@ -481,16 +481,29 @@ public:
         {
             case WIDX_LOAN_INCREASE:
             {
+                // If loan can be increased, do so.
+                // If not, action shows error message.
                 auto newLoan = gBankLoan + 1000.00_GBP;
+                if (gBankLoan < gMaxBankLoan)
+                {
+                    newLoan = std::min(gMaxBankLoan, newLoan);
+                }
                 auto gameAction = ParkSetLoanAction(newLoan);
                 GameActions::Execute(&gameAction);
                 break;
             }
             case WIDX_LOAN_DECREASE:
             {
-                if (gBankLoan > 0)
+                // If loan is positive, decrease it.
+                // If loan is negative, action shows error message.
+                // If loan is exactly 0, prevent error message.
+                if (gBankLoan != 0)
                 {
                     auto newLoan = gBankLoan - 1000.00_GBP;
+                    if (gBankLoan > 0)
+                    {
+                        newLoan = std::max(static_cast<money64>(0LL), newLoan);
+                    }
                     auto gameAction = ParkSetLoanAction(newLoan);
                     GameActions::Execute(&gameAction);
                 }

src/openrct2/actions/ParkSetLoanAction.cpp

@@ -42,21 +42,20 @@ GameActions::Result ParkSetLoanAction::Query() const
 {
     auto currentLoan = gBankLoan;
     auto loanDifference = currentLoan - _value;
-    if (_value > currentLoan)
+    if (_value > currentLoan && _value > gMaxBankLoan)
     {
-        if (_value > gMaxBankLoan)
+        return GameActions::Result(
-        {
+            GameActions::Status::Disallowed, STR_CANT_BORROW_ANY_MORE_MONEY, STR_BANK_REFUSES_TO_INCREASE_LOAN);
-            return GameActions::Result(
-                GameActions::Status::Disallowed, STR_CANT_BORROW_ANY_MORE_MONEY, STR_BANK_REFUSES_TO_INCREASE_LOAN);
-        }
     }
-    else
+    // FIXME: use money64 literal once it is implemented
+    if (_value < currentLoan && _value < 0)
     {
-        if (loanDifference > gCash)
+        return GameActions::Result(GameActions::Status::InvalidParameters, STR_CANT_PAY_BACK_LOAN, STR_LOAN_CANT_BE_NEGATIVE);
-        {
+    }
-            return GameActions::Result(
+    if (loanDifference > gCash)
-                GameActions::Status::InsufficientFunds, STR_CANT_PAY_BACK_LOAN, STR_NOT_ENOUGH_CASH_AVAILABLE);
+    {
-        }
+        return GameActions::Result(
+            GameActions::Status::InsufficientFunds, STR_CANT_PAY_BACK_LOAN, STR_NOT_ENOUGH_CASH_AVAILABLE);
     }
     return GameActions::Result();
 }

src/openrct2/localisation/StringIds.h

@@ -3939,6 +3939,8 @@ enum : uint16_t
     STR_CONTRIBUTORS_WINDOW = 6542,
     STR_CONTRIBUTORS_WINDOW_BUTTON = 6543,
 
+    STR_LOAN_CANT_BE_NEGATIVE = 6544,
+
     // 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
 };