vcmi: more optionals in bonuses

lib/JsonNode.cpp

@@ -842,25 +842,12 @@ static BonusParams convertDeprecatedBonus(const JsonNode &ability)
 											   ability["subtype"].isNumber() ? ability["subtype"].Integer() : -1);
 		if(params.isConverted)
 		{
-			if(!params.valRelevant) {
+			if(ability["type"].String() == "SECONDARY_SKILL_PREMY" && bonusValueMap.find(ability["valueType"].String())->second == BonusValueType::PERCENT_TO_BASE) //assume secondary skill special
-				params.val = static_cast<si32>(ability["val"].Float());
-				params.valRelevant = true;
-			}
-			BonusValueType valueType = BonusValueType::ADDITIVE_VALUE;
-			if(!ability["valueType"].isNull())
-				valueType = bonusValueMap.find(ability["valueType"].String())->second;
-
-			if(ability["type"].String() == "SECONDARY_SKILL_PREMY" && valueType == BonusValueType::PERCENT_TO_BASE) //assume secondary skill special
 			{
 				params.valueType = BonusValueType::PERCENT_TO_TARGET_TYPE;
 				params.targetType = BonusSource::SECONDARY_SKILL;
-				params.targetTypeRelevant = true;
 			}
 
-			if(!params.valueTypeRelevant) {
-				params.valueType = valueType;
-				params.valueTypeRelevant = true;
-			}
 			logMod->warn("Please, use this bonus:\n%s\nConverted sucessfully!", params.toJson().toJson());
 			return params;
 		}
@@ -930,10 +917,10 @@ bool JsonUtils::parseBonus(const JsonNode &ability, Bonus *b)
 			return false;
 		}
 		b->type = params->type;
-		b->val = params->val;
+		b->val = params->val.value_or(0);
-		b->valType = params->valueType;
+		b->valType = params->valueType.value_or(BonusValueType::ADDITIVE_VALUE);
-		if(params->targetTypeRelevant)
+		if(params->targetType)
-			b->targetSourceType = params->targetType;
+			b->targetSourceType = params->targetType.value();
 	}
 	else
 		b->type = it->second;
@@ -1073,31 +1060,26 @@ CSelector JsonUtils::parseSelector(const JsonNode & ability)
 		ret = ret.And(Selector::subtype()(subtype));
 	}
 	value = &ability["sourceType"];
-	BonusSource src = BonusSource::OTHER; //Fixes for GCC false maybe-uninitialized
+	std::optional<BonusSource> src = std::nullopt; //Fixes for GCC false maybe-uninitialized
-	si32 id = 0;
+	std::optional<si32> id = std::nullopt;
-	auto sourceIDRelevant = false;
-	auto sourceTypeRelevant = false;
 	if(value->isString())
 	{
 		auto it = bonusSourceMap.find(value->String());
 		if(it != bonusSourceMap.end())
-		{
 			src = it->second;
-			sourceTypeRelevant = true;
 	}
 
-	}
 	value = &ability["sourceID"];
 	if(!value->isNull())
 	{
-		sourceIDRelevant = true;
+		id = -1;
-		resolveIdentifier(id, ability, "sourceID");
+		resolveIdentifier(*id, ability, "sourceID");
 	}
 
-	if(sourceIDRelevant && sourceTypeRelevant)
+	if(src && id)
-		ret = ret.And(Selector::source(src, id));
+		ret = ret.And(Selector::source(*src, *id));
-	else if(sourceTypeRelevant)
+	else if(src)
-		ret = ret.And(Selector::sourceTypeSel(src));
+		ret = ret.And(Selector::sourceTypeSel(*src));
 
 	
 	value = &ability["targetSourceType"];

lib/bonuses/BonusParams.cpp

@@ -72,7 +72,6 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 		{
 			type = BonusType::MANA_PER_KNOWLEDGE;
 			valueType = BonusValueType::PERCENT_TO_BASE;
-			valueTypeRelevant = true;
 		}
 		else if(deprecatedSubtype == SecondarySkill::SORCERY || deprecatedSubtypeStr == "skill.sorcery")
 			type = BonusType::SPELL_DAMAGE;
