Merge pull request #5690 from IvanSavenko/rewardable_improve

Extend functionality of rewardable objects

AI/Nullkiller/Engine/PriorityEvaluator.cpp

@@ -206,6 +206,11 @@ int getDwellingArmyCost(const CGObjectInstance * target)
 	return cost;
 }
 
+static uint64_t evaluateSpellScrollArmyValue(const SpellID &)
+{
+	return 1500;
+}
+
 static uint64_t evaluateArtifactArmyValue(const CArtifact * art)
 {
 	if(art->getId() == ArtifactID::SPELL_SCROLL)
@@ -237,7 +242,7 @@ uint64_t RewardEvaluator::getArmyReward(
 	case Obj::CREATURE_GENERATOR4:
 		return getDwellingArmyValue(ai->cb.get(), target, checkGold);
 	case Obj::SPELL_SCROLL:
-		//FALL_THROUGH
+		return evaluateSpellScrollArmyValue(dynamic_cast<const CGArtifact *>(target)->getArtifactInstance()->getScrollSpellID());
 	case Obj::ARTIFACT:
 		return evaluateArtifactArmyValue(dynamic_cast<const CGArtifact *>(target)->getArtifactInstance()->getType());
 	case Obj::HERO:
@@ -265,25 +270,16 @@ uint64_t RewardEvaluator::getArmyReward(
 
 			auto rewardValue = 0;
 
-			if(!info.reward.artifacts.empty())
-			{
-				for(auto artID : info.reward.artifacts)
-				{
-					const auto * art = artID.toArtifact();
+			for(auto artID : info.reward.grantedArtifacts)
+				rewardValue += evaluateArtifactArmyValue(artID.toArtifact());
 
-					rewardValue += evaluateArtifactArmyValue(art);
-				}
-			}
+			for(auto scroll : info.reward.grantedScrolls)
+				rewardValue += evaluateSpellScrollArmyValue(scroll);
 
-			if(!info.reward.creatures.empty())
-			{
-				for(const auto & stackInfo : info.reward.creatures)
-				{
-					rewardValue += stackInfo.getType()->getAIValue() * stackInfo.getCount();
-				}
-			}
+			for(const auto & stackInfo : info.reward.creatures)
+				rewardValue += stackInfo.getType()->getAIValue() * stackInfo.getCount();
 
-			totalValue += rewardValue > 0 ? rewardValue / (info.reward.artifacts.size() + info.reward.creatures.size()) : 0;
+			totalValue += rewardValue > 0 ? rewardValue / (info.reward.grantedArtifacts.size() + info.reward.creatures.size()) : 0;
 		}
 
 		return totalValue;

client/Client.h

@@ -188,7 +188,7 @@ public:
 	void giveResources(PlayerColor player, TResources resources) override {};
 
 	void giveCreatures(const CArmedInstance * objid, const CGHeroInstance * h, const CCreatureSet & creatures, bool remove) override {};
-	void takeCreatures(ObjectInstanceID objid, const std::vector<CStackBasicDescriptor> & creatures) override {};
+	void takeCreatures(ObjectInstanceID objid, const std::vector<CStackBasicDescriptor> & creatures, bool forceRemoval) override {};
 	bool changeStackType(const StackLocation & sl, const CCreature * c) override {return false;};
 	bool changeStackCount(const StackLocation & sl, TQuantity count, bool absoluteValue = false) override {return false;};
 	bool insertNewStack(const StackLocation & sl, const CCreature * c, TQuantity count) override {return false;};

config/gameConfig.json

@@ -463,6 +463,14 @@
 			"mergeOnRecruit" : true
 		},
 		
+		"mapObjects" : 
+		{
+			// Allow behavior that emulates h3 bug where quest from Seer Hut or Border Guard can take entire army from hero
+			// WARNING: handling of heroes without armies is not tested and may lead to bugs or crashes! Use at own risk!
+			// If this option is off, quests will only allow taking entire army if quest reward also gives creatures
+			"h3BugQuestTakesEntireArmy" : false
+		},
+
 		"markets" : 
 		{
 			// period between restocking of "Black Market" object found on adventure map
@@ -574,6 +582,12 @@
 					"type" : "MANA_PER_KNOWLEDGE_PERCENTAGE", //1000% mana per knowledge
 					"val" : 1000,
 					"valueType" : "BASE_NUMBER"
+				},
+				"spellCastsPerTurn" :
+				{
+					"type" : "HERO_SPELL_CASTS_PER_COMBAT_TURN", //1 spell can be cast by hero per turn during combat
+					"val" : 1,
+					"valueType" : "BASE_NUMBER"
 				}
 			}
 		},

config/objects/cartographer.json

@@ -18,7 +18,7 @@
 					"rarity" : 20
 				},
 				"compatibilityIdentifiers" : [ "water" ],
-				"visitMode" : "unlimited",
+				"visitMode" : "playerGlobal",
 				"canRefuse" : true,
 				"rewards" : [
 					{
@@ -44,7 +44,7 @@
 					"rarity" : 2
 				},
 				"compatibilityIdentifiers" : [ "land" ],
-				"visitMode" : "unlimited",
+				"visitMode" : "playerGlobal",
 				"canRefuse" : true,
 				"rewards" : [
 					{
@@ -72,7 +72,7 @@
 					"rarity" : 20
 				},
 				"compatibilityIdentifiers" : [ "subterra" ],
-				"visitMode" : "unlimited",
+				"visitMode" : "playerGlobal",
 				"canRefuse" : true,
 				"rewards" : [
 					{
@@ -94,4 +94,4 @@
 			}
 		}
 	}
-}
+}

config/schemas/gameSettings.json

@@ -111,6 +111,13 @@
 				"blackMarketRestockPeriod" : { "type" : "number" }
 			}
 		},
+		"mapObjects": {
+			"type" : "object",
+			"additionalProperties" : false,
+			"properties" : {
+				"h3BugQuestTakesEntireArmy" : { "type" : "boolean" }
+			}
+		},
 		"banks": {
 			"type" : "object",
 			"additionalProperties" : false,

config/schemas/rewardable.json

@@ -119,13 +119,28 @@
 					"description": "List of bonuses that will be granted to visiting hero",
 					"items": { "$ref" : "bonusInstance.json" }
 				},
+				"commanderBonuses" : {
+					"type":"array",
+					"description": "List of bonuses that will be granted to commander of a visiting hero",
+					"items": { "$ref" : "bonusInstance.json" }
+				},
+				"playerBonuses" : {
+					"type":"array",
+					"description": "List of bonuses that will be granted to player that owns visiting hero",
+					"items": { "$ref" : "bonusInstance.json" }
+				},
 
 				"resources" : { "$ref" : "#/definitions/identifierWithValueList" },
 				"secondary" : { "$ref" : "#/definitions/identifierWithValueList" },
 				"creatures" : { "$ref" : "#/definitions/identifierWithValueList" },
+				"takenCreatures" : { "$ref" : "#/definitions/identifierWithValueList" },
 				"primary" : { "$ref" : "#/definitions/identifierWithValueList" },
 
 				"artifacts" : { "$ref" : "#/definitions/identifierList" },
+				"takenArtifacts" : { "$ref" : "#/definitions/identifierList" },
+				"takenArtifactSlots" : { "$ref" : "#/definitions/identifierList" },
+				"scrolls" : { "$ref" : "#/definitions/identifierList" },
+				"takenScrolls" : { "$ref" : "#/definitions/identifierList" },
 				"spells" : { "$ref" : "#/definitions/identifierList" },
 
 				"spellCast" : {
@@ -166,18 +181,23 @@
 				"manaPoints" : { "$ref" : "#/definitions/value" },
 
 				"canLearnSkills" : { "type" : "boolean" },
+				"commanderAlive" : { "type" : "boolean" },
+				"hasExtraCreatures" : { "type" : "boolean" },
 
 				"resources" : { "$ref" : "#/definitions/identifierWithValueList" },
 				"secondary" : { "$ref" : "#/definitions/identifierWithValueList" },
 				"creatures" : { "$ref" : "#/definitions/identifierWithValueList" },
+				"canReceiveCreatures" : { "$ref" : "#/definitions/identifierWithValueList" },
 				"primary" : { "$ref" : "#/definitions/identifierWithValueList" },
 
 				"canLearnSpells" : { "$ref" : "#/definitions/identifierList" },
 				"heroClasses" : { "$ref" : "#/definitions/identifierList" },
 				"artifacts" : { "$ref" : "#/definitions/identifierList" },
+				"scrolls" : { "$ref" : "#/definitions/identifierList" },
 				"spells" : { "$ref" : "#/definitions/identifierList" },
 				"colors" : { "$ref" : "#/definitions/identifierList" },
 				"heroes" : { "$ref" : "#/definitions/identifierList" },
+				"availableSlots" : { "$ref" : "#/definitions/identifierList" },
 				
 				"anyOf" : {
 					"type" : "array",
@@ -285,6 +305,10 @@
 			"type" : "boolean"
 		},
 		
+		"forceCombat": {
+			"type" : "boolean"
+		},
+		
 		"showScoutedPreview": {
 			"type" : "boolean"
 		},
@@ -298,7 +322,7 @@
 		},
 		
 		"visitMode": {
-			"enum" : [ "unlimited", "once", "hero", "bonus", "limiter", "player" ],
+			"enum" : [ "unlimited", "once", "hero", "bonus", "limiter", "player", "playerGlobal" ],
 			"type" : "string"
 		},
 		

docs/modders/Map_Objects/Rewardable.md

@@ -97,6 +97,9 @@ Rewardable object is defined similarly to other objects, with key difference bei
 // Object description that will be shown when player right-clicks object
 "description" : "",
 
+// If set to true, and objects is guarded, then combat will start immediately, without asking player for confirmation on whether to attack guardians
+"forceCombat" : true,
+
 // If set to true, right-clicking previously visited object would show preview of its content. For example, Witch Hut will show icon with provided skill
 "showScoutedPreview" : true,
 
@@ -152,12 +155,13 @@ Rewardable object is defined similarly to other objects, with key difference bei
 }
 
 // determines who can revisit object before reset
-// "once",     - object can only be visited once. First visitor takes it all.
-// "hero",     - object can be visited if this hero has not visited it before
-// "limiter",  - object can be visited if hero fails to fulfill provided limiter
-// "player",   - object can be visited if this player has not visited it before
-// "bonus"     - object can be visited if hero no longer has bonus from this object (including any other object of the same type)
-// "unlimited" - no restriction on revisiting.
+// "once",         - object can only be visited once. First visitor takes it all.
+// "hero",         - object can be visited if this hero has not visited it before
+// "limiter",      - object can be visited if hero fails to fulfill provided limiter
+// "player",       - object can be visited if this player has not visited it before
+// "playerGlobal", - object can be visited if this player has not visited this object or any other object of this type before
+// "bonus"         - object can be visited if hero no longer has bonus from this object (including any other object of the same type)
+// "unlimited"     - no restriction on revisiting.
 "visitMode" : "unlimited", 
 
 //determines way to select granted rewards if multiple options are available
@@ -303,7 +307,7 @@ Keep in mind, that all randomization is performed on map load and on object rese
 ],
 ```
 
-### Experience
+### Hero Experience
 
 - Can be used as limiter
 - Can be used as reward to grant experience to hero
@@ -364,6 +368,15 @@ Keep in mind, that all randomization is performed on map load and on object rese
 "movePercentage": 50,
 ```
 
+### Commander
+
+- Can be used as limiter, hero must have alive commander
+- If hero does not have commander (for example, in games without them), limiter will always fail
+
+```json
+"commanderAlive" : true
+```
+
 ### Primary Skills
 
 - Can be used as limiter, hero must have primary skill at least at specified level
@@ -457,6 +470,49 @@ Keep in mind, that all randomization is performed on map load and on object rese
 ]
 ```
 
+### Artifact Slots
+
+- Can be used as limiter, hero must have listed slots empty to pass the limiter
+- Slots occupied by components of combined artifacts are considered to be full
+
+```json
+"artifactSlots": [
+    "LEFT_HAND",
+    "RIGHT_HAND"
+],
+```
+
+- Can be used as a reward, to remove artifact that is present in slot
+- Components of a combined artifact can not be removed in this way
+
+```json
+"takenArtifactSlots" : [
+	"LEFT_HAND"
+],
+```
+
+List of supported slots names:
+
+- `HEAD` - helmet slot
+- `SHOULDERS` - slot used by capes
+- `NECK` - slot used by neclaces artifacts
+- `RIGHT_HAND` - slot for weapons / swords
+- `LEFT_HAND` - slot for shields
+- `TORSO` - chest slot for armors
+- `RIGHT_RING` - ring slot located next to weapon slot
+- `LEFT_RING` - ring slot located next to shield slot
+- `FEET` - slot for boots
+- `MISC1` - top-most miscellaneous slot
+- `MISC2` - 2nd from top miscellaneous slot
+- `MISC3` - 3rd from top miscellaneous slot
+- `MISC4` - bottom-right miscellaneous slot
+- `MISC5` - bottom-left miscellaneous slot
+- `SPELLBOOK` - Hero's spellbook. Its removal is untested and may lead to unintended side effects
+- `MACH1` - Ballista, or alternative war machine from mod
+- `MACH2` - Ammo Cart, or alternative war machine from mod
+- `MACH3` - First Aid tent, or alternative war machine from mod
+- `MACH4` - Catapult slot. Its removal is untested and may lead to unintended side effects
+
 ### Artifacts
 
 - Can be used as limiter, hero must have artifact either equipped or in backpack
@@ -483,6 +539,42 @@ Keep in mind, that all randomization is performed on map load and on object rese
 ],
 ```
 
