[Spells] Added "summonByHealth" option to Summon effect

* with summonByHealth on, effect power (levelPower * SP) treated as HP
* default behavior (effect power = amount) not changed
* tests included

lib/spells/effects/Summon.cpp

@@ -18,9 +18,11 @@
 #include "../../NetPacks.h"
 #include "../../serializer/JsonSerializeFormat.h"
 
+#include "../../CCreatureHandler.h"
 #include "../../CHeroHandler.h"
 #include "../../mapObjects/CGHeroInstance.h"
 
+
 static const std::string EFFECT_NAME = "core:summon";
 
 namespace spells
@@ -34,7 +36,8 @@ Summon::Summon()
 	: Effect(),
 	creature(),
 	permanent(false),
-	exclusive(true)
+	exclusive(true),
+	summonByHealth(false)
 {
 }
 
@@ -52,41 +55,41 @@ void Summon::adjustTargetTypes(std::vector<TargetType> & types) const
 
 bool Summon::applicable(Problem & problem, const Mechanics * m) const
 {
-	if(!exclusive)
-		return true;
-
-	//check if there are summoned elementals of other type
-
-	auto otherSummoned = m->cb->battleGetUnitsIf([m, this](const battle::Unit * unit)
-	{
-		return (unit->unitOwner() == m->getCasterColor())
-			&& (unit->unitSlot() == SlotID::SUMMONED_SLOT_PLACEHOLDER)
-			&& (!unit->isClone())
-			&& (unit->creatureId() != creature);
-	});
-
-	if(!otherSummoned.empty())
+	if(exclusive)
 	{
-		auto elemental = otherSummoned.front();
+		//check if there are summoned creatures of other type
 
-		MetaString text;
-		text.addTxt(MetaString::GENERAL_TXT, 538);
+		auto otherSummoned = m->cb->battleGetUnitsIf([m, this](const battle::Unit * unit)
+		{
+			return (unit->unitOwner() == m->getCasterColor())
+				&& (unit->unitSlot() == SlotID::SUMMONED_SLOT_PLACEHOLDER)
+				&& (!unit->isClone())
+				&& (unit->creatureId() != creature);
+		});
 
-		auto caster = dynamic_cast<const CGHeroInstance *>(m->caster);
-		if(caster)
+		if(!otherSummoned.empty())
 		{
-			text.addReplacement(caster->name);
+			auto elemental = otherSummoned.front();
 
-			text.addReplacement(MetaString::CRE_PL_NAMES, elemental->creatureIndex());
+			MetaString text;
+			text.addTxt(MetaString::GENERAL_TXT, 538);
 
-			if(caster->type->sex)
-				text.addReplacement(MetaString::GENERAL_TXT, 540);
-			else
-				text.addReplacement(MetaString::GENERAL_TXT, 539);
+			auto caster = dynamic_cast<const CGHeroInstance *>(m->caster);
+			if(caster)
+			{
+				text.addReplacement(caster->name);
+
+				text.addReplacement(MetaString::CRE_PL_NAMES, elemental->creatureIndex());
 
+				if(caster->type->sex)
+					text.addReplacement(MetaString::GENERAL_TXT, 540);
+				else
+					text.addReplacement(MetaString::GENERAL_TXT, 539);
+
+			}
+			problem.add(std::move(text), Problem::NORMAL);
+			return false;
 		}
-		problem.add(std::move(text), Problem::NORMAL);
-		return false;
 	}
 
 	return true;
@@ -95,12 +98,7 @@ bool Summon::applicable(Problem & problem, const Mechanics * m) const
 void Summon::apply(BattleStateProxy * battleState, RNG & rng, const Mechanics * m, const EffectTarget & target) const
 {
 	//new feature - percentage bonus
-	auto amount = m->applySpecificSpellBonus(m->calculateRawEffectValue(0, m->getEffectPower()));
-	if(amount < 1)
-	{
-		battleState->complain("Summoning didn't summon any!");
-		return;
-	}
+	auto valueWithBonus = m->applySpecificSpellBonus(m->calculateRawEffectValue(0, m->getEffectPower()));//TODO: consider use base power too
 
 	BattleUnitsChanged pack;
 
@@ -110,13 +108,32 @@ void Summon::apply(BattleStateProxy * battleState, RNG & rng, const Mechanics *
 		{
 			const battle::Unit * summoned = dest.unitValue;
 			std::shared_ptr<battle::Unit> state = summoned->acquire();
-			int64_t healthValue = amount * summoned->MaxHealth();
+			int64_t healthValue = (summonByHealth ? valueWithBonus : (valueWithBonus * summoned->MaxHealth()));
 			state->heal(healthValue, EHealLevel::OVERHEAL, (permanent ? EHealPower::PERMANENT : EHealPower::ONE_BATTLE));
 			pack.changedStacks.emplace_back(summoned->unitId(), UnitChanges::EOperation::RESET_STATE);
 			state->save(pack.changedStacks.back().data);
 		}
 		else
 		{
+			int32_t amount = 0;
+
+			if(summonByHealth)
+			{
+				auto creatureType = creature.toCreature();
+				auto creatureMaxHealth = creatureType->MaxHealth();
+				amount = valueWithBonus / creatureMaxHealth;
+			}
+			else
+			{
+				amount = static_cast<int32_t>(valueWithBonus);
+			}
+
+			if(amount < 1)
+			{
+				battleState->complain("Summoning didn't summon any!");
+				continue;
+			}
+
 			battle::UnitInfo info;
 			info.id = m->cb->battleNextUnitId();
 			info.count = amount;
@@ -144,6 +161,7 @@ void Summon::serializeJsonEffect(JsonSerializeFormat & handler)
 	handler.serializeId("id", creature, CreatureID());
 	handler.serializeBool("permanent", permanent, false);
 	handler.serializeBool("exclusive", exclusive, true);
+	handler.serializeBool("summonByHealth", summonByHealth, false);
 }
 
 EffectTarget Summon::transformTarget(const Mechanics * m, const Target & aimPoint, const Target & spellTarget) const

lib/spells/effects/Summon.h

@@ -21,8 +21,6 @@ namespace effects
 class Summon : public Effect
 {
 public:
-	CreatureID creature;
-
 	Summon();
 	virtual ~Summon();
 
@@ -41,8 +39,11 @@ protected:
 	void serializeJsonEffect(JsonSerializeFormat & handler) override final;
 
 private:
+	CreatureID creature;
+
 	bool permanent;
 	bool exclusive;
+	bool summonByHealth;
 };
 
 }

test/spells/effects/SummonTest.cpp

@@ -13,6 +13,8 @@
 
 #include <vstd/RNG.h>
 
+#include "../../../lib/CCreatureHandler.h"
+
 namespace test
 {
 using namespace ::spells;
@@ -133,20 +135,28 @@ INSTANTIATE_TEST_CASE_P
 	)
 );
 
-class SummonApplyTest : public TestWithParam<bool>, public EffectFixture
+class SummonApplyTest : public TestWithParam<::testing::tuple<bool, bool>>, public EffectFixture
 {
 public:
 	CreatureID toSummon;
 	bool permanent = false;
+	bool summonByHealth = false;
 
+	const uint32_t unitId = 42;
 	const int32_t unitAmount = 123;
+	const int32_t unitHealth;
+	const int64_t unitTotalHealth;
+
+	const BattleHex unitPosition = BattleHex(5,5);
 
 	std::shared_ptr<::battle::UnitInfo> unitAddInfo;
 	std::shared_ptr<UnitFake> acquired;
 
 	SummonApplyTest()
 		: EffectFixture("core:summon"),
-		toSummon(creature1)
+		toSummon(creature1),
+		unitHealth(toSummon.toCreature()->MaxHealth()),
+		unitTotalHealth(unitAmount * unitHealth)
 	{
 	}
 
@@ -155,7 +165,15 @@ public:
 		InSequence local;
 		EXPECT_CALL(mechanicsMock, getEffectPower()).WillOnce(Return(38));
 		EXPECT_CALL(mechanicsMock, calculateRawEffectValue(Eq(0), Eq(38))).WillOnce(Return(567));
-		EXPECT_CALL(mechanicsMock, applySpecificSpellBonus(Eq(567))).WillOnce(Return(unitAmount));
+
+		if(summonByHealth)
+		{
+			EXPECT_CALL(mechanicsMock, applySpecificSpellBonus(Eq(567))).WillOnce(Return(unitTotalHealth));
+		}
+		else
+		{
+			EXPECT_CALL(mechanicsMock, applySpecificSpellBonus(Eq(567))).WillOnce(Return(unitAmount));
+		}
 	}
 
     void onUnitAdded(uint32_t id, const JsonNode & data)
@@ -168,11 +186,13 @@ protected:
 	{
 		EffectFixture::setUp();
 
-		permanent = GetParam();
+		permanent = ::testing::get<0>(GetParam());
+		summonByHealth = ::testing::get<1>(GetParam());
 
 		JsonNode options(JsonNode::JsonType::DATA_STRUCT);
 		options["id"].String() = "airElemental";
 		options["permanent"].Bool() = permanent;
+		options["summonByHealth"].Bool() = summonByHealth;
 
 		EffectFixture::setupEffect(options);
 
@@ -182,6 +202,7 @@ protected:
 	void TearDown() override
 	{
 		acquired.reset();
+		unitAddInfo.reset();
 	}
 };
 
@@ -189,20 +210,17 @@ TEST_P(SummonApplyTest, SpawnsNewUnit)
 {
 	expectAmountCalculation();
 
-	BattleHex position(5,5);
-	const uint32_t unitId = 42;
-
 	EXPECT_CALL(*battleFake, nextUnitId()).WillOnce(Return(unitId));
 	EXPECT_CALL(*battleFake, addUnit(Eq(unitId), _)).WillOnce(Invoke(this, &SummonApplyTest::onUnitAdded));
 
 	EffectTarget target;
-	target.emplace_back(position);
+	target.emplace_back(unitPosition);
 
 	subject->apply(battleProxy.get(), rngMock, &mechanicsMock, target);
 
 	EXPECT_EQ(unitAddInfo->count, unitAmount);
 	EXPECT_EQ(unitAddInfo->id, unitId);
-	EXPECT_EQ(unitAddInfo->position, position);
+	EXPECT_EQ(unitAddInfo->position, unitPosition);
 	EXPECT_EQ(unitAddInfo->side, mechanicsMock.casterSide);
 	EXPECT_EQ(unitAddInfo->summoned, !permanent);
 	EXPECT_EQ(unitAddInfo->type, toSummon);
@@ -212,10 +230,6 @@ TEST_P(SummonApplyTest, UpdatesOldUnit)
 {
 	expectAmountCalculation();
 
-	BattleHex position(5,5);
-	const uint32_t unitId = 42;
-	const int32_t unitHealth = 234;
-
 	acquired = std::make_shared<UnitFake>();
 	acquired->addNewBonus(std::make_shared<Bonus>(Bonus::PERMANENT, Bonus::STACK_HEALTH, Bonus::CREATURE_ABILITY, unitHealth, 0));
 	acquired->redirectBonusesToFake();
@@ -226,14 +240,13 @@ TEST_P(SummonApplyTest, UpdatesOldUnit)
 
 	{
 		EXPECT_CALL(unit, acquire()).WillOnce(Return(acquired));
-		EXPECT_CALL(*acquired, heal(Eq(unitHealth * unitAmount), Eq(EHealLevel::OVERHEAL), Eq(permanent ? EHealPower::PERMANENT : EHealPower::ONE_BATTLE)));
+		EXPECT_CALL(*acquired, heal(Eq(unitTotalHealth), Eq(EHealLevel::OVERHEAL), Eq(permanent ? EHealPower::PERMANENT : EHealPower::ONE_BATTLE)));
 		EXPECT_CALL(*acquired, save(_));
 		EXPECT_CALL(*battleFake, setUnitState(Eq(unitId), _, _));
 	}
 
 	EXPECT_CALL(unit, unitId()).WillOnce(Return(unitId));
 
-
 	unitsFake.setDefaultBonusExpectations();
 
 	EffectTarget target;
@@ -242,12 +255,15 @@ TEST_P(SummonApplyTest, UpdatesOldUnit)
 	subject->apply(battleProxy.get(), rngMock, &mechanicsMock, target);
 }
 
-
 INSTANTIATE_TEST_CASE_P
 (
 	ByConfig,
 	SummonApplyTest,
-	Values(false, true)
+	Combine
+	(
+		Values(false, true),
+		Values(false, true)
+	)
 );
 
 }