@@ -81,83 +80,68 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 		else if(deprecatedSubtype == SecondarySkill::ARCHERY|| deprecatedSubtypeStr == "skill.archery")
 		{
 			subtype = 1;
-			subtypeRelevant = true;
 			type = BonusType::PERCENTAGE_DAMAGE_BOOST;
 		}
 		else if(deprecatedSubtype == SecondarySkill::OFFENCE || deprecatedSubtypeStr == "skill.offence")
 		{
 			subtype = 0;
-			subtypeRelevant = true;
 			type = BonusType::PERCENTAGE_DAMAGE_BOOST;
 		}
 		else if(deprecatedSubtype == SecondarySkill::ARMORER || deprecatedSubtypeStr == "skill.armorer")
 		{
 			subtype = -1;
-			subtypeRelevant = true;
 			type = BonusType::GENERAL_DAMAGE_REDUCTION;
 		}
 		else if(deprecatedSubtype == SecondarySkill::NAVIGATION || deprecatedSubtypeStr == "skill.navigation")
 		{
 			subtype = 0;
-			subtypeRelevant = true;
 			valueType = BonusValueType::PERCENT_TO_BASE;
-			valueTypeRelevant = true;
 			type = BonusType::MOVEMENT;
 		}
 		else if(deprecatedSubtype == SecondarySkill::LOGISTICS || deprecatedSubtypeStr == "skill.logistics")
 		{
 			subtype = 1;
-			subtypeRelevant = true;
 			valueType = BonusValueType::PERCENT_TO_BASE;
-			valueTypeRelevant = true;
 			type = BonusType::MOVEMENT;
 		}
 		else if(deprecatedSubtype == SecondarySkill::ESTATES || deprecatedSubtypeStr == "skill.estates")
 		{
 			type = BonusType::GENERATE_RESOURCE;
 			subtype = GameResID(EGameResID::GOLD);
-			subtypeRelevant = true;
 		}
 		else if(deprecatedSubtype == SecondarySkill::AIR_MAGIC || deprecatedSubtypeStr == "skill.airMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtypeRelevant = true;
 			subtype = 4;
 		}
 		else if(deprecatedSubtype == SecondarySkill::WATER_MAGIC || deprecatedSubtypeStr == "skill.waterMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtypeRelevant = true;
 			subtype = 1;
 		}
 		else if(deprecatedSubtype == SecondarySkill::FIRE_MAGIC || deprecatedSubtypeStr == "skill.fireMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtypeRelevant = true;
 			subtype = 2;
 		}
 		else if(deprecatedSubtype == SecondarySkill::EARTH_MAGIC || deprecatedSubtypeStr == "skill.earthMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtypeRelevant = true;
 			subtype = 8;
 		}
 		else if (deprecatedSubtype == SecondarySkill::ARTILLERY || deprecatedSubtypeStr == "skill.artillery")
 		{
 			type = BonusType::BONUS_DAMAGE_CHANCE;
-			subtypeRelevant = true;
 			subtypeStr = "core:creature.ballista";
 		}
 		else if (deprecatedSubtype == SecondarySkill::FIRST_AID || deprecatedSubtypeStr == "skill.firstAid")
 		{
 			type = BonusType::SPECIFIC_SPELL_POWER;
-			subtypeRelevant = true;
 			subtypeStr = "core:spell.firstAid";
 		}
 		else if (deprecatedSubtype == SecondarySkill::BALLISTICS || deprecatedSubtypeStr == "skill.ballistics")
 		{
 			type = BonusType::CATAPULT_EXTRA_SHOTS;
-			subtypeRelevant = true;
 			subtypeStr = "core:spell.catapultShot";
 		}
 		else
@@ -170,7 +154,6 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 		else if (deprecatedSubtype == SecondarySkill::ARTILLERY || deprecatedSubtypeStr == "skill.artillery")
 		{
 			type = BonusType::HERO_GRANTS_ATTACKS;
-			subtypeRelevant = true;
 			subtypeStr = "core:creature.ballista";
 		}
 		else
@@ -179,52 +162,41 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 	else if (deprecatedTypeStr == "SEA_MOVEMENT")
 	{
 		subtype = 0;
-		subtypeRelevant = true;
 		valueType = BonusValueType::ADDITIVE_VALUE;
-		valueTypeRelevant = true;
 		type = BonusType::MOVEMENT;
 	}
 	else if (deprecatedTypeStr == "LAND_MOVEMENT")
 	{
 		subtype = 1;
-		subtypeRelevant = true;
 		valueType = BonusValueType::ADDITIVE_VALUE;
-		valueTypeRelevant = true;
 		type = BonusType::MOVEMENT;
 	}
 	else if (deprecatedTypeStr == "MAXED_SPELL")
 	{
 		type = BonusType::SPELL;
 		subtypeStr = deprecatedSubtypeStr;
-		subtypeRelevant = true;
 		valueType = BonusValueType::INDEPENDENT_MAX;
-		valueTypeRelevant = true;
 		val = 3;
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "FULL_HP_REGENERATION")
 	{
 		type = BonusType::HP_REGENERATION;
 		val = 100000; //very high value to always chose stack health
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "KING1")
 	{
 		type = BonusType::KING;
 		val = 0;
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "KING2")
 	{
 		type = BonusType::KING;
 		val = 2;
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "KING3")
 	{
 		type = BonusType::KING;
 		val = 3;
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "SIGHT_RADIOUS")
 		type = BonusType::SIGHT_RADIUS;
