[Patch] More realistic loans system [Update: r14536]

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

PhilSophus
Chairman
Chairman
Posts: 776
Joined: 20 Jan 2007 12:08
Location: Germany

Re: [Patch] More realistic loans system [Update: r12164]

Post by PhilSophus »

nexsis wrote:is there like a tax like deduction on the patch?
Sorry, I don't understand what you mean. Could you elaborate on what you mean by a tax-like deduction in the context of loans.
"The bigger the island of our knowledge, the longer the shore of our ignorance" - John A. Wheeler, Physicist, 1911-2008
DaleStan
TTDPatch Developer
TTDPatch Developer
Posts: 10285
Joined: 18 Feb 2004 03:06
Contact:

Re: [Patch] More realistic loans system [Update: r12164]

Post by DaleStan »

That would require that there be a tax deduction in Patch.
To get a good answer, ask a Smart Question. Similarly, if you want a bug fixed, write a Useful Bug Report. No TTDPatch crashlog? Then follow directions.
Projects: NFORenum (download) | PlaneSet (Website) | grfcodec (download) | grfdebug.log parser
User avatar
Bilbo
Tycoon
Tycoon
Posts: 1710
Joined: 06 Jun 2007 21:07
Location: Czech Republic

Re: [Patch] More realistic loans system [Update: r12164]

Post by Bilbo »

PhilSophus wrote: The only other solution I can see is to provide small - and + buttons around the take loan and repay loan buttons, but I guess that wouldn't look very good.
I don't think they will be ugly and advantage is that people will quickly figure out what they do. Unlike the solution with SHIFT/CTRL modifier on clicks, which tend to hide the functionality a bit.
If you need something, do it yourself or it will be never done.

