Improved smooth_economy patch

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

therax
Engineer
Engineer
Posts: 12
Joined: 16 Apr 2005 21:17

Post by therax »

Code: Select all

            if ((i->pct_transported[j] < 153) ^ CHANCE16I(1,3,r)) 
               new -= ((RandomRange(50)+10)*old)>>8 && new >= 6; 
            if ((i->pct_transported[j] == 0) ^ CHANCE16I(1,3,r)) 
               new -= ((RandomRange(50)+10)*old)>>8 && new == (RandomRange(6)+6);
I'm not sure what you're trying to do with the logical AND operators (&&) here, but it's almost certainly not doing what you want it to do.

&& takes its left side and right side, and if they both evaluate to logical true (!= 0), returns the value 1, taking the value 0 otherwise. So both of these lines are basically:

Code: Select all

if (new >= 6)
    new -= 1;
since the first expression will always evaluate to a non-zero value.
Nanaki13
Traffic Manager
Traffic Manager
Posts: 151
Joined: 08 Jan 2005 16:08

Post by Nanaki13 »

Found another problem with this patch.
When you start servicing an industry in the middle or at the end of the month the % transported for that month is low like 20%, that lowers the production considerably and then it's hard to get it up again.
Reaction to low ratings should be slower.
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

therax wrote:

Code: Select all

            if ((i->pct_transported[j] < 153) ^ CHANCE16I(1,3,r)) 
               new -= ((RandomRange(50)+10)*old)>>8 && new >= 6; 
            if ((i->pct_transported[j] == 0) ^ CHANCE16I(1,3,r)) 
               new -= ((RandomRange(50)+10)*old)>>8 && new == (RandomRange(6)+6);
I'm not sure what you're trying to do with the logical AND operators (&&) here, but it's almost certainly not doing what you want it to do.

&& takes its left side and right side, and if they both evaluate to logical true (!= 0), returns the value 1, taking the value 0 otherwise. So both of these lines are basically:

Code: Select all

if (new >= 6)
    new -= 1;
since the first expression will always evaluate to a non-zero value.
It's certainly doing what I intended it to do, but not completely. I have changed it - now the industry is more responsive. :)
Look for the change down below. :)
Last edited by SirkoZ on 28 Apr 2005 22:21, edited 1 time in total.
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

Nanaki13 wrote:Found another problem with this patch.
When you start servicing an industry in the middle or at the end of the month the % transported for that month is low like 20%, that lowers the production considerably and then it's hard to get it up again.
Reaction to low ratings should be slower.
First you say "the industry goes BOOM" and now not fast enough? :lol:
Please - it's quite easy to get it up and, well yes - it is more difficult than before, but not too much. It's certainly better, than original, when industry went up only if the station ratings were lower than 50%.
I have tested it quite extensively.

Any better solution is welcome, however, the industry has to respond to transport, but not too late and not the one, which isn't used (chance16(i)).
therax
Engineer
Engineer
Posts: 12
Joined: 16 Apr 2005 21:17

Post by therax »

SirkoZ wrote:It's certainly doing what I intended it to do. However, if you have better solution, you can always present it.
SirkoZ wrote:Thank you for your comments - I will change them, however I have to warn you - I'm not really a programmer - more of an avid fan, so I may ask you for some help.
I'm trying to help you here, so I'll say it again: the && operator is not doing what you think it's doing. All the logical operators in C return is a true/false value, 1 or 0. All they care about for input is a true/false value, non-zero or zero.

To show that this doesn't work, I built a test program that exercises your code repeatedly. I set it for 1000 trials, where the "old" production is 128 units, and the amount transported is set to 25 (a paltry 10%). This is exercising the second clause of your if tree (that isn't actually a tree, but I digress.) You would expect that this would cause "new" to be less than "old" as the industry reduces production in response to the terrible service.

Here are the results I got:

Code: Select all

