Disable "Repay loan" if funds are insufficient

[Graphite]

I've been studying the openTTD code for a few days now and, though I still have a lot of puzzling to do, I have figured out a little bit of how things work. To see whether I could affect the game at all I decided to fix something that has been bothering me for a long time, namely that the "repay loan" button stays enabled even if you don't have the funds to repay it. I know it's nothing glorious or ground-breaking (in fact I only changed one line of code), but it's one less annoyance in the game (as far as I'm concerned).

P.S. I'll post this on sourceforge as soon as I'm able to. I forgot my username there and unfortunately their "forgot my username?" service is currently down.

EDIT: Placed the new version in the first post.

[Darkvater]

Nice and clean patch, although unneeded

Can you tell me why you find it annoying? It's not like it's bothering anyone with being enabled. It is not saying "Oh, click me, please, click me!"

[MeusH]

Very good work We need more people like you fixing so-called "small bugs", which exist in infinite amounts

[Alltaken]

It's not like it's bothering anyone with being enabled.

really?

Alltaken

[Pilatus]

players are lazy.. So I like it.

[Graphite]

Nice and clean patch, although unneeded

Can you tell me why you find it annoying? It's not like it's bothering anyone with being enabled. It is not saying "Oh, click me, please, click me!"

Actually, when a button like that is enabled it does say "Oh, click me, please, click me!". Obviously you want to keep your loan as minimal as possible and, if that button is enabled, it seems to suggest that you can in fact make your loan less. Only upon either manually checking your current finances or upon actually clicking it will you find out that it is in fact not possible.

You can also think of it the other way around. If you have the window open and the "repay loan" button suddenly becomes available it tells you that "Yes, you've made enough money to repay part of your loan again!"

Anyhow... from my computer science background I have learned that a button should be disabled if it can't do what it should do at that point in time. The button already did so when you have fully repayed your loan. I just made it does so when you don't have enough money as well.

[MeusH]

Actually, bank taxes your loan only if loan > your money.
Also, the loan tax doesn't change if you've borrowed $10000 or $10000000, if your bank balace is $100000000000.

This way I'm not bothering with "Repay" button for all the game, because I know I won't be taxed having billions of dollars.

However, OTTD needs these patches, especially if there is no way for this patch to cause bugs. Merge it to the trunk now

[Born Acorn]

repaying your loan = more score. Thats why I do it in Multiplayer. SIngleplayer, Im less inclined to.

Nice patch btw. Wouldn't mind seeing it committed.

[Graphite]

Actually, bank taxes your loan only if loan > your money.
Also, the loan tax doesn't change if you've borrowed $10000 or $10000000, if your bank balace is $100000000000.

Hmm... I'll have to keep that in mind in future games, though the whole idea behind that seems a bit dodgy. The way I see it having a loan means that you pay interest, regardless of how much money you might have elsewhere. Ah well. I still say it makes the interface more consistent, so I stand behind my patch.

[EDIT] I decided to test this to see whether it was true. I started a new game, took out a big loan, then just fast-forwarded without using any of the money. First of all I noticed that I seem to pay 25 pounds a month for no particular reason (it's listed under other). Also, at the end of the year, I had paid 2964 pounds of loan interest. Considering my loan was 100,000.- and the interest rate 3%, I had expected to see 3000 here. So I have two questions now:

1. What is that 25 pound charge each month? (I figured this out, see below)
2. Why are the taxes 2964, instead of 3000?

[EDIT 2] I had a look through the code to see whether I could figure those two questions out myself. I've concluded that question one comes from the following snippet of code in PlayersPayInterest() in economy.c.

Code: Select all

SET_EXPENSES_TYPE(EXPENSES_OTHER);
SubtractMoneyFromPlayer(_price.station_value >> 2);

Unfortunately now I'm just wondering why this bit is here and why exactly the station-value (which is 100) divided by 4 is charged on your 'other' account?

As for question 2. I'm still working on that one.

[Graphite]

Right... I still don't know why that odd bit of other expenses is there, but I did figure out what was causing the discrepancy in the interest calculations. The fixed point calculation of the interest amount had an accuracy of 16 bits, which was not nearly enough to represent the 1/1200 that it needed to use. I upped this to 20 bits of accuracy and the calculations are a lot more accurate now. I have also placed a comment in the code there explaining how the calculation of the interest was done, because it took me a while to figure out what was happening.

I've added this fix to my earlier patch. The new version of it is attached to this message.

[Graphite]

I know, I know... Third post in a row in this thread, but they're all separate concerns. I was wondering about the following bit of code this time:

Code: Select all

static inline int64 BIGMULS(int32 a, int32 b) {
    return (int32)(((int64)(a) * (int64)(b)));
}

From what I gather it's supposed to return an int64, but for some reason it gets cast to an int32 before that is done. I'd imagine that this might cause problems, because it might truncate large values while users of it don't expect it.

[webfreakz.nl]

Graphite, could you please add the latest patch to the first post in this topic?

Thx

[Paul Nieuwkamp]

Code: Select all

SET_EXPENSES_TYPE(EXPENSES_OTHER);
SubtractMoneyFromPlayer(_price.station_value >> 2);

Unfortunately now I'm just wondering why this bit is here and why exactly the station-value (which is 100) divided by 4 is charged on your 'other' account?

As for question 2. I'm still working on that one.

Probably to bankrupt you if you do just that: nothing

25 quid a month isn't much, but even if you repay your loan before you got billed interest, it's enough to bankrupt you your first year. (Hmm, I don't know if you go bankrupt with negative funds or if it actually has to be below $max_loan)

Edit: Damn, that was a very old thread :X Sorry

[Juhl]

First, sorry to bump this old thread

Like Graphite I'm also very bothered by the un-disabled (nice word by the way ) loan buttons. So instead of fire that very annoying pop-up, I have made some code that;

• disables the borrow button if there aren't any more money to borrow
• disables the repay button if you haven't got enough money to repay

There has been some changes to the code since Graphite's patch, but this should work with r10044.

[benc]

Thanks for the patch! IMHO, half of it is useful.

disables the borrow button if there aren't any more money to borrow

This is a useful tweak and there's no reason it shouldn't go in trunk.

disables the repay button if you haven't got enough money to repay

I'm not sold on this half of the patch, though. All other UI buttons stay enabled even when you don't have enough money to perform the action. E.g., raise land or build vehicle. Consistency is important.

Also, if you're going to the trouble of replacing the hard-coded 10000 with a symbol, do it globally! The value is also used in misc_cmd.cpp.

[Juhl]

Thanks for the patch! IMHO, half of it is useful.

disables the borrow button if there aren't any more money to borrow

This is a useful tweak and there's no reason it shouldn't go in trunk.

disables the repay button if you haven't got enough money to repay

I'm not sold on this half of the patch, though. All other UI buttons stay enabled even when you don't have enough money to perform the action. E.g., raise land or build vehicle. Consistency is important.

Also, if you're going to the trouble of replacing the hard-coded 10000 with a symbol, do it globally! The value is also used in misc_cmd.cpp.

Took your advise and modified the patch, it's now in trunk (r10158).