From 34bbae05dbb8997e65271afdef65e5526968d260 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sat, 1 Nov 2025 14:25:24 +0000 Subject: [PATCH] Codechange: Use enum/EnumBitSet for livery in use flags. (#14746) Replaces magic numbers. --- src/company_cmd.cpp | 22 +++++++++++----------- src/company_gui.cpp | 10 +++++----- src/group_cmd.cpp | 18 +++++++++--------- src/livery.h | 9 ++++++++- src/saveload/company_sl.cpp | 4 ++-- src/script/api/script_group.cpp | 4 ++-- src/vehicle.cpp | 6 +++--- 7 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/company_cmd.cpp b/src/company_cmd.cpp index 577f54b4b4..c336e4bf21 100644 --- a/src/company_cmd.cpp +++ b/src/company_cmd.cpp @@ -574,14 +574,14 @@ restart:; void ResetCompanyLivery(Company *c) { for (LiveryScheme scheme = LS_BEGIN; scheme < LS_END; scheme++) { - c->livery[scheme].in_use = 0; + c->livery[scheme].in_use.Reset(); c->livery[scheme].colour1 = c->colour; c->livery[scheme].colour2 = c->colour; } for (Group *g : Group::Iterate()) { if (g->owner == c->index) { - g->livery.in_use = 0; + g->livery.in_use.Reset(); g->livery.colour1 = c->colour; g->livery.colour2 = c->colour; } @@ -1077,8 +1077,8 @@ CommandCost CmdSetCompanyManagerFace(DoCommandFlags flags, uint style, uint32_t void UpdateCompanyLiveries(Company *c) { for (int i = 1; i < LS_END; i++) { - if (!HasBit(c->livery[i].in_use, 0)) c->livery[i].colour1 = c->livery[LS_DEFAULT].colour1; - if (!HasBit(c->livery[i].in_use, 1)) c->livery[i].colour2 = c->livery[LS_DEFAULT].colour2; + if (!c->livery[i].in_use.Test(Livery::Flag::Primary)) c->livery[i].colour1 = c->livery[LS_DEFAULT].colour1; + if (!c->livery[i].in_use.Test(Livery::Flag::Secondary)) c->livery[i].colour2 = c->livery[LS_DEFAULT].colour2; } UpdateCompanyGroupLiveries(c); } @@ -1109,7 +1109,7 @@ CommandCost CmdSetCompanyColour(DoCommandFlags flags, LiveryScheme scheme, bool if (flags.Test(DoCommandFlag::Execute)) { if (primary) { - if (scheme != LS_DEFAULT) AssignBit(c->livery[scheme].in_use, 0, colour != INVALID_COLOUR); + if (scheme != LS_DEFAULT) c->livery[scheme].in_use.Set(Livery::Flag::Primary, colour != INVALID_COLOUR); if (colour == INVALID_COLOUR) colour = c->livery[LS_DEFAULT].colour1; c->livery[scheme].colour1 = colour; @@ -1122,7 +1122,7 @@ CommandCost CmdSetCompanyColour(DoCommandFlags flags, LiveryScheme scheme, bool CompanyAdminUpdate(c); } } else { - if (scheme != LS_DEFAULT) AssignBit(c->livery[scheme].in_use, 1, colour != INVALID_COLOUR); + if (scheme != LS_DEFAULT) c->livery[scheme].in_use.Set(Livery::Flag::Secondary, colour != INVALID_COLOUR); if (colour == INVALID_COLOUR) colour = c->livery[LS_DEFAULT].colour2; c->livery[scheme].colour2 = colour; @@ -1131,16 +1131,16 @@ CommandCost CmdSetCompanyColour(DoCommandFlags flags, LiveryScheme scheme, bool } } - if (c->livery[scheme].in_use != 0) { + if (c->livery[scheme].in_use.Any({Livery::Flag::Primary, Livery::Flag::Secondary})) { /* If enabling a scheme, set the default scheme to be in use too */ - c->livery[LS_DEFAULT].in_use = 1; + c->livery[LS_DEFAULT].in_use.Set(Livery::Flag::Primary); } else { /* Else loop through all schemes to see if any are left enabled. * If not, disable the default scheme too. */ - c->livery[LS_DEFAULT].in_use = 0; + c->livery[LS_DEFAULT].in_use.Reset({Livery::Flag::Primary, Livery::Flag::Secondary}); for (scheme = LS_DEFAULT; scheme < LS_END; scheme++) { - if (c->livery[scheme].in_use != 0) { - c->livery[LS_DEFAULT].in_use = 1; + if (c->livery[scheme].in_use.Any({Livery::Flag::Primary, Livery::Flag::Secondary})) { + c->livery[LS_DEFAULT].in_use.Set(Livery::Flag::Primary); break; } } diff --git a/src/company_gui.cpp b/src/company_gui.cpp index bf336ed35a..b302f5874e 100644 --- a/src/company_gui.cpp +++ b/src/company_gui.cpp @@ -655,7 +655,7 @@ private: } uint8_t sel; - if (default_livery == nullptr || HasBit(livery->in_use, primary ? 0 : 1)) { + if (default_livery == nullptr || livery->in_use.Test(primary ? Livery::Flag::Primary : Livery::Flag::Secondary)) { sel = primary ? livery->colour1 : livery->colour2; } else { sel = default_col; @@ -819,7 +819,7 @@ public: } if (scheme == LS_END) scheme = LS_DEFAULT; const Livery *livery = &c->livery[scheme]; - if (scheme == LS_DEFAULT || HasBit(livery->in_use, primary ? 0 : 1)) { + if (scheme == LS_DEFAULT || livery->in_use.Test(primary ? Livery::Flag::Primary : Livery::Flag::Secondary)) { colour = STR_COLOUR_DARK_BLUE + (primary ? livery->colour1 : livery->colour2); } } @@ -827,7 +827,7 @@ public: if (this->sel != GroupID::Invalid()) { const Group *g = Group::Get(this->sel); const Livery *livery = &g->livery; - if (HasBit(livery->in_use, primary ? 0 : 1)) { + if (livery->in_use.Test(primary ? Livery::Flag::Primary : Livery::Flag::Secondary)) { colour = STR_COLOUR_DARK_BLUE + (primary ? livery->colour1 : livery->colour2); } } @@ -875,12 +875,12 @@ public: /* Text below the first dropdown. */ DrawSprite(SPR_SQUARE, GetColourPalette(livery.colour1), pri_squ.left, y + square_offs); - DrawString(pri.left, pri.right, y + text_offs, (is_default_scheme || HasBit(livery.in_use, 0)) ? STR_COLOUR_DARK_BLUE + livery.colour1 : STR_COLOUR_DEFAULT, is_selected ? TC_WHITE : TC_GOLD); + DrawString(pri.left, pri.right, y + text_offs, (is_default_scheme || livery.in_use.Test(Livery::Flag::Primary)) ? STR_COLOUR_DARK_BLUE + livery.colour1 : STR_COLOUR_DEFAULT, is_selected ? TC_WHITE : TC_GOLD); /* Text below the second dropdown. */ if (sec.right > sec.left) { // Second dropdown has non-zero size. DrawSprite(SPR_SQUARE, GetColourPalette(livery.colour2), sec_squ.left, y + square_offs); - DrawString(sec.left, sec.right, y + text_offs, (is_default_scheme || HasBit(livery.in_use, 1)) ? STR_COLOUR_DARK_BLUE + livery.colour2 : STR_COLOUR_DEFAULT, is_selected ? TC_WHITE : TC_GOLD); + DrawString(sec.left, sec.right, y + text_offs, (is_default_scheme || livery.in_use.Test(Livery::Flag::Secondary)) ? STR_COLOUR_DARK_BLUE + livery.colour2 : STR_COLOUR_DEFAULT, is_selected ? TC_WHITE : TC_GOLD); } y += this->line_height; diff --git a/src/group_cmd.cpp b/src/group_cmd.cpp index b0e4ce4ee5..2ba83c4dd8 100644 --- a/src/group_cmd.cpp +++ b/src/group_cmd.cpp @@ -306,8 +306,8 @@ static void PropagateChildLivery(const Group *g, bool reset_cache) for (const GroupID &childgroup : g->children) { Group *cg = Group::Get(childgroup); - if (!HasBit(cg->livery.in_use, 0)) cg->livery.colour1 = g->livery.colour1; - if (!HasBit(cg->livery.in_use, 1)) cg->livery.colour2 = g->livery.colour2; + if (!cg->livery.in_use.Test(Livery::Flag::Primary)) cg->livery.colour1 = g->livery.colour1; + if (!cg->livery.in_use.Test(Livery::Flag::Secondary)) cg->livery.colour2 = g->livery.colour2; PropagateChildLivery(cg, reset_cache); } } @@ -321,8 +321,8 @@ void UpdateCompanyGroupLiveries(const Company *c) { for (Group *g : Group::Iterate()) { if (g->owner == c->index && g->parent == GroupID::Invalid()) { - if (!HasBit(g->livery.in_use, 0)) g->livery.colour1 = c->livery[LS_DEFAULT].colour1; - if (!HasBit(g->livery.in_use, 1)) g->livery.colour2 = c->livery[LS_DEFAULT].colour2; + if (!g->livery.in_use.Test(Livery::Flag::Primary)) g->livery.colour1 = c->livery[LS_DEFAULT].colour1; + if (!g->livery.in_use.Test(Livery::Flag::Secondary)) g->livery.colour2 = c->livery[LS_DEFAULT].colour2; PropagateChildLivery(g, false); } } @@ -480,11 +480,11 @@ CommandCost CmdAlterGroup(DoCommandFlags flags, AlterGroupMode mode, GroupID gro GroupStatistics::UpdateAutoreplace(g->owner); - if (!HasBit(g->livery.in_use, 0) || !HasBit(g->livery.in_use, 1)) { + if (!g->livery.in_use.All({Livery::Flag::Primary, Livery::Flag::Secondary})) { /* Update livery with new parent's colours if either colour is default. */ const Livery *livery = GetParentLivery(g); - if (!HasBit(g->livery.in_use, 0)) g->livery.colour1 = livery->colour1; - if (!HasBit(g->livery.in_use, 1)) g->livery.colour2 = livery->colour2; + if (!g->livery.in_use.Test(Livery::Flag::Primary)) g->livery.colour1 = livery->colour1; + if (!g->livery.in_use.Test(Livery::Flag::Secondary)) g->livery.colour2 = livery->colour2; PropagateChildLivery(g, true); MarkWholeScreenDirty(); @@ -684,11 +684,11 @@ CommandCost CmdSetGroupLivery(DoCommandFlags flags, GroupID group_id, bool prima if (flags.Test(DoCommandFlag::Execute)) { if (primary) { - AssignBit(g->livery.in_use, 0, colour != INVALID_COLOUR); + g->livery.in_use.Set(Livery::Flag::Primary, colour != INVALID_COLOUR); if (colour == INVALID_COLOUR) colour = GetParentLivery(g)->colour1; g->livery.colour1 = colour; } else { - AssignBit(g->livery.in_use, 1, colour != INVALID_COLOUR); + g->livery.in_use.Set(Livery::Flag::Secondary, colour != INVALID_COLOUR); if (colour == INVALID_COLOUR) colour = GetParentLivery(g)->colour2; g->livery.colour2 = colour; } diff --git a/src/livery.h b/src/livery.h index 2de27555d9..31c39f7840 100644 --- a/src/livery.h +++ b/src/livery.h @@ -10,6 +10,7 @@ #ifndef LIVERY_H #define LIVERY_H +#include "core/enum_type.hpp" #include "company_type.h" #include "gfx_type.h" @@ -76,7 +77,13 @@ DECLARE_ENUM_AS_ADDABLE(LiveryClass) /** Information about a particular livery. */ struct Livery { - uint8_t in_use = 0; ///< Bit 0 set if this livery should override the default livery first colour, Bit 1 for the second colour. + enum class Flag : uint8_t { + Primary = 0, ///< Primary colour is set. + Secondary = 1, ///< Secondary colour is set. + }; + using Flags = EnumBitSet; + + Flags in_use{}; ///< Livery flags. Colours colour1 = COLOUR_BEGIN; ///< First colour, for all vehicles. Colours colour2 = COLOUR_BEGIN; ///< Second colour, for vehicles with 2CC support. }; diff --git a/src/saveload/company_sl.cpp b/src/saveload/company_sl.cpp index 679b42bded..5ed2a63111 100644 --- a/src/saveload/company_sl.cpp +++ b/src/saveload/company_sl.cpp @@ -445,11 +445,11 @@ public: for (size_t i = 0; i < num_liveries; i++) { SlObject(&c->livery[i], this->GetLoadDescription()); if (update_in_use && i != LS_DEFAULT) { - if (c->livery[i].in_use == 0) { + if (!c->livery[i].in_use.Any({Livery::Flag::Primary, Livery::Flag::Secondary})) { c->livery[i].colour1 = c->livery[LS_DEFAULT].colour1; c->livery[i].colour2 = c->livery[LS_DEFAULT].colour2; } else { - c->livery[i].in_use = 3; + c->livery[i].in_use = {Livery::Flag::Primary, Livery::Flag::Secondary}; } } } diff --git a/src/script/api/script_group.cpp b/src/script/api/script_group.cpp index 486032cd70..5713fddc35 100644 --- a/src/script/api/script_group.cpp +++ b/src/script/api/script_group.cpp @@ -241,7 +241,7 @@ EnforcePrecondition(ScriptCompany::Colours::COLOUR_INVALID, IsValidGroup(group_id)); const Group *g = ::Group::Get(group_id); - if (!HasBit(g->livery.in_use, 0)) return ScriptCompany::Colours::COLOUR_INVALID; + if (!g->livery.in_use.Test(Livery::Flag::Primary)) return ScriptCompany::Colours::COLOUR_INVALID; return (ScriptCompany::Colours)g->livery.colour1; } @@ -250,6 +250,6 @@ EnforcePrecondition(ScriptCompany::Colours::COLOUR_INVALID, IsValidGroup(group_id)); const Group *g = ::Group::Get(group_id); - if (!HasBit(g->livery.in_use, 1)) return ScriptCompany::Colours::COLOUR_INVALID; + if (!g->livery.in_use.Test(Livery::Flag::Secondary)) return ScriptCompany::Colours::COLOUR_INVALID; return (ScriptCompany::Colours)g->livery.colour2; } diff --git a/src/vehicle.cpp b/src/vehicle.cpp index bb1973e56e..95764752d2 100644 --- a/src/vehicle.cpp +++ b/src/vehicle.cpp @@ -2052,16 +2052,16 @@ const Livery *GetEngineLivery(EngineID engine_type, CompanyID company, EngineID const Group *g = Group::GetIfValid(v->First()->group_id); if (g != nullptr) { /* Traverse parents until we find a livery or reach the top */ - while (g->livery.in_use == 0 && g->parent != GroupID::Invalid()) { + while (!g->livery.in_use.Any({Livery::Flag::Primary, Livery::Flag::Secondary}) && g->parent != GroupID::Invalid()) { g = Group::Get(g->parent); } - if (g->livery.in_use != 0) return &g->livery; + if (g->livery.in_use.Any({Livery::Flag::Primary, Livery::Flag::Secondary})) return &g->livery; } } /* The default livery is always available for use, but its in_use flag determines * whether any _other_ liveries are in use. */ - if (c->livery[LS_DEFAULT].in_use != 0) { + if (c->livery[LS_DEFAULT].in_use.Any({Livery::Flag::Primary, Livery::Flag::Secondary})) { /* Determine the livery scheme to use */ scheme = GetEngineLiveryScheme(engine_type, parent_engine_type, v); }