Simplify Necromancy code, update docs

docs/modders/Bonus/Bonus_Types.md

@@ -138,18 +138,23 @@ Changes surrender cost for affected heroes
 
 ### IMPROVED_NECROMANCY
 
-Allows to raise different creatures than Skeletons after battle.
+Bonus allows the hero to raise specific creatures from corpses after battle.
+
+If the hero has multiple bonuses of the same type, the game will select the unit with the higher level. If the units have the same level, the game will select the unit with the higher market value (the total cost of the unit in gold, including converted resources).
+
+If the hero has no free space for the target creature but has space for its upgrade (including subsequent upgrades), the upgraded unit will be raised instead at a rate of two-thirds.
 
 - subtype: creature raised
 - val: Necromancer power
-- addInfo: Level of Necromancy secondary skill (1 - Basic, 3 - Expert)
-- Example (from Cloak Of The Undead King):
+- addInfo: Requried total level of Necromancer power for this bonus to be active (val of all bonuses of this type)
 
+Example (from Cloak Of The Undead King):
 ```json
 {
     "type" : "IMPROVED_NECROMANCY",
     "subtype" : "creature.walkingDead",
-    "addInfo" : 1
+    "addInfo" : 1, // requires 1 val of IMPROVED_NECROMANCY from other source, e.g. skill
+    "val" : 0 // does not provides levels of necromancer power on its own
 }
 ```
 

lib/json/JsonBonus.cpp

