Codechange: Use EnumBitSet for TownActions. (#13542)

src/script/api/script_town.cpp

@@ -258,7 +258,7 @@
 	EnforceCompanyModeValid(false);
 	if (!IsValidTown(town_id)) return false;
 
-	return HasBit(::GetMaskOfTownActions(ScriptObject::GetCompany(), ::Town::Get(town_id)), town_action);
+	return ::GetMaskOfTownActions(ScriptObject::GetCompany(), ::Town::Get(town_id)).Test(::TownAction(town_action));
 }
 
 /* static */ bool ScriptTown::PerformTownAction(TownID town_id, TownAction town_action)
@@ -267,7 +267,7 @@
 	EnforcePrecondition(false, IsValidTown(town_id));
 	EnforcePrecondition(false, IsActionAvailable(town_id, town_action));
 
-	return ScriptObject::Command<CMD_DO_TOWN_ACTION>::Do(town_id, town_action);
+	return ScriptObject::Command<CMD_DO_TOWN_ACTION>::Do(town_id, ::TownAction(town_action));
 }
 
 /* static */ bool ScriptTown::ExpandTown(TownID town_id, SQInteger houses)

src/script/api/script_town.hpp

@@ -12,6 +12,7 @@
 
 #include "script_cargo.hpp"
 #include "script_company.hpp"
+#include "../../town.h"
 #include "../../town_type.h"
 
 /**
@@ -31,49 +32,49 @@ public:
 		 * absolute percentage, so 10% becomes 35%, with a max of 99%)
 		 * for all stations within 10 tiles.
 		 */
-		TOWN_ACTION_ADVERTISE_SMALL  = 0,
+		TOWN_ACTION_ADVERTISE_SMALL  = to_underlying(::TownAction::AdvertiseSmall),
 
 		/**
 		 * The cargo ratings temporary gains 44% of rating (in
 		 * absolute percentage, so 10% becomes 54%, with a max of 99%)
 		 * for all stations within 15 tiles.
 		 */
-		TOWN_ACTION_ADVERTISE_MEDIUM = 1,
+		TOWN_ACTION_ADVERTISE_MEDIUM = to_underlying(::TownAction::AdvertiseMedium),
 
 		/**
 		 * The cargo ratings temporary gains 63% of rating (in
 		 * absolute percentage, so 10% becomes 73%, with a max of 99%)
 		 * for all stations within 20 tiles.
 		 */
-		TOWN_ACTION_ADVERTISE_LARGE  = 2,
+		TOWN_ACTION_ADVERTISE_LARGE  = to_underlying(::TownAction::AdvertiseLarge),
 
 		/**
 		 * Rebuild the roads of this town for 6 economy-months.
 		 * @see \ref ScriptEconomyTime
 		 */
-		TOWN_ACTION_ROAD_REBUILD     = 3,
+		TOWN_ACTION_ROAD_REBUILD     = to_underlying(::TownAction::RoadRebuild),
 
 		/**
 		 * Build a statue in this town.
 		 */
-		TOWN_ACTION_BUILD_STATUE     = 4,
+		TOWN_ACTION_BUILD_STATUE     = to_underlying(::TownAction::BuildStatue),
 
 		/**
 		 * Fund the creation of extra buildings for 3 economy-months.
 		 * @see \ref ScriptEconomyTime
 		 */
-		TOWN_ACTION_FUND_BUILDINGS   = 5,
+		TOWN_ACTION_FUND_BUILDINGS   = to_underlying(::TownAction::FundBuildings),
 
 		/**
 		 * Buy exclusive rights for this town for 12 economy-months.
 		 * @see \ref ScriptEconomyTime
 		 */
-		TOWN_ACTION_BUY_RIGHTS       = 6,
+		TOWN_ACTION_BUY_RIGHTS       = to_underlying(::TownAction::BuyRights),
 
 		/**
 		 * Bribe the town in order to get a higher rating.
 		 */
