Skip to content
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

Jump to bottom

Newobjects on river slopes - unintended behavior #5441

Closed
DorpsGek opened this issue Jan 10, 2013 · 5 comments
Closed

Newobjects on river slopes - unintended behavior #5441

DorpsGek opened this issue Jan 10, 2013 · 5 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

Supercheese opened the ticket and wrote:

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.

Attachments

Reported version: other
Operating system: Windows

This issue was imported from FlySpray: https://bugs.openttd.org/task/5441
@DorpsGek
Copy link
Member Author

Supercheese wrote:

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.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5441#comment11868

@DorpsGek
Copy link
Member Author

Supercheese wrote:

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.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5441#comment11869

@DorpsGek
Copy link
Member Author

Supercheese wrote:

Here's a patch that seems to resolve the only remaining issue of unintended river-destruction. Made against r24915.

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/5441#comment11871

@DorpsGek
Copy link
Member Author

planetmaker wrote:

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 :-)

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/5441#comment11881

@DorpsGek
Copy link
Member Author

planetmaker closed the ticket.

Reason for closing: Fixed

In r24923. Thanks for initial patch

This comment was imported from FlySpray: https://bugs.openttd.org/task/5441

@DorpsGek DorpsGek added flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) bug labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@DorpsGek