News:

SimuTranslator
Make Simutrans speak your language.

BUG: Invalid demolishion costs of station/stop buildings.

Started by Hanczar, July 30, 2017, 08:15:49 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Hanczar

Pak.Britain-128Ex
Starting date: 1750.
Build from git revision: 2853944ed141f9f44b9672441da7a0f1691452a4 (git master, last change Sun Jul 30 17:10:06 2017 +0100)
1. It occurs on default settings either, but setting higher 'buy land' costs helps to spot it. I set it to 1000 to make game more challenging.

Steps to reproduce:
1. Put any staging post on city road.
2. Demolish post from point 1.

Result:
You have more money than before point 1. (e.g. 2000-7000 more ) ,   repeat 1 and 2 to earn unlimited money.

It doesn't matter if road belongs to me(player) of city(public), I earn money -  although in city more as land there has greater value.



git show 63a80d48d
commit 63a80d48db87930397d193127347892ac88c5dde
Author: James E. Petts <jamespetts@yahoo.com>
Date:   Tue May 16 23:06:29 2017 +0100

    FIX: The land value was not taken into account when deleting buildings
    FIX: The land value was not taken into account when building signalboxes

diff --git a/obj/gebaeude.cc b/obj/gebaeude.cc
index a516450..831fa88 100644
--- a/obj/gebaeude.cc
+++ b/obj/gebaeude.cc
@@ -1528,7 +1528,7 @@ void gebaeude_t::cleanup(player_t *player)
                // If the player does already own the building, the player is refunded the empty tile cost, as bulldozing a tile with a building
                // means that the player no longer owns the tile, and will have to pay again to purcahse it.
                const sint64 land_value = welt->get_land_value(get_pos()) * desc->get_size().x * desc->get_size().y;
-               cost = player == get_owner() ? bulldoze_cost : bulldoze_cost + land_value; // land_valueand bulldoze_cost are *both* negative numbers.
+               cost = player == get_owner() ? bulldoze_cost + abs(land_value) : bulldoze_cost; // land_valueand bulldoze_cost are *both* negative numbers.
                player_t::book_construction_costs(player, cost, get_pos().get_2d(), tile->get_desc()->get_finance_waytype());
                if(player != get_owner())
                {
diff --git a/simtool.cc b/simtool.cc
index 8cdeafb..2135449 100644
--- a/simtool.cc
+++ b/simtool.cc
@@ -5863,8 +5863,11 @@ const char* tool_signalbox_t::tool_signalbox_aux(player_t* player, koord3d pos,
        {
                cost = -welt->get_settings().cst_multiply_station * desc->get_level();
        }
+
+       cost += welt->get_land_value(pos);
       
-       if ( !player_t::can_afford(player, -cost) ) {
+       if ( !player_t::can_afford(player, -cost) )
+       {
                return NOTICE_INSUFFICIENT_FUNDS;
        }


From what I notice problem with above change is in file gebaeude.cc - it causes that    bulldoze_cost + abs(land_value)  is positive when land_value is greater than bulldoze_cost.
I'm not sure if reversing this condition - land_value is taken into account when current player own this title , previously it was taken into account when other player own this title - was really intended, but it's hard to me say how it should look.
At least reversing posted above change in this file make game again playable for me, without extra strange money inflow.

jamespetts

Thank you for the report: I think that I have fixed this now. I should be grateful if you could re-test and confirm with the latest build (to-morrow's nightly if you are not compiling yourself).
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Hanczar

Thank for quick response.

For me problem with last patch is fixed, below values which I observed just for check if such behaviour - total refund for roads, no refund for buildings - is intended:

- I build 'Unsurfaced road' it cost me 197.00 per title , removing it returns 185.75. So in general wrong placing a road is totally refunded with minor loss of 8.00 (ok).
- I build staging post - it cost me 5.00 , destroying it, cost me additional 124.00. So by wrong placing  a staging post worth of 5.00 - I lost 129.00 .
- I build 'Livestock pen' near my station, it cost 1624.50 ( 1437.50 for building, 187.00 for land) - destroying it cost additional 310.00. So wrong placing 'Livestock pen' cost in total 1934.50.
I use default Pak128-Britain.Ex settings, started 1750.

jamespetts

Thank you for that: that is most helpful. The first and third are intended (demolishing a stop should, before land value is taken into account, cost half what it costs to build it), but the second was in error and has now been fixed.

I should be grateful if you could re-test to confirm the fixes. Thank you again for your help.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.