-		TOWN_ACTION_BRIBE            = 7,
+		TOWN_ACTION_BRIBE            = to_underlying(::TownAction::Bribe),
 	};
 
 	/**

src/town.h

@@ -207,26 +207,20 @@ Town *CalcClosestTownFromTile(TileIndex tile, uint threshold = UINT_MAX);
 void ResetHouses();
 
 /** Town actions of a company. */
-enum TownActions {
+enum class TownAction : uint8_t {
-	TACT_NONE             = 0x00, ///< Empty action set.
+	AdvertiseSmall, ///< Small advertising campaign.
-
+	AdvertiseMedium, ///< Medium advertising campaign.
-	TACT_ADVERTISE_SMALL  = 0x01, ///< Small advertising campaign.
+	AdvertiseLarge, ///< Large advertising campaign.
-	TACT_ADVERTISE_MEDIUM = 0x02, ///< Medium advertising campaign.
+	RoadRebuild, ///< Rebuild the roads.
-	TACT_ADVERTISE_LARGE  = 0x04, ///< Large advertising campaign.
+	BuildStatue, ///< Build a statue.
-	TACT_ROAD_REBUILD     = 0x08, ///< Rebuild the roads.
+	FundBuildings, ///< Fund new buildings.
-	TACT_BUILD_STATUE     = 0x10, ///< Build a statue.
+	BuyRights, ///< Buy exclusive transport rights.
-	TACT_FUND_BUILDINGS   = 0x20, ///< Fund new buildings.
+	Bribe, ///< Try to bribe the council.
-	TACT_BUY_RIGHTS       = 0x40, ///< Buy exclusive transport rights.
+	End,
-	TACT_BRIBE            = 0x80, ///< Try to bribe the council.
-
-	TACT_COUNT            = 8,    ///< Number of available town actions.
-
-	TACT_ADVERTISE        = TACT_ADVERTISE_SMALL | TACT_ADVERTISE_MEDIUM | TACT_ADVERTISE_LARGE, ///< All possible advertising actions.
-	TACT_CONSTRUCTION     = TACT_ROAD_REBUILD | TACT_BUILD_STATUE | TACT_FUND_BUILDINGS,         ///< All possible construction actions.
-	TACT_FUNDS            = TACT_BUY_RIGHTS | TACT_BRIBE,                                        ///< All possible funding actions.
-	TACT_ALL              = TACT_ADVERTISE | TACT_CONSTRUCTION | TACT_FUNDS,                     ///< All possible actions.
 };
-DECLARE_ENUM_AS_BIT_SET(TownActions)
+using TownActions = EnumBitSet<TownAction, uint8_t>;
+
+DECLARE_INCREMENT_DECREMENT_OPERATORS(TownAction);
 
 void ClearTownHouse(Town *t, TileIndex tile);
 void UpdateTownMaxPass(Town *t);
@@ -241,7 +235,7 @@ bool GenerateTowns(TownLayout layout);
 const CargoSpec *FindFirstCargoWithTownAcceptanceEffect(TownAcceptanceEffect effect);
 CargoArray GetAcceptedCargoOfHouse(const HouseSpec *hs);
 
-extern const uint8_t _town_action_costs[TACT_COUNT];
+uint8_t GetTownActionCost(TownAction action);
 
 /**
  * Set the default name for a depot/waypoint

src/town_cmd.cpp

@@ -3333,12 +3333,24 @@ CommandCost CmdDeleteTown(DoCommandFlag flags, TownID town_id)
 }
 
 /**
- * Factor in the cost of each town action.
+ * Get cost factors for a TownAction
- * @see TownActions
+ * @param action TownAction to get cost factor for.
+ * @returns Cost factor.
  */
-const uint8_t _town_action_costs[TACT_COUNT] = {
+uint8_t GetTownActionCost(TownAction action)
-	2, 4, 9, 35, 48, 53, 117, 175
+{
-};
+	/**
+	 * Factor in the cost of each town action.
+	 * @see TownActions
+	 */
+	static const uint8_t town_action_costs[] = {
+		2, 4, 9, 35, 48, 53, 117, 175
+	};
+	static_assert(std::size(town_action_costs) == to_underlying(TownAction::End));
+
+	assert(to_underlying(action) < std::size(town_action_costs));
+	return town_action_costs[to_underlying(action)];
+}
 
 /**
  * Perform the "small advertising campaign" town action.
@@ -3620,6 +3632,7 @@ static TownActionProc * const _town_action_proc[] = {
 	TownActionBuyRights,
 	TownActionBribe
 };
+static_assert(std::size(_town_action_proc) == to_underlying(TownAction::End));
 
 /**
  * Get a list of available town authority actions.
@@ -3629,7 +3642,7 @@ static TownActionProc * const _town_action_proc[] = {
  */
 TownActions GetMaskOfTownActions(CompanyID cid, const Town *t)
 {
-	TownActions buttons = TACT_NONE;
+	TownActions buttons{};
 
 	/* Spectators and unwanted have no options */
 	if (cid != COMPANY_SPECTATOR && !(_settings_game.economy.bribe && t->unwanted[cid])) {
@@ -3639,11 +3652,10 @@ TownActions GetMaskOfTownActions(CompanyID cid, const Town *t)
 
 		/* Check the action bits for validity and
 		 * if they are valid add them */
-		for (uint i = 0; i != lengthof(_town_action_costs); i++) {
+		for (TownAction cur = {}; cur != TownAction::End; ++cur) {
-			const TownActions cur = (TownActions)(1 << i);
 
 			/* Is the company prohibited from bribing ? */
-			if (cur == TACT_BRIBE) {
+			if (cur == TownAction::Bribe) {
 				/* Company can't bribe if setting is disabled */
 				if (!_settings_game.economy.bribe) continue;
 				/* Company can bribe if another company has exclusive transport rights,
@@ -3655,19 +3667,19 @@ TownActions GetMaskOfTownActions(CompanyID cid, const Town *t)
 			}
 
 			/* Is the company not able to buy exclusive rights ? */
-			if (cur == TACT_BUY_RIGHTS && (!_settings_game.economy.exclusive_rights || t->exclusive_counter != 0)) continue;
+			if (cur == TownAction::BuyRights && (!_settings_game.economy.exclusive_rights || t->exclusive_counter != 0)) continue;
 
 			/* Is the company not able to fund buildings ? */
-			if (cur == TACT_FUND_BUILDINGS && !_settings_game.economy.fund_buildings) continue;
+			if (cur == TownAction::FundBuildings && !_settings_game.economy.fund_buildings) continue;
 
 			/* Is the company not able to fund local road reconstruction? */
-			if (cur == TACT_ROAD_REBUILD && !_settings_game.economy.fund_roads) continue;
+			if (cur == TownAction::RoadRebuild && !_settings_game.economy.fund_roads) continue;
 
 			/* Is the company not able to build a statue ? */
-			if (cur == TACT_BUILD_STATUE && t->statues.Test(cid)) continue;
+			if (cur == TownAction::BuildStatue && t->statues.Test(cid)) continue;
 
-			if (avail >= _town_action_costs[i] * _price[PR_TOWN_ACTION] >> 8) {
+			if (avail >= GetTownActionCost(cur) * _price[PR_TOWN_ACTION] >> 8) {
-				buttons |= cur;
+				buttons.Set(cur);
 			}
 		}
 	}
@@ -3684,16 +3696,16 @@ TownActions GetMaskOfTownActions(CompanyID cid, const Town *t)
  * @param action action to perform, @see _town_action_proc for the list of available actions
  * @return the cost of this operation or an error
  */
-CommandCost CmdDoTownAction(DoCommandFlag flags, TownID town_id, uint8_t action)
+CommandCost CmdDoTownAction(DoCommandFlag flags, TownID town_id, TownAction action)
 {
 	Town *t = Town::GetIfValid(town_id);
-	if (t == nullptr || action >= lengthof(_town_action_proc)) return CMD_ERROR;
+	if (t == nullptr || to_underlying(action) >= std::size(_town_action_proc)) return CMD_ERROR;
 
-	if (!HasBit(GetMaskOfTownActions(_current_company, t), action)) return CMD_ERROR;
+	if (!GetMaskOfTownActions(_current_company, t).Test(action)) return CMD_ERROR;
 
-	CommandCost cost(EXPENSES_OTHER, _price[PR_TOWN_ACTION] * _town_action_costs[action] >> 8);
+	CommandCost cost(EXPENSES_OTHER, _price[PR_TOWN_ACTION] * GetTownActionCost(action) >> 8);
 
-	CommandCost ret = _town_action_proc[action](t, flags);
+	CommandCost ret = _town_action_proc[to_underlying(action)](t, flags);
 	if (ret.Failed()) return ret;
 
 	if (flags & DC_EXEC) {

src/town_cmd.h

@@ -12,6 +12,7 @@
 
 #include "command_type.h"
 #include "company_type.h"
+#include "town.h"
 #include "town_type.h"
 
 enum TownAcceptanceEffect : uint8_t;
@@ -19,7 +20,7 @@ using HouseID = uint16_t;
 
 std::tuple<CommandCost, Money, TownID> CmdFoundTown(DoCommandFlag flags, TileIndex tile, TownSize size, bool city, TownLayout layout, bool random_location, uint32_t townnameparts, const std::string &text);
 CommandCost CmdRenameTown(DoCommandFlag flags, TownID town_id, const std::string &text);
-CommandCost CmdDoTownAction(DoCommandFlag flags, TownID town_id, uint8_t action);
+CommandCost CmdDoTownAction(DoCommandFlag flags, TownID town_id, TownAction action);
 CommandCost CmdTownGrowthRate(DoCommandFlag flags, TownID town_id, uint16_t growth_rate);
 CommandCost CmdTownRating(DoCommandFlag flags, TownID town_id, CompanyID company_id, int16_t rating);
 CommandCost CmdTownCargoGoal(DoCommandFlag flags, TownID town_id, TownAcceptanceEffect tae, uint32_t goal);

src/town_gui.cpp

@@ -80,11 +80,11 @@ static constexpr NWidgetPart _nested_town_authority_widgets[] = {
 struct TownAuthorityWindow : Window {
 private:
 	Town *town;    ///< Town being displayed.
-	int sel_index; ///< Currently selected town action, \c 0 to \c TACT_COUNT-1, \c -1 means no action selected.
+	TownAction sel_action = TownAction::End; ///< Currently selected town action, TownAction::End means no action selected.
-	uint displayed_actions_on_previous_painting; ///< Actions that were available on the previous call to OnPaint()
+	TownActions displayed_actions_on_previous_painting{}; ///< Actions that were available on the previous call to OnPaint()
 	TownActions enabled_actions; ///< Actions that are enabled in settings.
-	TownActions available_actions; ///< Actions that are available to execute for the current company.
+	TownActions available_actions{}; ///< Actions that are available to execute for the current company.
-	StringID action_tooltips[TACT_COUNT];
+	StringID action_tooltips[to_underlying(TownAction::End)];
 
 	Dimension icon_size;      ///< Dimensions of company icon
 	Dimension exclusive_size; ///< Dimensions of exclusive icon
@@ -100,7 +100,7 @@ private:
 	int GetNthSetBit(int n)
 	{
 		if (n >= 0) {
-			for (uint i : SetBitIterator(this->enabled_actions)) {
+			for (uint i : SetBitIterator(this->enabled_actions.base())) {
 				n--;
 				if (n < 0) return i;
 			}
@@ -115,18 +115,19 @@ private:
 	 */
 	static TownActions GetEnabledActions()
 	{
-		TownActions enabled = TACT_ALL;
+		TownActions enabled{};
+		enabled.Set();
 
-		if (!_settings_game.economy.fund_roads) CLRBITS(enabled, TACT_ROAD_REBUILD);
+		if (!_settings_game.economy.fund_roads) enabled.Reset(TownAction::RoadRebuild);
-		if (!_settings_game.economy.fund_buildings) CLRBITS(enabled, TACT_FUND_BUILDINGS);
+		if (!_settings_game.economy.fund_buildings) enabled.Reset(TownAction::FundBuildings);
-		if (!_settings_game.economy.exclusive_rights) CLRBITS(enabled, TACT_BUY_RIGHTS);
+		if (!_settings_game.economy.exclusive_rights) enabled.Reset(TownAction::BuyRights);
-		if (!_settings_game.economy.bribe) CLRBITS(enabled, TACT_BRIBE);
+		if (!_settings_game.economy.bribe) enabled.Reset(TownAction::Bribe);
 
 		return enabled;
 	}
 
 public:
-	TownAuthorityWindow(WindowDesc &desc, WindowNumber window_number) : Window(desc), sel_index(-1), displayed_actions_on_previous_painting(0), available_actions(TACT_NONE)
+	TownAuthorityWindow(WindowDesc &desc, WindowNumber window_number) : Window(desc)
 	{
 		this->town = Town::Get(window_number);
 		this->enabled_actions = GetEnabledActions();
@@ -157,7 +158,7 @@ public:
 		displayed_actions_on_previous_painting = this->available_actions;
 
 		this->SetWidgetLoweredState(WID_TA_ZONE_BUTTON, this->town->show_zone);
-		this->SetWidgetDisabledState(WID_TA_EXECUTE, (this->sel_index == -1) || !HasBit(this->available_actions, this->sel_index));
+		this->SetWidgetDisabledState(WID_TA_EXECUTE, (this->sel_action == TownAction::End) || !this->available_actions.Test(this->sel_action));
 
 		this->DrawWidgets();
 		if (!this->IsShaded())
@@ -228,16 +229,16 @@ public:
 		r.top += GetCharacterHeight(FS_NORMAL);
 
 		/* Draw list of actions */
-		for (int i = 0; i < TACT_COUNT; i++) {
+		for (TownAction i = {}; i != TownAction::End; ++i) {
 			/* Don't show actions if disabled in settings. */
-			if (!HasBit(this->enabled_actions, i)) continue;
+			if (!this->enabled_actions.Test(i)) continue;
 
 			/* Set colour of action based on ability to execute and if selected. */
 			TextColour action_colour = TC_GREY | TC_NO_SHADE;
-			if (HasBit(this->available_actions, i)) action_colour = TC_ORANGE;
+			if (this->available_actions.Test(i)) action_colour = TC_ORANGE;
-			if (this->sel_index == i) action_colour = TC_WHITE;
+			if (this->sel_action == i) action_colour = TC_WHITE;
 
-			DrawString(r, STR_LOCAL_AUTHORITY_ACTION_SMALL_ADVERTISING_CAMPAIGN + i, action_colour);
+			DrawString(r, STR_LOCAL_AUTHORITY_ACTION_SMALL_ADVERTISING_CAMPAIGN + to_underlying(i), action_colour);
 			r.top += GetCharacterHeight(FS_NORMAL);
 		}
 	}
@@ -251,13 +252,13 @@ public:
 	{
 		switch (widget) {
 			case WID_TA_ACTION_INFO:
-				if (this->sel_index != -1) {
+				if (this->sel_action != TownAction::End) {
-					Money action_cost = _price[PR_TOWN_ACTION] * _town_action_costs[this->sel_index] >> 8;
+					Money action_cost = _price[PR_TOWN_ACTION] * GetTownActionCost(this->sel_action) >> 8;
 					bool affordable = Company::IsValidID(_local_company) && action_cost < GetAvailableMoney(_local_company);
 
 					SetDParam(0, action_cost);
 					DrawStringMultiLine(r.Shrink(WidgetDimensions::scaled.framerect),
-						this->action_tooltips[this->sel_index],
+						this->action_tooltips[to_underlying(this->sel_action)],
 						affordable ? TC_YELLOW : TC_RED);
 				}
 				break;
@@ -270,9 +271,9 @@ public:
 			case WID_TA_ACTION_INFO: {
 				assert(size.width > padding.width && size.height > padding.height);
 				Dimension d = {0, 0};
-				for (int i = 0; i < TACT_COUNT; i++) {
+				for (TownAction i = {}; i != TownAction::End; ++i) {
-					SetDParam(0, _price[PR_TOWN_ACTION] * _town_action_costs[i] >> 8);
+					SetDParam(0, _price[PR_TOWN_ACTION] * GetTownActionCost(i) >> 8);
-					d = maxdim(d, GetStringMultiLineBoundingBox(this->action_tooltips[i], size));
+					d = maxdim(d, GetStringMultiLineBoundingBox(this->action_tooltips[to_underlying(i)], size));
 				}
 				d.width += padding.width;
 				d.height += padding.height;
@@ -281,10 +282,10 @@ public:
 			}
 
 			case WID_TA_COMMAND_LIST:
-				size.height = (TACT_COUNT + 1) * GetCharacterHeight(FS_NORMAL) + padding.height;
+				size.height = (to_underlying(TownAction::End) + 1) * GetCharacterHeight(FS_NORMAL) + padding.height;
 				size.width = GetStringBoundingBox(STR_LOCAL_AUTHORITY_ACTIONS_TITLE).width;
-				for (uint i = 0; i < TACT_COUNT; i++ ) {
+				for (TownAction i = {}; i != TownAction::End; ++i) {
-					size.width = std::max(size.width, GetStringBoundingBox(STR_LOCAL_AUTHORITY_ACTION_SMALL_ADVERTISING_CAMPAIGN + i).width + padding.width);
+					size.width = std::max(size.width, GetStringBoundingBox(STR_LOCAL_AUTHORITY_ACTION_SMALL_ADVERTISING_CAMPAIGN + to_underlying(i)).width + padding.width);
 				}
 				size.width += padding.width;
 				break;
@@ -316,17 +317,17 @@ public:
 
 				y = GetNthSetBit(y);
 				if (y >= 0) {
-					this->sel_index = y;
+					this->sel_action = static_cast<TownAction>(y);
 					this->SetDirty();
 				}
 
 				/* When double-clicking, continue */
-				if (click_count == 1 || y < 0 || !HasBit(this->available_actions, y)) break;
+				if (click_count == 1 || y < 0 || !this->available_actions.Test(this->sel_action)) break;
 				[[fallthrough]];
 			}
 
 			case WID_TA_EXECUTE:
-				Command<CMD_DO_TOWN_ACTION>::Post(STR_ERROR_CAN_T_DO_THIS, this->town->xy, static_cast<TownID>(this->window_number), this->sel_index);
+				Command<CMD_DO_TOWN_ACTION>::Post(STR_ERROR_CAN_T_DO_THIS, this->town->xy, static_cast<TownID>(this->window_number), this->sel_action);
 				break;
 		}
 	}
@@ -341,8 +342,8 @@ public:
 		if (!gui_scope) return;
 
 		this->enabled_actions = this->GetEnabledActions();
-		if (!HasBit(this->enabled_actions, this->sel_index)) {
+		if (!this->enabled_actions.Test(this->sel_action)) {
-			this->sel_index = -1;
+			this->sel_action = TownAction::End;
 		}
 	}
 };