127: 168
128: 292
133:  16
134:  20
135:  21
136:  23
137:  25
138:  25
139:  21
140:  23
141:  18
142:  15
143:  20
144:  13
145:  17
146:  24
147:  30
148:  19
149:  19
150:  28
151:  24
152:  26
153:  23
154:  22
155:  15
156:  33
157:  20
So, approximately 30% of the time (292 out of 1000 trials), production stayed the same at 128. 17% of the time production went down, but only by one unit, to 127. Production never went down more than this, despite what you'd expect. The remaining 53% of the time, production actually went up to a random value between 133 and 157.

Now, does it make sense that the most likely outcome when 10% of the industry's production is being picked up, that the industry should increase production? I can provide the test code if you like. Your code isn't doing what you think it does.
It's certainly doing what I intended it to do. However, if you have better solution, you can always present it.
I'm not going to present a better solution, because wizeman already has.
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

Well here's the improved one - even better, than wizeman's (more industry which-is-used-specific): (please, test it)

Code: Select all

default: /* INDUSTRY_PRODUCTION */ 
         for (j = 0; j < 2 && i->produced_cargo[j] != 255; j++){ 
            uint32 r = Random();
            int old, new, percent; 
            int mag; 

            new = old = i->production_rate[j];                  
            if ((i->pct_transported[j] > 153) ^ CHANCE16I(1,50,r)) 
               new += ((RandomRange(50)+10)*old)>>8; 
            if ((i->pct_transported[j] < 153) ^ CHANCE16I(1,3,r)) 
               new -= ((RandomRange(50)+10)*old)>>8; 
            if ((i->pct_transported[j] == 0) ^ CHANCE16I(1,3,r)) 
               new -= ((RandomRange(50)+10)*old)>>8;

				// make sure it doesn't exceed 255 or goes below 0
				new = clamp(new, 6, 255);
				if (new == old) {
					closeit = false;
					continue;
If it's sometimes hard to get the industry up, you just fund a statue in town. :))

Here's the diff:
Attachments
sir_ind_i.diff
(1.42 KiB) Downloaded 320 times
wizeman
Engineer
Engineer
Posts: 18
Joined: 28 Jun 2004 22:53
Location: Portugal
Contact:

Post by wizeman »

SirkoZ, you're really on a trial and error kind of coding, aren't you? :P

See, that xor operator makes it a little difficult to understand, but if I'm not mistaken, your latest code increases production only 1 in 50 times,but it decreases production 1 in 3 times..
So, if I'm not mistaken, the production will decrease a lot faster than increase..

You should check the code I've posted. It does exactly what you want, but I've used a 1 in 3 chance of going the opposite way, just to make things a little unpredictable. Of course, if that's not what you want, you can replace the CHANCE(1, 3) to CHANCE(1, 10) or so (to make it more responsive), or you can even take that part out. ;)

I think it works pretty well. 8)
wizeman
Engineer
Engineer
Posts: 18
Joined: 28 Jun 2004 22:53
Location: Portugal
Contact:

Post by wizeman »

Ok, I might have read the code wrong. Here's what your code is doing:

If the transported cargo is >60%, you've got a 49/50 chance of increasing production (by a small random percentage), then a 1/3 chance of decreasing, and then another 1/3 chance of decreasing it further (actually, there's a minor detail here, but let's not go into it).

If the transported cargo is between 0% and 60%, you've got a 1/50 chance of increasing production, then a 2/3 chance of decreasing, and then another 1/3 chance of decreasing it further.

If the transported cargo is exactly 0, you've got a 1/50 chance of increasing production, then a 2/3 chance of decreasing, and then another 2/3 chance of decreasing it further.

I think that's a little weird actually, and doesn't always do what you want.. :shock:

Just for reference, here's what my code does:

If the transported cargo is >60%, there's a 2/3 chance of increasing and a 1/3 chance of decreasing.
If the transported cargo is <60%, there's a 2/3 chance of decreasing and a 1/3 chance of increasing.

A little simpler and more effective ;)
Of course, like I said, if you put CHANCE(1,10), there would be a 9/10 chance of doing the right thing instead of a 2/3 chance.
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

