Fix #8453: [Script] Don't truncate loan variation to 32bit #8456
Conversation
src/misc_cmd.cpp
Outdated
| * @param p1 amount to decrease the loan with, multitude of LOAN_INTERVAL. Only used when p2 == 2. | ||
| * @param p2 when 0: pays back LOAN_INTERVAL | ||
| * when 1: pays back the maximum loan permitting money (press CTRL), | ||
| * when 2: pays back the amount specified in p1 | ||
| * @param p1 higher half of amount to increase the loan with, multitude of LOAN_INTERVAL. Only used when (p2 & 3) == 2. | ||
| * @param p2 (bit 2-31) - lower half of amount (lower 2 bits assumed to be 0) | ||
| * (bit 0-1) - when 0: loans LOAN_INTERVAL | ||
| * when 1: loans the maximum loan permitting money (press CTRL), | ||
| * when 2: loans the amount specified in p1 |
There was a problem hiding this comment.
The comments are about decreasing loan, not increasing loan.
EDIT: Also the amount specified is not only in p1 anymore
| @@ -204,8 +204,10 @@ | |||
|
|
|||
| if (loan == GetLoanAmount()) return true; | |||
|
|
|||
| Money amount = abs(loan - GetLoanAmount()); | |||
|
|
|||
There was a problem hiding this comment.
| assert(amount >> 62 == 0); |
There was a problem hiding this comment.
No, that suggestion is totally wrong.
There was a problem hiding this comment.
what about
assert(GB((int64)amount, 62, 2) == 0);
There was a problem hiding this comment.
Checking highest 2 bits is useless.
There was a problem hiding this comment.
I am guessing he was trying to ensure the lower (not higher) bits were zero'd. This is pointless, as we already check that the modulo of the interval is zero, and the interval is validated to have its lower bits zero. And yes, you can make code too pedantic :D
| @@ -204,8 +204,10 @@ | |||
|
|
|||
| if (loan == GetLoanAmount()) return true; | |||
|
|
|||
| Money amount = abs(loan - GetLoanAmount()); | |||
|
|
|||
There was a problem hiding this comment.
I am guessing he was trying to ensure the lower (not higher) bits were zero'd. This is pointless, as we already check that the modulo of the interval is zero, and the interval is validated to have its lower bits zero. And yes, you can make code too pedantic :D
Motivation / Problem
Difference between requested new loan and current loan may not fit in 32 bits.
Description
Replaced p1 with higher half of loan variation.
Used free bits in p2 to pass 30 highest bits of lower half of loan variation (assuming lower 2 bits are always 0)
Limitations
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.