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.

Sign up for GitHub

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

Codechange: Use vector for airport tile layouts. #12607

Merged
merged 2 commits into from
May 2, 2024
Merged

Codechange: Use vector for airport tile layouts. #12607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
745435d
Codechange: Use vector for airport tile layouts.
PeterN May 1, 2024
6978fb6
Codefix: Use static_cast instead of C-cast.
PeterN May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/airport_gui.cpp
View file
Open in desktop
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++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be nicer to change layout to be a size_t, rather than have the cast? Might need some signature changes as well I suppose

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does lead to a bit of a rabbit hole, when there's no need for for a size_t field further down.

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
Open in desktop
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;
Comment on lines +3941 to +3944
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but if the tile table is now a vector or span then the termination sentinel does not need to be saved in OpenTTD's state.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I figured this already touches enough :)

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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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