Winks aside - the problem with your code is, that it's not responsive enough (for specific industry). Have you even tried my latest? I know I have yours and it's so random, there's really no response.
I don't see what's wrong with industry decreasing, when it's not used and it always increases, if it in fact is being used. With this combination you get a nice response to station ratings and to cargo waiting on the station (though it's not included in code :) ).

Anyone else any thoughts, Darkvater, anyone?

P.S.: I might delete the last line (%=0), but that just might make it too high again, even though not transported...
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

Final patch (best):
It also corrects the maize farm behaviour - now it makes fields, so if they are destroyed in the counstruction, the farm has a place to grow it's "cargo". :wink:
Attachments
smooth_economy_improved.patch
SIR_industries_patch
(1.62 KiB) Downloaded 365 times
wizeman
Engineer
Engineer
Posts: 18
Joined: 28 Jun 2004 22:53
Location: Portugal
Contact:

Post by wizeman »

Code: Select all

if ((i->pct_transported[j] < 153) & (i->pct_transported[j] > 0) & CHANCE16I(1,3,r))
Be careful, the & operator is a bitwise "and", not the logical "and" operator.

What you want to use is the && operator :P
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

If anything is to be fixed, the developers will say so.

You go play with your patch if you don't like smooth industry. :roll:
wizeman
Engineer
Engineer
Posts: 18
Joined: 28 Jun 2004 22:53
Location: Portugal
Contact:

Post by wizeman »

Look, I'm just trying to help you here, I am already pretty happy with my patch.
I was just pointing out your mistake, because clearly (and as you said before) you don't have much experience programming (at least in C).

If you don't understand what I said, go read this page (for example).
I will quote a relevant section, just to make it clearer:
One common mistake made by programmers is to confuse the logical operators, &&, || and ! with the bitwise operators &, |, and ~.
Don't worry, the code will behave the same as it's now, because the comparisons (<, >) and the CHANCE16I() macro only return 0 or 1.
Trust me on this, ok? :P

You see, there's a very big difference between making code that works, and making good code.
That's what Computer Science is all about!
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

I'm sorry - I was too hasty with my reply and I didn't really have the time to do the testing before - when I read you post and it made me a bit upset.

Thanky you for your explanation - I will change the code right away and test it. I just hope the developers take me seriously after all this "improving". :D

I especially like your statement
You see, there's a very big difference between making code that works, and making good code.
That's what Computer Science is all about!
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

OK - now I have changed the "connected" lines (station ratings > 0) both to Chance16I (1,50) and all the & into && and it's all OK - even more - it works so nicely now - increases come randomly across a longer period of time - one could say it's even more realistic. Very nice indeed. :D
Of course random up's and down's of unused industry are still quite often (33%), but they stay nicely around 100 or 150 units.

Here is the code and patch for the latest source (same as for 0.3.6, just know, that new=change :)) ):

Code: Select all

           if ((i->pct_transported[j] > 153) && CHANCE16I(1,50,r))
				  new += ((RandomRange(50) + 10)*old) >> 8;
			  if ((i->pct_transported[j] < 153) && (i->pct_transported[j] > 0) && CHANCE16I(1,50,r))
				  new -= ((RandomRange(50) + 10)*old) >> 8;
			  if ((i->pct_transported[j] == 0) && CHANCE16I(1,6,r))
				  new += 2-(RandomRange(5));
Attachments
smooth_economy_improved.patch
(1.63 KiB) Downloaded 399 times
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

Here's the 0.3.6 OpenTTD_exe with the above patch, renamed to sirottd.exe, not to interfere with original openttd.exe.
Attachments
sirottd.zip
Improved_industry_exe_0.3.6
(399.69 KiB) Downloaded 374 times
Szenty
Engineer
Engineer
Posts: 2
Joined: 15 May 2005 09:47
Location: Budapest, Hungary

Post by Szenty »

SirkoZ, I'm impressed with your idea and I'm going to launch a server with your patch today evening. Anybody interested? :)
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