My patches: Extra large maps (1048576 high, 1048576 wide) (FS#1059), Vehicle + Town + Industry console commands (FS#1060), few minor patches (FS#2820, FS#1521, FS#2837, FS#2843), AI debugging facility

Other: Very large ships NewGRF, Bilbo's multiplayer patch pack v5 (for OpenTTD 0.7.3)
PhilSophus
Chairman
Chairman
Posts: 776
Joined: 20 Jan 2007 12:08
Location: Germany

Re: [Patch] More realistic loans system [Update: r12187]

Post by PhilSophus »

After a bit of experimenting and fiddling around with the GUI code :roll: I found a solution satisfying my aesthetic demands :wink: The amount for borrow/repay can now be changed separately via buttons. The <CTRL>-click functionality of maximum borrow/repay now also can be reached via these buttons (<CTRL>-click still works). Thank you to Bilbo for pushing me to that :P

See first post for details and download.
"The bigger the island of our knowledge, the longer the shore of our ignorance" - John A. Wheeler, Physicist, 1911-2008
PhilSophus
Chairman
Chairman
Posts: 776
Joined: 20 Jan 2007 12:08
Location: Germany

Re: [Patch] More realistic loans system [Update: r12795]

Post by PhilSophus »

Update to trunk (r12795) once more (see first post).
"The bigger the island of our knowledge, the longer the shore of our ignorance" - John A. Wheeler, Physicist, 1911-2008
wleader
Engineer
Engineer
Posts: 123
Joined: 18 May 2007 09:04

Re: [Patch] More realistic loans system [Update: r12795]

Post by wleader »

When I apply this patch and compile with MSVC++ Express 2005, I get the following errors below. I must admit I don't understand templates well enough to even have a clue about what the compiler doesn't like about this code. Whats odd is that overflowsafe_type.hpp isn't even modified by this patch, so I suspect that the code was already broken in the trunk, but somehow nothing called it before so it was never compiled before. But again, I really don't understand whats going on here.

src\core/overflowsafe_type.hpp(142) : error C2666: 'OverflowSafeInt<T,T_MAX,T_MIN>::operator +' : 4 overloads have similar conversions
with
[
T=int64,
T_MAX=0x7fffffffffffffff,
T_MIN=0x800000000
]
src\core/overflowsafe_type.hpp(54): could be 'OverflowSafeInt<T,T_MAX,T_MIN> OverflowSafeInt<T,T_MAX,T_MIN>::operator +(const uint) const'
with
[
T=int64,
T_MAX=0x7fffffffffffffff,
T_MIN=0x800000000
]
src\core/overflowsafe_type.hpp(53): or 'OverflowSafeInt<T,T_MAX,T_MIN> OverflowSafeInt<T,T_MAX,T_MIN>::operator +(const int) const'
with
[
T=int64,
T_MAX=0x7fffffffffffffff,
T_MIN=0x800000000
]
src\core/overflowsafe_type.hpp(52): or 'OverflowSafeInt<T,T_MAX,T_MIN> OverflowSafeInt<T,T_MAX,T_MIN>::operator +(const OverflowSafeInt<T,T_MAX,T_MIN> &) const'
with
[
T=int64,
T_MAX=0x7fffffffffffffff,
T_MIN=0x800000000
]
or 'built-in C++ operator+(int64, byte)'
while trying to match the argument list '(OverflowSafeInt<T,T_MAX,T_MIN>, byte)'
with
[
T=int64,
T_MAX=0x7fffffffffffffff,
T_MIN=0x800000000
]
..\src\economy.cpp(150) : see reference to function template instantiation 'OverflowSafeInt<T,T_MAX,T_MIN> operator +<int64,0x7fffffffffffffff,0x800000000>(byte,OverflowSafeInt<T,T_MAX,T_MIN>)' being compiled
with
[
T=int64,
T_MAX=0x7fffffffffffffff,
T_MIN=0x800000000
]


Upon further investigation, I have found that the offending line is this from the CalculateCurrentInterest function added by the patch:
return _economy.min_interest_rate + ((int64)(_economy.max_interest_rate - _economy.min_interest_rate) * p->current_loan + p->max_loan / 2 ) / p->max_loan;

Changing this to return 0; causes the errors to go away, but as that is not an acceptable solution, I need something else.
PhilSophus
Chairman
Chairman
Posts: 776
Joined: 20 Jan 2007 12:08
Location: Germany

Re: [Patch] More realistic loans system [Update: r12795]

Post by PhilSophus »

Thanks for your report.

I think, I know what the problem is and how to fix it (explicitly casting the operands to the appropriate type to remove the ambiguity). I'll do so this evening.

Are there any other places where this occurs? I'm using Linux and gcc does not complain about it. Maybe I find a compiler switch to make gcc stricter. In fact, I still do not know who is right (i.e. ISO-conforming) gcc or MSVC++ Express 2005 or both.
"The bigger the island of our knowledge, the longer the shore of our ignorance" - John A. Wheeler, Physicist, 1911-2008
User avatar
Roest
Traffic Manager
Traffic Manager
Posts: 215
Joined: 03 Apr 2008 08:18

Re: [Patch] More realistic loans system [Update: r12795]

Post by Roest »

Your changes of english.txt also create lots of warnings when compiling with other language files around. Doesn't do real harm but is pretty ugly.
PhilSophus
Chairman
Chairman
Posts: 776
Joined: 20 Jan 2007 12:08
Location: Germany

Re: [Patch] More realistic loans system [Update: r12795]

Post by PhilSophus »

Roest wrote:Your changes of english.txt also create lots of warnings when compiling with other language files around. Doesn't do real harm but is pretty ugly.
Yes, I know, but I don't see an easy way to avoid it at the moment. To show the now varying amount of money you borrow or repay on the buttons, I had to change the number or type of parameters for some strings, IIRC. That's what causes these warnings. OTTD handles this gracefully and falls back to the English string in this case (meaning you get some English buttons among the ones in your selected language). I'll have a look at that too, this evening, but can't promise to solve it.
"The bigger the island of our knowledge, the longer the shore of our ignorance" - John A. Wheeler, Physicist, 1911-2008
User avatar
Roest
Traffic Manager
Traffic Manager
Posts: 215
Joined: 03 Apr 2008 08:18

Re: [Patch] More realistic loans system [Update: r12795]

Post by Roest »

Maybe simply create two new strings and use them for your patch and leave the original untouched.
PhilSophus
Chairman
Chairman
Posts: 776
Joined: 20 Jan 2007 12:08
Location: Germany

Re: [Patch] More realistic loans system [Update: r12795]

Post by PhilSophus »

Roest wrote:Maybe simply create two new strings and use them for your patch and leave the original untouched.
Okay, that would have been the lazy way. I used a Perl one-liner to change the two affected strings in all languages. So, now you see the buttons in your language again and the warnings are also gone.

As for the compiler error with VC++ Express 2005: I suspect it is a bug in the compiler. At least applying the user-defined operators I arrived at Money::operator+(Money) which is not ambiguous at all (Money being the same as OverflowSaveInt<int64,...>).

I tried to cast all suspects to int64 and hope this avoids the bug, but since I can not reproduce the error with gcc I'm not sure. So, please test to compile with VC++ Express 2005 and report your results.

See top post for new version.
"The bigger the island of our knowledge, the longer the shore of our ignorance" - John A. Wheeler, Physicist, 1911-2008
DaleStan
TTDPatch Developer
TTDPatch Developer
Posts: 10285
Joined: 18 Feb 2004 03:06
Contact:

Re: [Patch] More realistic loans system [Update: r12795]

Post by DaleStan »

PhilSophus wrote:At least applying the user-defined operators
I'm pretty sure you can't do that. The argument list in question is "Money, byte". The available overloads are:
const Money, const uint
const Money, const int
const Money, const Money
int64, byte

All involve exactly one implicit cast. Which one should be selected?
To get a good answer, ask a Smart Question. Similarly, if you want a bug fixed, write a Useful Bug Report. No TTDPatch crashlog? Then follow directions.
Projects: NFORenum (download) | PlaneSet (Website) | grfcodec (download) | grfdebug.log parser
PhilSophus
Chairman
Chairman
Posts: 776
Joined: 20 Jan 2007 12:08
Location: Germany

Re: [Patch] More realistic loans system [Update: r12841]

Post by PhilSophus »

DaleStan wrote:
PhilSophus wrote:At least applying the user-defined operators
I'm pretty sure you can't do that. The argument list in question is "Money, byte". The available overloads are:
const Money, const uint
const Money, const int
const Money, const Money
int64, byte

All involve exactly one implicit cast. Which one should be selected?
This is all quite clear if you only read the error messages (that's why I wrote earlier, I know how to solve it), but reading the affected code I don't see such an ambiguity. Maybe, you can cure my blindness:

Code: Select all

return _economy.min_interest_rate + (int64)(_economy.max_interest_rate-_economy.min_interest_rate)*p->current_loan+p->max_loan/2)/p->max_loan;
So we have:

Code: Select all

byte + ((int64)(byte-byte)*Money + Money/int)/Money
The only sums I see are (Money+Money) and (byte+Money), the latter being defined in core/overflowsafe_type.hpp:142.

Oh, now I see: operator+(byte,OverflowSafeInt) calls OverflowSafeInt::operator+(integral type) and there are only overloads for int and uint, but not for byte, making it ambiguous. So the problem finally occurs in core/overflowsafe_type.hpp not in my code (and I looked in the wrong place).

Nevertheless, I'm still not sure, if VC++ 2005 should resolve the ambiguity in favor of one of the overloads (like gcc obviously does) or if gcc is more intelligent than the standard allows. Anyone knows, if the original code compiled cleanly in newer VC++ versions (e.g. 2008)? If it did I would assume a bug in VC++ Express 2005.

EDIT: Doing some research, it seems that according to ISO C++ standard (4.5/1) byte (which is a typedef to unsigned char) should be promoted to int and not unsigned int. So, VC++ Express 2005 is buggy in this case.
"The bigger the island of our knowledge, the longer the shore of our ignorance" - John A. Wheeler, Physicist, 1911-2008
DaleStan
TTDPatch Developer
TTDPatch Developer
Posts: 10285
Joined: 18 Feb 2004 03:06
Contact:

Re: [Patch] More realistic loans system [Update: r12841]

Post by DaleStan »

PhilSophus wrote:Doing some research, it seems that according to ISO C++ standard (4.5/1) byte (which is a typedef to unsigned char) should be promoted to int and not unsigned int.
Beg pardon? unsigned char is promoted to signed int?

If that's the case (I don't have a copy on hand to check) I think I'd agree with MS here: This is stupid. The user should be required to explicitly specify this promotion if that's what he really wants.
To get a good answer, ask a Smart Question. Similarly, if you want a bug fixed, write a Useful Bug Report. No TTDPatch crashlog? Then follow directions.
Projects: NFORenum (download) | PlaneSet (Website) | grfcodec (download) | grfdebug.log parser
wleader
Engineer
Engineer
Posts: 123
Joined: 18 May 2007 09:04

