OpenTTD

Tasklist

FS#5441 - Newobjects on river slopes - unintended behavior

Attached to Project: OpenTTD
Opened by Supercheese (Supercheese) - Thursday, 10 January 2013, 01:10 GMT
Last edited by Ingo von Borstel (planetmaker) - Sunday, 20 January 2013, 12:43 GMT
Type Bug
Category NewGRF → NewObject
Status Closed
Assigned To No-one
Operating System Windows
Severity Low
Priority Normal
Reported Version other
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

As of OTTD 1.3.0 beta1, clearing a Newobject on a river slope also destroys the river beneath it. Normally, clearing a Newobject on an unsloped water tile using the dynamite tool preserves the water underneath, and I believe this is also the intended behavior for sloped river tiles, so the current behavior seems to be a bug.

Additionally, building an object that normally has zero cost on a river slope incurs a small cost. I'm not sure where this cost comes from, and it seems to be unintended behavior.


Steps to reproduce the unintended behavior, either:

1a) Load the attached .grf in OTTD 1.3.0 beta1 and load the attached .sav

2a) Clear a Newobject (circling seagulls) on a river slope using the dynamite tool; the river beneath is destroyed.

3a) Build a Newobject on a river slope to observe a small cost, whilst building it on an unsloped water tile has no cost.


Or, alternatively:

1b) Load the attached GRF in OTTD 1.3.0 beta1

2b) Generate a map with sloped river tiles

3b) Build one of the Newobjects on a sloped river tile to observe a small cost, whilst building it on an unsloped water tile has no cost.

4b) Clear the object using the dynamite tool; the river beneath is destroyed.


In case it helps, the Newobjects in question have the following NML flags set:

OBJ_FLAG_ON_WATER, OBJ_FLAG_DRAW_WATER, OBJ_FLAG_NOT_ON_LAND, OBJ_FLAG_ANIMATED, OBJ_FLAG_NO_FOUNDATIONS

...and the autoslope and tile_check callbacks are not used.
This task depends upon

Closed by  Ingo von Borstel (planetmaker)
Sunday, 20 January 2013, 12:43 GMT
Reason for closing:  Fixed
Additional comments about closing:  In r24923. Thanks for initial patch
Comment by Supercheese (Supercheese) - Tuesday, 15 January 2013, 03:48 GMT
Ok, I'm pretty sure I've identified the river-clearing problem, it is in the MakeWaterKeepingClass function in water_cmd.cpp:

That function checks if (GetTileSlope(tile, &z) != SLOPE_FLAT) and then always sets wc = WATER_CLASS_INVALID;

Commenting out line 173:
wc = WATER_CLASS_INVALID;
stops the unintended river-destruction and cost when clearing the sloped river tile. Of course, just commenting that out is not a real fix, as there likely is a reason that line is there, but it does appear to be the source of the problem.

I'll investigate the build cost problem next.

Comment by Supercheese (Supercheese) - Tuesday, 15 January 2013, 09:58 GMT
Seems the added build cost comes from not setting the tile_check callback, as lines 244 and 253-255 of object_cmd.cpp:

if (GetTileSlope(tile, &allowed_z) != SLOPE_FLAT) allowed_z++;

...

if (callback == CALLBACK_FAILED) {
cost.AddCost(CheckBuildableTile(t, 0, allowed_z, false, false));
}


... add the observed cost for slopes as observed in the steps detailed above. Setting the tile_check callback removes the build cost, so that's not directly an OTTD bug, but a bug in the grf, I suppose.
Comment by Supercheese (Supercheese) - Tuesday, 15 January 2013, 20:28 GMT
Here's a patch that seems to resolve the only remaining issue of unintended river-destruction. Made against r24915.
Comment by Ingo von Borstel (planetmaker) - Sunday, 20 January 2013, 11:21 GMT
The action sequence as shown in the three screenshots will lead to invalid river slopes. Thus if autoslope for the object is enabled, the river should not be restored, if its restoration would result in a slope forbidden for water.

Thus you might want to check for allowed water slopes before WATER_CLASS_RIVER is restored. The check is not needed for WATER_CLASS_CANAL, but could be applied there just the same.

Also configure your editor so that it automatically removes trailing whitespace :-)

Loading...