FS#6378 - Erroneous behavior of Get/SetGrowthRate GS methods
There are lots of cases SetGrowthRate can be used and, turns out, not all of them work well. Main problem that it leads to is town growth progress (grow_counter) being lost when you switch growth (in cb for example). Sometimes it even results in town not being able to grow at all. But generally it is just a bunch of closely intertwined bugs, so instead of trying to report them separately I did a comprehensive patch and will explain what's happening for every case possible (unless I missed some :).
For illustration I will be using test results that I got, they are all in form
<initial town state> -> <SetGrowthRate argument> | <current result town state> <patched result town state>
Where town state is `<growth_rate>:<grow_counter>`
Having * after growth_rate means it has TOWN_GROW_RATE_CUSTOM flag, [N] means town is not growing and [U] indicates that town state is not updated (can happen near the end of month for example).
Switching to normal growth
32:4[U] -> NORMAL | 12:4 12:4
12:4 -> NORMAL | 12:4 12:4
32*:4 -> NORMAL | 12:4 12:4
NONE:4[N] -> NORMAL | 12:4 12:4
12:4[N] -> NORMAL | 0:4[N] UNDEF:4[N]
Everything works fine except for the last test. When town is not growing UpdateTownGrowRate doesn't recalculate growth_rate leaving it at 0. It may seem harmless as it will be updated when it starts growing, but if another growth command is issued before that happens then grow_counter gets reset (loosing building progress):
0:4[N] -> 12 | 12*:12 12*:12
So I have to introduce a special value for this (TOWN_GROW_RATE_UNDEFINED) and check for it in CmdTownGrowthRate. This is done internally and doesn't affect GS at all.
UNDEF:4[N] -> 12 | 12*:0 12*:4
UNDEF:4[N] -> NORMAL | 0:4[N] UNDEF:4[N]
12:4 -> NONE | NONE:21845[N] NONE:4[N]
NONE:4[N] -> NONE | NONE:8[N] NONE:4[N]
16*:4 -> NONE | NONE:16383[N] NONE:4[N]
UNDEF:4[N] -> NONE | NONE:8[N] NONE:4[N]
As you can see grow_counter is being recalculated to some nonsense, because CmdTownGrowthRate doesn't check for TOWN_GROW_RATE_CUSTOM_NONE and treats it as normal value in calculations. Once again it leads to loosing building progress. So I fixed that by leaving growth_counter intact.
Switching to custom growth rate
NONE:4[N] -> 32 | 33*:0 33*:4
12:4 -> 32 | 33*:11 33*:11
16*:4 -> 0 | 1*:0 0*:0
16*:4 -> 1 | 1*:0 0*:0
16*:4 -> 2 | 2*:0 1*:0
16*:4 -> 8 | 8*:2 7*:1
16*:4 -> 70 | 74*:18 73*:18
16*:4 -> 10700 | 11311*:2827 11310*:2827
31713*:31713 -> 30000 | 31714*:31714 31713*:31713
UNDEF:20[N] -> 12 | 12*:0 12*:12
1*:0 -> 96 | 101*:0 100*:0
2*:1 -> 96 | 101*:50 100*:50
First of all, and I already mentioned this in FS#5827, formula used for grow_rate conversion from days to tics is wrong. And that is particularly noticeable for small rates where town grows approximately every (growth_rate + 1) days. Also there clearly is a misconception as fastest growth speed is achieved when growth_rate is 0, not 1.
Next, for whatever reason (realism? ;) CmdTownGrowthRate actually tries to scale growth_counter (unfinished buildings progress) proportionally to growth rate. While there is nothing wrong with this on its own it seems a bit weird that this is the only case where it happens. Neither UpdateTownGrowRate nor switching from custom growth to normal does that (and that's understandable as it's kinda tricky and would require smth like saving prev growth_rate in town). Also, formula used for this scaling is also questionable. Furthermore due to values being discrete there is no clear notion of what that progress exactly is. For example (last two tests), with growth_rate 1 having growth_counter at 0 may mean any progress between 50% and 100% and for 2*:1 it is 33%-66%. Current values (100% and 50% respectively) fall into that range, so it's more or less fine. Due to all that I'm not sure what to do with this scaling, so I left it as it is. Probably would be better to just to remove it and leave grow_counter intact.
Also when switching from stopped growth (first test), grow_counter is once again "scaled" using special value. Fixed that one.
GetGrowthRate sometimes is not returning the same value that was used by SetGrowthRate (for 35, 70, 105 ...). So I fixed it to be the exact opposite of conversion I used in SetGrowthRate.
SetGrowthRate doc says days_between_town_growth <= 30000 so I relied on that and didn't design and test it with larger values. But it is not actually enforced in code, so it can break if that happens (as it could before).
Regarding GS compatibiliy I don't think this patch breaks anything. It does change TOWN_GROWTH_NORMAL value but that's quite weird if any GS rely on particular value of constant.
Also I attached some files I used for testing (too bag openttd doesn't have unit tests of its own).
This task depends upon