Re: [Patch] More realistic loans system [Update: r12841]

Post by wleader »

Not that I am knowledgeable about the specifics of explicit and implicit type casting, I want to let you know that the changes did clear up the compile errors in MSVC++ 2005.
PhilSophus
Chairman
Chairman
Posts: 776
Joined: 20 Jan 2007 12:08
Location: Germany

Re: [Patch] More realistic loans system [Update: r12841]

Post by PhilSophus »

DaleStan wrote:
PhilSophus wrote:Doing some research, it seems that according to ISO C++ standard (4.5/1) byte (which is a typedef to unsigned char) should be promoted to int and not unsigned int.
Beg pardon? unsigned char is promoted to signed int?
Yes, unsigned char is promoted to signed int if (and only if) signed int can represent all possible values of unsigned char (which is the case on most architectures), otherwise to unsigned int (which here isn't the case). All other conversions between integral types that are not integral promotions are called integral conversions.

According to the overload resolution rules, integral promotions have a higher rank than integral conversions, so an overload requiring an integer promotion from unsigned char to int is considered a better match than one requiring an integer conversion from unsigned char to unsigned int (13.3.3.1.1/3).

To be honest, I'm not a C++ standard expert, so that's only what I'm reading from the standard (and to be honest not even the final standard, but the final draft, which is freely available in the Internet). So, I could of course misinterpret something.
DaleStan wrote: If that's the case (I don't have a copy on hand to check) I think I'd agree with MS here: This is stupid. The user should be required to explicitly specify this promotion if that's what he really wants.
Well, the implicit conversion rules of C++ are known to be a real nightmare. That's why you have to be really careful when providing overloads for multiple but not all integral types. But I don't think that it is good style for a compiler to reject standard-conforming code with an error (it could provide a warning of this pitfall, but should accept it).

