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
Fix #9579: Object and HQ construction is Construction cost, not Property Ma… #9673
Conversation
Hmm, this is failing the regression test. Any idea how I'd fix that? |
The failure occurs in GetQuarterlyExpenses(), which takes into account property maintenance but not construction costs, both of which this PR touches. Therefore, the regressions will need to be recorded again. |
If you run regression locally it should keep the output. |
c250b88
to
aca5e33
Compare
I know it's documented as such, but GetQuarterlyExpenses not including construction costs seems unexpected to me. Also, I question the use of that regression test, now that it's (almost) all 0s for all 24 quarters... |
I don't disagree, but I think improving regression tests would be out of scope for this PR, no? |
@@ -204,7 +204,7 @@ static CommandCost ClearTile_Object(TileIndex tile, DoCommandFlag flags); | |||
*/ | |||
CommandCost CmdBuildObject(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32 p2, const std::string &text) | |||
{ | |||
CommandCost cost(EXPENSES_PROPERTY); | |||
CommandCost cost(EXPENSES_CONSTRUCTION); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also puts "purchase land" under "construction". Intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not intended because I forgot that's an object, but I would also consider this a bug which is fixed by this PR. Currently purchasing land falls under Property Maintenance, then selling that land counts as income under Construction. Changing it to all be Construction makes better sense to me.
…t Property Maintenance (OpenTTD#9673)
…intenance
Motivation / Problem
Per #9579:
The cost of clearing objects is already counted under Construction.
Description
Building objects and moving the company HQ is now considered under Construction, not Property Maintenance.
Closes #9579
Limitations
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.