Here is the slightly improved Smooth_economy (Improved_industry) patch for 0.4.0 - code and .diff/.patch and .exe with version string 0.4.0_ind. If everyone's interested - the server is named [HUN] Improved Industry Patch only.

(my) Code (line 1703 to 1709):

Code: Select all

new = old = i->production_rate[j];
			  if ((i->pct_transported[j] > 153) && CHANCE16I(1,50,r))
				  new += ((RandomRange(50) + 10)*old) >> 8;
			  if ((i->pct_transported[j] < 153) && (i->pct_transported[j] > 0) && CHANCE16I(1,50,r) && (old > 5))
				  new -= ((RandomRange(50) + 10)*old) >> 8;
			  if ((i->pct_transported[j] == 0) && CHANCE16I(1,10,r) && (old > 6)) 
				  new += 2-(RandomRange(5));
Then here are the .patch and the .exe.
Attachments
sirottd.zip
(417.71 KiB) Downloaded 374 times
smooth_economy_improved.patch
improved_industry_diff
(1.28 KiB) Downloaded 345 times
Aloysha
Engineer
Engineer
Posts: 26
Joined: 26 May 2005 12:21
Location: Sydney, Australia

Post by Aloysha »

I just want to say, I was looking forward to putting this on my server tonight.

But I'm not really going to alienate the majority of OpenTTD players who probably don't want to dig through the forums, find the exe or update their source/compile for something which while cool isnt that major =(

so here's a request - 0.4.1, please may we have this!
Cura Internet OpenTTD
User avatar
SirkoZ
Tycoon
Tycoon
Posts: 1518
Joined: 06 Mar 2004 23:51
Location: The sunny side of Alps

Post by SirkoZ »

Sort of a BUMP + comments on workings of patch.

It would be nice if it were included in nightly already. :roll:

1.) Farm patch:

Code: Select all

if (i->type == IT_FARM || i->type == IT_FARM_2)
			MaybePlantFarmField(i);

Comment: Both - ordinary and sub-tropical (maize) farm will plant new fields, if the old ones get
destroyed by building railways etc. around the farm.

2.) Smooth_economy patch (increase/decrease conditions)

The industry in O/TTD gives cargo out to station 8 or 9 times per month (depends on DAY_TICKS
in ttd.h). These amounts of cargo are called production_rate (in the game - the production, that
is displayed in an industry window is the total_production).
Conditions for changes of production_rate (new = old = i->production_rate[j]):

A) Increase of production:

Code: Select all

if ((i->pct_transported[j] > 153) && CHANCE16I(1,50,r)) 
    new += ((RandomRange(50) + 10)*old) >> 8;

Comment: There is 2% chance of increase (from 10 to 22%) if industry gives out (to stations)
over 60% of its monthly production (pct_transported = percent of total production,
that ranges from 0-255, i.e. 255 = 100%).

B) Decrease of production:

Code: Select all

if ((i->pct_transported[j] < 153) && (i->pct_transported[j] > 0) && CHANCE16I(1,50,r) && (old > 5)) 
    new -= ((RandomRange(50) + 10)*old) >> 8; 

Comment: There is 2% chance of decrease (from 10 to 22%) if industry gives out less than 60%
and more than 0% of its monthly production.
"Safety switch" (old > 5):
Old production_rate has to be more than 5 or the industry will no longer
increase with new production_rate being 4 or less than 4.

C) The industry is not used by anyone (waiting for better times :-))

Code: Select all

 if ((i->pct_transported[j] == 0) && CHANCE16I(1,10,r) && (old > 6)) 
    new += 2-(RandomRange(5));

Comment: There is a 10% chance, that production_rate changes each month [-3, +2] (units),
if noone uses the industry. Decrease is a bit more favoured over the increase, though in game
it works quite randomly and also quite balanced.
Again - there is a "safety switch" (old > 6) - if old production was under 6 and production_rate
decreased to 4 or below (units), it would - again - not increase anymore.
Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: Bing [Bot] and 13 guests