Most C++-compilers are (in their standard configuration) even more permissive than the standard (i.e. allow more than allowed by the standard).
wleader wrote: Not that I am knowledgeable about the specifics of explicit and implicit type casting, I want to let you know that the changes did clear up the compile errors in MSVC++ 2005.
Thanks for your report. Well, I casted almost every operand in this expression to int64 to be sure. So, not that unexpected :wink:

EDIT: Just for the records: I filed a bug report.
"The bigger the island of our knowledge, the longer the shore of our ignorance" - John A. Wheeler, Physicist, 1911-2008
User avatar
AlexW
Traffic Manager
Traffic Manager
Posts: 213
Joined: 20 Feb 2007 07:59
Location: Newbury, UK

Re: [Patch] More realistic loans system

Post by AlexW »

An interesting patch although, as someone who works in Financial Services, it is worth pointing out that interest rates for new companies are much higher than rates for established companies. Also, the trend tends to be that the more you borrow, the smaller the rate of interest...

Also - would a "credit crunch" type event be possible to make interest rates change? Like the current "economy goes into recession" event.
DaleStan wrote:
Bilbo wrote:Just one note: currently you can repay or borrow by small steps (10K pounds) or with ctrl do "borrow/repay max at once". But once you have some wealthy company (say, 500M pounds value), allowing to borrow 250M pounds and you want, say, borrow 60M. But - if you click ctrl + borrow, then borrowing 50% is too much, especially with high interests. And borrowing by ordinary clicks ... you need 6000 clicks.
Then borrow max, spend what you need, and repay the rest; you shouldn't lose more than 100k that way, and if you're borrowing 60M, you can afford 100k.
Ah, Dalestan, if only my customers thought like that.
Hooked on Transport Tycoon since 1994
PhilSophus
Chairman
Chairman
Posts: 776
Joined: 20 Jan 2007 12:08
Location: Germany

