From bb670cfb8256001a198266a12bf5760a17380612 Mon Sep 17 00:00:00 2001
From: Ivan Savenko <saven.ivan@gmail.com>
Date: Sat, 13 Jan 2024 15:55:07 +0200
Subject: [PATCH 1/3] Merged accurate shot bonus into death stare bonus

---
 docs/modders/Bonus/Bonus_Types.md        | 23 +++++-----
 lib/JsonNode.cpp                         |  1 -
 lib/bonuses/BonusCustomTypes.cpp         |  4 ++
 lib/bonuses/BonusCustomTypes.h           |  4 ++
 lib/bonuses/BonusEnum.h                  |  1 -
 lib/modding/IdentifierStorage.cpp        |  4 ++
 server/battles/BattleActionProcessor.cpp | 54 ++++++++++++------------
 server/battles/BattleActionProcessor.h   |  2 +-
 8 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/docs/modders/Bonus/Bonus_Types.md b/docs/modders/Bonus/Bonus_Types.md
index a51e1bf5c..33a2df2df 100644
--- a/docs/modders/Bonus/Bonus_Types.md
+++ b/docs/modders/Bonus/Bonus_Types.md
@@ -727,14 +727,20 @@ Affected unit will deal additional damage after attack
 
 ### DEATH_STARE
 
-Affected unit will kill additional units after attack
+Affected unit will kill additional units after attack. Used for Death stare (Mighty Gorgon) ability and for Accurate Shot (Pirates, HotA)
 
 - subtype: 
-	- deathStareGorgon: random amount
-	- deathStareCommander: fixed amount
+	- deathStareGorgon: only melee attack, random amount of killed units
+	- deathStareNoRangePenalty: only ranged attacks without obstacle (walls) or range penalty
+	- deathStareRangePenalty: only ranged attacks with range penalty
+	- deathStareObstaclePenalty: only ranged attacks with obstacle (walls) penalty
+	- deathStareRangeObstaclePenalty: only ranged attacks with both range and obstacle penalty
+	- deathStareCommander: fixed amount, both melee and ranged attacks
 - val: 
-	- for deathStareGorgon: chance to kill, counted separately for each unit in attacking stack, percentage. At most (stack size \* chance) units can be killed at once. TODO: recheck formula
 	- for deathStareCommander: number of creatures to kill, total amount of killed creatures is (attacker level / defender level) \* val
+	- for all other subtypes: chance to kill, counted separately for each unit in attacking stack, percentage. At most (stack size \* chance) units can be killed at once, rounded up
+- addInfo:
+	- SpellID to be used as hit effect. If not set - 'deathStare' spell will be used
 
 ### SPECIAL_CRYSTAL_GENERATION
 
@@ -744,15 +750,6 @@ If player has affected unit under his control in any army, he will receive addit
 
 Affected unit will not use spellcast as default attack option
 
-### ACCURATE_SHOT
-
-Affected unit will kill additional units after attack, similar to death stare - works only for ranged attack
-
-- subtype:
-	spell identifier for spell that receives value that should be killed on input, spell.deathStare is used by default, use 'accurateShot' as part of spell name to allow detection for proper battle log description
-- val:
-	chance to kill, counted separately for each unit in attacking stack, percentage. Chance gets lessened by 2/3 with range penalty and effect won't trigger with wall penalty. At most (stack size \* chance / 100 **[rounded up]**) units can be killed at once. TODO: recheck formula
-
 ## Creature spellcasting and activated abilities
 
 ### SPELLCASTER
