Merge pull request #532 from Wargus/clean_up

Create `enum class ECondition`.

src/include/spells.h

@@ -41,6 +41,9 @@
 #include "unitsound.h"
 #include "vec2i.h"
 
+#include <string_view>
+#include <variant>
+
 /*----------------------------------------------------------------------------
 --  Declarations
 ----------------------------------------------------------------------------*/
@@ -83,6 +86,13 @@ enum TargetType {
 	TargetUnit
 };
 
+enum class ECondition
+{
+	Ignore,
+	ShouldBeFalse,
+	ShouldBeTrue
+};
+
 /*
 ** *******************
 ** Target definition.
@@ -111,7 +121,7 @@ class ConditionInfoVariable
 public:
 	ConditionInfoVariable() = default;
 
-	char Enable = 0;                /// Target is 'user defined variable'.
+	ECondition Enable = ECondition::Ignore; /// Target is 'user defined variable'.
 	bool Check = false;             /// True if need to check that variable.
 
 	int ExactValue = 0;             /// Target must have exactly ExactValue of it's value.
@@ -138,14 +148,11 @@ class ConditionInfo
 	//
 	//  Conditions that check specific flags. Possible values are the defines below.
 	//
-#define CONDITION_FALSE 1
-#define CONDITION_TRUE  0
-#define CONDITION_ONLY  2
-	char Alliance = 0;          /// Target is allied. (neutral is neither allied, nor opponent)
-	char Opponent = 0;          /// Target is opponent. (neutral is neither allied, nor opponent)
-	char TargetSelf = 1;        /// Target is the same as the caster.
+	ECondition Alliance = ECondition::Ignore; /// Target is allied. (neutral is neither allied, nor opponent)
+	ECondition Opponent = ECondition::Ignore; /// Target is opponent. (neutral is neither allied, nor opponent)
+	ECondition TargetSelf = ECondition::ShouldBeFalse; /// Target is the same as the caster.
 
-	std::vector<char> BoolFlag;         /// User defined boolean flag.
+	std::vector<ECondition> BoolFlag; /// User defined boolean flag.
 
 	std::vector<ConditionInfoVariable> Variable;
 	std::unique_ptr<LuaCallback> CheckFunc;
@@ -176,9 +183,9 @@ class AutoCastInfo
 
 	/// Detailed generic conditions (not per-target, where Condition is evaluated.)
 	/// Combat mode is when there are hostile non-coward units around
-	int Combat = 0;                  /// If it should be casted in combat
-	int Attacker = 0;                /// If it should be casted on unit which attacks
-	int Corpse = CONDITION_FALSE;    /// If it should be casted on corpses
+	ECondition Combat = ECondition::Ignore; /// If it should be casted in combat
+	ECondition Attacker = ECondition::Ignore; /// If it should be casted on unit which attacks
+	ECondition Corpse = ECondition::ShouldBeFalse; /// If it should be casted on corpses
 
 	// Position autocast callback
 	std::unique_ptr<LuaCallback> PositionAutoCast;
@@ -263,8 +270,11 @@ extern bool AutoCastSpell(CUnit &caster, const SpellType &spell);
 /// return spell type by ident string
 extern SpellType &SpellTypeByIdent(const std::string_view &ident);
 
-/// return 0, 1, 2 for true, only, false.
-extern char Ccl2Condition(lua_State *l, std::string_view value);
+/// return ECondition.
+extern ECondition Ccl2Condition(lua_State *l, std::string_view value);
+
+std::variant<ECondition, int> Ccl2ConditionOrNumber(lua_State *l, std::string_view value);
+
 
 //@}
 

src/include/ui.h

@@ -70,6 +70,7 @@ class CFile;
 class CFont;
 class LuaActionListener;
 class CPopup;
+enum class ECondition;
 
 /*----------------------------------------------------------------------------
 --  Definitions
@@ -185,8 +186,8 @@ class ConditionPanel
 	bool HideAllied = false;       /// if true, don't show for allied unit. (but show own units)
 	bool ShowOpponent = false;     /// if true, show for opponent unit.
 
-	std::vector<char> BoolFlags;   /// array of condition about user flags.
-	std::vector<char> Variables;   /// array of variable to verify (enable and max > 0)
+	std::vector<ECondition> BoolFlags; /// array of condition about user flags.
+	std::vector<std::variant<ECondition, int>> Variables;   /// array of variable to verify (enable and max > 0)
 };
 
 /**

src/include/ui/popup.h

@@ -49,6 +49,7 @@ class ButtonAction;
 class CFont;
 class CPopup;
 enum class ButtonCmd;
+enum class ECondition;
 
 #define MARGIN_X 4
 #define MARGIN_Y 2
@@ -65,8 +66,8 @@ class PopupConditionPanel
 	std::optional<ButtonCmd> ButtonAction; /// action type of button
 	std::string ButtonValue;    /// value used in ValueStr field of button
 
-	std::vector<char> BoolFlags; /// array of condition about user flags.
-	std::vector<char> Variables; /// array of variable to verify (enable and max > 0)
+	std::vector<ECondition> BoolFlags; /// array of condition about user flags.
+	std::vector<ECondition> Variables; /// array of variable to verify (enable and max > 0)
 };
 
 class CPopupContentType

src/include/unittype.h

@@ -51,6 +51,7 @@
 #include "color.h"
 #include "luacallback.h"
 #include "missileconfig.h"
+#include "spells.h"
 #include "vec2i.h"
 
 #include <climits>
@@ -562,7 +563,7 @@ class CUnitType
 	Vec2i GetHalfTileSize() const { return Vec2i(TileWidth / 2, TileHeight / 2); }
 	PixelSize GetPixelSize() const;
 
-	bool CheckUserBoolFlags(const std::vector<char> &BoolFlags) const;
+	bool CheckUserBoolFlags(const std::vector<ECondition> &BoolFlags) const;
 	bool CanTransport() const { return MaxOnBoard > 0 && !GivesResource; }
 	bool CanMove() const;
 
@@ -677,9 +678,9 @@ class CUnitType
 	CUnitStats MapDefaultStat;
 	struct BoolFlags {
 		bool value = false;             /// User defined flag. Used for (dis)allow target.
-		char CanTransport = 0;      /// Can transport units with this flag.
-		char CanTargetFlag = 0;     /// Flag needed to target with missile.
-		char AiPriorityTarget = 0;  /// Attack this units first.
+		ECondition CanTransport = ECondition::Ignore; /// Can transport units with this flag.
+		ECondition CanTargetFlag = ECondition::Ignore; /// Flag needed to target with missile.
+		ECondition AiPriorityTarget = ECondition::Ignore; /// Attack this units first.
 	};
 	std::vector<BoolFlags> BoolFlag;
 

src/spell/script_spell.cpp

@@ -111,19 +111,33 @@ static std::unique_ptr<SpellActionType> CclSpellAction(lua_State *l)
 **  @param l      Lua state.
 **  @param value  scm value to convert.
 **
-**  @return CONDITION_TRUE, CONDITION_FALSE, CONDITION_ONLY or -1 on error.
-**  @note This is a helper function to make CclSpellCondition shorter
-**        and easier to understand.
+**  @return ECondition.
 */
