Skip to content

Commit

Permalink
Codechange: Use vector for airport tile layouts. (OpenTTD#12607)
Browse files Browse the repository at this point in the history 
Simplify AirportSpec data by storing layout information together in a vector, instead of separate arrays.

This removes manual memory management and separate count members.

The default layouts will be copied instead of always referring to the originals.
  • Loading branch information
@PeterN
PeterN authored May 2, 2024
1 parent 65c9df4 commit cf96d49
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 180 deletions.
12 changes: 6 additions & 6 deletions src/airport_gui.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ class BuildAirportWindow : public PickerWindowBase {
const AirportSpec *as = ac->GetSpec(_selected_airport_index);
if (as->IsAvailable()) {
/* Ensure the airport layout is valid. */
_selected_airport_layout = Clamp(_selected_airport_layout, 0, as->num_table - 1);
_selected_airport_layout = Clamp(_selected_airport_layout, 0, static_cast<uint8_t>(as->layouts.size() - 1));
selectFirstAirport = false;
this->UpdateSelectSize();
}
Expand Down Expand Up @@ -306,7 +306,7 @@ class BuildAirportWindow : public PickerWindowBase {
StringID string = GetAirportTextCallback(as, _selected_airport_layout, CBID_AIRPORT_LAYOUT_NAME);
if (string != STR_UNDEFINED) {
SetDParam(0, string);
} else if (as->num_table > 1) {
} else if (as->layouts.size() > 1) {
SetDParam(0, STR_STATION_BUILD_AIRPORT_LAYOUT_NAME);
SetDParam(1, _selected_airport_layout + 1);
}
Expand Down Expand Up @@ -348,7 +348,7 @@ class BuildAirportWindow : public PickerWindowBase {
for (int i = 0; i < NUM_AIRPORTS; i++) {
const AirportSpec *as = AirportSpec::Get(i);
if (!as->enabled) continue;
for (uint8_t layout = 0; layout < as->num_table; layout++) {
for (uint8_t layout = 0; layout < static_cast<uint8_t>(as->layouts.size()); layout++) {
SpriteID sprite = GetCustomAirportSprite(as, layout);
if (sprite != 0) {
Dimension d = GetSpriteSize(sprite);
Expand All @@ -364,7 +364,7 @@ class BuildAirportWindow : public PickerWindowBase {
for (int i = NEW_AIRPORT_OFFSET; i < NUM_AIRPORTS; i++) {
const AirportSpec *as = AirportSpec::Get(i);
if (!as->enabled) continue;
for (uint8_t layout = 0; layout < as->num_table; layout++) {
for (uint8_t layout = 0; layout < static_cast<uint8_t>(as->layouts.size()); layout++) {
StringID string = GetAirportTextCallback(as, layout, CBID_AIRPORT_ADDITIONAL_TEXT);
if (string == STR_UNDEFINED) continue;

Expand Down Expand Up @@ -474,14 +474,14 @@ class BuildAirportWindow : public PickerWindowBase {
const AirportSpec *as = AirportClass::Get(_selected_airport_class)->GetSpec(_selected_airport_index);
int w = as->size_x;
int h = as->size_y;
Direction rotation = as->rotation[_selected_airport_layout];
Direction rotation = as->layouts[_selected_airport_layout].rotation;
if (rotation == DIR_E || rotation == DIR_W) Swap(w, h);
SetTileSelectSize(w, h);

this->preview_sprite = GetCustomAirportSprite(as, _selected_airport_layout);

this->SetWidgetDisabledState(WID_AP_LAYOUT_DECREASE, _selected_airport_layout == 0);
this->SetWidgetDisabledState(WID_AP_LAYOUT_INCREASE, _selected_airport_layout + 1 >= as->num_table);
this->SetWidgetDisabledState(WID_AP_LAYOUT_INCREASE, _selected_airport_layout + 1U >= as->layouts.size());

int rad = _settings_game.station.modified_catchment ? as->catchment : (uint)CA_UNMODIFIED;
if (_settings_client.gui.station_show_coverage) SetTileSelectBigSize(-rad, -rad, 2 * rad, 2 * rad);
Expand Down
153 changes: 47 additions & 106 deletions src/newgrf.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3861,32 +3861,6 @@ static ChangeInfoResult IndustriesChangeInfo(uint indid, int numinfo, int prop,
return ret;
}

/**
* Create a copy of the tile table so it can be freed later
* without problems.
* @param as The AirportSpec to copy the arrays of.
*/
static void DuplicateTileTable(AirportSpec *as)
{
AirportTileTable **table_list = MallocT<AirportTileTable*>(as->num_table);
for (int i = 0; i < as->num_table; i++) {
uint num_tiles = 1;
const AirportTileTable *it = as->table[0];
do {
num_tiles++;
} while ((++it)->ti.x != -0x80);
table_list[i] = MallocT<AirportTileTable>(num_tiles);
MemCpyT(table_list[i], as->table[i], num_tiles);
}
as->table = table_list;
HangarTileTable *depot_table = MallocT<HangarTileTable>(as->nof_depots);
MemCpyT(depot_table, as->depot_table, as->nof_depots);
as->depot_table = depot_table;
Direction *rotation = MallocT<Direction>(as->num_table);
MemCpyT(rotation, as->rotation, as->num_table);
as->rotation = rotation;
}

/**
* Define properties for airports
* @param airport Local ID of the airport.
Expand Down Expand Up @@ -3942,89 +3916,68 @@ static ChangeInfoResult AirportChangeInfo(uint airport, int numinfo, int prop, B
as->grf_prop.grffile = _cur.grffile;
/* override the default airport */
_airport_mngr.Add(airport + i, _cur.grffile->grfid, subs_id);
/* Create a copy of the original tiletable so it can be freed later. */
DuplicateTileTable(as);
}
break;
}

case 0x0A: { // Set airport layout
uint8_t old_num_table = as->num_table;
free(as->rotation);
as->num_table = buf->ReadByte(); // Number of layaouts
as->rotation = MallocT<Direction>(as->num_table);
uint32_t defsize = buf->ReadDWord(); // Total size of the definition
AirportTileTable **tile_table = CallocT<AirportTileTable*>(as->num_table); // Table with tiles to compose the airport
AirportTileTable *att = CallocT<AirportTileTable>(defsize); // Temporary array to read the tile layouts from the GRF
int size;
const AirportTileTable *copy_from;
try {
for (uint8_t j = 0; j < as->num_table; j++) {
const_cast<Direction&>(as->rotation[j]) = (Direction)buf->ReadByte();
for (int k = 0;; k++) {
att[k].ti.x = buf->ReadByte(); // Offsets from northermost tile
att[k].ti.y = buf->ReadByte();

if (att[k].ti.x == 0 && att[k].ti.y == 0x80) {
/* Not the same terminator. The one we are using is rather
* x = -80, y = 0 . So, adjust it. */
att[k].ti.x = -0x80;
att[k].ti.y = 0;
att[k].gfx = 0;

size = k + 1;
copy_from = att;
break;
}
uint8_t num_layouts = buf->ReadByte();
buf->ReadDWord(); // Total size of definition, unneeded.
uint8_t size_x = 0;
uint8_t size_y = 0;

att[k].gfx = buf->ReadByte();
std::vector<AirportTileLayout> layouts;
layouts.reserve(num_layouts);

if (att[k].gfx == 0xFE) {
/* Use a new tile from this GRF */
int local_tile_id = buf->ReadWord();
for (uint8_t j = 0; j != num_layouts; ++j) {
auto &layout = layouts.emplace_back();
layout.rotation = static_cast<Direction>(buf->ReadByte() & 6); // Rotation can only be DIR_NORTH, DIR_EAST, DIR_SOUTH or DIR_WEST.

/* Read the ID from the _airporttile_mngr. */
uint16_t tempid = _airporttile_mngr.GetID(local_tile_id, _cur.grffile->grfid);
for (;;) {
auto &tile = layout.tiles.emplace_back();
tile.ti.x = buf->ReadByte();
tile.ti.y = buf->ReadByte();
if (tile.ti.x == 0 && tile.ti.y == 0x80) {
/* Convert terminator to our own. */
tile.ti.x = -0x80;
tile.ti.y = 0;
tile.gfx = 0;
break;
}

if (tempid == INVALID_AIRPORTTILE) {
GrfMsg(2, "AirportChangeInfo: Attempt to use airport tile {} with airport id {}, not yet defined. Ignoring.", local_tile_id, airport + i);
} else {
/* Declared as been valid, can be used */
att[k].gfx = tempid;
}
} else if (att[k].gfx == 0xFF) {
att[k].ti.x = (int8_t)GB(att[k].ti.x, 0, 8);
att[k].ti.y = (int8_t)GB(att[k].ti.y, 0, 8);
}
tile.gfx = buf->ReadByte();

if (as->rotation[j] == DIR_E || as->rotation[j] == DIR_W) {
as->size_x = std::max<uint8_t>(as->size_x, att[k].ti.y + 1);
as->size_y = std::max<uint8_t>(as->size_y, att[k].ti.x + 1);
if (tile.gfx == 0xFE) {
/* Use a new tile from this GRF */
int local_tile_id = buf->ReadWord();

/* Read the ID from the _airporttile_mngr. */
uint16_t tempid = _airporttile_mngr.GetID(local_tile_id, _cur.grffile->grfid);

if (tempid == INVALID_AIRPORTTILE) {
GrfMsg(2, "AirportChangeInfo: Attempt to use airport tile {} with airport id {}, not yet defined. Ignoring.", local_tile_id, airport + i);
} else {
as->size_x = std::max<uint8_t>(as->size_x, att[k].ti.x + 1);
as->size_y = std::max<uint8_t>(as->size_y, att[k].ti.y + 1);
/* Declared as been valid, can be used */
tile.gfx = tempid;
}
} else if (tile.gfx == 0xFF) {
tile.ti.x = static_cast<int8_t>(GB(tile.ti.x, 0, 8));
tile.ti.y = static_cast<int8_t>(GB(tile.ti.y, 0, 8));
}

/* Determine largest size. */
if (layout.rotation == DIR_E || layout.rotation == DIR_W) {
size_x = std::max<uint8_t>(size_x, tile.ti.y + 1);
size_y = std::max<uint8_t>(size_y, tile.ti.x + 1);
} else {
size_x = std::max<uint8_t>(size_x, tile.ti.x + 1);
size_y = std::max<uint8_t>(size_y, tile.ti.y + 1);
}
tile_table[j] = CallocT<AirportTileTable>(size);
memcpy(tile_table[j], copy_from, sizeof(*copy_from) * size);
}
/* Free old layouts in the airport spec */
for (int j = 0; j < old_num_table; j++) {
/* remove the individual layouts */
free(as->table[j]);
}
free(as->table);
/* Install final layout construction in the airport spec */
as->table = tile_table;
free(att);
} catch (...) {
for (int i = 0; i < as->num_table; i++) {
free(tile_table[i]);
}
free(tile_table);
free(att);
throw;
}
as->layouts = std::move(layouts);
as->size_x = size_x;
as->size_y = size_y;
break;
}

Expand Down Expand Up @@ -8726,18 +8679,6 @@ static void ResetCustomHouses()
static void ResetCustomAirports()
{
for (GRFFile * const file : _grf_files) {
for (auto &as : file->airportspec) {
if (as != nullptr) {
/* We need to remove the tiles layouts */
for (int j = 0; j < as->num_table; j++) {
/* remove the individual layouts */
free(as->table[j]);
}
free(as->table);
free(as->depot_table);
free(as->rotation);
}
}
file->airportspec.clear();
file->airtspec.clear();
}
Expand Down
4 changes: 2 additions & 2 deletions src/newgrf_airport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ bool AirportSpec::IsAvailable() const
*/
bool AirportSpec::IsWithinMapBounds(uint8_t table, TileIndex tile) const
{
if (table >= this->num_table) return false;
if (table >= this->layouts.size()) return false;

uint8_t w = this->size_x;
uint8_t h = this->size_y;
if (this->rotation[table] == DIR_E || this->rotation[table] == DIR_W) Swap(w, h);
if (this->layouts[table].rotation == DIR_E || this->layouts[table].rotation == DIR_W) Swap(w, h);

return TileX(tile) + w < Map::SizeX() &&
TileY(tile) + h < Map::SizeY();
Expand Down
12 changes: 7 additions & 5 deletions src/newgrf_airport.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,18 @@ struct HangarTileTable {
uint8_t hangar_num; ///< The hangar to which this tile belongs.
};

struct AirportTileLayout {
std::vector<AirportTileTable> tiles; ///< List of all tiles in this layout.
Direction rotation; ///< The rotation of this layout.
};

/**
* Defines the data structure for an airport.
*/
struct AirportSpec {
const struct AirportFTAClass *fsm; ///< the finite statemachine for the default airports
const AirportTileTable * const *table; ///< list of the tiles composing the airport
const Direction *rotation; ///< the rotation of each tiletable
uint8_t num_table; ///< number of elements in the table
const HangarTileTable *depot_table; ///< gives the position of the depots on the airports
uint8_t nof_depots; ///< the number of hangar tiles in this airport
std::vector<AirportTileLayout> layouts; ///< List of layouts composing the airport.
std::span<const HangarTileTable> depots; ///< Position of the depots on the airports.
uint8_t size_x; ///< size of airport in x direction
uint8_t size_y; ///< size of airport in y direction
uint8_t noise_level; ///< noise that this airport generates
Expand Down
6 changes: 4 additions & 2 deletions src/script/api/script_airport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@

if (_settings_game.economy.station_noise_level) {
uint dist;
AirportGetNearestTown(as, as->rotation[0], tile, AirportTileTableIterator(as->table[0], tile), dist);
const auto &layout = as->layouts[0];
AirportGetNearestTown(as, layout.rotation, tile, AirportTileTableIterator(layout.tiles.data(), tile), dist);
return GetAirportNoiseLevelForDistance(as, dist);
}

Expand All @@ -154,7 +155,8 @@
if (!as->IsWithinMapBounds(0, tile)) return INVALID_TOWN;

uint dist;
return AirportGetNearestTown(as, as->rotation[0], tile, AirportTileTableIterator(as->table[0], tile), dist)->index;
const auto &layout = as->layouts[0];
return AirportGetNearestTown(as, layout.rotation, tile, AirportTileTableIterator(layout.tiles.data(), tile), dist)->index;
}

/* static */ SQInteger ScriptAirport::GetMaintenanceCostFactor(AirportType type)
Expand Down
25 changes: 11 additions & 14 deletions src/station_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ struct Airport : public TileArea {
/** Check if this airport has at least one hangar. */
inline bool HasHangar() const
{
return this->GetSpec()->nof_depots > 0;
return !this->GetSpec()->depots.empty();
}

/**
Expand Down Expand Up @@ -357,10 +357,9 @@ struct Airport : public TileArea {
*/
inline TileIndex GetHangarTile(uint hangar_num) const
{
const AirportSpec *as = this->GetSpec();
for (uint i = 0; i < as->nof_depots; i++) {
if (as->depot_table[i].hangar_num == hangar_num) {
return this->GetRotatedTileFromOffset(as->depot_table[i].ti);
for (const auto &depot : this->GetSpec()->depots) {
if (depot.hangar_num == hangar_num) {
return this->GetRotatedTileFromOffset(depot.ti);
}
}
NOT_REACHED();
Expand All @@ -376,7 +375,7 @@ struct Airport : public TileArea {
{
const AirportSpec *as = this->GetSpec();
const HangarTileTable *htt = GetHangarDataByTile(tile);
return ChangeDir(htt->dir, DirDifference(this->rotation, as->rotation[0]));
return ChangeDir(htt->dir, DirDifference(this->rotation, as->layouts[0].rotation));
}

/**
Expand All @@ -396,11 +395,10 @@ struct Airport : public TileArea {
{
uint num = 0;
uint counted = 0;
const AirportSpec *as = this->GetSpec();
for (uint i = 0; i < as->nof_depots; i++) {
if (!HasBit(counted, as->depot_table[i].hangar_num)) {
for (const auto &depot : this->GetSpec()->depots) {
if (!HasBit(counted, depot.hangar_num)) {
num++;
SetBit(counted, as->depot_table[i].hangar_num);
SetBit(counted, depot.hangar_num);
}
}
return num;
Expand All @@ -415,10 +413,9 @@ struct Airport : public TileArea {
*/
inline const HangarTileTable *GetHangarDataByTile(TileIndex tile) const
{
const AirportSpec *as = this->GetSpec();
for (uint i = 0; i < as->nof_depots; i++) {
if (this->GetRotatedTileFromOffset(as->depot_table[i].ti) == tile) {
return as->depot_table + i;
for (const auto &depot : this->GetSpec()->depots) {
if (this->GetRotatedTileFromOffset(depot.ti) == tile) {
return &depot;
}
}
NOT_REACHED();
Expand Down
Loading

0 comments on commit cf96d49

Please sign in to comment.