From c0ab436077d2e2f425d8b3969de392a640256b88 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sun, 21 Jan 2024 13:23:04 +0000 Subject: [PATCH] Codechange: Store Colours in Colours type. (#11625) This reduces casts, some magic numbers, and introduces a bit of type-safety. --- src/company_base.h | 4 ++-- src/company_cmd.cpp | 8 ++++---- src/company_gui.cpp | 18 ++++++++++++------ src/company_manager_face.h | 2 +- src/group_cmd.cpp | 4 ++-- src/house.h | 2 +- src/industry.h | 2 +- src/industry_cmd.cpp | 4 ++-- src/linkgraph/linkgraph_gui.cpp | 2 +- src/livery.h | 5 +++-- src/main_gui.cpp | 2 +- src/newgrf.cpp | 11 ++++++----- src/newgrf_house.cpp | 2 +- src/news_type.h | 3 ++- src/openttd.cpp | 4 ++-- src/saveload/afterload.cpp | 2 +- src/saveload/company_sl.cpp | 2 +- src/saveload/oldloader_sl.cpp | 6 +++--- src/settings_type.h | 4 ++-- src/story.cpp | 2 +- src/table/town_land.h | 2 +- 21 files changed, 50 insertions(+), 41 deletions(-) diff --git a/src/company_base.h b/src/company_base.h index eea9974089..bcdbeef11a 100644 --- a/src/company_base.h +++ b/src/company_base.h @@ -67,7 +67,7 @@ struct CompanyProperties { byte money_fraction; ///< Fraction of money of the company, too small to represent in #money. Money current_loan; ///< Amount of money borrowed from the bank. - byte colour; ///< Company colour. + Colours colour; ///< Company colour. byte block_preview; ///< Number of quarters that the company is not allowed to get new exclusive engine previews (see CompaniesGenStatistics). @@ -105,7 +105,7 @@ struct CompanyProperties { // TODO: Change some of these member variables to use relevant INVALID_xxx constants CompanyProperties() : name_2(0), name_1(0), president_name_1(0), president_name_2(0), - face(0), money(0), money_fraction(0), current_loan(0), colour(0), block_preview(0), + face(0), money(0), money_fraction(0), current_loan(0), colour(COLOUR_BEGIN), block_preview(0), location_of_HQ(0), last_build_coordinate(0), inaugurated_year(0), months_of_bankruptcy(0), bankrupt_asked(0), bankrupt_timeout(0), bankrupt_value(0), terraform_limit(0), clear_limit(0), tree_limit(0), build_object_limit(0), is_ai(false), engine_renew_list(nullptr) {} diff --git a/src/company_cmd.cpp b/src/company_cmd.cpp index 7d14fafbd1..d6166d4571 100644 --- a/src/company_cmd.cpp +++ b/src/company_cmd.cpp @@ -457,7 +457,7 @@ static Colours GenerateCompanyColour() /* Move the colours that look similar to each company's colour to the side */ for (const Company *c : Company::Iterate()) { - Colours pcolour = (Colours)c->colour; + Colours pcolour = c->colour; for (uint i = 0; i < COLOUR_END; i++) { if (colours[i] == pcolour) { @@ -559,7 +559,7 @@ Company *DoStartupNewCompany(bool is_ai, CompanyID company = INVALID_COMPANY) c->colour = colour; ResetCompanyLivery(c); - _company_colours[c->index] = (Colours)c->colour; + _company_colours[c->index] = c->colour; /* Scale the initial loan based on the inflation rounded down to the loan interval. The maximum loan has already been inflation adjusted. */ c->money = c->current_loan = std::min((INITIAL_LOAN * _economy.inflation_prices >> 16) / LOAN_INTERVAL * LOAN_INTERVAL, _economy.max_loan); @@ -991,7 +991,7 @@ CommandCost CmdSetCompanyColour(DoCommandFlag flags, LiveryScheme scheme, bool p if (flags & DC_EXEC) { if (primary) { if (scheme != LS_DEFAULT) SB(c->livery[scheme].in_use, 0, 1, colour != INVALID_COLOUR); - if (colour == INVALID_COLOUR) colour = (Colours)c->livery[LS_DEFAULT].colour1; + if (colour == INVALID_COLOUR) colour = c->livery[LS_DEFAULT].colour1; c->livery[scheme].colour1 = colour; /* If setting the first colour of the default scheme, adjust the @@ -1004,7 +1004,7 @@ CommandCost CmdSetCompanyColour(DoCommandFlag flags, LiveryScheme scheme, bool p } } else { if (scheme != LS_DEFAULT) SB(c->livery[scheme].in_use, 1, 1, colour != INVALID_COLOUR); - if (colour == INVALID_COLOUR) colour = (Colours)c->livery[LS_DEFAULT].colour2; + if (colour == INVALID_COLOUR) colour = c->livery[LS_DEFAULT].colour2; c->livery[scheme].colour2 = colour; if (scheme == LS_DEFAULT) { diff --git a/src/company_gui.cpp b/src/company_gui.cpp index 272dbda3b5..0e4ac392a4 100644 --- a/src/company_gui.cpp +++ b/src/company_gui.cpp @@ -582,7 +582,7 @@ static const LiveryClass _livery_class[LS_END] = { template class DropDownListColourItem : public DropDownIcon> { public: - DropDownListColourItem(int colour, bool masked) : DropDownIcon>(TSprite, PALETTE_RECOLOUR_START + (colour % COLOUR_END), colour < COLOUR_END ? _colour_dropdown[colour] : STR_COLOUR_DEFAULT, colour, masked) + DropDownListColourItem(int colour, bool masked) : DropDownIcon>(TSprite, GENERAL_SPRITE_COLOUR(colour % COLOUR_END), colour < COLOUR_END ? _colour_dropdown[colour] : STR_COLOUR_DEFAULT, colour, masked) { } }; @@ -647,7 +647,12 @@ private: list.push_back(std::make_unique>(i, HasBit(used_colours, i))); } - byte sel = (default_livery == nullptr || HasBit(livery->in_use, primary ? 0 : 1)) ? (primary ? livery->colour1 : livery->colour2) : default_col; + byte sel; + if (default_livery == nullptr || HasBit(livery->in_use, primary ? 0 : 1)) { + sel = primary ? livery->colour1 : livery->colour2; + } else { + sel = default_col; + } ShowDropDownList(this, std::move(list), sel, widget); } @@ -1034,19 +1039,20 @@ public: bool local = (CompanyID)this->window_number == _local_company; if (!local) return; - if (index >= COLOUR_END) index = INVALID_COLOUR; + Colours colour = static_cast(index); + if (colour >= COLOUR_END) colour = INVALID_COLOUR; if (this->livery_class < LC_GROUP_RAIL) { /* Set company colour livery */ for (LiveryScheme scheme = LS_DEFAULT; scheme < LS_END; scheme++) { /* Changed colour for the selected scheme, or all visible schemes if CTRL is pressed. */ if (HasBit(this->sel, scheme) || (_ctrl_pressed && _livery_class[scheme] == this->livery_class && HasBit(_loaded_newgrf_features.used_liveries, scheme))) { - Command::Post(scheme, widget == WID_SCL_PRI_COL_DROPDOWN, (Colours)index); + Command::Post(scheme, widget == WID_SCL_PRI_COL_DROPDOWN, colour); } } } else { /* Setting group livery */ - Command::Post(this->sel, widget == WID_SCL_PRI_COL_DROPDOWN, (Colours)index); + Command::Post(this->sel, widget == WID_SCL_PRI_COL_DROPDOWN, colour); } } @@ -1149,7 +1155,7 @@ void ShowCompanyLiveryWindow(CompanyID company, GroupID group) * @param colour the (background) colour of the gradient * @param r position to draw the face */ -void DrawCompanyManagerFace(CompanyManagerFace cmf, int colour, const Rect &r) +void DrawCompanyManagerFace(CompanyManagerFace cmf, Colours colour, const Rect &r) { GenderEthnicity ge = (GenderEthnicity)GetCompanyManagerFaceBits(cmf, CMFV_GEN_ETHN, GE_WM); diff --git a/src/company_manager_face.h b/src/company_manager_face.h index 0d8cc68da6..113a4c728b 100644 --- a/src/company_manager_face.h +++ b/src/company_manager_face.h @@ -236,6 +236,6 @@ inline SpriteID GetCompanyManagerFaceSprite(CompanyManagerFace cmf, CompanyManag return _cmf_info[cmfv].first_sprite[ge] + GB(cmf, _cmf_info[cmfv].offset, _cmf_info[cmfv].length); } -void DrawCompanyManagerFace(CompanyManagerFace face, int colour, const Rect &r); +void DrawCompanyManagerFace(CompanyManagerFace face, Colours colour, const Rect &r); #endif /* COMPANY_MANAGER_FACE_H */ diff --git a/src/group_cmd.cpp b/src/group_cmd.cpp index a96d65585c..b0091ab303 100644 --- a/src/group_cmd.cpp +++ b/src/group_cmd.cpp @@ -672,11 +672,11 @@ CommandCost CmdSetGroupLivery(DoCommandFlag flags, GroupID group_id, bool primar if (flags & DC_EXEC) { if (primary) { SB(g->livery.in_use, 0, 1, colour != INVALID_COLOUR); - if (colour == INVALID_COLOUR) colour = (Colours)GetParentLivery(g)->colour1; + if (colour == INVALID_COLOUR) colour = GetParentLivery(g)->colour1; g->livery.colour1 = colour; } else { SB(g->livery.in_use, 1, 1, colour != INVALID_COLOUR); - if (colour == INVALID_COLOUR) colour = (Colours)GetParentLivery(g)->colour2; + if (colour == INVALID_COLOUR) colour = GetParentLivery(g)->colour2; g->livery.colour2 = colour; } diff --git a/src/house.h b/src/house.h index 2724506686..4831e289ac 100644 --- a/src/house.h +++ b/src/house.h @@ -113,7 +113,7 @@ struct HouseSpec { /* NewHouses properties */ GRFFileProps grf_prop; ///< Properties related the the grf file uint16_t callback_mask; ///< Bitmask of house callbacks that have to be called - byte random_colour[4]; ///< 4 "random" colours + Colours random_colour[4]; ///< 4 "random" colours byte probability; ///< Relative probability of appearing (16 is the standard value) HouseExtraFlags extra_flags; ///< some more flags HouseClassID class_id; ///< defines the class this house has (not grf file based) diff --git a/src/industry.h b/src/industry.h index bc6f79bb23..0d1bbe29b1 100644 --- a/src/industry.h +++ b/src/industry.h @@ -102,7 +102,7 @@ struct Industry : IndustryPool::PoolItem<&_industry_pool> { IndustryType type; ///< type of industry. Owner owner; ///< owner of the industry. Which SHOULD always be (imho) OWNER_NONE - byte random_colour; ///< randomized colour of the industry, for display purpose + Colours random_colour; ///< randomized colour of the industry, for display purpose TimerGameCalendar::Year last_prod_year; ///< last year of production byte was_cargo_delivered; ///< flag that indicate this has been the closest industry chosen for cargo delivery by a station. see DeliverGoodsToIndustry IndustryControlFlags ctlflags; ///< flags overriding standard behaviours diff --git a/src/industry_cmd.cpp b/src/industry_cmd.cpp index 92861e29ec..68cfe9f312 100644 --- a/src/industry_cmd.cpp +++ b/src/industry_cmd.cpp @@ -1778,7 +1778,7 @@ static void DoCreateNewIndustry(Industry *i, TileIndex tile, IndustryType type, i->owner = OWNER_NONE; uint16_t r = Random(); - i->random_colour = GB(r, 0, 4); + i->random_colour = static_cast(GB(r, 0, 4)); i->counter = GB(r, 4, 12); i->random = initial_random_bits; i->was_cargo_delivered = false; @@ -1832,7 +1832,7 @@ static void DoCreateNewIndustry(Industry *i, TileIndex tile, IndustryType type, uint16_t res = GetIndustryCallback(CBID_INDUSTRY_DECIDE_COLOUR, 0, 0, i, type, INVALID_TILE); if (res != CALLBACK_FAILED) { if (GB(res, 4, 11) != 0) ErrorUnknownCallbackResult(indspec->grf_prop.grffile->grfid, CBID_INDUSTRY_DECIDE_COLOUR, res); - i->random_colour = GB(res, 0, 4); + i->random_colour = static_cast(GB(res, 0, 4)); } } diff --git a/src/linkgraph/linkgraph_gui.cpp b/src/linkgraph/linkgraph_gui.cpp index 883484c26a..6bb9dd60a9 100644 --- a/src/linkgraph/linkgraph_gui.cpp +++ b/src/linkgraph/linkgraph_gui.cpp @@ -332,7 +332,7 @@ void LinkGraphOverlay::DrawStationDots(const DrawPixelInfo *dpi) const LinkGraphOverlay::DrawVertex(pt.x, pt.y, r, _colour_gradient[st->owner != OWNER_NONE ? - (Colours)Company::Get(st->owner)->colour : COLOUR_GREY][5], + Company::Get(st->owner)->colour : COLOUR_GREY][5], _colour_gradient[COLOUR_GREY][1]); } } diff --git a/src/livery.h b/src/livery.h index cfb54aa182..302d40e404 100644 --- a/src/livery.h +++ b/src/livery.h @@ -11,6 +11,7 @@ #define LIVERY_H #include "company_type.h" +#include "gfx_type.h" static const byte LIT_NONE = 0; ///< Don't show the liveries at all static const byte LIT_COMPANY = 1; ///< Show the liveries of your own company @@ -76,8 +77,8 @@ DECLARE_ENUM_AS_ADDABLE(LiveryClass) /** Information about a particular livery. */ struct Livery { byte in_use; ///< Bit 0 set if this livery should override the default livery first colour, Bit 1 for the second colour. - byte colour1; ///< First colour, for all vehicles. - byte colour2; ///< Second colour, for vehicles with 2CC support. + Colours colour1; ///< First colour, for all vehicles. + Colours colour2; ///< Second colour, for vehicles with 2CC support. }; void ResetCompanyLivery(Company *c); diff --git a/src/main_gui.cpp b/src/main_gui.cpp index d6ffa71ba2..aaac9a11eb 100644 --- a/src/main_gui.cpp +++ b/src/main_gui.cpp @@ -541,7 +541,7 @@ void ShowSelectGameWindow(); void SetupColoursAndInitialWindow() { for (uint i = 0; i != 16; i++) { - const byte *b = GetNonSprite(PALETTE_RECOLOUR_START + i, SpriteType::Recolour); + const byte *b = GetNonSprite(GENERAL_SPRITE_COLOUR(i), SpriteType::Recolour); assert(b); memcpy(_colour_gradient[i], b + 0xC6, sizeof(_colour_gradient[i])); diff --git a/src/newgrf.cpp b/src/newgrf.cpp index e38027190a..621dd971f2 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -2405,10 +2405,11 @@ static ChangeInfoResult TownHouseChangeInfo(uint hid, int numinfo, int prop, Byt housespec->grf_prop.local_id = hid + i; housespec->grf_prop.subst_id = subs_id; housespec->grf_prop.grffile = _cur.grffile; - housespec->random_colour[0] = 0x04; // those 4 random colours are the base colour - housespec->random_colour[1] = 0x08; // for all new houses - housespec->random_colour[2] = 0x0C; // they stand for red, blue, orange and green - housespec->random_colour[3] = 0x06; + /* Set default colours for randomization, used if not overridden. */ + housespec->random_colour[0] = COLOUR_RED; + housespec->random_colour[1] = COLOUR_BLUE; + housespec->random_colour[2] = COLOUR_ORANGE; + housespec->random_colour[3] = COLOUR_GREEN; /* House flags 40 and 80 are exceptions; these flags are never set automatically. */ housespec->building_flags &= ~(BUILDING_IS_CHURCH | BUILDING_IS_STADIUM); @@ -2502,7 +2503,7 @@ static ChangeInfoResult TownHouseChangeInfo(uint hid, int numinfo, int prop, Byt break; case 0x17: // Four random colours to use - for (uint j = 0; j < 4; j++) housespec->random_colour[j] = buf->ReadByte(); + for (uint j = 0; j < 4; j++) housespec->random_colour[j] = static_cast(GB(buf->ReadByte(), 0, 4)); break; case 0x18: // Relative probability of appearing diff --git a/src/newgrf_house.cpp b/src/newgrf_house.cpp index 63e7c44d69..6175920ca5 100644 --- a/src/newgrf_house.cpp +++ b/src/newgrf_house.cpp @@ -429,7 +429,7 @@ static void DrawTileLayout(const TileInfo *ti, const TileLayoutSpriteGroup *grou const DrawTileSprites *dts = group->ProcessRegisters(&stage); const HouseSpec *hs = HouseSpec::Get(house_id); - PaletteID palette = hs->random_colour[TileHash2Bit(ti->x, ti->y)] + PALETTE_RECOLOUR_START; + PaletteID palette = GENERAL_SPRITE_COLOUR(hs->random_colour[TileHash2Bit(ti->x, ti->y)]); if (HasBit(hs->callback_mask, CBM_HOUSE_COLOUR)) { uint16_t callback = GetHouseCallback(CBID_HOUSE_COLOUR, 0, 0, house_id, Town::GetByTile(ti->tile), ti->tile); if (callback != CALLBACK_FAILED) { diff --git a/src/news_type.h b/src/news_type.h index e2d32a5ae7..ca6343d6d6 100644 --- a/src/news_type.h +++ b/src/news_type.h @@ -11,6 +11,7 @@ #define NEWS_TYPE_H #include "core/enum_type.hpp" +#include "gfx_type.h" #include "timer/timer_game_calendar.h" #include "strings_type.h" #include "sound_type.h" @@ -161,7 +162,7 @@ struct CompanyNewsInformation : NewsAllocatedData { std::string other_company_name; ///< The name of the company taking over this one uint32_t face; ///< The face of the president - byte colour; ///< The colour related to the company + Colours colour; ///< The colour related to the company CompanyNewsInformation(const struct Company *c, const struct Company *other = nullptr); }; diff --git a/src/openttd.cpp b/src/openttd.cpp index 37729c82ca..542952b5ec 100644 --- a/src/openttd.cpp +++ b/src/openttd.cpp @@ -876,11 +876,11 @@ static void MakeNewGameDone() if (_settings_client.gui.starting_colour != COLOUR_END) { c->colour = _settings_client.gui.starting_colour; ResetCompanyLivery(c); - _company_colours[c->index] = (Colours)c->colour; + _company_colours[c->index] = c->colour; } if (_settings_client.gui.starting_colour_secondary != COLOUR_END && HasBit(_loaded_newgrf_features.used_liveries, LS_DEFAULT)) { - Command::Post(LS_DEFAULT, false, (Colours)_settings_client.gui.starting_colour_secondary); + Command::Post(LS_DEFAULT, false, _settings_client.gui.starting_colour_secondary); } OnStartGame(false); diff --git a/src/saveload/afterload.cpp b/src/saveload/afterload.cpp index 22a0ca811b..33d12648b5 100644 --- a/src/saveload/afterload.cpp +++ b/src/saveload/afterload.cpp @@ -2463,7 +2463,7 @@ bool AfterLoadGame() if (IsSavegameVersionBefore(SLV_148)) { for (Object *o : Object::Iterate()) { Owner owner = GetTileOwner(o->location.tile); - o->colour = (owner == OWNER_NONE) ? Random() & 0xF : Company::Get(owner)->livery->colour1; + o->colour = (owner == OWNER_NONE) ? static_cast(GB(Random(), 0, 4)) : Company::Get(owner)->livery->colour1; } } diff --git a/src/saveload/company_sl.cpp b/src/saveload/company_sl.cpp index 9f65198f9e..6ed8c5a8c4 100644 --- a/src/saveload/company_sl.cpp +++ b/src/saveload/company_sl.cpp @@ -516,7 +516,7 @@ struct PLYRChunkHandler : ChunkHandler { while ((index = SlIterateArray()) != -1) { Company *c = new (index) Company(); SlObject(c, slt); - _company_colours[index] = (Colours)c->colour; + _company_colours[index] = c->colour; } } diff --git a/src/saveload/oldloader_sl.cpp b/src/saveload/oldloader_sl.cpp index 06c0af9084..9ba3397412 100644 --- a/src/saveload/oldloader_sl.cpp +++ b/src/saveload/oldloader_sl.cpp @@ -455,10 +455,10 @@ static void FixTTOCompanies() } } -static inline byte RemapTTOColour(byte tto) +static inline Colours RemapTTOColour(Colours tto) { /** Lossy remapping of TTO colours to TTD colours. SVXConverter uses the same conversion. */ - static const byte tto_colour_remap[] = { + static const Colours tto_colour_remap[] = { COLOUR_DARK_BLUE, COLOUR_GREY, COLOUR_YELLOW, COLOUR_RED, COLOUR_PURPLE, COLOUR_DARK_GREEN, COLOUR_ORANGE, COLOUR_PALE_GREEN, COLOUR_BLUE, COLOUR_GREEN, COLOUR_CREAM, COLOUR_BROWN, @@ -1033,7 +1033,7 @@ static bool LoadOldCompany(LoadgameState *ls, int num) if (c->money == 893288) c->money = c->current_loan = 100000; } - _company_colours[num] = (Colours)c->colour; + _company_colours[num] = c->colour; c->inaugurated_year -= CalendarTime::ORIGINAL_BASE_YEAR; return true; diff --git a/src/settings_type.h b/src/settings_type.h index dcae83b814..b90eaacab3 100644 --- a/src/settings_type.h +++ b/src/settings_type.h @@ -186,8 +186,8 @@ struct GUISettings { byte missing_strings_threshold; ///< the number of missing strings before showing the warning uint8_t graph_line_thickness; ///< the thickness of the lines in the various graph guis uint8_t osk_activation; ///< Mouse gesture to trigger the OSK. - byte starting_colour; ///< default color scheme for the company to start a new game with - byte starting_colour_secondary; ///< default secondary color scheme for the company to start a new game with + Colours starting_colour; ///< default color scheme for the company to start a new game with + Colours starting_colour_secondary; ///< default secondary color scheme for the company to start a new game with bool show_newgrf_name; ///< Show the name of the NewGRF in the build vehicle window bool show_cargo_in_vehicle_lists; ///< Show the cargoes the vehicles can carry in the list windows bool auto_remove_signals; ///< automatically remove signals when in the way during rail construction diff --git a/src/story.cpp b/src/story.cpp index a03c2a12f7..ad732f9d37 100644 --- a/src/story.cpp +++ b/src/story.cpp @@ -143,7 +143,7 @@ void StoryPageButtonData::SetVehicleType(VehicleType vehtype) /** Get the button background colour. */ Colours StoryPageButtonData::GetColour() const { - Colours colour = (Colours)GB(this->referenced_id, 0, 8); + Colours colour = static_cast(GB(this->referenced_id, 0, 8)); if (!IsValidColours(colour)) return INVALID_COLOUR; return colour; } diff --git a/src/table/town_land.h b/src/table/town_land.h index 83b663add1..d2d991c328 100644 --- a/src/table/town_land.h +++ b/src/table/town_land.h @@ -1813,7 +1813,7 @@ static_assert(lengthof(_town_draw_tile_data) == (NEW_HOUSE_OFFSET) * 4 * 4); {mnd, mxd, p, rc, bn, rr, mg, \ {ca1, ca2, ca3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, \ {cg1, cg2, cg3, CT_INVALID, CT_INVALID, CT_INVALID, CT_INVALID, CT_INVALID, CT_INVALID, CT_INVALID, CT_INVALID, CT_INVALID, CT_INVALID, CT_INVALID, CT_INVALID, CT_INVALID}, \ - bf, ba, true, GRFFileProps(INVALID_HOUSE_ID), 0, {0, 0, 0, 0}, \ + bf, ba, true, GRFFileProps(INVALID_HOUSE_ID), 0, {COLOUR_BEGIN, COLOUR_BEGIN, COLOUR_BEGIN, COLOUR_BEGIN}, \ 16, NO_EXTRA_FLAG, HOUSE_NO_CLASS, {0, 2, 0, 0}, 0, 0, 0} /** House specifications from original data */ static const HouseSpec _original_house_specs[] = {