@@ -115,6 +115,7 @@ static void loadBonusSubtype(BonusSubtypeID & subtype, BonusType type, const Jso
 		case BonusType::HATE:
 		case BonusType::SUMMON_GUARDIANS:
 		case BonusType::MANUAL_CONTROL:
+		case BonusType::SKELETON_TRANSFORMER_TARGET:
 		{
 			LIBRARY->identifiers()->requestIdentifier( "creature", node, [&subtype](int32_t identifier)
 			{

lib/mapObjects/CGHeroInstance.cpp

@@ -981,83 +981,79 @@ bool CGHeroInstance::canLearnSpell(const spells::Spell * spell, bool allowBanned
  */
 CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &battleResult) const
 {
-	bool hasImprovedNecromancy = hasBonusOfType(BonusType::IMPROVED_NECROMANCY);
+	TConstBonusListPtr improvedNecromancy = getBonusesOfType(BonusType::IMPROVED_NECROMANCY);
 
 	// need skill or cloak of undead king - lesser artifacts don't work without skill
-	if (hasImprovedNecromancy)
-	{
-		double necromancySkill = valOfBonuses(BonusType::UNDEAD_RAISE_PERCENTAGE) / 100.0;
-		const ui8 necromancyLevel = valOfBonuses(BonusType::IMPROVED_NECROMANCY);
-		vstd::amin(necromancySkill, 1.0); //it's impossible to raise more creatures than all...
-		const std::map<CreatureID,si32> &casualties = battleResult.casualties[CBattleInfoEssentials::otherSide(battleResult.winner)];
-		if(casualties.empty())
-			return CStackBasicDescriptor();
-		// figure out what to raise - pick strongest creature meeting requirements
-		CreatureID creatureTypeRaised = CreatureID::NONE; //now we always have IMPROVED_NECROMANCY, no need for hardcode
-		int requiredCasualtyLevel = 1;
-		TConstBonusListPtr improvedNecromancy = getBonusesOfType(BonusType::IMPROVED_NECROMANCY);
-		if(!improvedNecromancy->empty())
+	if (improvedNecromancy->empty())
+		return CStackBasicDescriptor();
+
+	int raisedUnitsPercentage = std::clamp(valOfBonuses(BonusType::UNDEAD_RAISE_PERCENTAGE), 0, 100);
+	if (raisedUnitsPercentage == 0)
+		return CStackBasicDescriptor();
+
+	const std::map<CreatureID,si32> &casualties = battleResult.casualties[CBattleInfoEssentials::otherSide(battleResult.winner)];
+	if(casualties.empty())
+		return CStackBasicDescriptor();
+
+	// figure out what to raise - pick strongest creature meeting requirements
+	CreatureID bestCreature = CreatureID::NONE;
+	int necromancerPower = improvedNecromancy->totalValue();
+
+	// pick best bonus available
+	for(const std::shared_ptr<Bonus> & newPick : *improvedNecromancy)
+	{
+		// addInfo[0] = required necromancy skill
+		if(newPick->additionalInfo[0] > necromancerPower)
+			continue;
+
+		CreatureID newCreature = newPick->subtype.as<CreatureID>();;
+
+		if(!bestCreature.hasValue())
 		{
-			int maxCasualtyLevel = 1;
-			for(const auto & casualty : casualties)
-				vstd::amax(maxCasualtyLevel, LIBRARY->creatures()->getById(casualty.first)->getLevel());
-			// pick best bonus available
-			std::shared_ptr<Bonus> topPick;
-			for(const std::shared_ptr<Bonus> & newPick : *improvedNecromancy)
-			{
-				// addInfo[0] = required necromancy skill, addInfo[1] = required casualty level
-				if(newPick->additionalInfo[0] > necromancyLevel || newPick->additionalInfo[1] > maxCasualtyLevel)
-					continue;
-				if(!topPick)
-				{
-					topPick = newPick;
-				}
-				else
-				{
-					auto quality = [](const std::shared_ptr<Bonus> & pick) -> std::tuple<int, int, int>
-					{
-						const auto * c = pick->subtype.as<CreatureID>().toCreature();
-						return std::tuple<int, int, int> {c->getLevel(), static_cast<int>(c->getFullRecruitCost().marketValue()), -pick->additionalInfo[1]};
-					};
-					if(quality(topPick) < quality(newPick))
-						topPick = newPick;
-				}
-			}
-			if(topPick)
-			{
-				creatureTypeRaised = topPick->subtype.as<CreatureID>();
-				requiredCasualtyLevel = std::max(topPick->additionalInfo[1], 1);
-			}
+			bestCreature = newCreature;
 		}
-		assert(creatureTypeRaised != CreatureID::NONE);
-		// raise upgraded creature (at 2/3 rate) if no space available otherwise
-		if(getSlotFor(creatureTypeRaised) == SlotID())
+		else
 		{
-			for (const auto & slot : Slots())
+			auto quality = [](CreatureID pick) -> std::tuple<int, int>
 			{
-				if (creatureTypeRaised.toCreature()->isMyDirectOrIndirectUpgrade(slot.second->getCreature()))
-				{
-					creatureTypeRaised = slot.second->getCreatureID();
-					necromancySkill *= 2/3.0;
-					break;
-				}
-			}
+				const auto * c = pick.toCreature();
+				return std::tuple<int, int> {c->getLevel(), static_cast<int>(c->getFullRecruitCost().marketValue())};
+			};
+			if(quality(bestCreature) < quality(newCreature))
+				bestCreature = newCreature;
 		}
-		// calculate number of creatures raised - low level units contribute at 50% rate
-		const double raisedUnitHealth = creatureTypeRaised.toCreature()->getMaxHealth();
-		double raisedUnits = 0;
-		for(const auto & casualty : casualties)
+	}
+
+	assert(bestCreature != CreatureID::NONE);
+	CreatureID selectedCreature = bestCreature;
+
+	// raise upgraded creature (at 2/3 rate) if no space available otherwise
+	if(getSlotFor(selectedCreature) == SlotID())
+	{
+		for (const auto & slot : Slots())
 		{
-			const CCreature * c = casualty.first.toCreature();
-			double raisedFromCasualty = std::min(c->getMaxHealth() / raisedUnitHealth, 1.0) * casualty.second * necromancySkill;
-			if(c->getLevel() < requiredCasualtyLevel)
-				raisedFromCasualty *= 0.5;
-			raisedUnits += raisedFromCasualty;
+			if (selectedCreature.toCreature()->isMyDirectOrIndirectUpgrade(slot.second->getCreature()))
+			{
+				selectedCreature = slot.second->getCreatureID();
+				break;
+			}
 		}
-		return CStackBasicDescriptor(creatureTypeRaised, std::max(static_cast<int>(raisedUnits), 1));
 	}
 
-	return CStackBasicDescriptor();
+	// calculate number of creatures raised - low level units contribute at 50% rate
+	const double raisedUnitHealth = selectedCreature.toCreature()->getMaxHealth();
+	double raisedUnits = 0;
+	for(const auto & casualty : casualties)
+	{
+		const CCreature * c = casualty.first.toCreature();
+		double raisedFromCasualty = std::min(c->getMaxHealth() / raisedUnitHealth, 1.0) * casualty.second * raisedUnitsPercentage;
+		raisedUnits += raisedFromCasualty;
+	}
+
+	if (bestCreature != selectedCreature)
+		return CStackBasicDescriptor(selectedCreature, std::max(static_cast<int>(raisedUnits * 2 / 3), 1));
+	else
+		return CStackBasicDescriptor(selectedCreature, std::max(static_cast<int>(raisedUnits), 1));
 }
 
 int CGHeroInstance::getSightRadius() const