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

Incorrect value for "Cost to Clear" for Company Headquarters in tile inspector #6938

Closed
James103 opened this Issue Oct 11, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@James103

James103 commented Oct 11, 2018

I notice that when I use the tile inspector on my company headquarters (if I have one), then the "Cost to Clear" value is incorrect. Specifically, it shows up as £92 quadrillion instead of (for a company value of £1 million) £10,000. If I were to a actually demolish/relocate the headquarters, it would deduct £10,000 (for a company value of £1 million) from my bank balance.

What should have happened was that the "Cost to Clear" shows up as 1% of my company value for my headquarters, or "N/A" for another company's headquarters. What actually happened was it still shows "N/A" for another company's headquarters (expected), but it always shows £92 quadrillion as the "Cost to Clear" for my headquarters, regardless of my company's value.

image

To reproduce this unexpected behaviour, do the following:

  1. Build your headquarters (go into your company overview window, click "Build HQ", then click on a free space).
  2. Activate the tile inspector by clicking on the question mark at the far right of the main toolbar and selecting "Land area information" in the dropdown.
  3. Click on the headquarters you just built.
  4. A "Land Area Information" window pops up. Look at the "Cost to Clear" field in the newly opened window. It should say something like "£92,233,720,368,546,758" or the equivalent in your local game currency.
  5. Switch to another company. To do this (single player), open up the console and type "startai". Then, open up the cheat menu (CTRL + ALT + C) and change "Playing as company" to the newly started company.
  6. While playing as another company, use the tile inspector on your own headquarters. The "Cost to Clear" field should now say "N/A" as expected.

A possible cause of this bug is that the Land Area Information tool gives the "Cost to Clear" as if your company value is always £9 quintillion (64-bit signed max) instead of (for example) £1 million. If that's the case, possible solutions include:

  • A. return N/A always for "Cost to Clear" for a company headquarters (even your own)
  • B. return the proper value based on your company value * 1% if it's your own HQ, or N/A if it's not your own HQ.

I'm leaning towards the following: If the magic bulldozer cheat is enabled, use plan B. Otherwise, use plan A.

@MiguelHorta

This comment has been minimized.

Show comment
Hide comment
@MiguelHorta

MiguelHorta Oct 11, 2018

Contributor

It seems the issue is here:

OpenTTD/src/misc_gui.cpp

Lines 196 to 200 in fbfa4eb

Money old_money = c->money;
c->money = INT64_MAX;
assert(_current_company == _local_company);
CommandCost costclear = DoCommand(tile, 0, 0, DC_NONE, CMD_LANDSCAPE_CLEAR);
c->money = old_money;

I guess the logic is to raise temporarily the company value to its maximum, so no operation can fail. Anyway 1% of INT64_MAX are the 92,233,720,368,546,758 here in question.

I'm not really seasoned in the openttd code, but why can't DC_QUERY_COST flag be used here?

Contributor

MiguelHorta commented Oct 11, 2018

It seems the issue is here:

OpenTTD/src/misc_gui.cpp

Lines 196 to 200 in fbfa4eb

Money old_money = c->money;
c->money = INT64_MAX;
assert(_current_company == _local_company);
CommandCost costclear = DoCommand(tile, 0, 0, DC_NONE, CMD_LANDSCAPE_CLEAR);
c->money = old_money;

I guess the logic is to raise temporarily the company value to its maximum, so no operation can fail. Anyway 1% of INT64_MAX are the 92,233,720,368,546,758 here in question.

I'm not really seasoned in the openttd code, but why can't DC_QUERY_COST flag be used here?

@LordAro

This comment has been minimized.

Show comment
Hide comment
@LordAro

LordAro Oct 11, 2018

Member

Summary of IRC:

This bug only appears when magic bulldozer is enabled. Otherwise, the HQ is unmovable and doesn't have a removal cost associated with it

Using git blame on misc_gui.cpp, you can see that DC_NONE usage dates from before svn r1, and maybe even TTD itself (hacking the player's amount of money appears in Ludde's original cvs repo), so I'd say yes, just use DC_QUERY_COST instead

Member

LordAro commented Oct 11, 2018

Summary of IRC:

This bug only appears when magic bulldozer is enabled. Otherwise, the HQ is unmovable and doesn't have a removal cost associated with it

Using git blame on misc_gui.cpp, you can see that DC_NONE usage dates from before svn r1, and maybe even TTD itself (hacking the player's amount of money appears in Ludde's original cvs repo), so I'd say yes, just use DC_QUERY_COST instead

@MiguelHorta

This comment has been minimized.

Show comment
Hide comment
@MiguelHorta

MiguelHorta Oct 11, 2018

Contributor

@James103 Can you confirm it is only reproducible if the "magic bulldozer" is enabled?

Contributor

MiguelHorta commented Oct 11, 2018

@James103 Can you confirm it is only reproducible if the "magic bulldozer" is enabled?

@James103

This comment has been minimized.

Show comment
Hide comment
@James103

James103 Oct 11, 2018

Yes, it is only reproducible when the magic bulldozer cheat is enabled.
This is when the magic bulldozer cheat is disabled:
image

And this is when the magic bulldozer cheat is enabled:
image

James103 commented Oct 11, 2018

Yes, it is only reproducible when the magic bulldozer cheat is enabled.
This is when the magic bulldozer cheat is disabled:
image

And this is when the magic bulldozer cheat is enabled:
image

MiguelHorta added a commit to MiguelHorta/OpenTTD that referenced this issue Oct 11, 2018

Fix OpenTTD#6938: Don't change company value to perform cost estimations
These meddling kids dared to play with magic, but the ancients texts
weren't ready for that.
Refactors logic to estimate costs that dated to even before OpenTTD 0.1;

@frosch123 frosch123 closed this in 42b00c3 Oct 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment