FS#6525 - Rating bonus for default economy also

Attached to Project: OpenTTD
Opened by SirkoZ (SirkoZ) - Wednesday, 07 December 2016, 12:59 GMT
Type Patch
Category Core
Status New
Assigned To No-one
Operating System All
Severity Low
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No


I suggest rating bonus for default economy also because I feel it needs it.
This task depends upon

2017-08-15: A task closure has been requested. Reason for request: AIUI, the point is that default economy is not adjusted from original TTD. Whether that's wise is a different question. IMO both provided economies suck in different ways. NewGRF can't control economy, and GS has no useful industry control. But those are different questions, and meanwhile tweaking the built-in economies is putting lipstick on a pig :) This should be closed because it doesn't meet a current OpenTTD goal. Thanks for the patch though SirkoZ.
Comment by Grzegorz DuczyƄski (adf88) - Monday, 26 December 2016, 06:32 GMT
"bonus for default economy also" - what do you mean by that? What is your patch supposed to do?
Comment by SirkoZ (SirkoZ) - Monday, 26 December 2016, 19:03 GMT
Well same as with smooth economy - if over 80% is transported, industry is even more likely to increase production.
Adds to playability.
Comment by Alberth (Alberth) - Friday, 26 May 2017, 07:18 GMT
Not sure how much sense it makes to improve non-smooth economy, but I can see the point.
@adf88: Compare smooth case with non-smooth case (industry_cmd.cpp, lines 2536-ish versus 2573-ish).

I think the 'else' branch needs to be cleaned up though, an "if (only_decrease || ..)" and a "if (!only_decrease && ..)" inside doesn't make a lot of sense there. Your patch now adds a hack rather than a proper fix.
Comment by SirkoZ (SirkoZ) - Sunday, 28 May 2017, 09:54 GMT
You have to admit, Alberth, that non-smooth portion is well-condensed and therefore might look as hacked together, however you have much inside:

Line 2572 - first if temperate Oil Wells (or other industry with only_decrease behaviour) or 1 out of 3 (as it was in TTD - I've gone through the Marcin's exe) then

again Line 2574 - if !only_decrease because you don't want to increase only_decrease industry (same as in TTD exe) and of course the
chance16 portion that I extended without adding all kinds of extra code - it should be concise and to the point, don't you agree?

Regards S/Z
Comment by Alberth (Alberth) - Wednesday, 14 June 2017, 04:42 GMT
The entire code should be easily readable, which is not the same as concise as possible. I believe the current code is too dense, too many cases folded into one.

You have to think which cases exist, and how the execution path runs for each case. If you change a statement, you have to think very careful which cases are affected. It's too easy to mess up there.
By expanding the various cases clearly, it's trivial to understand what code belongs in what case, much less room for errors.

No doubt the compiler will smash it all back into something smaller and more efficient like what we have now, but I'd like that to be a compiler problem and not mine.
Comment by SirkoZ (SirkoZ) - Thursday, 15 June 2017, 01:23 GMT
Again - no doubt it's condensed, however it's easy to comprehend.
There is no room for errors in the current code. Try expanding - oh there shall be errors. :-)
I did try and not that it can't be done, it's unnecessarily complicated in my opinion.
Comment by SirkoZ (SirkoZ) - Thursday, 15 June 2017, 01:24 GMT
In the last sentence I should have said it _becomes_ unnecessarily complicated.
Comment by SirkoZ (SirkoZ) - Tuesday, 07 November 2017, 01:22 GMT
Perhaps the simplest fix to date would be to replace this:
if (!only_decrease && (i->last_month_pct_transported[0] > PERCENT_TRANSPORTED_60) != Chance16(1, 3)) {

with this (calculate adjustment based on last_month_%_transported):
if (!only_decrease && Chance16((85 + ((i->last_month_pct_transported[0] * 2) / 3)), 256)) {

Achieves the purpose and simplifies the code. First you take 1/3 as with default line (for non-used industries) and
then add the remaining 2/3rds based on how much industry is actually used...
Comment by SirkoZ (SirkoZ) - Tuesday, 07 November 2017, 01:24 GMT
LOL - lipstick on a pig - good one. :-)
Well I tried. =)