@@ -232,70 +204,57 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 	{
 		type = BonusType::MORALE;
 		val = 1;
-		valRelevant = true;
 		valueType = BonusValueType::INDEPENDENT_MAX;
-		valueTypeRelevant = true;
 	}
 	else if (deprecatedTypeStr == "SELF_LUCK")
 	{
 		type = BonusType::LUCK;
 		val = 1;
-		valRelevant = true;
 		valueType = BonusValueType::INDEPENDENT_MAX;
-		valueTypeRelevant = true;
 	}
 	else if (deprecatedTypeStr == "DIRECT_DAMAGE_IMMUNITY")
 	{
 		type = BonusType::SPELL_DAMAGE_REDUCTION;
 		val = 100;
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "AIR_SPELL_DMG_PREMY")
 	{
 		type = BonusType::SPELL_DAMAGE;
-		subtypeRelevant = true;
 		subtype = 0;
 	}
 	else if (deprecatedTypeStr == "FIRE_SPELL_DMG_PREMY")
 	{
 		type = BonusType::SPELL_DAMAGE;
-		subtypeRelevant = true;
 		subtype = 1;
 	}
 	else if (deprecatedTypeStr == "WATER_SPELL_DMG_PREMY")
 	{
 		type = BonusType::SPELL_DAMAGE;
-		subtypeRelevant = true;
 		subtype = 2;
 	}
 	else if (deprecatedTypeStr == "EARTH_SPELL_DMG_PREMY")
 	{
 		type = BonusType::SPELL_DAMAGE;
-		subtypeRelevant = true;
 		subtype = 3;
 	}
 	else if (deprecatedTypeStr == "AIR_SPELLS")
 	{
 		type = BonusType::SPELLS_OF_SCHOOL;
-		subtypeRelevant = true;
 		subtype = 0;
 	}
 	else if (deprecatedTypeStr == "FIRE_SPELLS")
 	{
 		type = BonusType::SPELLS_OF_SCHOOL;
-		subtypeRelevant = true;
 		subtype = 1;
 	}
 	else if (deprecatedTypeStr == "WATER_SPELLS")
 	{
 		type = BonusType::SPELLS_OF_SCHOOL;
-		subtypeRelevant = true;
 		subtype = 2;
 	}
 	else if (deprecatedTypeStr == "EARTH_SPELLS")
 	{
 		type = BonusType::SPELLS_OF_SCHOOL;
-		subtypeRelevant = true;
 		subtype = 3;
 	}
 	else
@@ -308,16 +267,16 @@ const JsonNode & BonusParams::toJson()
 	if(ret.isNull())
 	{
 		ret["type"].String() = vstd::findKey(bonusNameMap, type);
-		if(subtypeRelevant && !subtypeStr.empty())
+		if(subtypeStr)
-			ret["subtype"].String() = subtypeStr;
+			ret["subtype"].String() = *subtypeStr;
-		else if(subtypeRelevant)
+		else if(subtype)
-			ret["subtype"].Integer() = subtype;
+			ret["subtype"].Integer() = *subtype;
-		if(valueTypeRelevant)
+		if(valueType)
-			ret["valueType"].String() = vstd::findKey(bonusValueMap, valueType);
+			ret["valueType"].String() = vstd::findKey(bonusValueMap, *valueType);
-		if(valRelevant)
+		if(val)
-			ret["val"].Float() = val;
+			ret["val"].Float() = *val;
-		if(targetTypeRelevant)
+		if(targetType)
-			ret["targetSourceType"].String() = vstd::findKey(bonusSourceMap, targetType);
+			ret["targetSourceType"].String() = vstd::findKey(bonusSourceMap, *targetType);
 		jsonCreated = true;
 	}
 	return ret;
@@ -326,16 +285,19 @@ const JsonNode & BonusParams::toJson()
 CSelector BonusParams::toSelector()
 {
 	assert(isConverted);
-	if(subtypeRelevant && !subtypeStr.empty())
+	if(subtypeStr)
-		JsonUtils::resolveIdentifier(subtype, toJson(), "subtype");
+	{
+		subtype = -1;
+		JsonUtils::resolveIdentifier(*subtype, toJson(), "subtype");
+	}
 
 	auto ret = Selector::type()(type);
-	if(subtypeRelevant)
+	if(subtype)
-		ret = ret.And(Selector::subtype()(subtype));
+		ret = ret.And(Selector::subtype()(*subtype));
-	if(valueTypeRelevant)
+	if(valueType)
-		ret = ret.And(Selector::valueType(valueType));
+		ret = ret.And(Selector::valueType(*valueType));
-	if(targetTypeRelevant)
+	if(targetType)
-		ret = ret.And(Selector::targetSourceType()(targetType));
+		ret = ret.And(Selector::targetSourceType()(*targetType));
 	return ret;
 }
 