Re: [Patch] More realistic loans system [Update: r12841]

Post by PhilSophus »

AlexW wrote:An interesting patch although, as someone who works in Financial Services, it is worth pointing out that interest rates for new companies are much higher than rates for established companies. Also, the trend tends to be that the more you borrow, the smaller the rate of interest...
...if the risk stays the same, I guess. But giving the company 10% of its Net Asset Value is certainly lower risk than giving it 50% of NAV. And the interest increases with risk, so not that unrealistic in this aspect.

Of course, the loans system of OpenTTD (also with this patch) is completely unrealistic. I'm well aware of this, but, hey, it's a game after all, and not even a game on economy but on transport. Making it much more realistic doesn't give that much benefit from a game-play point of view, IMHO (esp. compared to the effort needed).

That is to say, I probably won't go into this direction, but if you (or anyone else) wants to do that (e.g. by adding a more sophisticated rating model based on company income, cash flow, ...) you may use whatever you need from this patch.
AlexW wrote:Also - would a "credit crunch" type event be possible to make interest rates change? Like the current "economy goes into recession" event.
Not a bad idea. I might have a look at it.
"The bigger the island of our knowledge, the longer the shore of our ignorance" - John A. Wheeler, Physicist, 1911-2008
User avatar
AlexW
Traffic Manager
Traffic Manager
Posts: 213
Joined: 20 Feb 2007 07:59
Location: Newbury, UK

Re: [Patch] More realistic loans system [Update: r12841]

Post by AlexW »

Oh I quite agree - the benefit of your patch as I see it is to make it just a bit harder on bigger companies, which might increase the difficulty slightly. Keep up the good work.
Hooked on Transport Tycoon since 1994
Anunnaki
Engineer
Engineer
Posts: 64
Joined: 16 Aug 2005 16:09
Location: Prague, Czech Republic

Re: [Patch] More realistic loans system [Update: r12841]

Post by Anunnaki »

Hi there,

can you update your PATCH to be compatible with some newest version of OpenTTD ?
I was trying to use your PATCH on latest build OpenTTD-r12932 but i have got error message "The patch seems outdated !".

Problematics files:
italian.txt (EDIT2: problem with reading/opening file due to bad UTF-8 check, appears only in Tortoise GUI win klient)
saveload.cpp
openttd.cpp

THX for reply.

EDIT1:
I finaly did it, here is version which is compatible with SVN OpenTTD_r12933 (i mean merging withouth errors).

Fixed:
1. italian.txt EDIT2: (no change)
2. saveload.cpp changed line: -extern const uint16 SAVEGAME_VERSION = 94; to -extern const uint16 SAVEGAME_VERSION = 95;
3. openttd.cpp right placing block of code (no change, only new text block position in SVN openttd.cpp code)
(PS: it is my first update/change, so please be patient, i was starting only 4 hours ago with "learning")
Last edited by Anunnaki on 07 May 2008 21:56, edited 3 times in total.
Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 16 guests