diff --git a/lib/JsonNode.cpp b/lib/JsonNode.cpp
index eb3ec93b4..593cbdf19 100644
--- a/lib/JsonNode.cpp
+++ b/lib/JsonNode.cpp
@@ -515,7 +515,6 @@ static void loadBonusSubtype(BonusSubtypeID & subtype, BonusType type, const Jso
 		case BonusType::SPECIFIC_SPELL_POWER:
 		case BonusType::ENCHANTED:
 		case BonusType::MORE_DAMAGE_FROM_SPELL:
-		case BonusType::ACCURATE_SHOT:
 		case BonusType::NOT_ACTIVE:
 		{
 			VLC->identifiers()->requestIdentifier( "spell", node, [&subtype](int32_t identifier)
diff --git a/lib/bonuses/BonusCustomTypes.cpp b/lib/bonuses/BonusCustomTypes.cpp
index 4e793ac21..6f6ac83b0 100644
--- a/lib/bonuses/BonusCustomTypes.cpp
+++ b/lib/bonuses/BonusCustomTypes.cpp
@@ -23,6 +23,10 @@ const BonusCustomSubtype BonusCustomSubtype::heroMovementLand(1);
 const BonusCustomSubtype BonusCustomSubtype::heroMovementSea(0);
 const BonusCustomSubtype BonusCustomSubtype::deathStareGorgon(0);
 const BonusCustomSubtype BonusCustomSubtype::deathStareCommander(1);
+const BonusCustomSubtype BonusCustomSubtype::deathStareNoRangePenalty(2);
+const BonusCustomSubtype BonusCustomSubtype::deathStareRangePenalty(3);
+const BonusCustomSubtype BonusCustomSubtype::deathStareObstaclePenalty(4);
+const BonusCustomSubtype BonusCustomSubtype::deathStareRangeObstaclePenalty(5);
 const BonusCustomSubtype BonusCustomSubtype::rebirthRegular(0);
 const BonusCustomSubtype BonusCustomSubtype::rebirthSpecial(1);
 const BonusCustomSubtype BonusCustomSubtype::visionsMonsters(0);
diff --git a/lib/bonuses/BonusCustomTypes.h b/lib/bonuses/BonusCustomTypes.h
index a6a2c96e2..824561929 100644
--- a/lib/bonuses/BonusCustomTypes.h
+++ b/lib/bonuses/BonusCustomTypes.h
@@ -45,6 +45,10 @@ public:
 
 	static const BonusCustomSubtype deathStareGorgon; // 0
 	static const BonusCustomSubtype deathStareCommander;
+	static const BonusCustomSubtype deathStareNoRangePenalty;
+	static const BonusCustomSubtype deathStareRangePenalty;
+	static const BonusCustomSubtype deathStareObstaclePenalty;
+	static const BonusCustomSubtype deathStareRangeObstaclePenalty;
 
 	static const BonusCustomSubtype rebirthRegular; // 0
 	static const BonusCustomSubtype rebirthSpecial; // 1
diff --git a/lib/bonuses/BonusEnum.h b/lib/bonuses/BonusEnum.h
index 92068d7be..f6cb258e0 100644
--- a/lib/bonuses/BonusEnum.h
+++ b/lib/bonuses/BonusEnum.h
@@ -174,7 +174,6 @@ class JsonNode;
 	BONUS_NAME(MAX_MORALE) /*cheat bonus*/ \
 	BONUS_NAME(MAX_LUCK) /*cheat bonus*/ \
 	BONUS_NAME(FEROCITY) /*extra attacks, only if at least some creatures killed while attacking target unit, val = amount of additional attacks, additional info = amount of creatures killed to trigger (default 1)*/ \
-	BONUS_NAME(ACCURATE_SHOT) /*HotA Sea Dog-like ability - ranged only, val = full arrow trigger percent, subtype = spell identifier that killed value goes through (death stare by default) - use 'accurateShot' as part of spell name for proper battle log description*/ \
 	BONUS_NAME(ENEMY_ATTACK_REDUCTION) /*in % (value) eg. Nix (HotA)*/ \
 	BONUS_NAME(REVENGE) /*additional damage based on how many units in stack died - formula: sqrt((number of creatures at battle start + 1) * creature health) / (total health now + 1 creature health) - 1) * 100% */ \
 	/* end of list */
diff --git a/lib/modding/IdentifierStorage.cpp b/lib/modding/IdentifierStorage.cpp
index fb26568e4..d8b16d7d0 100644
--- a/lib/modding/IdentifierStorage.cpp
+++ b/lib/modding/IdentifierStorage.cpp
@@ -53,6 +53,10 @@ CIdentifierStorage::CIdentifierStorage()
 	registerObject(ModScope::scopeBuiltin(), "bonusSubtype", "heroMovementSea", 0);
 	registerObject(ModScope::scopeBuiltin(), "bonusSubtype", "deathStareGorgon", 0);
 	registerObject(ModScope::scopeBuiltin(), "bonusSubtype", "deathStareCommander", 1);
+	registerObject(ModScope::scopeBuiltin(), "bonusSubtype", "deathStareNoRangePenalty", 2);
+	registerObject(ModScope::scopeBuiltin(), "bonusSubtype", "deathStareRangePenalty", 3);
+	registerObject(ModScope::scopeBuiltin(), "bonusSubtype", "deathStareObstaclePenalty", 4);
+	registerObject(ModScope::scopeBuiltin(), "bonusSubtype", "deathStareRangeObstaclePenalty", 5);
 	registerObject(ModScope::scopeBuiltin(), "bonusSubtype", "rebirthRegular", 0);
 	registerObject(ModScope::scopeBuiltin(), "bonusSubtype", "rebirthSpecial", 1);
 	registerObject(ModScope::scopeBuiltin(), "bonusSubtype", "visionsMonsters", 0);
diff --git a/server/battles/BattleActionProcessor.cpp b/server/battles/BattleActionProcessor.cpp
index 67b1d5be7..5e6f00a4f 100644
--- a/server/battles/BattleActionProcessor.cpp
+++ b/server/battles/BattleActionProcessor.cpp
@@ -1242,7 +1242,7 @@ void BattleActionProcessor::handleAttackBeforeCasting(const CBattleInfoCallback
 	attackCasting(battle, ranged, BonusType::SPELL_BEFORE_ATTACK, attacker, defender); //no death stare / acid breath needed?
 }
 
-void BattleActionProcessor::HandleDeathStareAndPirateShot(const CBattleInfoCallback & battle, bool ranged, const CStack * attacker, const CStack * defender)
+void BattleActionProcessor::handleDeathStare(const CBattleInfoCallback & battle, bool ranged, const CStack * attacker, const CStack * defender)
 {
 	// mechanics of Death Stare as in H3:
 	// each gorgon have 10% chance to kill (counted separately in H3) -> binomial distribution
@@ -1254,28 +1254,30 @@ void BattleActionProcessor::HandleDeathStareAndPirateShot(const CBattleInfoCallb
 		* X = 3 multiplier for shooting without penalty and X = 2 if shooting with penalty. Ability doesn't work if shooting at creatures behind walls.
 		*/
 
-	auto bonus = attacker->getBonus(Selector::type()(BonusType::DEATH_STARE));
-	if(bonus == nullptr)
-		bonus = attacker->getBonus(Selector::type()(BonusType::ACCURATE_SHOT));
+	auto subtype = BonusCustomSubtype::deathStareGorgon;
 
-	if(bonus->type == BonusType::ACCURATE_SHOT) //should not work from behind walls, except when being defender or under effect of golden bow etc.
+	if (ranged)
 	{
-		if(!ranged)
-			return;
-		if(battle.battleHasWallPenalty(attacker, attacker->getPosition(), defender->getPosition()))
-			return;
+		bool rangePenalty = battle.battleHasDistancePenalty(attacker, attacker->getPosition(), defender->getPosition());
+		bool obstaclePenalty = battle.battleHasWallPenalty(attacker, attacker->getPosition(), defender->getPosition());
+
+		if(rangePenalty)
+		{
+			if(obstaclePenalty)
+				subtype = BonusCustomSubtype::deathStareRangeObstaclePenalty;
+			else
+				subtype = BonusCustomSubtype::deathStareRangePenalty;
+		}
+		else
+		{
+			if(obstaclePenalty)
+				subtype = BonusCustomSubtype::deathStareObstaclePenalty;
+			else
+				subtype = BonusCustomSubtype::deathStareNoRangePenalty;
+		}
 	}
 
-	int singleCreatureKillChancePercent;
-	if(bonus->type == BonusType::ACCURATE_SHOT)
-	{
-		singleCreatureKillChancePercent = attacker->valOfBonuses(BonusType::ACCURATE_SHOT);
-		if(battle.battleHasDistancePenalty(attacker, attacker->getPosition(), defender->getPosition()))
-			singleCreatureKillChancePercent = (singleCreatureKillChancePercent * 2) / 3;
-	}
-	else //DEATH_STARE
-		singleCreatureKillChancePercent = attacker->valOfBonuses(BonusType::DEATH_STARE, BonusCustomSubtype::deathStareGorgon);
-
+	int singleCreatureKillChancePercent = attacker->valOfBonuses(BonusType::DEATH_STARE, subtype);
 	double chanceToKill = singleCreatureKillChancePercent / 100.0;
 	vstd::amin(chanceToKill, 1); //cap at 100%
 	std::binomial_distribution<> distribution(attacker->getCount(), chanceToKill);
@@ -1284,16 +1286,16 @@ void BattleActionProcessor::HandleDeathStareAndPirateShot(const CBattleInfoCallb
 	int maxToKill = (attacker->getCount() * singleCreatureKillChancePercent + 99) / 100;
 	vstd::amin(killedCreatures, maxToKill);
 
-	if(bonus->type == BonusType::DEATH_STARE)
-		killedCreatures += (attacker->level() * attacker->valOfBonuses(BonusType::DEATH_STARE, BonusCustomSubtype::deathStareCommander)) / defender->level();
+	killedCreatures += (attacker->level() * attacker->valOfBonuses(BonusType::DEATH_STARE, BonusCustomSubtype::deathStareCommander)) / defender->level();
 
 	if(killedCreatures)
 	{
 		//TODO: death stare or accurate shot was not originally available for multiple-hex attacks, but...
 
 		SpellID spellID = SpellID(SpellID::DEATH_STARE); //also used as fallback spell for ACCURATE_SHOT
-		if(bonus->type == BonusType::ACCURATE_SHOT && bonus->subtype.as<SpellID>() != SpellID::NONE)
-			spellID = bonus->subtype.as<SpellID>();
+		auto bonus = attacker->getBonus(Selector::typeSubtype(BonusType::DEATH_STARE, subtype));
+		if(bonus && bonus->additionalInfo[0] != SpellID::NONE)
+			spellID = SpellID(bonus->additionalInfo[0]);
 
 		const CSpell * spell = spellID.toSpell();
 		spells::AbilityCaster caster(attacker, 0);
@@ -1319,10 +1321,8 @@ void BattleActionProcessor::handleAfterAttackCasting(const CBattleInfoCallback &
 		return;
 	}
 
-	if(attacker->hasBonusOfType(BonusType::DEATH_STARE) || attacker->hasBonusOfType(BonusType::ACCURATE_SHOT))
-	{
-		HandleDeathStareAndPirateShot(battle, ranged, attacker, defender);
-	}
+	if(attacker->hasBonusOfType(BonusType::DEATH_STARE))
+		handleDeathStare(battle, ranged, attacker, defender);
 
 	if(!defender->alive())
 		return;
diff --git a/server/battles/BattleActionProcessor.h b/server/battles/BattleActionProcessor.h
index b8d4ede5e..6c28b5950 100644
--- a/server/battles/BattleActionProcessor.h
+++ b/server/battles/BattleActionProcessor.h
@@ -45,7 +45,7 @@ class BattleActionProcessor : boost::noncopyable
 
 	void handleAttackBeforeCasting(const CBattleInfoCallback & battle, bool ranged, const CStack * attacker, const CStack * defender);
 
-	void HandleDeathStareAndPirateShot(const CBattleInfoCallback &battle, bool ranged, const CStack *attacker, const CStack *defender);
+	void handleDeathStare(const CBattleInfoCallback &battle, bool ranged, const CStack *attacker, const CStack *defender);
 
 	void handleAfterAttackCasting(const CBattleInfoCallback & battle, bool ranged, const CStack * attacker, const CStack * defender);
 	void attackCasting(const CBattleInfoCallback & battle, bool ranged, BonusType attackMode, const battle::Unit * attacker, const CStack * defender);

From fc6a9924eebd6b159ef0e7ac2c23979aac8ed921 Mon Sep 17 00:00:00 2001
From: Ivan Savenko <saven.ivan@gmail.com>
Date: Sat, 13 Jan 2024 15:55:38 +0200
Subject: [PATCH 2/3] FIRST_STRIKE now supports ranged / melee subtypes,
 BLOCKS_RETALIATION will properly block FIRST_STRIKE

---
 docs/modders/Bonus/Bonus_Types.md        |  5 +++++
 lib/JsonNode.cpp                         |  1 +
 server/battles/BattleActionProcessor.cpp | 14 ++++++++++----
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/docs/modders/Bonus/Bonus_Types.md b/docs/modders/Bonus/Bonus_Types.md
index 33a2df2df..0aaddfd4e 100644
--- a/docs/modders/Bonus/Bonus_Types.md
+++ b/docs/modders/Bonus/Bonus_Types.md
@@ -587,6 +587,11 @@ Affected unit will attack units on all hexes that surround attacked hex
 
 Affected unit will retaliate before enemy attacks, if able
 
+- subtype: 
+	- damageTypeMelee: only melee attacks affected
+	- damageTypeRanged: only ranged attacks affected. Note that unit also requires ability to retaliate in ranged, such as RANGED_RETALIATION bonus
+	- damageTypeAll: any attacks are affected
+
 ### SHOOTS_ALL_ADJACENT
 
 Affected unit will attack units on all hexes that surround attacked hex in ranged attacks
diff --git a/lib/JsonNode.cpp b/lib/JsonNode.cpp
index 593cbdf19..3a5850041 100644
--- a/lib/JsonNode.cpp
+++ b/lib/JsonNode.cpp
@@ -537,6 +537,7 @@ static void loadBonusSubtype(BonusSubtypeID & subtype, BonusType type, const Jso
 		case BonusType::NEGATE_ALL_NATURAL_IMMUNITIES:
 		case BonusType::CREATURE_DAMAGE:
 		case BonusType::FLYING:
+		case BonusType::FIRST_STRIKE:
 		case BonusType::GENERAL_DAMAGE_REDUCTION:
 		case BonusType::PERCENTAGE_DAMAGE_BOOST:
 		case BonusType::SOUL_STEAL:
diff --git a/server/battles/BattleActionProcessor.cpp b/server/battles/BattleActionProcessor.cpp
index 5e6f00a4f..deab770e2 100644
--- a/server/battles/BattleActionProcessor.cpp
+++ b/server/battles/BattleActionProcessor.cpp
@@ -268,7 +268,9 @@ bool BattleActionProcessor::doAttackAction(const CBattleInfoCallback & battle, c
 		totalAttacks += attackingHero->valOfBonuses(BonusType::HERO_GRANTS_ATTACKS, BonusSubtypeID(stack->creatureId()));
 	}
 
-	const bool firstStrike = destinationStack->hasBonusOfType(BonusType::FIRST_STRIKE);
+	static const auto firstStrikeSelector = Selector::typeSubtype(BonusType::FIRST_STRIKE, BonusCustomSubtype::damageTypeAll).Or(Selector::typeSubtype(BonusType::FIRST_STRIKE, BonusCustomSubtype::damageTypeMelee));
+	const bool firstStrike = destinationStack->hasBonus(firstStrikeSelector);
+
 	const bool retaliation = destinationStack->ableToRetaliate();
 	bool ferocityApplied = false;
 	int32_t defenderInitialQuantity = destinationStack->getCount();
@@ -276,7 +278,7 @@ bool BattleActionProcessor::doAttackAction(const CBattleInfoCallback & battle, c
 	for (int i = 0; i < totalAttacks; ++i)
 	{
 		//first strike
-		if(i == 0 && firstStrike && retaliation)
+		if(i == 0 && firstStrike && retaliation && !stack->hasBonusOfType(BonusType::BLOCKS_RETALIATION))
 		{
 			makeAttack(battle, destinationStack, stack, 0, stack->getPosition(), true, false, true);
 		}
@@ -353,7 +355,11 @@ bool BattleActionProcessor::doShootAction(const CBattleInfoCallback & battle, co
 		return false;
 	}
 
-	makeAttack(battle, stack, destinationStack, 0, destination, true, true, false);
+	static const auto firstStrikeSelector = Selector::typeSubtype(BonusType::FIRST_STRIKE, BonusCustomSubtype::damageTypeAll).Or(Selector::typeSubtype(BonusType::FIRST_STRIKE, BonusCustomSubtype::damageTypeRanged));
+	const bool firstStrike = destinationStack->hasBonus(firstStrikeSelector);
+
+	if (!firstStrike)
+		makeAttack(battle, stack, destinationStack, 0, destination, true, true, false);
 
 	//ranged counterattack
 	if (destinationStack->hasBonusOfType(BonusType::RANGED_RETALIATION)
@@ -375,7 +381,7 @@ bool BattleActionProcessor::doShootAction(const CBattleInfoCallback & battle, co
 		totalRangedAttacks += attackingHero->valOfBonuses(BonusType::HERO_GRANTS_ATTACKS, BonusSubtypeID(stack->creatureId()));
 	}
 
-	for(int i = 1; i < totalRangedAttacks; ++i)
+	for(int i = firstStrike ? 0:1; i < totalRangedAttacks; ++i)
 	{
 		if(
 			stack->alive()

From 9acd436a09b90b2c7823b3dca3534facb2485307 Mon Sep 17 00:00:00 2001
From: Ivan Savenko <saven.ivan@gmail.com>
Date: Sun, 14 Jan 2024 15:51:49 +0200
Subject: [PATCH 3/3] Review suggestion

---
 Mods/vcmi/config/vcmi/english.json | 2 --
 config/bonuses.json                | 7 -------
 docs/modders/Bonus/Bonus_Types.md  | 2 +-
 3 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/Mods/vcmi/config/vcmi/english.json b/Mods/vcmi/config/vcmi/english.json
index db1ab3c05..4098fb24f 100644
--- a/Mods/vcmi/config/vcmi/english.json
+++ b/Mods/vcmi/config/vcmi/english.json
@@ -333,8 +333,6 @@
 	"vcmi.stackExperience.rank.9" : "Master",
 	"vcmi.stackExperience.rank.10" : "Ace",
 
-	"core.bonus.ACCURATE_SHOT.name": "Accurate Shot",
-	"core.bonus.ACCURATE_SHOT.description": "Has (${val}% - penalties) extra kills chance",
 	"core.bonus.ADDITIONAL_ATTACK.name": "Double Strike",
 	"core.bonus.ADDITIONAL_ATTACK.description": "Attacks twice",
 	"core.bonus.ADDITIONAL_RETALIATION.name": "Additional retaliations",
diff --git a/config/bonuses.json b/config/bonuses.json
index 8a3ecca59..350ec5d5a 100644
--- a/config/bonuses.json
+++ b/config/bonuses.json
@@ -3,13 +3,6 @@
 // LEVEL_SPELL_IMMUNITY
 
 {
-	"ACCURATE_SHOT":
-	{
-		"graphics":
-		{
-			"icon":  "zvs/Lib1.res/E_DIST"
-		}
-	},
 	"ADDITIONAL_ATTACK":
 	{
 		"graphics":
diff --git a/docs/modders/Bonus/Bonus_Types.md b/docs/modders/Bonus/Bonus_Types.md
index 0aaddfd4e..7452aa448 100644
--- a/docs/modders/Bonus/Bonus_Types.md
+++ b/docs/modders/Bonus/Bonus_Types.md
@@ -745,7 +745,7 @@ Affected unit will kill additional units after attack. Used for Death stare (Mig
 	- for deathStareCommander: number of creatures to kill, total amount of killed creatures is (attacker level / defender level) \* val
 	- for all other subtypes: chance to kill, counted separately for each unit in attacking stack, percentage. At most (stack size \* chance) units can be killed at once, rounded up
 - addInfo:
-	- SpellID to be used as hit effect. If not set - 'deathStare' spell will be used
+	- SpellID to be used as hit effect. If not set - 'deathStare' spell will be used. If set to "accurateShot" battle log messages will use alternative description
 
 ### SPECIAL_CRYSTAL_GENERATION