lib/bonuses/BonusParams.h

@@ -19,15 +19,11 @@ VCMI_LIB_NAMESPACE_BEGIN
 struct DLL_LINKAGE BonusParams {
 	bool isConverted;
 	BonusType type = BonusType::NONE;
-	TBonusSubtype subtype = -1;
+	std::optional<TBonusSubtype> subtype = std::nullopt;
-	std::string subtypeStr;
+	std::optional<std::string> subtypeStr = std::nullopt;
-	bool subtypeRelevant = false;
+	std::optional<BonusValueType> valueType = std::nullopt;
-	BonusValueType valueType = BonusValueType::BASE_NUMBER;
+	std::optional<si32> val = std::nullopt;
-	bool valueTypeRelevant = false;
+	std::optional<BonusSource> targetType = std::nullopt;
-	si32 val = 0;
-	bool valRelevant = false;
-	BonusSource targetType = BonusSource::SECONDARY_SKILL;
-	bool targetTypeRelevant = false;
 
 	BonusParams(bool isConverted = true) : isConverted(isConverted) {};
 	BonusParams(std::string deprecatedTypeStr, std::string deprecatedSubtypeStr = "", int deprecatedSubtype = 0);

lib/bonuses/IBonusBearer.cpp

@@ -15,22 +15,21 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-int IBonusBearer::valOfBonuses(BonusType type, int subtype) const
+int IBonusBearer::valOfBonuses(BonusType type, std::optional<int> subtype) const
 {
 	//This part is performance-critical
-	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + std::to_string(subtype);
+	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + (subtype ?  "_" + std::to_string(*subtype) : "");
 
 	CSelector s = Selector::type()(type);
-	if(subtype != -1)
+	if(subtype)
-		s = s.And(Selector::subtype()(subtype));
+		s = s.And(Selector::subtype()(*subtype));
 
 	return valOfBonuses(s, cachingStr);
 }
 
 int IBonusBearer::valOfBonuses(const CSelector &selector, const std::string &cachingStr) const
 {
-	CSelector limit = nullptr;
+	TConstBonusListPtr hlp = getAllBonuses(selector, nullptr, nullptr, cachingStr);
-	TConstBonusListPtr hlp = getAllBonuses(selector, limit, nullptr, cachingStr);
 	return hlp->totalValue();
 }
 bool IBonusBearer::hasBonus(const CSelector &selector, const std::string &cachingStr) const
@@ -44,14 +43,14 @@ bool IBonusBearer::hasBonus(const CSelector &selector, const CSelector &limit, c
 	return getBonuses(selector, limit, cachingStr)->size() > 0;
 }
 
-bool IBonusBearer::hasBonusOfType(BonusType type, int subtype) const
+bool IBonusBearer::hasBonusOfType(BonusType type, std::optional<int> subtype) const
 {
 	//This part is performance-ciritcal
-	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + std::to_string(subtype);
+	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + (subtype ?  "_" + std::to_string(*subtype) : "");
 
 	CSelector s = Selector::type()(type);
-	if(subtype != -1)
+	if(subtype)
-		s = s.And(Selector::subtype()(subtype));
+		s = s.And(Selector::subtype()(*subtype));
 
 	return hasBonus(s, cachingStr);
 }

lib/bonuses/IBonusBearer.h

@@ -33,8 +33,8 @@ public:
 	std::shared_ptr<const Bonus> getBonus(const CSelector &selector) const; //returns any bonus visible on node that matches (or nullptr if none matches)
 
 	//Optimized interface (with auto-caching)
-	int valOfBonuses(BonusType type, int subtype = -1) const; //subtype -> subtype of bonus, if -1 then anyt;
+	int valOfBonuses(BonusType type, std::optional<int> subtype = std::nullopt) const; //subtype -> subtype of bonus;
-	bool hasBonusOfType(BonusType type, int subtype = -1) const;//determines if hero has a bonus of given type (and optionally subtype)
+	bool hasBonusOfType(BonusType type, std::optional<int> subtype = std::nullopt) const;//determines if hero has a bonus of given type (and optionally subtype)
 	bool hasBonusFrom(BonusSource source, ui32 sourceID) const;
 
 	virtual int64_t getTreeVersion() const = 0;

lib/spells/TargetCondition.cpp

@@ -362,8 +362,8 @@ public:
 			auto params = BonusParams(identifier, "", -1);
 			if(params.isConverted)
 			{
-				if(params.valRelevant)
+				if(params.val)
-					return std::make_shared<SelectorCondition>(params.toSelector(), params.val, params.val);
+					return std::make_shared<SelectorCondition>(params.toSelector(), *params.val, *params.val);
 				return std::make_shared<SelectorCondition>(params.toSelector());
 			}