Merge pull request #5620 from IvanSavenko/adela_fix

Better support for Adela specialty (+new modding functionality for it)

config/heroes/castle.json

@@ -182,7 +182,7 @@
 						{
 							"type" : "HAS_ANOTHER_BONUS_LIMITER",
 							"parameters" : [
-								"GENERAL_DAMAGE_PREMY",
+								null,
 								null,
 								{
 									"type" : "SPELL_EFFECT",
@@ -191,7 +191,7 @@
 							]
 						}
 					],
-					"updater" : "TIMES_HERO_LEVEL",
+					"updater" : "TIMES_HERO_LEVEL_DIVIDE_STACK_LEVEL",
 					"val" : 3
 				}
 			}

config/schemas/bonusInstance.json

@@ -72,7 +72,7 @@
 			"anyOf" : [
 				{
 					"type" : "string",
-					"enum" : [ "TIMES_HERO_LEVEL", "TIMES_STACK_LEVEL", "ARMY_MOVEMENT", "BONUS_OWNER_UPDATER" ]
+					"enum" : [ "TIMES_HERO_LEVEL", "TIMES_STACK_LEVEL", "DIVIDE_STACK_LEVEL", "BONUS_OWNER_UPDATER", "TIMES_HERO_LEVEL_DIVIDE_STACK_LEVEL" ]
 				},
 				{
 					"description" : "updater",
@@ -82,7 +82,7 @@
 					"properties" : {
 						"type" : {
 							"type" : "string",
-							"enum" : [ "GROWS_WITH_LEVEL", "ARMY_MOVEMENT" ],
+							"enum" : [ "GROWS_WITH_LEVEL" ],
 							"description" : "type"
 						},
 						"parameters" : {

docs/modders/Bonus/Bonus_Updaters.md

@@ -63,20 +63,27 @@ Usage:
 
 Remark: The stack level for war machines is 0.
 
-## ARMY_MOVEMENT
+## DIVIDE_STACK_LEVEL
 
-- Type: Complex
-- Parameters: basePerSpeed, dividePerSpeed, additionalMultiplier, maxValue
-- Effect: Updates val to `val+= max((floor(basePerSpeed / dividePerSpeed) * additionalMultiplier), maxValue)`
-- Remark: this updater is designed for MOVEMENT bonus to match H3 army movement rules (in the example - actual movement updater, which produces values same as in default movement.txt).
-- Example:
+- Type: Simple
+- Effect: Updates val to `val / stackLevel`
 
-```json
-"updater" : {
-    "parameters" : [ 20, 3, 10, 700 ],
-    "type" : "ARMY_MOVEMENT"
-}
-```
+Usage:
+
+`"updater" : "DIVIDE_STACK_LEVEL"`
+
+Remark: The stack level for war machines is 0.
+
+## TIMES_HERO_LEVEL_DIVIDE_STACK_LEVEL
+
+- Type: Simple
+- Effect: Same effect as `TIMES_HERO_LEVEL` combined with `DIVIDE_STACK_LEVEL`: `val * heroLevel / stackLevel`
+
+Intended to be used as hero bonus (such as specialty, skill, or artifact), for bonuses that affect units (Example: Adela Bless specialty)
+
+Usage:
+
+`"updater" : "TIMES_HERO_LEVEL_DIVIDE_STACK_LEVEL"`
 
 ## BONUS_OWNER_UPDATER
 

docs/modders/Bonus_Format.md

@@ -40,25 +40,39 @@ All parameters but type are optional.
 	// TODO
 	"effectRange" : "EFFECT_RANGE",
 
-	// TODO
+	// This sections allows to define 'limiter', which allows to limit bonus and only apply it under specific conditions
+	// Typical conditions are "affect only specific creature", or "affect only if target has another, specific bonus"
+	// See Bonus Limiters list below for full list of supported limiters 
 	"limiters" : [
 		"PREDEFINED_LIMITER", optional_parameters (...), //whhich one is preferred?
 		{"type" : LIMITER_TYPE, "parameters" : [1,2,3]}
 	],
 	
-	// TODO
+	// Normally, only entities that are located "below" bonus source are affected by the bonus
+	// For example, bonus on creature will only affect creature itself,
+	// bonus on hero will affect hero itself and all its units, and player bonus will affect all objects owned by player
+	// Propagator allows bonus to affect "upwards" entities. 
+	// For example, creature that has bonus with battle-wide propagator will affect all units in combat, and not just unit itself
+	// See Bonus Propagators list below for full list of supported propagators
 	"propagator" : 	["PROPAGATOR_TYPE", optional_parameters (...)],
 	
-	// TODO
+	// Updaters allow to modify bonus depending on context. 
+	// For example, it can be used to scale bonus based on hero or unit level
+	// See Bonus Updaters list below for full list of supported updaters
 	"updater" :	    {Bonus Updater},
 	
-	// TODO
+	// This is special type of propagator, that is only activated when bonus is being propagated upwards,
+	// using its propagator. It has no effect on bonuses without propagator
 	"propagationUpdater" :	{Bonus Updater, but works during propagation},
 	
 	// TODO
 	"description" : "",
 	
-	// TODO
+	// Stacking string allows to block stacking of bonuses from different entities
+	// For example, devils and archdevils (different entities) both have battle-wide debuff to luck
+	// Normally, having both such units in combat would result in bonus stacking, providing -2 debuff to luck in total
+	// If such behavior is undesired, both such unit must have same, non-empty stacking string
+	// String can contain any text, as long as it not empty and is same for both of such creatures
 	"stacking" :    ""
 }
 ```

lib/bonuses/CBonusSystemNode.cpp

@@ -55,6 +55,10 @@ void CBonusSystemNode::getAllParents(TCNodes & out) const //retrieves list of pa
 {
 	for(auto * parent : parentsToInherit)
 	{
+		// Diamond found! One of the parents of the targeted node can be discovered in two ways.
+		// For example, a hero has been attached to both the player node and the global node (to which the player node is also attached).
+		// This is illegal and can be a source of duplicate bonuses.
+		assert(out.count(parent) == 0);
 		out.insert(parent);
 		parent->getAllParents(out);
 	}
@@ -63,13 +67,10 @@ void CBonusSystemNode::getAllParents(TCNodes & out) const //retrieves list of pa
 void CBonusSystemNode::getAllBonusesRec(BonusList &out, const CSelector & selector) const
 {
 	BonusList beforeUpdate;
-	TCNodes lparents;
-	getAllParents(lparents);
 
-	for(const auto * parent : lparents)
-	{
+	for(const auto * parent : parentsToInherit)
 		parent->getAllBonusesRec(beforeUpdate, selector);
-	}
+
 	bonuses.getAllBonuses(beforeUpdate);
 
 	for(const auto & b : beforeUpdate)
@@ -79,18 +80,7 @@ void CBonusSystemNode::getAllBonusesRec(BonusList &out, const CSelector & select
 			? getUpdatedBonus(b, b->updater)
 			: b;
 
-		//do not add bonus with updater
-		bool bonusExists = false;
-		for(const auto & bonus : out)
-		{
-			if (bonus == updated)
-				bonusExists = true;
-			if (bonus->updater && bonus->updater == updated->updater)
-				bonusExists = true;
-		}
-
-		if (!bonusExists)
-			out.push_back(updated);
+		out.push_back(updated);
 	}
 }
 

lib/bonuses/Limiters.cpp

@@ -166,15 +166,21 @@ HasAnotherBonusLimiter::HasAnotherBonusLimiter(BonusType bonus, BonusSubtypeID _
 
 ILimiter::EDecision HasAnotherBonusLimiter::limit(const BonusLimitationContext &context) const
 {
-	//TODO: proper selector config with parsing of JSON
-	auto mySelector = Selector::type()(type);
+	boost::container::static_vector<CSelector, 4> selectorSegments;
 
+	if (type != BonusType::NONE)
+		selectorSegments.push_back(Selector::type()(type));
 	if(isSubtypeRelevant)
-		mySelector = mySelector.And(Selector::subtype()(subtype));
+		selectorSegments.push_back(Selector::subtype()(subtype));
 	if(isSourceRelevant && isSourceIDRelevant)
-		mySelector = mySelector.And(Selector::source(source, sid));
+		selectorSegments.push_back(Selector::source(source, sid));
 	else if (isSourceRelevant)
-		mySelector = mySelector.And(Selector::sourceTypeSel(source));
+		selectorSegments.push_back(Selector::sourceTypeSel(source));
+
+	auto mySelector = selectorSegments.empty() ? Selector::none : selectorSegments[0];
+
+	for (size_t i = 1; i <selectorSegments.size(); ++i)
+		mySelector = mySelector.And(selectorSegments[i]);
 
 	//if we have a bonus of required type accepted, limiter should accept also this bonus
 	if(context.alreadyAccepted.getFirst(mySelector))

lib/bonuses/Updaters.cpp

@@ -108,72 +108,53 @@ JsonNode TimesHeroLevelUpdater::toJsonNode() const
 	return JsonNode("TIMES_HERO_LEVEL");
 }
 
-ArmyMovementUpdater::ArmyMovementUpdater():
-	base(20),
-	divider(3),
-	multiplier(10),
-	max(700)
+std::shared_ptr<Bonus> TimesHeroLevelDivideStackLevelUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context)
 {
+	if(context.getNodeType() == CBonusSystemNode::HERO)
+	{
+		auto newBonus = TimesHeroLevelUpdater::createUpdatedBonus(b, context);
+		newBonus->updater = divideStackLevel;
+		return newBonus;
+	}
+	return b;
 }
 
-ArmyMovementUpdater::ArmyMovementUpdater(int base, int divider, int multiplier, int max):
-	base(base),
-	divider(divider),
-	multiplier(multiplier),
-	max(max)
+std::string TimesHeroLevelDivideStackLevelUpdater::toString() const
 {
+	return "TimesHeroLevelDivideStackLevelUpdater";
 }
 
-std::shared_ptr<Bonus> ArmyMovementUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context)
+JsonNode TimesHeroLevelDivideStackLevelUpdater::toJsonNode() const
 {
-	return b;
+	return JsonNode("TIMES_HERO_LEVEL_DIVIDE_STACK_LEVEL");
 }
 
-std::string ArmyMovementUpdater::toString() const
+std::shared_ptr<Bonus> TimesStackLevelUpdater::apply(const std::shared_ptr<Bonus> & b, int level) const
 {
-	return "ArmyMovementUpdater";
+	auto newBonus = std::make_shared<Bonus>(*b);
+	newBonus->val *= level;
+	newBonus->updater = nullptr; // prevent double-apply
+	return newBonus;
 }
 
-JsonNode ArmyMovementUpdater::toJsonNode() const
-{
-	JsonNode root;
-
-	root["type"].String() = "ARMY_MOVEMENT";
-	root["parameters"].Vector().emplace_back(base);
-	root["parameters"].Vector().emplace_back(divider);
-	root["parameters"].Vector().emplace_back(multiplier);
-	root["parameters"].Vector().emplace_back(max);
-
-	return root;
-}
 std::shared_ptr<Bonus> TimesStackLevelUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context)
 {
 	if(context.getNodeType() == CBonusSystemNode::STACK_INSTANCE || context.getNodeType() == CBonusSystemNode::COMMANDER)
 	{
 		int level = dynamic_cast<const CStackInstance &>(context).getLevel();
-		auto newBonus = std::make_shared<Bonus>(*b);
-		newBonus->val *= level;
-		return newBonus;
+		return apply(b, level);
 	}
-	else if(context.getNodeType() == CBonusSystemNode::STACK_BATTLE)
+
+	if(context.getNodeType() == CBonusSystemNode::STACK_BATTLE)
 	{
 		const auto & stack = dynamic_cast<const CStack &>(context);
 		//update if stack doesn't have an instance (summons, war machines)
 		if(stack.base == nullptr)
-		{
-			int level = stack.unitType()->getLevel();
-			auto newBonus = std::make_shared<Bonus>(*b);
-			newBonus->val *= level;
-			return newBonus;
-		}
+			return apply(b, stack.unitType()->getLevel());
+
 		// If these are not handled here, the final outcome may potentially be incorrect.
-		else
-		{
-			int level = dynamic_cast<const CStackInstance*>(stack.base)->getLevel();
-			auto newBonus = std::make_shared<Bonus>(*b);
-			newBonus->val *= level;
-			return newBonus;
-		}
+		int level = dynamic_cast<const CStackInstance*>(stack.base)->getLevel();
+		return apply(b, level);
 	}
 	return b;
 }
@@ -188,6 +169,46 @@ JsonNode TimesStackLevelUpdater::toJsonNode() const
 	return JsonNode("TIMES_STACK_LEVEL");
 }
 
+std::shared_ptr<Bonus> DivideStackLevelUpdater::apply(const std::shared_ptr<Bonus> & b, int level) const
+{
+	auto newBonus = std::make_shared<Bonus>(*b);
+	newBonus->val /= level;
+	newBonus->updater = nullptr; // prevent double-apply
+	return newBonus;
+}
+
+std::shared_ptr<Bonus> DivideStackLevelUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context)
+{
+	if(context.getNodeType() == CBonusSystemNode::STACK_INSTANCE || context.getNodeType() == CBonusSystemNode::COMMANDER)
+	{
+		int level = dynamic_cast<const CStackInstance &>(context).getLevel();
+		return apply(b, level);
+	}
+
+	if(context.getNodeType() == CBonusSystemNode::STACK_BATTLE)
+	{
+		const auto & stack = dynamic_cast<const CStack &>(context);
+		//update if stack doesn't have an instance (summons, war machines)
+		if(stack.base == nullptr)
+			return apply(b, stack.unitType()->getLevel());
+
+		// If these are not handled here, the final outcome may potentially be incorrect.
+		int level = dynamic_cast<const CStackInstance*>(stack.base)->getLevel();
+		return apply(b, level);
+	}
+	return b;
+}
+
+std::string DivideStackLevelUpdater::toString() const
+{
+	return "DivideStackLevelUpdater";
+}
+
+JsonNode DivideStackLevelUpdater::toJsonNode() const
+{
+	return JsonNode("DIVIDE_STACK_LEVEL");
+}
+
 std::string OwnerUpdater::toString() const
 {
 	return "OwnerUpdater";

lib/bonuses/Updaters.h

@@ -88,6 +88,7 @@ public:
 
 class DLL_LINKAGE TimesStackLevelUpdater : public IUpdater
 {
+	std::shared_ptr<Bonus> apply(const std::shared_ptr<Bonus> & b, int level) const;
 public:
 	template <typename Handler> void serialize(Handler & h)
 	{
@@ -99,22 +100,13 @@ public:
 	JsonNode toJsonNode() const override;
 };
 
-class DLL_LINKAGE ArmyMovementUpdater : public IUpdater
+class DLL_LINKAGE DivideStackLevelUpdater : public IUpdater
 {
+	std::shared_ptr<Bonus> apply(const std::shared_ptr<Bonus> & b, int level) const;
 public:
-	si32 base;
-	si32 divider;
-	si32 multiplier;
-	si32 max;
-	ArmyMovementUpdater();
-	ArmyMovementUpdater(int base, int divider, int multiplier, int max);
 	template <typename Handler> void serialize(Handler & h)
 	{
 		h & static_cast<IUpdater &>(*this);
-		h & base;
-		h & divider;
-		h & multiplier;
-		h & max;
 	}
 
 	std::shared_ptr<Bonus> createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) override;
@@ -122,6 +114,25 @@ public:
 	JsonNode toJsonNode() const override;
 };
 
+class DLL_LINKAGE TimesHeroLevelDivideStackLevelUpdater : public TimesHeroLevelUpdater
+{
+	std::shared_ptr<DivideStackLevelUpdater> divideStackLevel;
+public:
+	template <typename Handler> void serialize(Handler & h)
+	{
+		h & static_cast<TimesHeroLevelUpdater &>(*this);
+		h & divideStackLevel;
+	}
+
+	TimesHeroLevelDivideStackLevelUpdater()
+		: divideStackLevel(std::make_shared<DivideStackLevelUpdater>())
+	{}
+
+	std::shared_ptr<Bonus> createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) override;
+	std::string toString() const override;
+	JsonNode toJsonNode() const override;
+};
+
 class DLL_LINKAGE OwnerUpdater : public IUpdater
 {
 protected:

lib/json/JsonBonus.cpp

@@ -328,8 +328,9 @@ static TUpdaterPtr parseUpdater(const JsonNode & updaterJson)
 	const std::map<std::string, std::function<TUpdaterPtr()>> bonusUpdaterMap =
 	{
 			{"TIMES_HERO_LEVEL", std::make_shared<TimesHeroLevelUpdater>},
+			{"TIMES_HERO_LEVEL_DIVIDE_STACK_LEVEL", std::make_shared<TimesHeroLevelDivideStackLevelUpdater>},
+			{"DIVIDE_STACK_LEVEL", std::make_shared<DivideStackLevelUpdater>},
 			{"TIMES_STACK_LEVEL", std::make_shared<TimesStackLevelUpdater>},
-			{"ARMY_MOVEMENT", std::make_shared<ArmyMovementUpdater>},
 			{"BONUS_OWNER_UPDATER", std::make_shared<OwnerUpdater>}
 	};
 
@@ -354,24 +355,6 @@ static TUpdaterPtr parseUpdater(const JsonNode & updaterJson)
 				updater->stepSize = static_cast<int>(param[1].Integer());
 			return updater;
 		}
-		else if (updaterJson["type"].String() == "ARMY_MOVEMENT")
-		{
-			auto updater = std::make_shared<ArmyMovementUpdater>();
-			if(updaterJson["parameters"].isVector())
-			{
-				const auto & param = updaterJson["parameters"].Vector();
-				if(param.size() < 4)
-					logMod->warn("Invalid ARMY_MOVEMENT parameters, using default!");
-				else
-				{
-					updater->base = static_cast<si32>(param.at(0).Integer());
-					updater->divider = static_cast<si32>(param.at(1).Integer());
-					updater->multiplier = static_cast<si32>(param.at(2).Integer());
-					updater->max = static_cast<si32>(param.at(3).Integer());
-				}
-				return updater;
-			}
-		}
 		else
 			logMod->warn("Unknown updater type \"%s\"", updaterJson["type"].String());
 		break;
@@ -522,47 +505,52 @@ std::shared_ptr<ILimiter> JsonUtils::parseLimiter(const JsonNode & limiter)
 			}
 			else if(limiterType == "HAS_ANOTHER_BONUS_LIMITER")
 			{
-				std::string anotherBonusType = parameters[0].String();
-				auto it = bonusNameMap.find(anotherBonusType);
-				if(it == bonusNameMap.end())
+				auto bonusLimiter = std::make_shared<HasAnotherBonusLimiter>();
+
+				if (!parameters[0].isNull())
 				{
-					logMod->error("Error: invalid ability type %s.", anotherBonusType);
+					std::string anotherBonusType = parameters[0].String();
+					auto it = bonusNameMap.find(anotherBonusType);
+					if(it != bonusNameMap.end())
+					{
+						bonusLimiter->type = it->second;
+					}
+					else
+					{
+						logMod->error("Error: invalid ability type %s.", anotherBonusType);
+					}
 				}
-				else
+
+				auto findSource = [&](const JsonNode & parameter)
 				{
-					auto bonusLimiter = std::make_shared<HasAnotherBonusLimiter>();
-					bonusLimiter->type = it->second;
-					auto findSource = [&](const JsonNode & parameter)
+					if(parameter.getType() == JsonNode::JsonType::DATA_STRUCT)
 					{
-						if(parameter.getType() == JsonNode::JsonType::DATA_STRUCT)
+						auto sourceIt = bonusSourceMap.find(parameter["type"].String());
+						if(sourceIt != bonusSourceMap.end())
 						{
-							auto sourceIt = bonusSourceMap.find(parameter["type"].String());
-							if(sourceIt != bonusSourceMap.end())
-							{
-								bonusLimiter->source = sourceIt->second;
-								bonusLimiter->isSourceRelevant = true;
-								if(!parameter["id"].isNull()) {
-									loadBonusSourceInstance(bonusLimiter->sid, bonusLimiter->source, parameter["id"]);
-									bonusLimiter->isSourceIDRelevant = true;
-								}
+							bonusLimiter->source = sourceIt->second;
+							bonusLimiter->isSourceRelevant = true;
+							if(!parameter["id"].isNull()) {
+								loadBonusSourceInstance(bonusLimiter->sid, bonusLimiter->source, parameter["id"]);
+								bonusLimiter->isSourceIDRelevant = true;
 							}
 						}
-						return false;
-					};
-					if(parameters.size() > 1)
+					}
+					return false;
+				};
+				if(parameters.size() > 1)
+				{
+					if(findSource(parameters[1]) && parameters.size() == 2)
+						return bonusLimiter;
+					else
 					{
-						if(findSource(parameters[1]) && parameters.size() == 2)
-							return bonusLimiter;
-						else
-						{
-							loadBonusSubtype(bonusLimiter->subtype, bonusLimiter->type, parameters[1]);
-							bonusLimiter->isSubtypeRelevant = true;
-							if(parameters.size() > 2)
-								findSource(parameters[2]);
-						}
+						loadBonusSubtype(bonusLimiter->subtype, bonusLimiter->type, parameters[1]);
+						bonusLimiter->isSubtypeRelevant = true;
+						if(parameters.size() > 2)
+							findSource(parameters[2]);
 					}
-					return bonusLimiter;
 				}
+				return bonusLimiter;
 			}
 			else if(limiterType == "CREATURE_ALIGNMENT_LIMITER")
 			{

lib/serializer/RegisterTypes.h

@@ -104,7 +104,6 @@ void registerTypes(Serializer &s)
 	s.template registerType<TimesHeroLevelUpdater>(44);
 	s.template registerType<TimesStackLevelUpdater>(45);
 	s.template registerType<OwnerUpdater>(46);
-	s.template registerType<ArmyMovementUpdater>(47);
 	s.template registerType<ILimiter>(48);
 	s.template registerType<AnyOfLimiter>(49);
 	s.template registerType<NoneOfLimiter>(50);
@@ -294,6 +293,8 @@ void registerTypes(Serializer &s)
 	s.template registerType<SetResearchedSpells>(242);
 	s.template registerType<SaveLocalState>(243);
 	s.template registerType<LobbyDelete>(244);
+	s.template registerType<TimesHeroLevelDivideStackLevelUpdater>(245);
+	s.template registerType<DivideStackLevelUpdater>(246);
 }
 
 VCMI_LIB_NAMESPACE_END