Skip to content

Commit

Permalink
Fix OpenTTD#7043, OpenTTD#7274: Delete town owned bridge based on adj…
Browse files Browse the repository at this point in the history
…acent tile ownership, not nearest town.

The existing test performed badly with a large number of towns due to having to calculate the
nearest town, and also by its nature assumed a bridge was built by the nearest town, leading
to bridges of nearby large towns be removed incorrectly.
  • Loading branch information
PeterN committed Feb 26, 2019
1 parent 7ac17f5 commit b6dab5a
Showing 1 changed file with 38 additions and 6 deletions.
44 changes: 38 additions & 6 deletions src/town_cmd.cpp
Expand Up @@ -59,6 +59,31 @@ CargoTypes _town_cargoes_accepted; ///< Bitmap of all cargoes accepted by houses
TownPool _town_pool("Town");
INSTANTIATE_POOL_METHODS(Town)

/**
* Check if a town 'owns' a bridge.
* Bridges to not directly have an owner, so we check the tiles adjacent to the bridge ends.
* If either adjacent tile belongs to the town then it will be assumed that the town built
* the bridge.
* @param tile Bridge tile to test
* @param t Town we are interested in
* @return true if town 'owns' a bridge.
*/
static bool TestTownOwnsBridge(TileIndex tile, const Town *t)
{
if (!IsTileOwner(tile, OWNER_TOWN)) return false;

TileIndex adjacent = tile + TileOffsByDiagDir(ReverseDiagDir(GetTunnelBridgeDirection(tile)));
bool town_owned = IsTileType(adjacent, MP_ROAD) && IsTileOwner(adjacent, OWNER_TOWN) && GetTownIndex(adjacent) == t->index;

if (!town_owned) {
/* Or other adjacent road */
TileIndex adjacent = tile + TileOffsByDiagDir(ReverseDiagDir(GetTunnelBridgeDirection(GetOtherTunnelBridgeEnd(tile))));
town_owned = IsTileType(adjacent, MP_ROAD) && IsTileOwner(adjacent, OWNER_TOWN) && GetTownIndex(adjacent) == t->index;
}

return town_owned;
}

Town::~Town()
{
free(this->name);
Expand Down Expand Up @@ -90,7 +115,7 @@ Town::~Town()
break;

case MP_TUNNELBRIDGE:
assert(!IsTileOwner(tile, OWNER_TOWN) || ClosestTownFromTile(tile, UINT_MAX) != this);
assert(!TestTownOwnsBridge(tile, this));
break;

default:
Expand Down Expand Up @@ -2730,18 +2755,25 @@ CommandCost CmdDeleteTown(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32
if (d->town == t) return CMD_ERROR;
}

/* Check all tiles for town ownership. */
/* Check all tiles for town ownership. First check for bridge tiles, as
* these do not directly have an owner so we need to check adjacent
* tiles. This won't work correctly in the same loop if the adjacent
* tile was already deleted earlier in the loop. */
for (TileIndex tile = 0; tile < MapSize(); ++tile) {
if (IsTileType(tile, MP_TUNNELBRIDGE) && TestTownOwnsBridge(tile, t)) {
CommandCost ret = DoCommand(tile, 0, 0, flags, CMD_LANDSCAPE_CLEAR);
if (ret.Failed()) return ret;
}
}

/* Check all remaining tiles for town ownership. */
for (TileIndex tile = 0; tile < MapSize(); ++tile) {
bool try_clear = false;
switch (GetTileType(tile)) {
case MP_ROAD:
try_clear = HasTownOwnedRoad(tile) && GetTownIndex(tile) == t->index;
break;

case MP_TUNNELBRIDGE:
try_clear = IsTileOwner(tile, OWNER_TOWN) && ClosestTownFromTile(tile, UINT_MAX) == t;
break;

case MP_HOUSE:
try_clear = GetTownIndex(tile) == t->index;
break;
Expand Down

0 comments on commit b6dab5a

Please sign in to comment.