-char Ccl2Condition(lua_State *l, std::string_view value)
+ECondition Ccl2Condition(lua_State *l, std::string_view value)
 {
 	if (value == "true") {
-		return CONDITION_TRUE;
+		return ECondition::Ignore;
 	} else if (value == "false") {
-		return CONDITION_FALSE;
+		return ECondition::ShouldBeFalse;
 	} else if (value == "only") {
-		return CONDITION_ONLY;
-	} else if (value[0] == '<') {
+		return ECondition::ShouldBeTrue;
+	} else {
+		LuaError(l, "Bad condition result: %s", value.data());
+		ExitFatal(-1);
+	}
+}
+
+/**
+**  Get a condition value from a scm object.
+**
+**  @param l      Lua state.
+**  @param value  scm value to convert.
+**
+**  @return ECondition.
+*/
+std::variant<ECondition, int> Ccl2ConditionOrNumber(lua_State *l, std::string_view value)
+{
+	if (value[0] == '<') {
 		int v = to_number(value.substr(1));
 		if (v > 100) {
 			LuaError(l, "Can only encode condition '<' up to 100%%, got %d", v);
@@ -134,13 +148,13 @@ char Ccl2Condition(lua_State *l, std::string_view value)
 		if (v > 100) {
 			LuaError(l, "Can only encode condition '<' up to 100%%, got %d", v);
 		}
-		return v + CONDITION_ONLY;
+		return v;
 	} else {
-		LuaError(l, "Bad condition result: %s", value.data());
-		return -1;
+		return Ccl2Condition(l, value);
 	}
 }
 
+
 /**
 **  Parse the Condition for spell.
 **
@@ -151,11 +165,11 @@ char Ccl2Condition(lua_State *l, std::string_view value)
 */
 static void CclSpellCondition(lua_State *l, ConditionInfo *condition)
 {
-	// Flags are defaulted to 0(CONDITION_TRUE)
+	// Flags are defaulted to ECondition::Ignore
 	size_t new_bool_size = UnitTypeVar.GetNumberBoolFlag();
 
 	condition->BoolFlag.resize(new_bool_size);
-	std::fill(std::begin(condition->BoolFlag), std::end(condition->BoolFlag), 0);
+	std::fill(std::begin(condition->BoolFlag), std::end(condition->BoolFlag), ECondition::Ignore);
 
 	condition->Variable.resize(UnitTypeVar.GetNumberVariable());
 	// Initialize min/max stuff to values with no effect.

src/spell/spells.cpp

@@ -131,8 +131,9 @@ static bool PassCondition(const CUnit &caster, const SpellType &spell, const CUn
 		if (unit == nullptr) {
 			continue;
 		}
-		if (condition->Variable[i].Enable != CONDITION_TRUE) {
-			if ((condition->Variable[i].Enable == CONDITION_ONLY) ^ (unit->Variable[i].Enable)) {
+		if (condition->Variable[i].Enable != ECondition::Ignore) {
+			if ((condition->Variable[i].Enable == ECondition::ShouldBeTrue)
+			    ^ (unit->Variable[i].Enable)) {
 				return false;
 			}
 		}
@@ -184,21 +185,20 @@ static bool PassCondition(const CUnit &caster, const SpellType &spell, const CUn
 		return true;
 	}
 
-	if (condition->Alliance != CONDITION_TRUE) {
-		if ((condition->Alliance == CONDITION_ONLY) ^
-			// own units could be not allied ?
-			(caster.IsAllied(*target) || target->Player == caster.Player)) {
+	if (condition->Alliance != ECondition::Ignore) {
+		// own units could be not allied ?
+		if ((condition->Alliance == ECondition::ShouldBeTrue)
+		    ^ (caster.IsAllied(*target) || target->Player == caster.Player)) {
 			return false;
 		}
 	}
-	if (condition->Opponent != CONDITION_TRUE) {
-		if ((condition->Opponent == CONDITION_ONLY) ^
-			(caster.IsEnemy(*target) && 1)) {
+	if (condition->Opponent != ECondition::Ignore) {
+		if ((condition->Opponent == ECondition::ShouldBeTrue) ^ caster.IsEnemy(*target)) {
 			return false;
 		}
 	}
-	if (condition->TargetSelf != CONDITION_TRUE) {
-		if ((condition->TargetSelf == CONDITION_ONLY) ^ (&caster == target)) {
+	if (condition->TargetSelf != ECondition::Ignore) {
+		if ((condition->TargetSelf == ECondition::ShouldBeTrue) ^ (&caster == target)) {
 			return false;
 		}
 	}
@@ -263,7 +263,7 @@ static std::unique_ptr<Target> SelectTargetUnitsOfAutoCast(CUnit &caster, const
 		table.push_back(&caster); // Allow self as target (we check conditions later)
 
 	// Check generic conditions. FIXME: a better way to do this?
-	if (autocast->Combat != CONDITION_TRUE) {
+	if (autocast->Combat != ECondition::Ignore) {
 		// Check each unit if it is hostile.
 		const bool inCombat =
 			ranges::find_if(table,
@@ -275,7 +275,7 @@ static std::unique_ptr<Target> SelectTargetUnitsOfAutoCast(CUnit &caster, const
 			                            || CanTarget(*target->Type, *caster.Type));
 							})
 			!= table.end();
-		if ((autocast->Combat == CONDITION_ONLY) ^ (inCombat)) {
+		if ((autocast->Combat == ECondition::ShouldBeTrue) ^ (inCombat)) {
 			return nullptr;
 		}
 	}
@@ -292,11 +292,11 @@ static std::unique_ptr<Target> SelectTargetUnitsOfAutoCast(CUnit &caster, const
 				size_t count = 0;
 				for (size_t i = 0; i != table.size(); ++i) {
 					// Check for corpse
-					if (autocast->Corpse == CONDITION_ONLY) {
+					if (autocast->Corpse == ECondition::ShouldBeTrue) {
 						if (table[i]->CurrentAction() != UnitAction::Die) {
 							continue;
 						}
-					} else if (autocast->Corpse == CONDITION_FALSE) {
+					} else if (autocast->Corpse == ECondition::ShouldBeFalse) {
 						if (table[i]->CurrentAction() == UnitAction::Die || table[i]->IsAlive() == false) {
 							continue;
 						}
@@ -332,7 +332,7 @@ static std::unique_ptr<Target> SelectTargetUnitsOfAutoCast(CUnit &caster, const
 			int n = 0;
 			for (size_t i = 0; i != table.size(); ++i) {
 				// Check if unit in battle
-				if (autocast->Attacker == CONDITION_ONLY) {
+				if (autocast->Attacker == ECondition::ShouldBeTrue) {
 					const int range = table[i]->Player->Type == PlayerTypes::PlayerPerson ? table[i]->Type->ReactRangePerson : table[i]->Type->ReactRangeComputer;
 					if ((table[i]->CurrentAction() != UnitAction::Attack
 						 && table[i]->CurrentAction() != UnitAction::AttackGround
@@ -343,11 +343,11 @@ static std::unique_ptr<Target> SelectTargetUnitsOfAutoCast(CUnit &caster, const
 					}
 				}
 				// Check for corpse
-				if (autocast->Corpse == CONDITION_ONLY) {
+				if (autocast->Corpse == ECondition::ShouldBeTrue) {
 					if (table[i]->CurrentAction() != UnitAction::Die) {
 						continue;
 					}
-				} else if (autocast->Corpse == CONDITION_FALSE) {
+				} else if (autocast->Corpse == ECondition::ShouldBeFalse) {
 					if (table[i]->CurrentAction() == UnitAction::Die) {
 						continue;
 					}

src/ui/botpanel.cpp

@@ -344,8 +344,9 @@ static bool CanShowPopupContent(const PopupConditionPanel *condition,
 
 	if (!condition->Variables.empty() && type) {
 		for (unsigned int i = 0; i < UnitTypeVar.GetNumberVariable(); ++i) {
-			if (condition->Variables[i] != CONDITION_TRUE) {
-				if ((condition->Variables[i] == CONDITION_ONLY) ^ type->Stats[ThisPlayer->Index].Variables[i].Enable) {
+			if (condition->Variables[i] != ECondition::Ignore) {
+				if ((condition->Variables[i] == ECondition::ShouldBeTrue)
+				    ^ type->Stats[ThisPlayer->Index].Variables[i].Enable) {
 					return false;
 				}
 			}