From 0d8924c6b024a8076b6cad55ac8a997af6c301ea Mon Sep 17 00:00:00 2001
From: Stephan Spengler <stephanspengler93@gmail.com>
Date: Sun, 15 Jan 2023 21:54:55 +0100
Subject: [PATCH] Fix: park loan is clamped to a 32-bit integer (#19025)

---
 data/language/en-GB.txt                    |  1 +
 distribution/changelog.txt                 |  1 +
 src/openrct2-ui/windows/Finances.cpp       | 15 +++++++++++++-
 src/openrct2/actions/ParkSetLoanAction.cpp | 23 +++++++++++-----------
 src/openrct2/localisation/StringIds.h      |  2 ++
 5 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/data/language/en-GB.txt b/data/language/en-GB.txt
index 8f942dd4c2..5acf00a7f3 100644
--- a/data/language/en-GB.txt
+++ b/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 #
diff --git a/distribution/changelog.txt b/distribution/changelog.txt
index c16fc22b38..33b1aa6835 100644
--- a/distribution/changelog.txt
+++ b/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.
diff --git a/src/openrct2-ui/windows/Finances.cpp b/src/openrct2-ui/windows/Finances.cpp
index 18badc60f2..5891e0912a 100644
--- a/src/openrct2-ui/windows/Finances.cpp
+++ b/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);
                 }
diff --git a/src/openrct2/actions/ParkSetLoanAction.cpp b/src/openrct2/actions/ParkSetLoanAction.cpp
index 733a96f499..bb468ff6b8 100644
--- a/src/openrct2/actions/ParkSetLoanAction.cpp
+++ b/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::InsufficientFunds, STR_CANT_PAY_BACK_LOAN, STR_NOT_ENOUGH_CASH_AVAILABLE);
-        }
+        return GameActions::Result(GameActions::Status::InvalidParameters, STR_CANT_PAY_BACK_LOAN, STR_LOAN_CANT_BE_NEGATIVE);
+    }
+    if (loanDifference > gCash)
+    {
+        return GameActions::Result(
+            GameActions::Status::InsufficientFunds, STR_CANT_PAY_BACK_LOAN, STR_NOT_ENOUGH_CASH_AVAILABLE);
     }
     return GameActions::Result();
 }
diff --git a/src/openrct2/localisation/StringIds.h b/src/openrct2/localisation/StringIds.h
index 5522e850b3..bf0e8acd05 100644
--- a/src/openrct2/localisation/StringIds.h
+++ b/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
 };