+### Taking Artifacts
+
+- Can be used as reward, to take artifact from hero
+- Taking part of a combined artifact would disassemble it
+- Artifacts can be taken from either equipment or from backpack slots
+- Format is identical to [Artifacts](#artifacts)
+
+```json
+"takenArtifacts": [
+    "ribCage"
+],
+```
+
+### Scrolls
+
+- Can be used as limiter, hero must have scroll either equipped or in backpack
+- Can be used as reward, to give new scroll to a hero
+- Format is identical to [Spells](#spells)
+
+```json
+"scrolls": [
+    "magicArrow"
+],
+```
+
+### Taking Scrolls
+
+- Can be used as reward, to take scroll from hero
+- Format is identical to [Spells](#spells)
+
+```json
+"takenScrolls": [
+    "magicArrow"
+],
+```
+
 ### Spells
 
 - Can be used as limiter
@@ -538,6 +630,50 @@ Keep in mind, that all randomization is performed on map load and on object rese
 ],
 ```
 
+- Additionally, it is possible to check whether tested troops can be removed, in other words - whether hero has any creatures other than those specified in `creatures` field using `hasExtraCreatures` key
+- Following check will pass only if hero either has more archers than 20, or has exactly 20 archers and any creatures other than that
+
+```json
+"creatures" : [
+    {
+        "type" : "archer",
+        "amount" : 20,
+    }
+],
+"hasExtraCreatures" : true
+```
+
+### Taking Creatures
+
+- Can be used as reward, to take creatures from a hero
+- Attempting to take more creatures than hero has, or creatures that hero does not have is legal
+- It is not possible to take entire army, however it is possible to replace entire army by providing new creatures.
+- Format is identical to [Creatures](#creatures)
+
+```json
+"takenCreatures" : [
+    {
+        "type" : "archer",
+        "amount" : 20,
+    }
+],
+```
+
+### Creature receiving test
+
+- Can be used as limiter, to test whether hero can accept specified creatures without abandoning any units
+- Check will pass if hero has enough free slots to accept creatures, or if hero already has specified creatures, or if hero has units that can be merged to create space for new troops
+- Note that attempting to give more troops than hero can accept is legal, and will show unit selection dialog
+
+```json
+"canReceiveCreatures" : [
+    {
+        "type" : "archer",
+        "amount" : 20,
+    }
+],
+```
+
 ### Guards
 
 - When used in a reward, these creatures will be added to guards of the objects

lib/CCreatureSet.cpp

@@ -612,6 +612,38 @@ bool CCreatureSet::canBeMergedWith(const CCreatureSet &cs, bool allowMergingStac
 	}
 }
 
+bool CCreatureSet::hasUnits(const std::vector<CStackBasicDescriptor> & units, bool requireLastStack) const
+{
+	bool foundExtraCreatures = false;
+	int testedSlots = 0;
+	for(const auto & reqStack : units)
+	{
+		size_t count = 0;
+		for(const auto & slot : Slots())
+		{
+			const auto & heroStack = slot.second;
+			if (heroStack->getType() == reqStack.getType())
+			{
+				count += heroStack->getCount();
+				testedSlots += 1;
+			}
+		}
+		if (count > reqStack.getCount())
+			foundExtraCreatures = true;
+
+		if (count < reqStack.getCount()) //not enough creatures of this kind
+			return false;
+	}
+
+	if (requireLastStack)
+	{
+		if (!foundExtraCreatures && testedSlots >= Slots().size())
+			return false;
+	}
+
+	return true;
+}
+
 bool CCreatureSet::hasStackAtSlot(const SlotID & slot) const
 {
 	return vstd::contains(stacks, slot);

lib/CCreatureSet.h

@@ -333,6 +333,11 @@ public:
 	bool contains(const CStackInstance *stack) const;
 	bool canBeMergedWith(const CCreatureSet &cs, bool allowMergingStacks = true) const;
 
+	/// Returns true if this creature set contains all listed units
+	/// If requireLastStack is true, then this function will also
+	/// require presence of any unit other than requested (or more units than requested)
+	bool hasUnits(const std::vector<CStackBasicDescriptor> & units, bool requireLastStack = true) const;
+
 	template <typename Handler> void serialize(Handler &h)
 	{
 		h & stacks;

lib/GameSettings.cpp

@@ -90,6 +90,7 @@ const std::vector<GameSettings::SettingOption> GameSettings::settingProperties =
 		{EGameSettings::MAP_FORMAT_JSON_VCMI,                             "mapFormat", "jsonVCMI"                             },
 		{EGameSettings::MAP_FORMAT_RESTORATION_OF_ERATHIA,                "mapFormat", "restorationOfErathia"                 },
 		{EGameSettings::MAP_FORMAT_SHADOW_OF_DEATH,                       "mapFormat", "shadowOfDeath"                        },
+		{EGameSettings::MAP_OBJECTS_H3_BUG_QUEST_TAKES_ENTIRE_ARMY,       "mapObjects","h3BugQuestTakesEntireArmy"            },
 		{EGameSettings::MARKETS_BLACK_MARKET_RESTOCK_PERIOD,              "markets",   "blackMarketRestockPeriod"             },
 		{EGameSettings::MODULE_COMMANDERS,                                "modules",   "commanders"                           },
 		{EGameSettings::MODULE_STACK_ARTIFACT,                            "modules",   "stackArtifact"                        },

lib/IGameCallback.h

@@ -103,7 +103,7 @@ public:
 	virtual void giveResources(PlayerColor player, TResources resources)=0;
 
 	virtual void giveCreatures(const CArmedInstance *objid, const CGHeroInstance * h, const CCreatureSet &creatures, bool remove) =0;
-	virtual void takeCreatures(ObjectInstanceID objid, const std::vector<CStackBasicDescriptor> &creatures) =0;
+	virtual void takeCreatures(ObjectInstanceID objid, const std::vector<CStackBasicDescriptor> &creatures, bool forceRemoval = false) =0;
 	virtual bool changeStackCount(const StackLocation &sl, TQuantity count, bool absoluteValue = false) =0;
 	virtual bool changeStackType(const StackLocation &sl, const CCreature *c) =0;
 	virtual bool insertNewStack(const StackLocation &sl, const CCreature *c, TQuantity count = -1) =0; //count -1 => moves whole stack

lib/IGameSettings.h

@@ -63,6 +63,7 @@ enum class EGameSettings
 	MAP_FORMAT_JSON_VCMI,
 	MAP_FORMAT_RESTORATION_OF_ERATHIA,
 	MAP_FORMAT_SHADOW_OF_DEATH,
+	MAP_OBJECTS_H3_BUG_QUEST_TAKES_ENTIRE_ARMY,
 	MARKETS_BLACK_MARKET_RESTOCK_PERIOD,
 	MODULE_COMMANDERS,
 	MODULE_STACK_ARTIFACT,

lib/battle/BattleInfo.cpp

@@ -602,7 +602,7 @@ EGateState BattleInfo::getGateState() const
 	return si.gateState;
 }
 
-uint32_t BattleInfo::getCastSpells(BattleSide side) const
+int32_t BattleInfo::getCastSpells(BattleSide side) const
 {
 	return getSide(side).castSpellsCount;
 }

lib/battle/BattleInfo.h

@@ -110,7 +110,7 @@ public:
 	EWallState getWallState(EWallPart partOfWall) const override;
 	EGateState getGateState() const override;
 
-	uint32_t getCastSpells(BattleSide side) const override;
+	int32_t getCastSpells(BattleSide side) const override;
 	int32_t getEnchanterCounter(BattleSide side) const override;
 
 	const IBonusBearer * getBonusBearer() const override;

lib/battle/BattleProxy.cpp

@@ -105,7 +105,7 @@ EGateState BattleProxy::getGateState() const
 	return subject->battleGetGateState();
 }
 
-uint32_t BattleProxy::getCastSpells(BattleSide side) const
+int32_t BattleProxy::getCastSpells(BattleSide side) const
 {
 	return subject->battleCastSpells(side);
 }

lib/battle/BattleProxy.h

@@ -49,7 +49,7 @@ public:
 	EWallState getWallState(EWallPart partOfWall) const override;
 	EGateState getGateState() const override;
 
-	uint32_t getCastSpells(BattleSide side) const override;
+	int32_t getCastSpells(BattleSide side) const override;
 	int32_t getEnchanterCounter(BattleSide side) const override;
 
 	const IBonusBearer * getBonusBearer() const override;

lib/battle/CBattleInfoCallback.cpp

@@ -114,17 +114,16 @@ ESpellCastProblem CBattleInfoCallback::battleCanCastSpell(const spells::Caster *
 	{
 	case spells::Mode::HERO:
 	{
-		if(battleCastSpells(side) > 0)
-			return ESpellCastProblem::CASTS_PER_TURN_LIMIT;
-
-		const auto * hero = dynamic_cast<const CGHeroInstance *>(caster);
+		const auto * hero = caster->getHeroCaster();
 
 		if(!hero)
 			return ESpellCastProblem::NO_HERO_TO_CAST_SPELL;
-		if(hero->hasBonusOfType(BonusType::BLOCK_ALL_MAGIC))
-			return ESpellCastProblem::MAGIC_IS_BLOCKED;
 		if(!hero->hasSpellbook())
 			return ESpellCastProblem::NO_SPELLBOOK;
+		if(hero->hasBonusOfType(BonusType::BLOCK_ALL_MAGIC))
+			return ESpellCastProblem::MAGIC_IS_BLOCKED;
+		if(battleCastSpells(side) >= hero->valOfBonuses(BonusType::HERO_SPELL_CASTS_PER_COMBAT_TURN))
+			return ESpellCastProblem::CASTS_PER_TURN_LIMIT;
 	}
 		break;
 	default:

lib/battle/CBattleInfoEssentials.cpp

@@ -256,7 +256,7 @@ InfoAboutHero CBattleInfoEssentials::battleGetHeroInfo(BattleSide side) const
 	return InfoAboutHero(hero, infoLevel);
 }
 
-uint32_t CBattleInfoEssentials::battleCastSpells(BattleSide side) const
+int32_t CBattleInfoEssentials::battleCastSpells(BattleSide side) const
 {
 	RETURN_IF_NOT_BATTLE(-1);
 	return getBattle()->getCastSpells(side);

lib/battle/CBattleInfoEssentials.h

@@ -78,7 +78,7 @@ public:
 	bool playerHasAccessToHeroInfo(const PlayerColor & player, const CGHeroInstance * h) const;
 	TownFortifications battleGetFortifications() const;
 	bool battleHasHero(BattleSide side) const;
-	uint32_t battleCastSpells(BattleSide side) const; //how many spells has given side cast
+	int32_t battleCastSpells(BattleSide side) const; //how many spells has given side cast
 	const CGHeroInstance * battleGetFightingHero(BattleSide side) const; //deprecated for players callback, easy to get wrong
 	const CArmedInstance * battleGetArmyObject(BattleSide side) const;
 	InfoAboutHero battleGetHeroInfo(BattleSide side) const;

lib/battle/IBattleState.h

@@ -63,7 +63,7 @@ public:
 	/// Returns list of all spells used by specified side (and that can be learned by opposite hero)
 	virtual std::vector<SpellID> getUsedSpells(BattleSide side) const = 0;
 
-	virtual uint32_t getCastSpells(BattleSide side) const = 0;
+	virtual int32_t getCastSpells(BattleSide side) const = 0;
 	virtual int32_t getEnchanterCounter(BattleSide side) const = 0;
 
 	virtual ui8 getTacticDist() const = 0;

lib/bonuses/BonusEnum.h

@@ -183,6 +183,7 @@ class JsonNode;
 	BONUS_NAME(MECHANICAL) /*eg. factory creatures, cannot be rised or healed, only neutral morale, repairable by engineer */ \
 	BONUS_NAME(PRISM_HEX_ATTACK_BREATH) /*eg. dragons*/	\
 	BONUS_NAME(BASE_TILE_MOVEMENT_COST) /*minimal cost for moving offroad*/	\
+	BONUS_NAME(HERO_SPELL_CASTS_PER_COMBAT_TURN) /**/	\
 	/* end of list */
 
 

lib/constants/EntityIdentifiers.h

@@ -756,6 +756,7 @@ public:
 
 	DLL_LINKAGE static si32 decode(const std::string & identifier);
 	DLL_LINKAGE static std::string encode(const si32 index);
+	DLL_LINKAGE static std::string entityType();
 };
 
 class ArtifactPosition : public StaticIdentifierWithEnum<ArtifactPosition, ArtifactPositionBase>

lib/entities/artifact/CArtHandler.cpp

@@ -235,6 +235,20 @@ int32_t ArtifactPositionBase::decode(const std::string & slotName)
 		return PRE_FIRST;
 }
 
+std::string ArtifactPositionBase::encode(int32_t index)
+{
+#define ART_POS(x) #x ,
+	const std::vector<std::string> artSlots = { ART_POS_LIST };
+#undef ART_POS
+
+	return artSlots.at(index);
+}
+
+std::string ArtifactPositionBase::entityType()
+{
+	return "artifactSlot";
+}
+
 void CArtHandler::addSlot(CArtifact * art, const std::string & slotID) const
 {
 	static const std::vector<ArtifactPosition> miscSlots =

lib/entities/artifact/CArtifactInstance.cpp

@@ -19,22 +19,22 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-CCombinedArtifactInstance::PartInfo::PartInfo(IGameCallback * cb)
-	:GameCallbackHolder(cb)
-{}
-
 CCombinedArtifactInstance::PartInfo::PartInfo(const CArtifactInstance * artifact, ArtifactPosition slot)
-	: GameCallbackHolder(artifact->cb)
-	, artifactID(artifact->getId())
+	: artifactID(artifact->getId())
+	, artifactPtr(artifact)
 	, slot(slot)
 {
 }
 
 const CArtifactInstance * CCombinedArtifactInstance::PartInfo::getArtifact() const
 {
-	if (artifactID.hasValue())
-		return cb->getArtInstance(artifactID);
-	return nullptr;
+	assert(artifactPtr != nullptr || !artifactID.hasValue());
+	return artifactPtr;
+}
+
+ArtifactInstanceID CCombinedArtifactInstance::PartInfo::getArtifactID() const
+{
+	return artifactID;
 }
 
 void CCombinedArtifactInstance::addPart(const CArtifactInstance * art, const ArtifactPosition & slot)
@@ -57,7 +57,7 @@ bool CCombinedArtifactInstance::isPart(const CArtifactInstance * supposedPart) c
 
 	for(const PartInfo & constituent : partsInfo)
 	{
-		if(constituent.artifactID == supposedPart->getId())
+		if(constituent.getArtifactID() == supposedPart->getId())
 			return true;
 	}
 
@@ -186,7 +186,10 @@ bool CArtifactInstance::isScroll() const
 void CArtifactInstance::attachToBonusSystem(CGameState & gs)
 {
 	for(PartInfo & part : partsInfo)
-		attachToSource(*gs.getArtInstance(part.artifactID));
+	{
+		part = PartInfo(gs.getArtInstance(part.getArtifactID()), part.slot);
+		attachToSource(*gs.getArtInstance(part.getArtifactID()));
+	}
 }
 
 void CArtifactInstance::saveCompatibilityFixArtifactID(std::shared_ptr<CArtifactInstance> self)

lib/entities/artifact/CArtifactInstance.h

@@ -22,18 +22,24 @@ class DLL_LINKAGE CCombinedArtifactInstance : public GameCallbackHolder
 {
 protected:
 	using GameCallbackHolder::GameCallbackHolder;
+
 public:
-	using ArtPlacementMap = std::map<const CArtifactInstance*, ArtifactPosition>;
+	using ArtPlacementMap = std::map<const CArtifactInstance *, ArtifactPosition>;
 
-	struct PartInfo : public GameCallbackHolder
+	struct PartInfo
 	{
-		explicit PartInfo(IGameCallback * cb);
+	private:
+		const CArtifactInstance * artifactPtr = nullptr;
+		ArtifactInstanceID artifactID;
+	public:
+
+		PartInfo() = default;
 		PartInfo(const CArtifactInstance * artifact, ArtifactPosition slot);
 
-		ArtifactInstanceID artifactID;
 		ArtifactPosition slot;
 
 		const CArtifactInstance * getArtifact() const;
+		ArtifactInstanceID getArtifactID() const;
 
 		template <typename Handler>
 		void serialize(Handler & h);

lib/entities/artifact/CArtifactSet.cpp

@@ -62,6 +62,32 @@ ArtifactPosition CArtifactSet::getArtPos(const ArtifactID & aid, bool onlyWorn,
 	return ArtifactPosition::PRE_FIRST;
 }
 
+bool CArtifactSet::hasScroll(const SpellID & spellID, bool onlyWorn) const
+{
+	return getScrollPos(spellID, onlyWorn) != ArtifactPosition::PRE_FIRST;
+}
+
+ArtifactPosition CArtifactSet::getScrollPos(const SpellID & spellID, bool onlyWorn) const
+{
+	for (const auto & slot : artifactsWorn)
+		if (slot.second.getArt() && slot.second.getArt()->isScroll() && slot.second.getArt()->getScrollSpellID() == spellID)
+			return slot.first;
+
+	if (!onlyWorn)
+	{
+		size_t backpackPositionIdx = ArtifactPosition::BACKPACK_START;
+
+		for (const auto & slot : artifactsInBackpack)
+		{
+			if (slot.getArt() && slot.getArt()->isScroll() && slot.getArt()->getScrollSpellID() == spellID)
+				return ArtifactPosition(backpackPositionIdx);
+			backpackPositionIdx++;
+		}
+	}
+
+	return ArtifactPosition::PRE_FIRST;
+}
+
 const CArtifactInstance * CArtifactSet::getArtByInstanceId(const ArtifactInstanceID & artInstId) const
 {
 	for(const auto & i : artifactsWorn)
@@ -255,7 +281,7 @@ bool CArtifactSet::isPositionFree(const ArtifactPosition & pos, bool onlyLockChe
 		return artifactsInBackpack.size() < GameConstants::ALTAR_ARTIFACTS_SLOTS;
 
 	if(const ArtSlotInfo *s = getSlot(pos))
-		return (onlyLockCheck || !s->getArt()) && !s->locked;
+		return (onlyLockCheck || !s->getID().hasValue()) && !s->locked;
 
 	return true; //no slot means not used
 }

lib/entities/artifact/CArtifactSet.h

@@ -33,9 +33,11 @@ public:
 	ArtifactInstanceID getArtID(const ArtifactPosition & pos, bool excludeLocked = true) const;
 	/// Looks for first artifact with given ID
 	ArtifactPosition getArtPos(const ArtifactID & aid, bool onlyWorn = true, bool allowLocked = true) const;
+	ArtifactPosition getScrollPos(const SpellID & aid, bool onlyWorn = true) const;
 	ArtifactPosition getArtPos(const CArtifactInstance * art) const;
 	const CArtifactInstance * getArtByInstanceId(const ArtifactInstanceID & artInstId) const;
 	bool hasArt(const ArtifactID & aid, bool onlyWorn = false, bool searchCombinedParts = false) const;
+	bool hasScroll(const SpellID & aid, bool onlyWorn = false) const;
 	bool isPositionFree(const ArtifactPosition & pos, bool onlyLockCheck = false) const;
 
 	virtual IGameCallback * getCallback() const = 0;

lib/gameState/CGameState.cpp

@@ -1569,9 +1569,6 @@ void CGameState::buildBonusSystemTree()
 	buildGlobalTeamPlayerTree();
 	for(auto & armed : map->getObjects<CArmedInstance>())
 		armed->attachToBonusSystem(*this);
-
-	for(auto & art : map->getArtifacts())
-		art->attachToBonusSystem(*this);
 }
 
 void CGameState::restoreBonusSystemTree()

lib/json/JsonRandom.cpp

@@ -113,6 +113,12 @@ JsonRandomizationException::JsonRandomizationException(const std::string & messa
 			return loadVariable(IdentifierType::entityType(), value.String(), variables, IdentifierType::NONE);
 	}
 
+	template<>
+	ArtifactPosition JsonRandom::decodeKey(const JsonNode & value, const Variables & variables)
+	{
+		return ArtifactPosition::decode(value.String());
+	}
+
 	template<>
 	PlayerColor JsonRandom::decodeKey(const JsonNode & value, const Variables & variables)
 	{
@@ -420,6 +426,21 @@ JsonRandomizationException::JsonRandomizationException(const std::string & messa
 		return ret;
 	}
 
+	std::vector<ArtifactPosition> JsonRandom::loadArtifactSlots(const JsonNode & value, vstd::RNG & rng, const Variables & variables)
+	{
+		std::set<ArtifactPosition> allowedSlots;
+		for(ArtifactPosition pos(0); pos < ArtifactPosition::BACKPACK_START; ++pos)
+			allowedSlots.insert(pos);
+
+		std::vector<ArtifactPosition> ret;
+		for (const JsonNode & entry : value.Vector())
+		{
+			std::set<ArtifactPosition> potentialPicks = filterKeys(entry, allowedSlots, variables);
+			ret.push_back(*RandomGeneratorUtil::nextItem(potentialPicks, rng));
+		}
+		return ret;
+	}
+
 	SpellID JsonRandom::loadSpell(const JsonNode & value, vstd::RNG & rng, const Variables & variables)
 	{
 		std::set<SpellID> defaultSpells;

lib/json/JsonRandom.h

@@ -75,6 +75,7 @@ public:
 
 	ArtifactID loadArtifact(const JsonNode & value, vstd::RNG & rng, const Variables & variables);
 	std::vector<ArtifactID> loadArtifacts(const JsonNode & value, vstd::RNG & rng, const Variables & variables);
+	std::vector<ArtifactPosition> loadArtifactSlots(const JsonNode & value, vstd::RNG & rng, const Variables & variables);
 
 	SpellID loadSpell(const JsonNode & value, vstd::RNG & rng, const Variables & variables);
 	std::vector<SpellID> loadSpells(const JsonNode & value, vstd::RNG & rng, const Variables & variables);

lib/mapObjectConstructors/CRewardableConstructor.cpp

@@ -47,6 +47,15 @@ std::shared_ptr<CGObjectInstance> CRewardableConstructor::create(IGameCallback *
 	return ret;
 }
 
+void CRewardableConstructor::assignBonuses(std::vector<Bonus> & bonuses, MapObjectID objectID) const
+{
+	for (auto & bonus : bonuses)
+	{
+		bonus.source = BonusSource::OBJECT_TYPE;
+		bonus.sid = BonusSourceID(objectID);
+	}
+}
+
 Rewardable::Configuration CRewardableConstructor::generateConfiguration(IGameCallback * cb, vstd::RNG & rand, MapObjectID objectID, const std::map<std::string, JsonNode> & presetVariables) const
 {
 	Rewardable::Configuration result;
@@ -62,11 +71,9 @@ Rewardable::Configuration CRewardableConstructor::generateConfiguration(IGameCal
 
 	for(auto & rewardInfo : result.info)
 	{
-		for (auto & bonus : rewardInfo.reward.bonuses)
-		{
-			bonus.source = BonusSource::OBJECT_TYPE;
-			bonus.sid = BonusSourceID(objectID);
-		}
+		assignBonuses(rewardInfo.reward.heroBonuses, objectID);
+		assignBonuses(rewardInfo.reward.commanderBonuses, objectID);
+		assignBonuses(rewardInfo.reward.playerBonuses, objectID);
 	}
 
 	return result;

lib/mapObjectConstructors/CRewardableConstructor.h

@@ -14,10 +14,13 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
+struct Bonus;
+
 class DLL_LINKAGE CRewardableConstructor : public AObjectTypeHandler
 {
 	Rewardable::Info objectInfo;
 
+	void assignBonuses(std::vector<Bonus> & bonuses, MapObjectID objectID) const;
 	void initTypeData(const JsonNode & config) override;
 	
 	bool blockVisit = false;

lib/mapObjects/CGHeroInstance.cpp

@@ -1337,7 +1337,8 @@ void CGHeroInstance::restoreBonusSystem(CGameState & gs)
 {
 	CArmedInstance::restoreBonusSystem(gs);
 	artDeserializationFix(gs, this);
-	this->commander->artDeserializationFix(gs, this->commander.get());
+	if (commander)
+		commander->artDeserializationFix(gs, this->commander.get());
 	if (boardedBoat.hasValue())
 	{
 		auto boat = gs.getObjInstance(boardedBoat);
@@ -1613,7 +1614,8 @@ void CGHeroInstance::levelUp(const std::vector<SecondarySkill> & skills)
 
 void CGHeroInstance::attachCommanderToArmy()
 {
-	commander->setArmy(this);
+	if (commander)
+		commander->setArmy(this);
 }
 
 void CGHeroInstance::levelUpAutomatically(vstd::RNG & rand)

lib/mapObjects/CGPandoraBox.cpp

@@ -83,7 +83,7 @@ void CGPandoraBox::grantRewardWithMessage(const CGHeroInstance * h, int index, b
 	temp.heroLevel = vi.reward.heroLevel;
 	temp.primary = vi.reward.primary;
 	temp.secondary = vi.reward.secondary;
-	temp.bonuses = vi.reward.bonuses;
+	temp.heroBonuses = vi.reward.heroBonuses;
 	temp.manaDiff = vi.reward.manaDiff;
 	temp.manaPercentage = vi.reward.manaPercentage;
 	
@@ -106,7 +106,7 @@ void CGPandoraBox::grantRewardWithMessage(const CGHeroInstance * h, int index, b
 	if(vi.reward.manaDiff || vi.reward.manaPercentage >= 0)
 		txt = setText(temp.manaDiff > 0, 177, 176, h);
 	
-	for(auto b : vi.reward.bonuses)
+	for(auto b : vi.reward.heroBonuses)
 	{
 		if(b.val && b.type == BonusType::MORALE)
 			txt = setText(b.val > 0, 179, 178, h);
@@ -122,7 +122,7 @@ void CGPandoraBox::grantRewardWithMessage(const CGHeroInstance * h, int index, b
 	
 	//artifacts message
 	temp = Rewardable::Reward{};
-	temp.artifacts = vi.reward.artifacts;
+	temp.grantedArtifacts = vi.reward.grantedArtifacts;
 	sendInfoWindow(setText(true, 183, 183, h), temp);
 	
 	//creatures message
@@ -160,8 +160,8 @@ void CGPandoraBox::grantRewardWithMessage(const CGHeroInstance * h, int index, b
 	temp.manaPercentage = -1;
 	temp.spells.clear();
 	temp.creatures.clear();
-	temp.bonuses.clear();
-	temp.artifacts.clear();
+	temp.heroBonuses.clear();
+	temp.grantedArtifacts.clear();
 	sendInfoWindow(setText(true, 175, 175, h), temp);
 	
 	// grant reward afterwards. Note that it may remove object
@@ -229,11 +229,11 @@ void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 		int val = 0;
 		handler.serializeInt("morale", val, 0);
 		if(val)
-			vinfo.reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
+			vinfo.reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
 		
 		handler.serializeInt("luck", val, 0);
 		if(val)
-			vinfo.reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
+			vinfo.reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
 		
 		vinfo.reward.resources.serializeJson(handler, "resources");
 		{
@@ -246,7 +246,7 @@ void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 			}
 		}
 		
-		handler.serializeIdArray("artifacts", vinfo.reward.artifacts);
+		handler.serializeIdArray("artifacts", vinfo.reward.grantedArtifacts);
 		handler.serializeIdArray("spells", vinfo.reward.spells);
 		handler.enterArray("creatures").serializeStruct(vinfo.reward.creatures);
 		
@@ -279,8 +279,8 @@ void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 		|| vinfo.reward.heroExperience
 		|| vinfo.reward.manaDiff
 		|| vinfo.reward.resources.nonZero()
-		|| !vinfo.reward.artifacts.empty()
-		|| !vinfo.reward.bonuses.empty()
+		|| !vinfo.reward.grantedArtifacts.empty()
+		|| !vinfo.reward.heroBonuses.empty()
 		|| !vinfo.reward.creatures.empty()
 		|| !vinfo.reward.secondary.empty();
 		

lib/mapObjects/CQuest.cpp

@@ -17,6 +17,7 @@
 #include "../texts/CGeneralTextHandler.h"
 #include "CGCreature.h"
 #include "../IGameCallback.h"
+#include "../IGameSettings.h"
 #include "../entities/artifact/CArtifact.h"
 #include "../entities/hero/CHeroHandler.h"
 #include "../mapObjectConstructors/CObjectClassesHandler.h"
@@ -101,29 +102,7 @@ const std::string & CQuest::missionState(int state)
 
 bool CQuest::checkMissionArmy(const CQuest * q, const CCreatureSet * army)
 {
-	std::vector<CStackBasicDescriptor>::const_iterator cre;
-	TSlots::const_iterator it;
-	ui32 count = 0;
-	ui32 slotsCount = 0;
-	bool hasExtraCreatures = false;
-	for(cre = q->mission.creatures.begin(); cre != q->mission.creatures.end(); ++cre)
-	{
-		for(count = 0, it = army->Slots().begin(); it != army->Slots().end(); ++it)
-		{
-			if(it->second->getType() == cre->getType())
-			{
-				count += it->second->getCount();
-				slotsCount++;
-			}
-		}
-
-		if(static_cast<TQuantity>(count) < cre->getCount()) //not enough creatures of this kind
-			return false;
-
-		hasExtraCreatures |= static_cast<TQuantity>(count) > cre->getCount();
-	}
-
-	return hasExtraCreatures || slotsCount < army->Slots().size();
+	return army->hasUnits(q->mission.creatures, true);
 }
 
 bool CQuest::checkQuest(const CGHeroInstance * h) const
@@ -140,7 +119,7 @@ bool CQuest::checkQuest(const CGHeroInstance * h) const
 	return true;
 }
 
-void CQuest::completeQuest(IGameCallback * cb, const CGHeroInstance *h) const
+void CQuest::completeQuest(IGameCallback * cb, const CGHeroInstance *h, bool allowFullArmyRemoval) const
 {
 	// FIXME: this should be part of 'reward', and not hacking into limiter state that should only limit access to such reward
 
@@ -174,7 +153,7 @@ void CQuest::completeQuest(IGameCallback * cb, const CGHeroInstance *h) const
 		logGlobal->error("Failed to find artifact %s in inventory of hero %s", elem.toEntity(LIBRARY)->getJsonKey(), h->getHeroTypeID());
 	}
 
-	cb->takeCreatures(h->id, mission.creatures);
+	cb->takeCreatures(h->id, mission.creatures, allowFullArmyRemoval);
 	cb->giveResources(h->getOwner(), -mission.resources);
 }
 
@@ -457,7 +436,8 @@ void CGSeerHut::init(vstd::RNG & rand)
 	seerName = LIBRARY->generaltexth->translate(seerNameID);
 	getQuest().textOption = rand.nextInt(2);
 	getQuest().completedOption = rand.nextInt(1, 3);
-	
+	getQuest().mission.hasExtraCreatures = !allowsFullArmyRemoval();
+
 	configuration.canRefuse = true;
 	configuration.visitMode = Rewardable::EVisitMode::VISIT_ONCE;
 	configuration.selectMode = Rewardable::ESelectMode::SELECT_PLAYER;
@@ -667,14 +647,21 @@ const CGCreature * CGSeerHut::getCreatureToKill(bool allowNull) const
 	return dynamic_cast<const CGCreature *>(o);
 }
 
+bool CGSeerHut::allowsFullArmyRemoval() const
+{
+	bool seerGivesUnits = !configuration.info.empty() && !configuration.info.back().reward.creatures.empty();
+	bool h3BugSettingEnabled = cb->getSettings().getBoolean(EGameSettings::MAP_OBJECTS_H3_BUG_QUEST_TAKES_ENTIRE_ARMY);
+	return seerGivesUnits || h3BugSettingEnabled;
+}
+
 void CGSeerHut::blockingDialogAnswered(const CGHeroInstance *hero, int32_t answer) const
 {
-	CRewardableObject::blockingDialogAnswered(hero, answer);
 	if(answer)
 	{
-		getQuest().completeQuest(cb, hero);
+		getQuest().completeQuest(cb, hero, allowsFullArmyRemoval());
 		cb->setObjPropertyValue(id, ObjProperty::SEERHUT_COMPLETE, !getQuest().repeatedQuest); //mission complete
 	}
+	CRewardableObject::blockingDialogAnswered(hero, answer);
 }
 
 void CGSeerHut::serializeJsonOptions(JsonSerializeFormat & handler)
@@ -714,9 +701,9 @@ void CGSeerHut::serializeJsonOptions(JsonSerializeFormat & handler)
 		if(metaTypeName == "mana")
 			reward.manaDiff = val;
 		if(metaTypeName == "morale")
-			reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
+			reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
 		if(metaTypeName == "luck")
-			reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
+			reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
 		if(metaTypeName == "resource")
 		{
 			auto rawId = *LIBRARY->identifiers()->getIdentifier(ModScope::scopeMap(), fullIdentifier, false);
@@ -735,7 +722,7 @@ void CGSeerHut::serializeJsonOptions(JsonSerializeFormat & handler)
 		if(metaTypeName == "artifact")
 		{
 			auto rawId = *LIBRARY->identifiers()->getIdentifier(ModScope::scopeMap(), fullIdentifier, false);
-			reward.artifacts.push_back(rawId);
+			reward.grantedArtifacts.push_back(rawId);
 		}
 		if(metaTypeName == "spell")
 		{
@@ -758,6 +745,7 @@ void CGQuestGuard::init(vstd::RNG & rand)
 	blockVisit = true;
 	getQuest().textOption = rand.nextInt(3, 5);
 	getQuest().completedOption = rand.nextInt(4, 5);
+	getQuest().mission.hasExtraCreatures = !allowsFullArmyRemoval();
 	
 	configuration.info.push_back({});
 	configuration.info.back().visitType = Rewardable::EEventType::EVENT_FIRST_VISIT;
@@ -815,7 +803,7 @@ void CGKeymasterTent::onHeroVisit( const CGHeroInstance * h ) const
 	if (!wasMyColorVisited (h->getOwner()) )
 	{
 		ChangeObjectVisitors cow;
-		cow.mode = ChangeObjectVisitors::VISITOR_GLOBAL;
+		cow.mode = ChangeObjectVisitors::VISITOR_ADD_PLAYER;
 		cow.hero = h->id;
 		cow.object = id;
 		cb->sendAndApply(cow);

lib/mapObjects/CQuest.h

@@ -80,7 +80,7 @@ public:
 	void getVisitText(const CGameInfoCallback * cb, MetaString &text, std::vector<Component> & components, bool FirstVisit, const CGHeroInstance * h = nullptr) const;
 	void getCompletionText(const CGameInfoCallback * cb, MetaString &text) const;
 	void getRolloverText (const CGameInfoCallback * cb, MetaString &text, bool onHover) const; //hover or quest log entry
-	void completeQuest(IGameCallback *, const CGHeroInstance * h) const;
+	void completeQuest(IGameCallback *, const CGHeroInstance * h, bool allowFullArmyRemoval) const;
 	void addTextReplacements(const CGameInfoCallback * cb, MetaString &out, std::vector<Component> & components) const;
 	void addKillTargetReplacements(MetaString &out) const;
 	void defineQuestName();
@@ -166,6 +166,7 @@ public:
 		h & seerName;
 	}
 protected:
+	bool allowsFullArmyRemoval() const;
 	void setPropertyDer(ObjProperty what, ObjPropertyID identifier) override;
 
 	void serializeJsonOptions(JsonSerializeFormat & handler) override;

lib/mapObjects/CRewardableObject.cpp

@@ -21,6 +21,7 @@
 #include "../mapObjects/CGHeroInstance.h"
 #include "../networkPacks/PacksForClient.h"
 #include "../networkPacks/PacksForClientBattle.h"
+#include "../networkPacks/StackLocation.h"
 #include "../serializer/JsonSerializeFormat.h"
 
 #include <vstd/RNG.h>
@@ -51,7 +52,15 @@ void CRewardableObject::onHeroVisit(const CGHeroInstance *hero) const
 		cb->sendAndApply(cov);
 	}
 
-	if (isGuarded())
+	if (!isGuarded())
+	{
+		doHeroVisit(hero);
+	}
+	else if (configuration.forceCombat)
+	{
+		doStartBattle(hero);
+	}
+	else
 	{
 		auto guardedIndexes = getAvailableRewards(hero, Rewardable::EEventType::EVENT_GUARDED);
 		auto guardedReward = configuration.info.at(guardedIndexes.at(0));
@@ -64,10 +73,6 @@ void CRewardableObject::onHeroVisit(const CGHeroInstance *hero) const
 
 		cb->showBlockingDialog(this, &bd);
 	}
-	else
-	{
-		doHeroVisit(hero);
-	}
 }
 
 void CRewardableObject::heroLevelUpDone(const CGHeroInstance *hero) const
@@ -83,15 +88,26 @@ void CRewardableObject::battleFinished(const CGHeroInstance *hero, const BattleR
 	}
 }
 
+void CRewardableObject::garrisonDialogClosed(const CGHeroInstance *hero) const
+{
+	// if visitor received creatures as rewards, but does not have free slots, he will leave some units
+	// inside rewardable object, which might get treated as guards later
+	while(!stacks.empty())
+		cb->eraseStack(StackLocation(id, stacks.begin()->first));
+}
+
+void CRewardableObject::doStartBattle(const CGHeroInstance * hero) const
+{
+	auto layout = BattleLayout::createLayout(cb, configuration.guardsLayout, hero, this);
+	cb->startBattle(hero, this, visitablePos(), hero, nullptr, layout, nullptr);
+}
+
 void CRewardableObject::blockingDialogAnswered(const CGHeroInstance * hero, int32_t answer) const
 {
 	if(isGuarded())
 	{
 		if (answer)
-		{
-			auto layout = BattleLayout::createLayout(cb, configuration.guardsLayout, hero, this);
-			cb->startBattle(hero, this, visitablePos(), hero, nullptr, layout, nullptr);
-		}
+			doStartBattle(hero);
 	}
 	else
 	{
@@ -128,7 +144,9 @@ bool CRewardableObject::wasVisitedBefore(const CGHeroInstance * contextHero) con
 		case Rewardable::VISIT_ONCE:
 			return onceVisitableObjectCleared;
 		case Rewardable::VISIT_PLAYER:
-			return vstd::contains(cb->getPlayerState(contextHero->getOwner())->visitedObjects, ObjectInstanceID(id));
+			return cb->getPlayerState(contextHero->getOwner())->visitedObjects.count(ObjectInstanceID(id)) != 0;
+		case Rewardable::VISIT_PLAYER_GLOBAL:
+			return cb->getPlayerState(contextHero->getOwner())->visitedObjectsGlobal.count({ID, subID}) != 0;
 		case Rewardable::VISIT_BONUS:
 			return contextHero->hasBonusFrom(BonusSource::OBJECT_TYPE, BonusSourceID(ID));
 		case Rewardable::VISIT_HERO:
@@ -151,7 +169,9 @@ bool CRewardableObject::wasVisited(PlayerColor player) const
 			return false;
 		case Rewardable::VISIT_ONCE:
 		case Rewardable::VISIT_PLAYER:
-			return vstd::contains(cb->getPlayerState(player)->visitedObjects, ObjectInstanceID(id));
+			return cb->getPlayerState(player)->visitedObjects.count(ObjectInstanceID(id)) != 0;
+		case Rewardable::VISIT_PLAYER_GLOBAL:
+			return cb->getPlayerState(player)->visitedObjectsGlobal.count({ID, subID}) != 0;
 		default:
 			return false;
 	}
@@ -196,7 +216,7 @@ std::string CRewardableObject::getDisplayTextImpl(PlayerColor player, const CGHe
 	}
 	else
 	{
-		if(configuration.visitMode == Rewardable::VISIT_PLAYER || configuration.visitMode == Rewardable::VISIT_ONCE)
+		if(configuration.visitMode == Rewardable::VISIT_PLAYER || configuration.visitMode == Rewardable::VISIT_ONCE || configuration.visitMode == Rewardable::VISIT_PLAYER_GLOBAL)
 		{
 			if (wasVisited(player))
 				result += "\n" + configuration.visitedTooltip.toString();

lib/mapObjects/CRewardableObject.h

@@ -25,6 +25,8 @@ protected:
 	/// reward selected by player, no serialize
 	ui16 selectedReward = 0;
 	
+	void doStartBattle(const CGHeroInstance * hero) const;
+
 	void grantReward(ui32 rewardID, const CGHeroInstance * hero) const override;
 	void markAsVisited(const CGHeroInstance * hero) const override;
 
@@ -56,6 +58,7 @@ public:
 	void onHeroVisit(const CGHeroInstance *h) const override;
 
 	void battleFinished(const CGHeroInstance *hero, const BattleResult &result) const override;
+	void garrisonDialogClosed(const CGHeroInstance *hero) const override;
 
 	///possibly resets object state
 	void newTurn(vstd::RNG & rand) const override;

lib/mapObjects/TownBuildingInstance.cpp

@@ -72,27 +72,38 @@ TownRewardableBuildingInstance::TownRewardableBuildingInstance(CGTownInstance *
 	configuration = generateConfiguration(rand);
 }
 
+void TownRewardableBuildingInstance::assignBonuses(std::vector<Bonus> & bonuses) const
+{
+	const auto & building = town->getTown()->buildings.at(getBuildingType());
+
+	for (auto & bonus : bonuses)
+	{
+		if (building->mapObjectLikeBonuses.hasValue())
+		{
+			bonus.source = BonusSource::OBJECT_TYPE;
+			bonus.sid = BonusSourceID(building->mapObjectLikeBonuses);
+		}
+		else
+		{
+			bonus.source = BonusSource::TOWN_STRUCTURE;
+			bonus.sid = BonusSourceID(building->getUniqueTypeID());
+		}
+	}
+}
+
 Rewardable::Configuration TownRewardableBuildingInstance::generateConfiguration(vstd::RNG & rand) const
 {
 	Rewardable::Configuration result;
 	const auto & building = town->getTown()->buildings.at(getBuildingType());
 
+	// force modal info window instead of displaying in inactive info box on adventure map
+	result.infoWindowType = EInfoWindowMode::MODAL;
 	building->rewardableObjectInfo.configureObject(result, rand, cb);
 	for(auto & rewardInfo : result.info)
 	{
-		for (auto & bonus : rewardInfo.reward.bonuses)
-		{
-			if (building->mapObjectLikeBonuses.hasValue())
-			{
-				bonus.source = BonusSource::OBJECT_TYPE;
-				bonus.sid = BonusSourceID(building->mapObjectLikeBonuses);
-			}
-			else
-			{
-				bonus.source = BonusSource::TOWN_STRUCTURE;
-				bonus.sid = BonusSourceID(building->getUniqueTypeID());
-			}
-		}
+		assignBonuses(rewardInfo.reward.heroBonuses);
+		assignBonuses(rewardInfo.reward.commanderBonuses);
+		assignBonuses(rewardInfo.reward.playerBonuses);
 	}
 	return result;
 }
@@ -162,6 +173,7 @@ bool TownRewardableBuildingInstance::wasVisitedBefore(const CGHeroInstance * con
 		case Rewardable::VISIT_ONCE:
 			return !visitors.empty();
 		case Rewardable::VISIT_PLAYER:
+		case Rewardable::VISIT_PLAYER_GLOBAL:
 			return false; //not supported
 		case Rewardable::VISIT_BONUS:
 		{
@@ -201,6 +213,7 @@ bool TownRewardableBuildingInstance::wasVisited(PlayerColor player) const
 		case Rewardable::VISIT_BONUS:
 		case Rewardable::VISIT_HERO:
 		case Rewardable::VISIT_LIMITER:
+		case Rewardable::VISIT_PLAYER_GLOBAL:
 			return false;
 		case Rewardable::VISIT_ONCE:
 		case Rewardable::VISIT_PLAYER:

lib/mapObjects/TownBuildingInstance.h

@@ -58,6 +58,7 @@ class DLL_LINKAGE TownRewardableBuildingInstance : public TownBuildingInstance,
 	bool wasVisitedBefore(const CGHeroInstance * contextHero) const override;
 	void grantReward(ui32 rewardID, const CGHeroInstance * hero) const override;
 	Rewardable::Configuration generateConfiguration(vstd::RNG & rand) const;
+	void assignBonuses(std::vector<Bonus> & bonuses) const;
 
 	const IObjectInterface * getObject() const override;
 	bool wasVisited(PlayerColor player) const override;

lib/mapping/CMap.cpp

@@ -873,7 +873,7 @@ const std::vector<ObjectInstanceID> & CMap::getHeroesOnMap()
 void CMap::addToHeroPool(std::shared_ptr<CGHeroInstance> hero)
 {
 	assert(hero->getHeroTypeID().isValid());
-	assert(!vstd::contains(heroesOnMap, hero->getHeroTypeID()));
+	assert(!vstd::contains(heroesOnMap, hero->id));
 	assert(heroesPool.at(hero->getHeroTypeID().getNum()) == nullptr);
 
 	heroesPool.at(hero->getHeroTypeID().getNum()) = hero;

lib/mapping/MapFormatH3M.cpp

@@ -1111,9 +1111,9 @@ void CMapLoaderH3M::readBoxContent(CGPandoraBox * object, const int3 & mapPositi
 	reward.heroExperience = reader->readUInt32();
 	reward.manaDiff = reader->readInt32();
 	if(auto val = reader->readInt8Checked(-3, 3))
-		reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven));
+		reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven));
 	if(auto val = reader->readInt8Checked(-3, 3))
-		reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven));
+		reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven));
 
 	reader->readResources(reward.resources);
 	for(int x = 0; x < GameConstants::PRIMARY_SKILLS; ++x)
@@ -1130,13 +1130,16 @@ void CMapLoaderH3M::readBoxContent(CGPandoraBox * object, const int3 & mapPositi
 	size_t gart = reader->readUInt8(); //number of gained artifacts
 	for(size_t oo = 0; oo < gart; ++oo)
 	{
-		reward.artifacts.push_back(reader->readArtifact());
+		ArtifactID grantedArtifact = reader->readArtifact();
+
 		if (features.levelHOTA5)
 		{
 			SpellID scrollSpell = reader->readSpell16();
-			if (reward.artifacts.back() == ArtifactID::SPELL_SCROLL)
-				logGlobal->warn("Map '%s': Pandora/Event at %s Option to give spell scroll (%s) via event or pandora is not implemented!", mapName, mapPosition.toString(), scrollSpell.toEntity(LIBRARY)->getJsonKey());
+			if (grantedArtifact == ArtifactID::SPELL_SCROLL)
+				reward.grantedScrolls.push_back(scrollSpell);
 		}
+		else
+			reward.grantedArtifacts.push_back(grantedArtifact);
 	}
 
 	size_t gspel = reader->readUInt8(); //number of gained spells
@@ -2300,12 +2303,12 @@ void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, con
 			}
 			case ESeerHutRewardType::MORALE:
 			{
-				reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven));
+				reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven));
 				break;
 			}
 			case ESeerHutRewardType::LUCK:
 			{
-				reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven));
+				reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven));
 				break;
 			}
 			case ESeerHutRewardType::RESOURCES:
@@ -2334,15 +2337,15 @@ void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, con
 			}
 			case ESeerHutRewardType::ARTIFACT:
 			{
-				reward.artifacts.push_back(reader->readArtifact());
+				ArtifactID grantedArtifact = reader->readArtifact();
 				if (features.levelHOTA5)
 				{
 					SpellID scrollSpell = reader->readSpell16();
-					if (reward.artifacts.back() == ArtifactID::SPELL_SCROLL)
-						logGlobal->warn("Map '%s': Seer Hut at %s: Option to give spell scroll (%s) as a reward is not implemented!", mapName, position.toString(), scrollSpell.toEntity(LIBRARY)->getJsonKey());
-
+					if (grantedArtifact == ArtifactID::SPELL_SCROLL)
+						reward.grantedScrolls.push_back(scrollSpell);
 				}
-
+				else
+					reward.grantedArtifacts.push_back(grantedArtifact);
 				break;
 			}
 			case ESeerHutRewardType::SPELL:
@@ -2407,16 +2410,18 @@ EQuestMission CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & positi
 			size_t artNumber = reader->readUInt8();
 			for(size_t yy = 0; yy < artNumber; ++yy)
 			{
-				auto artid = reader->readArtifact();
+				ArtifactID requiredArtifact = reader->readArtifact();
+
 				if (features.levelHOTA5)
 				{
 					SpellID scrollSpell = reader->readSpell16();
-					if (artid == ArtifactID::SPELL_SCROLL)
-						logGlobal->warn("Map '%s': Seer Hut at %s: Quest to find scroll '%s' is not implemented!", mapName, position.toString(), scrollSpell.toEntity(LIBRARY)->getJsonKey());
-
+					if (requiredArtifact == ArtifactID::SPELL_SCROLL)
+						guard->getQuest().mission.scrolls.push_back(scrollSpell);
 				}
-				guard->getQuest().mission.artifacts.push_back(artid);
-				map->allowedArtifact.erase(artid); //these are unavailable for random generation
+				else
+					guard->getQuest().mission.artifacts.push_back(requiredArtifact);
+
+				map->allowedArtifact.erase(requiredArtifact); //these are unavailable for random generation
 			}
 			break;
 		}

lib/networkPacks/NetPacksLib.cpp

@@ -1014,6 +1014,9 @@ void GiveBonus::applyGs(CGameState *gs)
 	case ETarget::OBJECT:
 		cbsn = dynamic_cast<CBonusSystemNode*>(gs->getObjInstance(id.as<ObjectInstanceID>()));
 		break;
+	case ETarget::HERO_COMMANDER:
+		cbsn = gs->getHero(id.as<ObjectInstanceID>())->getCommander();
+		break;
 	case ETarget::PLAYER:
 		cbsn = gs->getPlayerState(id.as<PlayerColor>());
 		break;
@@ -1045,16 +1048,16 @@ void ChangeObjPos::applyGs(CGameState *gs)
 
 void ChangeObjectVisitors::applyGs(CGameState *gs)
 {
+	auto objectPtr = gs->getObjInstance(object);
+
 	switch (mode) {
 		case VISITOR_ADD_HERO:
-			gs->getPlayerTeam(gs->getHero(hero)->tempOwner)->scoutedObjects.insert(object);
 			gs->getHero(hero)->visitedObjects.insert(object);
-			gs->getPlayerState(gs->getHero(hero)->tempOwner)->visitedObjects.insert(object);
-			break;
+			[[fallthrough]];
 		case VISITOR_ADD_PLAYER:
 			gs->getPlayerTeam(gs->getHero(hero)->tempOwner)->scoutedObjects.insert(object);
-			for(const auto & color : gs->getPlayerTeam(gs->getHero(hero)->tempOwner)->players)
-				gs->getPlayerState(color)->visitedObjects.insert(object);
+			gs->getPlayerState(gs->getHero(hero)->tempOwner)->visitedObjects.insert(object);
+			gs->getPlayerState(gs->getHero(hero)->tempOwner)->visitedObjectsGlobal.insert({objectPtr->ID, objectPtr->subID});
 
 			break;
 		case VISITOR_CLEAR:
@@ -1076,12 +1079,6 @@ void ChangeObjectVisitors::applyGs(CGameState *gs)
 			gs->getPlayerTeam(gs->getHero(hero)->tempOwner)->scoutedObjects.insert(object);
 
 			break;
-		case VISITOR_GLOBAL:
-			{
-				CGObjectInstance * objectPtr = gs->getObjInstance(object);
-				gs->getPlayerState(gs->getHero(hero)->tempOwner)->visitedObjectsGlobal.insert({objectPtr->ID, objectPtr->subID});
-				break;
-			}
 	}
 }
 

lib/networkPacks/PacksForClient.h

@@ -408,17 +408,31 @@ struct DLL_LINKAGE SetAvailableHero : public CPackForClient
 
 struct DLL_LINKAGE GiveBonus : public CPackForClient
 {
-	enum class ETarget : int8_t { OBJECT, PLAYER, BATTLE };
-	
+	using VariantType = VariantIdentifier<ObjectInstanceID, PlayerColor, BattleID>;
+	enum class ETarget : int8_t
+	{
+		OBJECT,
+		PLAYER,
+		BATTLE,
+		HERO_COMMANDER
+	};
+
 	explicit GiveBonus(ETarget Who = ETarget::OBJECT)
 		:who(Who)
 	{
 	}
 
+	GiveBonus(ETarget who, const VariantType & id, const Bonus & bonus)
+		: who(who)
+		, id(id)
+		, bonus(bonus)
+	{
+	}
+
 	void applyGs(CGameState * gs) override;
 
 	ETarget who = ETarget::OBJECT;
-	VariantIdentifier<ObjectInstanceID, PlayerColor, BattleID> id;
+	VariantType id;
 	Bonus bonus;
 
 	void visitTyped(ICPackVisitor & visitor) override;
@@ -1207,7 +1221,6 @@ struct DLL_LINKAGE ChangeObjectVisitors : public CPackForClient
 	{
 		VISITOR_ADD_HERO,   // mark hero as one that have visited this object
 		VISITOR_ADD_PLAYER, // mark player as one that have visited this object instance
-		VISITOR_GLOBAL,     // mark player as one that have visited object of this type
 		VISITOR_SCOUTED,    // marks targeted team as having scouted this object
 		VISITOR_CLEAR,      // clear all visitors from this object (object reset)
 	};

lib/rewardable/Configuration.cpp

@@ -103,6 +103,7 @@ void Rewardable::Configuration::serializeJson(JsonSerializeFormat & handler)
 	handler.serializeStruct("resetParameters", resetParameters);
 	handler.serializeBool("canRefuse", canRefuse);
 	handler.serializeBool("showScoutedPreview", showScoutedPreview);
+	handler.serializeBool("forceCombat", forceCombat);
 	handler.serializeBool("coastVisitable", coastVisitable);
 	handler.serializeInt("infoWindowType", infoWindowType);
 }

lib/rewardable/Configuration.h

@@ -22,18 +22,19 @@ VCMI_LIB_NAMESPACE_BEGIN
 namespace Rewardable
 {
 
-enum EVisitMode
+enum EVisitMode : uint8_t
 {
 	VISIT_UNLIMITED, // any number of times. Side effect - object hover text won't contain visited/not visited text
 	VISIT_ONCE,      // only once, first to visit get all the rewards
 	VISIT_HERO,      // every hero can visit object once
 	VISIT_BONUS,     // can be visited by any hero that don't have bonus from this object
 	VISIT_LIMITER,   // can be visited by heroes that don't fulfill provided limiter
-	VISIT_PLAYER     // every player can visit object once
+	VISIT_PLAYER,     // every player can visit object once
+	VISIT_PLAYER_GLOBAL // every player can visit object once. All objects of the same type will be considered as visited
 };
 
 /// controls selection of reward granted to player
-enum ESelectMode
+enum ESelectMode : uint8_t
 {
 	SELECT_FIRST,  // first reward that matches limiters
 	SELECT_PLAYER, // player can select from all allowed rewards
@@ -41,7 +42,7 @@ enum ESelectMode
 	SELECT_ALL // grant all rewards that match limiters
 };
 
-enum class EEventType
+enum class EEventType : uint8_t
 {
 	EVENT_INVALID = 0,
 	EVENT_FIRST_VISIT,
@@ -51,7 +52,7 @@ enum class EEventType
 };
 
 constexpr std::array<std::string_view, 4> SelectModeString{"selectFirst", "selectPlayer", "selectRandom", "selectAll"};
-constexpr std::array<std::string_view, 6> VisitModeString{"unlimited", "once", "hero", "bonus", "limiter", "player"};
+constexpr std::array<std::string_view, 7> VisitModeString{"unlimited", "once", "hero", "bonus", "limiter", "player", "playerGlobal" };
 
 struct DLL_LINKAGE ResetInfo
 {
@@ -144,10 +145,10 @@ struct DLL_LINKAGE Configuration
 	std::vector<Rewardable::VisitInfo> info;
 
 	/// how reward will be selected, uses ESelectMode enum
-	ui8 selectMode = Rewardable::SELECT_FIRST;
+	ESelectMode selectMode = Rewardable::SELECT_FIRST;
 
 	/// controls who can visit an object, uses EVisitMode enum
-	ui8 visitMode = Rewardable::VISIT_UNLIMITED;
+	EVisitMode visitMode = Rewardable::VISIT_UNLIMITED;
 
 	/// how and when should the object be reset
 	Rewardable::ResetInfo resetParameters;
@@ -163,6 +164,9 @@ struct DLL_LINKAGE Configuration
 	/// if true - player can refuse visiting an object (e.g. Tomb)
 	bool canRefuse = false;
 
+	/// if set to true and object is guarded, then hero visit will immediately start combat without confirmation
+	bool forceCombat = false;
+
 	/// if true - right-clicking object will show preview of object rewards
 	bool showScoutedPreview = false;
 
@@ -194,6 +198,8 @@ struct DLL_LINKAGE Configuration
 		h & variables;
 		h & visitLimiter;
 		h & canRefuse;
+		if (h.version >= Handler::Version::REWARDABLE_EXTENSIONS)
+			h & forceCombat;
 		h & showScoutedPreview;
 		h & infoWindowType;
 		h & coastVisitable;

lib/rewardable/Info.cpp

@@ -134,6 +134,8 @@ void Rewardable::Info::configureLimiter(Rewardable::Configuration & object, vstd
 	limiter.heroExperience = randomizer.loadValue(source["heroExperience"], rng, variables);
 	limiter.heroLevel = randomizer.loadValue(source["heroLevel"], rng, variables);
 	limiter.canLearnSkills = source["canLearnSkills"].Bool();
+	limiter.commanderAlive = source["commanderAlive"].Bool();
+	limiter.hasExtraCreatures = source["hasExtraCreatures"].Bool();
 
 	limiter.manaPercentage = randomizer.loadValue(source["manaPercentage"], rng, variables);
 	limiter.manaPoints = randomizer.loadValue(source["manaPoints"], rng, variables);
@@ -143,9 +145,12 @@ void Rewardable::Info::configureLimiter(Rewardable::Configuration & object, vstd
 	limiter.primary = randomizer.loadPrimaries(source["primary"], rng, variables);
 	limiter.secondary = randomizer.loadSecondaries(source["secondary"], rng, variables);
 	limiter.artifacts = randomizer.loadArtifacts(source["artifacts"], rng, variables);
+	limiter.availableSlots = randomizer.loadArtifactSlots(source["availableSlots"], rng, variables);
 	limiter.spells  = randomizer.loadSpells(source["spells"], rng, variables);
+	limiter.scrolls  = randomizer.loadSpells(source["scrolls"], rng, variables);
 	limiter.canLearnSpells  = randomizer.loadSpells(source["canLearnSpells"], rng, variables);
 	limiter.creatures = randomizer.loadCreatures(source["creatures"], rng, variables);
+	limiter.canReceiveCreatures = randomizer.loadCreatures(source["canReceiveCreatures"], rng, variables);
 	
 	limiter.players = randomizer.loadColors(source["colors"], rng, variables);
 	limiter.heroes = randomizer.loadHeroes(source["heroes"], rng);
@@ -174,16 +179,23 @@ void Rewardable::Info::configureReward(Rewardable::Configuration & object, vstd:
 	reward.movePercentage = randomizer.loadValue(source["movePercentage"], rng, variables, -1);
 
 	reward.removeObject = source["removeObject"].Bool();
-	reward.bonuses = randomizer.loadBonuses(source["bonuses"]);
+	reward.heroBonuses = randomizer.loadBonuses(source["bonuses"]);
+	reward.commanderBonuses = randomizer.loadBonuses(source["commanderBonuses"]);
+	reward.playerBonuses = randomizer.loadBonuses(source["playerBonuses"]);
 
 	reward.guards = randomizer.loadCreatures(source["guards"], rng, variables);
 
 	reward.primary = randomizer.loadPrimaries(source["primary"], rng, variables);
 	reward.secondary = randomizer.loadSecondaries(source["secondary"], rng, variables);
 
-	reward.artifacts = randomizer.loadArtifacts(source["artifacts"], rng, variables);
+	reward.grantedArtifacts = randomizer.loadArtifacts(source["artifacts"], rng, variables);
+	reward.takenArtifacts = randomizer.loadArtifacts(source["takenArtifacts"], rng, variables);
+	reward.takenArtifactSlots = randomizer.loadArtifactSlots(source["takenArtifactSlots"], rng, variables);
+	reward.grantedScrolls = randomizer.loadSpells(source["scrolls"], rng, variables);
+	reward.takenScrolls = randomizer.loadSpells(source["takenScrolls"], rng, variables);
 	reward.spells = randomizer.loadSpells(source["spells"], rng, variables);
 	reward.creatures = randomizer.loadCreatures(source["creatures"], rng, variables);
+	reward.takenCreatures = randomizer.loadCreatures(source["takenCreatures"], rng, variables);
 	if(!source["spellCast"].isNull() && source["spellCast"].isStruct())
 	{
 		reward.spellCast.first = randomizer.loadSpell(source["spellCast"]["spell"], rng, variables);
@@ -293,12 +305,18 @@ void Rewardable::Info::replaceTextPlaceholders(MetaString & target, const Variab
 			}
 		}
 
-		for (const auto & artifact : info.reward.artifacts )
+		for (const auto & artifact : info.reward.grantedArtifacts )
 		{
 			loot.appendRawString("%s");
 			loot.replaceName(artifact);
 		}
 
+		for (const auto & scroll : info.reward.grantedScrolls )
+		{
+			loot.appendRawString("%s");
+			loot.replaceName(scroll);
+		}
+
 		for (const auto & spell : info.reward.spells )
 		{
 			loot.appendRawString("%s");
@@ -315,9 +333,12 @@ void Rewardable::Info::replaceTextPlaceholders(MetaString & target, const Variab
 	}
 	else
 	{
-		for (const auto & artifact : info.reward.artifacts )
+		for (const auto & artifact : info.reward.grantedArtifacts )
 			target.replaceName(artifact);
 
+		for (const auto & scroll : info.reward.grantedScrolls )
+			target.replaceName(scroll);
+
 		for (const auto & spell : info.reward.spells )
 			target.replaceName(spell);
 
@@ -444,6 +465,7 @@ void Rewardable::Info::configureObject(Rewardable::Configuration & object, vstd:
 
 	object.canRefuse = parameters["canRefuse"].Bool();
 	object.showScoutedPreview = parameters["showScoutedPreview"].Bool();
+	object.forceCombat = parameters["forceCombat"].Bool();
 	object.guardsLayout = parameters["guardsLayout"].String();
 	object.coastVisitable = parameters["coastVisitable"].Bool();
 
@@ -457,7 +479,7 @@ void Rewardable::Info::configureObject(Rewardable::Configuration & object, vstd:
 	{
 		if(Rewardable::VisitModeString[i] == visitMode)
 		{
-			object.visitMode = i;
+			object.visitMode = static_cast<EVisitMode>(i);
 			break;
 		}
 	}
@@ -467,7 +489,7 @@ void Rewardable::Info::configureObject(Rewardable::Configuration & object, vstd:
 	{
 		if(Rewardable::SelectModeString[i] == selectMode)
 		{
-			object.selectMode = i;
+			object.selectMode = static_cast<ESelectMode>(i);
 			break;
 		}
 	}

lib/rewardable/Info.h

@@ -32,7 +32,7 @@ struct Configuration;
 struct Variables;
 struct VisitInfo;
 struct ResetInfo;
-enum class EEventType;
+enum class EEventType : uint8_t;
 
 class DLL_LINKAGE Info : public IObjectInfo
 {

lib/rewardable/Interface.cpp

@@ -151,18 +151,67 @@ void Rewardable::Interface::grantRewardAfterLevelup(const Rewardable::VisitInfo
 		cb->setMovePoints(&smp);
 	}
 
-	for(const Bonus & bonus : info.reward.bonuses)
+	for(const Bonus & bonus : info.reward.heroBonuses)
 	{
-		GiveBonus gb;
-		gb.who = GiveBonus::ETarget::OBJECT;
-		gb.bonus = bonus;
-		gb.id = hero->id;
+		GiveBonus gb(GiveBonus::ETarget::OBJECT, hero->id, bonus);
 		cb->giveHeroBonus(&gb);
 	}
 
-	for(const ArtifactID & art : info.reward.artifacts)
+	if (hero->getCommander())
+	{
+		for(const Bonus & bonus : info.reward.commanderBonuses)
+		{
+			GiveBonus gb(GiveBonus::ETarget::HERO_COMMANDER, hero->id, bonus);
+			cb->giveHeroBonus(&gb);
+		}
+	}
+
+	for(const Bonus & bonus : info.reward.playerBonuses)
+	{
+		GiveBonus gb(GiveBonus::ETarget::PLAYER, hero->getOwner(), bonus);
+		cb->giveHeroBonus(&gb);
+	}
+
+	for(const ArtifactID & art : info.reward.takenArtifacts)
+	{
+		// hero does not have such artifact alone, but he might have it as part of assembled artifact
+		if(!hero->hasArt(art))
+		{
+			const auto * assembly = hero->getCombinedArtWithPart(art);
+			if (assembly)
+			{
+				DisassembledArtifact da;
+				da.al = ArtifactLocation(hero->id, hero->getArtPos(assembly));
+				cb->sendAndApply(da);
+			}
+		}
+
+		if(hero->hasArt(art))
+			cb->removeArtifact(ArtifactLocation(hero->id, hero->getArtPos(art, false)));
+	}
+
+	for(const ArtifactPosition & slot : info.reward.takenArtifactSlots)
+	{
+		const auto & slotContent = hero->getSlot(slot);
+
+		if (!slotContent->locked && slotContent->artifactID.hasValue())
+			cb->removeArtifact(ArtifactLocation(hero->id, slot));
+
+		// TODO: handle locked slots?
+	}
+
+	for(const SpellID & spell : info.reward.takenScrolls)
+	{
+		if(hero->hasScroll(spell, false))
+			cb->removeArtifact(ArtifactLocation(hero->id, hero->getScrollPos(spell, false)));
+	}
+
+	for(const ArtifactID & art : info.reward.grantedArtifacts)
 		cb->giveHeroNewArtifact(hero, art, ArtifactPosition::FIRST_AVAILABLE);
 
+	for(const SpellID & spell : info.reward.grantedScrolls)
+		cb->giveHeroNewScroll(hero, spell, ArtifactPosition::FIRST_AVAILABLE);
+
 	if(!info.reward.spells.empty())
 	{
 		std::set<SpellID> spellsToGive;
@@ -175,6 +224,11 @@ void Rewardable::Interface::grantRewardAfterLevelup(const Rewardable::VisitInfo
 			cb->changeSpells(hero, true, spellsToGive);
 	}
 
+	if (!info.reward.takenCreatures.empty())
+	{
+		cb->takeCreatures(hero->id, info.reward.takenCreatures, !info.reward.creatures.empty());
+	}
+
 	if(!info.reward.creaturesChange.empty())
 	{
 		for(const auto & slot : hero->Slots())

lib/rewardable/Limiter.cpp

@@ -30,6 +30,8 @@ Rewardable::Limiter::Limiter()
 	, manaPercentage(0)
 	, manaPoints(0)
 	, canLearnSkills(false)
+	, commanderAlive(false)
+	, hasExtraCreatures(false)
 	, primary(GameConstants::PRIMARY_SKILLS, 0)
 {
 }
@@ -40,20 +42,25 @@ bool operator==(const Rewardable::Limiter & l, const Rewardable::Limiter & r)
 {
 	return l.dayOfWeek == r.dayOfWeek
 	&& l.daysPassed == r.daysPassed
-	&& l.heroLevel == r.heroLevel
 	&& l.heroExperience == r.heroExperience
+	&& l.heroLevel == r.heroLevel
 	&& l.manaPoints == r.manaPoints
 	&& l.manaPercentage == r.manaPercentage
-	&& l.secondary == r.secondary
 	&& l.canLearnSkills == r.canLearnSkills
-	&& l.creatures == r.creatures
-	&& l.spells == r.spells
+	&& l.commanderAlive == r.commanderAlive
+	&& l.hasExtraCreatures == r.hasExtraCreatures
+	&& l.resources == r.resources
+	&& l.primary == r.primary
+	&& l.secondary == r.secondary
 	&& l.artifacts == r.artifacts
-	&& l.players == r.players
+	&& l.availableSlots == r.availableSlots
+	&& l.scrolls == r.scrolls
+	&& l.spells == r.spells
+	&& l.canLearnSpells == r.canLearnSpells
+	&& l.creatures == r.creatures
 	&& l.heroes == r.heroes
 	&& l.heroClasses == r.heroClasses
-	&& l.resources == r.resources
-	&& l.primary == r.primary
+	&& l.players == r.players
 	&& l.noneOf == r.noneOf
 	&& l.allOf == r.allOf
 	&& l.anyOf == r.anyOf;
@@ -78,16 +85,25 @@ bool Rewardable::Limiter::heroAllowed(const CGHeroInstance * hero) const
 			return false;
 	}
 
-	for(const auto & reqStack : creatures)
+	if (commanderAlive)
 	{
-		size_t count = 0;
-		for(const auto & slot : hero->Slots())
-		{
-			const auto & heroStack = slot.second;
-			if (heroStack->getType() == reqStack.getType())
-				count += heroStack->getCount();
-		}
-		if (count < reqStack.getCount()) //not enough creatures of this kind
+		if (!hero->getCommander() || !hero->getCommander()->alive)
+			return false;
+	}
+
+	if (!creatures.empty())
+	{
+		if (!hero->hasUnits(creatures, hasExtraCreatures))
+			return false;
+	}
+
+	if (!canReceiveCreatures.empty())
+	{
+		CCreatureSet setToTest;
+		for (const auto & unitToGive : canReceiveCreatures)
+			setToTest.addToSlot(setToTest.getSlotFor(unitToGive.getCreature()), unitToGive.getId(), unitToGive.getCount());
+
+		if (!hero->canBeMergedWith(setToTest))
 			return false;
 	}
 
@@ -133,6 +149,18 @@ bool Rewardable::Limiter::heroAllowed(const CGHeroInstance * hero) const
 			return false;
 	}
 
+	for(const auto & scroll : scrolls)
+	{
+		if (!hero->hasScroll(scroll, false))
+			return false;
+	}
+
+	for(const auto & slot : availableSlots)
+	{
+		if (hero->getSlot(slot)->artifactID.hasValue())
+			return false;
+	}
+
 	{
 		std::unordered_map<ArtifactID, unsigned int, ArtifactID::hash> artifactsRequirements; // artifact ID -> required count
 		for(const auto & art : artifacts)

lib/rewardable/Limiter.h

@@ -48,6 +48,13 @@ struct DLL_LINKAGE Limiter final : public Serializeable
 	/// Number of free secondary slots that hero needs to have
 	bool canLearnSkills;
 
+	/// Hero has commander, and commander is currently alive
+	bool commanderAlive;
+
+	/// Hero has creatures other than those requested in 'creatures' field
+	/// In other words, it is possible to take requested creatures from hero
+	bool hasExtraCreatures;
+
 	/// resources player needs to have in order to trigger reward
 	TResources resources;
 
@@ -59,6 +66,12 @@ struct DLL_LINKAGE Limiter final : public Serializeable
 	/// checks for artifacts copies if same artifact id is included multiple times
 	std::vector<ArtifactID> artifacts;
 
+	/// artifact slots that hero needs to have available (not locked and without any artifact) to pass the limiter
+	std::vector<ArtifactPosition> availableSlots;
+
+	/// Spell scrolls that hero must have in inventory (equipped or in backpack)
+	std::vector<SpellID> scrolls;
+
 	/// Spells that hero must have in the spellbook
 	std::vector<SpellID> spells;
 
@@ -68,6 +81,9 @@ struct DLL_LINKAGE Limiter final : public Serializeable
 	/// creatures that hero needs to have
 	std::vector<CStackBasicDescriptor> creatures;
 	
+	/// creatures that hero needs to have
+	std::vector<CStackBasicDescriptor> canReceiveCreatures;
+
 	/// only heroes/hero classes from list could pass limiter
 	std::vector<HeroTypeID> heroes;
 	std::vector<HeroClassID> heroClasses;
@@ -102,13 +118,27 @@ struct DLL_LINKAGE Limiter final : public Serializeable
 		h & manaPoints;
 		h & manaPercentage;
 		h & canLearnSkills;
+		if (h.version >= Handler::Version::REWARDABLE_EXTENSIONS)
+		{
+			h & commanderAlive;
+			h & hasExtraCreatures;
+		}
 		h & resources;
 		h & primary;
 		h & secondary;
 		h & artifacts;
+		if (h.version >= Handler::Version::REWARDABLE_EXTENSIONS)
+		{
+			h & availableSlots;
+			h & scrolls;
+		}
 		h & spells;
 		h & canLearnSpells;
 		h & creatures;
+		if (h.version >= Handler::Version::REWARDABLE_EXTENSIONS)
+		{
+			h & canReceiveCreatures;
+		}
 		h & heroes;
 		h & heroClasses;
 		h & players;

lib/rewardable/Reward.cpp

@@ -79,7 +79,7 @@ void Rewardable::Reward::loadComponents(std::vector<Component> & comps, const CG
 	for (auto comp : extraComponents)
 		comps.push_back(comp);
 	
-	for (auto & bonus : bonuses)
+	for (auto & bonus : heroBonuses)
 	{
 		if (bonus.type == BonusType::MORALE)
 			comps.emplace_back(ComponentType::MORALE, bonus.val);
@@ -111,9 +111,28 @@ void Rewardable::Reward::loadComponents(std::vector<Component> & comps, const CG
 		comps.emplace_back(ComponentType::SEC_SKILL, entry.first, finalLevel);
 	}
 
-	for(const auto & entry : artifacts)
+	for(const auto & entry : grantedArtifacts)
 		comps.emplace_back(ComponentType::ARTIFACT, entry);
 
+	for(const auto & entry : takenArtifacts)
+		comps.emplace_back(ComponentType::ARTIFACT, entry);
+
+	for(const auto & entry : takenArtifactSlots)
+	{
+		if (h)
+		{
+			const auto & slotContent = h->getSlot(entry);
+			if (slotContent->artifactID.hasValue())
+				comps.emplace_back(ComponentType::ARTIFACT, slotContent->getArt()->getTypeId());
+		}
+	}
+
+	for(const SpellID & spell : grantedScrolls)
+		comps.emplace_back(ComponentType::SPELL, spell);
+
+	for(const SpellID & spell : takenScrolls)
+		comps.emplace_back(ComponentType::SPELL, spell);
+
 	for(const auto & entry : spells)
 	{
 		bool learnable = !h || h->canLearnSpell(entry.toEntity(LIBRARY), true);
@@ -141,7 +160,11 @@ void Rewardable::Reward::serializeJson(JsonSerializeFormat & handler)
 	handler.serializeInt("manaDiff", manaDiff);
 	handler.serializeInt("manaOverflowFactor", manaOverflowFactor);
 	handler.serializeInt("movePoints", movePoints);
-	handler.serializeIdArray("artifacts", artifacts);
+	handler.serializeIdArray("artifacts", grantedArtifacts);
+	handler.serializeIdArray("takenArtifacts", takenArtifacts);
+	handler.serializeIdArray("takenArtifactSlots", takenArtifactSlots);
+	handler.serializeIdArray("scrolls", grantedScrolls);
+	handler.serializeIdArray("takenScrolls", takenScrolls);
 	handler.serializeIdArray("spells", spells);
 	handler.enterArray("creatures").serializeStruct(creatures);
 	handler.enterArray("primary").serializeArray(primary);

lib/rewardable/Reward.h

@@ -57,7 +57,6 @@ struct RewardRevealTiles
 };
 
 /// Reward that can be granted to a hero
-/// NOTE: eventually should replace seer hut rewards and events/pandoras
 struct DLL_LINKAGE Reward final
 {
 	/// resources that will be given to player
@@ -65,6 +64,7 @@ struct DLL_LINKAGE Reward final
 
 	/// received experience
 	si32 heroExperience;
+
 	/// received levels (converted into XP during grant)
 	si32 heroLevel;
 
@@ -86,7 +86,9 @@ struct DLL_LINKAGE Reward final
 	std::vector<CStackBasicDescriptor> guards;
 
 	/// list of bonuses, e.g. morale/luck
-	std::vector<Bonus> bonuses;
+	std::vector<Bonus> heroBonuses;
+	std::vector<Bonus> commanderBonuses;
+	std::vector<Bonus> playerBonuses;
 
 	/// skills that hero may receive or lose
 	std::vector<si32> primary;
@@ -96,9 +98,14 @@ struct DLL_LINKAGE Reward final
 	std::map<CreatureID, CreatureID> creaturesChange;
 
 	/// objects that hero may receive
-	std::vector<ArtifactID> artifacts;
+	std::vector<ArtifactID> grantedArtifacts;
+	std::vector<ArtifactID> takenArtifacts;
+	std::vector<ArtifactPosition> takenArtifactSlots;
+	std::vector<SpellID> grantedScrolls;
+	std::vector<SpellID> takenScrolls;
 	std::vector<SpellID> spells;
 	std::vector<CStackBasicDescriptor> creatures;
+	std::vector<CStackBasicDescriptor> takenCreatures;
 	
 	/// actions that hero may execute and object caster. Pair of spellID and school level
 	std::pair<SpellID, int> spellCast;
@@ -137,10 +144,24 @@ struct DLL_LINKAGE Reward final
 		h & movePoints;
 		h & primary;
 		h & secondary;
-		h & bonuses;
-		h & artifacts;
+		h & heroBonuses;
+		if (h.version >= Handler::Version::REWARDABLE_EXTENSIONS)
+		{
+			h & playerBonuses;
+			h & commanderBonuses;
+		}
+		h & grantedArtifacts;
+		if (h.version >= Handler::Version::REWARDABLE_EXTENSIONS)
+		{
+			h & takenArtifacts;
+			h & takenArtifactSlots;
+			h & grantedScrolls;
+			h & takenScrolls;
+		}
 		h & spells;
 		h & creatures;
+		if (h.version >= Handler::Version::REWARDABLE_EXTENSIONS)
+			h & takenCreatures;
 		h & creaturesChange;
 		h & revealTiles;
 		h & spellCast;

lib/serializer/ESerializationVersion.h

@@ -39,9 +39,9 @@ enum class ESerializationVersion : int32_t
 	STACK_INSTANCE_EXPERIENCE_FIX, // stack experience is stored as total, not as average
 	STACK_INSTANCE_ARMY_FIX, // remove serialization of army that owns stack instance
 	STORE_UID_COUNTER_IN_CMAP,  // fix crash caused by conflicting instanceName after loading game
+	REWARDABLE_EXTENSIONS, // new functionality for rewardable objects
 
-	
-	CURRENT = STORE_UID_COUNTER_IN_CMAP,
+	CURRENT = REWARDABLE_EXTENSIONS,
 };
 
 static_assert(ESerializationVersion::MINIMAL <= ESerializationVersion::CURRENT, "Invalid serialization version definition!");

mapeditor/inspector/rewardswidget.cpp

@@ -249,8 +249,8 @@ void RewardsWidget::obtainData()
 bool RewardsWidget::commitChanges()
 {
 	//common parameters
-	object.configuration.visitMode = ui->visitMode->currentIndex();
-	object.configuration.selectMode = ui->selectMode->currentIndex();
+	object.configuration.visitMode = static_cast<Rewardable::EVisitMode>(ui->visitMode->currentIndex());
+	object.configuration.selectMode = static_cast<Rewardable::ESelectMode>(ui->selectMode->currentIndex());
 	object.configuration.infoWindowType = EInfoWindowMode(ui->windowMode->currentIndex());
 	if(ui->onSelectText->text().isEmpty())
 		object.configuration.onSelect.clear();
@@ -297,11 +297,11 @@ void RewardsWidget::saveCurrentVisitInfo(int index)
 			vinfo.reward.resources[i] = widget->value();
 	}
 	
-	vinfo.reward.artifacts.clear();
+	vinfo.reward.grantedArtifacts.clear();
 	for(int i = 0; i < ui->rArtifacts->count(); ++i)
 	{
 		if(ui->rArtifacts->item(i)->checkState() == Qt::Checked)
-			vinfo.reward.artifacts.push_back(LIBRARY->artifacts()->getByIndex(i)->getId());
+			vinfo.reward.grantedArtifacts.push_back(LIBRARY->artifacts()->getByIndex(i)->getId());
 	}
 	vinfo.reward.spells.clear();
 	for(int i = 0; i < ui->rSpells->count(); ++i)
@@ -336,13 +336,13 @@ void RewardsWidget::saveCurrentVisitInfo(int index)
 		vinfo.reward.spellCast.second = ui->castLevel->currentIndex();
 	}
 	
-	vinfo.reward.bonuses.clear();
+	vinfo.reward.heroBonuses.clear();
 	for(int i = 0; i < ui->bonuses->rowCount(); ++i)
 	{
 		auto dur = bonusDurationMap.at(ui->bonuses->item(i, 0)->text().toStdString());
 		auto typ = bonusNameMap.at(ui->bonuses->item(i, 1)->text().toStdString());
 		auto val = ui->bonuses->item(i, 2)->data(Qt::UserRole).toInt();
-		vinfo.reward.bonuses.emplace_back(dur, typ, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(object.id));
+		vinfo.reward.heroBonuses.emplace_back(dur, typ, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(object.id));
 	}
 	
 	vinfo.limiter.dayOfWeek = ui->lDayOfWeek->currentIndex();
@@ -452,7 +452,7 @@ void RewardsWidget::loadCurrentVisitInfo(int index)
 			widget->setValue(vinfo.reward.resources[i]);
 	}
 	
-	for(auto i : vinfo.reward.artifacts)
+	for(auto i : vinfo.reward.grantedArtifacts)
 		ui->rArtifacts->item(LIBRARY->artifacts()->getById(i)->getIndex())->setCheckState(Qt::Checked);
 	for(auto i : vinfo.reward.spells)
 		ui->rArtifacts->item(LIBRARY->spells()->getById(i)->getIndex())->setCheckState(Qt::Checked);
@@ -478,7 +478,7 @@ void RewardsWidget::loadCurrentVisitInfo(int index)
 		ui->castLevel->setCurrentIndex(vinfo.reward.spellCast.second);
 	}
 	
-	for(auto & i : vinfo.reward.bonuses)
+	for(auto & i : vinfo.reward.heroBonuses)
 	{
 		auto dur = vstd::findKey(bonusDurationMap, i.duration);
 		for(int i = 0; i < ui->bonusDuration->count(); ++i)
@@ -786,7 +786,7 @@ void RewardsDelegate::updateModelData(QAbstractItemModel * model, const QModelIn
 		}
 		textList += QObject::tr("Resources: %1").arg(resourcesList.join(", "));
 		QStringList artifactsList;
-		for (auto artifact : vinfo.reward.artifacts)
+		for (auto artifact : vinfo.reward.grantedArtifacts)
 		{
 			artifactsList += QString::fromStdString(LIBRARY->artifacts()->getById(artifact)->getNameTranslated());
 		}
@@ -814,7 +814,7 @@ void RewardsDelegate::updateModelData(QAbstractItemModel * model, const QModelIn
 			textList += QObject::tr("Spell Cast: %1 (%2)").arg(QString::fromStdString(LIBRARY->spells()->getById(vinfo.reward.spellCast.first)->getNameTranslated())).arg(vinfo.reward.spellCast.second);
 		}
 		QStringList bonusesList;
-		for (auto & bonus : vinfo.reward.bonuses)
+		for (auto & bonus : vinfo.reward.heroBonuses)
 		{
 			bonusesList += QString("%1 %2 (%3)").arg(QString::fromStdString(vstd::findKey(bonusDurationMap, bonus.duration))).arg(QString::fromStdString(vstd::findKey(bonusNameMap, bonus.type))).arg(bonus.val);
 		}

server/CGameHandler.cpp

@@ -1157,26 +1157,36 @@ void CGameHandler::giveCreatures(const CArmedInstance *obj, const CGHeroInstance
 	tryJoiningArmy(obj, h, remove, true);
 }
 
-void CGameHandler::takeCreatures(ObjectInstanceID objid, const std::vector<CStackBasicDescriptor> &creatures)
+void CGameHandler::takeCreatures(ObjectInstanceID objid, const std::vector<CStackBasicDescriptor> &creatures, bool forceRemoval)
 {
-	std::vector<CStackBasicDescriptor> cres = creatures;
-	if (cres.size() <= 0)
+	std::vector<CStackBasicDescriptor> remainerForTaking = creatures;
+	if (remainerForTaking.empty())
 		return;
-	const CArmedInstance* obj = static_cast<const CArmedInstance*>(getObj(objid));
 
-	for (CStackBasicDescriptor &sbd : cres)
+	const CArmedInstance* army = static_cast<const CArmedInstance*>(getObj(objid));
+
+	for (const CStackBasicDescriptor &stackToTake : remainerForTaking)
 	{
 		TQuantity collected = 0;
-		while(collected < sbd.getCount())
+		while(collected < stackToTake.getCount())
 		{
 			bool foundSth = false;
-			for (auto i = obj->Slots().begin(); i != obj->Slots().end(); i++)
+			for (const auto & armySlot : army->Slots())
 			{
-				if (i->second->getType() == sbd.getType())
+				if (armySlot.second->getType() == stackToTake.getType())
 				{
-					TQuantity take = std::min(sbd.getCount() - collected, i->second->getCount()); //collect as much cres as we can
-					changeStackCount(StackLocation(obj->id, i->first), -take, false);
-					collected += take;
+					if (stackToTake.getCount() - collected >= armySlot.second->getCount())
+					{
+						// take entire stack
+						collected += armySlot.second->getCount();
+						eraseStack(StackLocation(army->id, armySlot.first), forceRemoval);
+					}
+					else
+					{
+						// take part of the stack
+						collected = stackToTake.getCount();
+						changeStackCount(StackLocation(army->id, armySlot.first), collected - stackToTake.getCount(), false);
+					}
 					foundSth = true;
 					break;
 				}

server/CGameHandler.h

@@ -125,7 +125,7 @@ public:
 	void giveResources(PlayerColor player, TResources resources) override;
 
 	void giveCreatures(const CArmedInstance *objid, const CGHeroInstance * h, const CCreatureSet &creatures, bool remove) override;
-	void takeCreatures(ObjectInstanceID objid, const std::vector<CStackBasicDescriptor> &creatures) override;
+	void takeCreatures(ObjectInstanceID objid, const std::vector<CStackBasicDescriptor> &creatures, bool forceRemoval) override;
 	bool changeStackType(const StackLocation &sl, const CCreature *c) override;
 	bool changeStackCount(const StackLocation &sl, TQuantity count, bool absoluteValue = false) override;
 	bool insertNewStack(const StackLocation &sl, const CCreature *c, TQuantity count) override;

server/processors/PlayerMessageProcessor.cpp

@@ -383,6 +383,9 @@ void PlayerMessageProcessor::cheatGiveSpells(PlayerColor player, const CGHeroIns
 		gameHandler->sendAndApply(giveBonus);
 	}
 
+	giveBonus.bonus = Bonus(BonusDuration::PERMANENT, BonusType::HERO_SPELL_CASTS_PER_COMBAT_TURN, BonusSource::OTHER, 99, BonusSourceID());
+	gameHandler->sendAndApply(giveBonus);
+
 	///Give mana
 	SetMana sm;
 	sm.hid = hero->id;

test/mock/mock_IGameCallback.h

@@ -62,7 +62,7 @@ public:
 	void giveResources(PlayerColor player, TResources resources) override {}
 
 	void giveCreatures(const CArmedInstance *objid, const CGHeroInstance * h, const CCreatureSet &creatures, bool remove) override {}
-	void takeCreatures(ObjectInstanceID objid, const std::vector<CStackBasicDescriptor> &creatures) override {}
+	void takeCreatures(ObjectInstanceID objid, const std::vector<CStackBasicDescriptor> &creatures, bool forceRemoval) override {}
 	bool changeStackCount(const StackLocation &sl, TQuantity count, bool absoluteValue = false) override {return false;}
 	bool changeStackType(const StackLocation &sl, const CCreature *c) override {return false;}
 	bool insertNewStack(const StackLocation &sl, const CCreature *c, TQuantity count = -1) override {return false;} //count -1 => moves whole stack

test/mock/mock_battle_IBattleState.h

@@ -29,7 +29,7 @@ public:
 	MOCK_CONST_METHOD1(getSidePlayer, PlayerColor(BattleSide));
 	MOCK_CONST_METHOD1(getSideArmy, const CArmedInstance *(BattleSide));
 	MOCK_CONST_METHOD1(getSideHero, const CGHeroInstance *(BattleSide));
-	MOCK_CONST_METHOD1(getCastSpells, uint32_t(BattleSide));
+	MOCK_CONST_METHOD1(getCastSpells, int32_t(BattleSide));
 	MOCK_CONST_METHOD1(getEnchanterCounter, int32_t(BattleSide));
 	MOCK_CONST_METHOD0(getTacticDist, ui8());
 	MOCK_CONST_METHOD0(getTacticsSide, BattleSide());