New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Commit fef8b83 broke GSTown.SetGrowthRate #7112
Comments
65,535 is TOWN_GROWTH_RATE_NONE |
Probable cause: While all town growth internal counters are scaled up by a factor of 70, the size of the integers used to hold those numbers remains the same, at 2^16 = 65536 (unit16). Solution: Increase the town growth counter variables to 24-bit or 32-bit (uint32) instead of 16-bit (uint16). This should allow for town growth rates of 30 million days. For compatibility reasons, the town growth rate should be limited to 65534 days as usual (65535 beng reserved for TOWN_GROWTN_RATE_NONE). A side effect of this change from town growth in days to ticks is that the town growth rate calculations could output a fractional number of days (i.e. be more accurate) then before commit fef8b83. This, if implemented, could theoretically raise the maximum population of a single town or city to tens of millions, from just hundreds of thousands as before. |
No when your call to GSTown.SetGrowthRate() returns false check the error (should be ERR_PRECONDITION_FAIL) and in this case growth rate is left unmodified. |
After more reading of the code there's a bug OpenTTD/src/script/api/script_town.cpp Line 175 in 654b635
DAY_TICKS should be TOWN_GROWTH_TICKS I think OpenTTD/src/script/api/script_town.cpp Line 190 in 654b635
this may be wrong too, but I'm not sure |
My previous comment is wrong, the problem is EnforcePrecondition() OpenTTD/src/script/api/script_town.cpp Line 173 in 654b635
doesn't match OpenTTD/src/script/api/script_town.hpp Line 262 in 654b635
allowing growth_rate to be at max 68819 (doesn't fit in 16bits), while it should be at max 65119 (based on the comment) The precondition check was already wrong before fef8b83 allowing up to 30996 when the comment says 30000, but was safe because still within 16bits |
Commit fef8b83 broke the way that GSTown.SetGrowthRate processes its input. Previous to this change, scripts could pass in numbers up to at least 50,000 (based on settings in Renewed City Growth), after the change, any value over 930 will cause the town growth to be set to 65,535 AND fail (return false). At the very least, this fail behavior is incorrect. It should definitely not display to the user that the value is 65,535, even if this is the internal state of the program.
Ideally, I don't see any reason why towns should be limited to growth rates of about 2.5 years. Less ideally, if the growth is going to be capped like that fail behavior should either disable growth or set it to the maximum time.
The text was updated successfully, but these errors were encountered: