Properly removing stuff from hero, fixes #1646.

lib/CGameState.cpp

@@ -1292,16 +1292,19 @@ void CGameState::prepareCrossoverHeroes(std::vector<CGameState::CampaignHeroRepl
 		for(CGHeroInstance * cgh : crossoverHeroes)
 		{
 			cgh->secSkills = cgh->type->secSkillsInit;
+			cgh->recreateSecondarySkillsBonuses();
 		}
 	}
 
-	if(!(travelOptions.whatHeroKeeps & 8))
+	if(!(travelOptions.whatHeroKeeps & 8)) 
 	{
-		//trimming spells
 		for(CGHeroInstance * cgh : crossoverHeroes)
 		{
+			// Trimming spells
 			cgh->spells.clear();
-			cgh->eraseArtSlot(ArtifactPosition(ArtifactPosition::SPELLBOOK)); // spellbook will also be removed
+
+			// Spellbook will also be removed
+			ArtifactLocation(cgh, ArtifactPosition(ArtifactPosition::SPELLBOOK)).removeArtifact();
 		}
 	}
 
@@ -1311,22 +1314,26 @@ void CGameState::prepareCrossoverHeroes(std::vector<CGameState::CampaignHeroRepl
 		for(CGHeroInstance * hero : crossoverHeroes)
 		{
 			size_t totalArts = GameConstants::BACKPACK_START + hero->artifactsInBackpack.size();
-			for (size_t i=0; i<totalArts; i++ )
+			for (size_t i = 0; i < totalArts; i++ )
 			{
 				auto artifactPosition = ArtifactPosition(i);
 				if(artifactPosition == ArtifactPosition::SPELLBOOK) continue; // do not handle spellbook this way
 
+				// TODO: why would there be nullptr artifacts?
 				const ArtSlotInfo *info = hero->getSlot(artifactPosition);
-				if(!info) continue;
+				if(!info) 
+					continue;
 
 				const CArtifactInstance *art = info->artifact;
-				if(!art) continue;
+				if(!art) 
+					continue;
 
 				int id  = art->artType->id;
 				assert( 8*18 > id );//number of arts that fits into h3m format
 				bool takeable = travelOptions.artifsKeptByHero[id / 8] & ( 1 << (id%8) );
 
-				if(!takeable) hero->eraseArtSlot(ArtifactPosition(i));
+				if(!takeable) 
+					ArtifactLocation(hero, artifactPosition).removeArtifact();
 			}
 		}
 	}
@@ -1334,11 +1341,16 @@ void CGameState::prepareCrossoverHeroes(std::vector<CGameState::CampaignHeroRepl
 	//trimming creatures
 	for(CGHeroInstance * cgh : crossoverHeroes)
 	{
-		vstd::erase_if(cgh->stacks, [&](const std::pair<SlotID, CStackInstance *> & j) -> bool
+		auto shouldSlotBeErased = [&](const std::pair<SlotID, CStackInstance *> & j) -> bool
 		{
 			CreatureID::ECreatureID crid = j.second->getCreatureID().toEnum();
-			return !(travelOptions.monstersKeptByHero[crid / 8] & (1 << (crid % 8)) );
-		});
+			return !(travelOptions.monstersKeptByHero[crid / 8] & (1 << (crid % 8)));
+		};
+
+		auto stacksCopy = cgh->stacks; //copy of the map, so we can iterate iover it and remove stacks
+		for(auto &slotPair : stacksCopy)
+			if(shouldSlotBeErased(slotPair))
+				cgh->eraseStack(slotPair.first);
 	}
 }
 

lib/CObjectHandler.cpp

@@ -1150,8 +1150,7 @@ void CGHeroInstance::initObj()
 	}
 
 	//initialize bonuses
-	for(auto skill_info : secSkills)
-		updateSkill(SecondarySkill(skill_info.first), skill_info.second);
+	recreateSecondarySkillsBonuses();
 	Updatespecialty();
 
 	mana = manaLimit(); //after all bonuses are taken into account, make sure this line is the last one
@@ -1212,6 +1211,17 @@ void CGHeroInstance::Updatespecialty() //TODO: calculate special value of bonuse
 		}
 	}
 }
+
+void CGHeroInstance::recreateSecondarySkillsBonuses()
+{
+	auto secondarySkillsBonuses = getBonuses(Selector::sourceType(Bonus::SECONDARY_SKILL));
+	for(auto bonus : *secondarySkillsBonuses)
+		removeBonus(bonus);
+
+	for(auto skill_info : secSkills)
+		updateSkill(SecondarySkill(skill_info.first), skill_info.second);
+}
+
 void CGHeroInstance::updateSkill(SecondarySkill which, int val)
 {
 	if(which == SecondarySkill::LEADERSHIP || which == SecondarySkill::LUCK)

lib/CObjectHandler.h

@@ -461,6 +461,7 @@ public:
 	ui8 maxlevelsToMagicSchool() const;
 	ui8 maxlevelsToWisdom() const;
 	void Updatespecialty();
+	void recreateSecondarySkillsBonuses();
 	void updateSkill(SecondarySkill which, int val);
 
 	CGHeroInstance();

lib/NetPacksBase.h

@@ -181,6 +181,8 @@ struct ArtifactLocation
 		return false;
 	}
 
+	DLL_LINKAGE void removeArtifact(); // BE CAREFUL, this operation modifies holder (gs)
+
 	DLL_LINKAGE const CArmedInstance *relatedObj() const; //hero or the stack owner
 	DLL_LINKAGE PlayerColor owningPlayer() const;
 	DLL_LINKAGE CArtifactSet *getHolderArtSet();

lib/NetPacksLib.cpp

@@ -638,6 +638,14 @@ struct GetBase : boost::static_visitor<T*>
 	}
 };
 
+
+DLL_LINKAGE void ArtifactLocation::removeArtifact()
+{
+	CArtifactInstance *a = getArt();
+	assert(a);
+	a->removeFrom(*this);
+}
+
 DLL_LINKAGE const CArmedInstance * ArtifactLocation::relatedObj() const
 {
 	return boost::apply_visitor(ObjectRetriever(), artHolder);
@@ -828,9 +836,7 @@ DLL_LINKAGE void PutArtifact::applyGs( CGameState *gs )
 
 DLL_LINKAGE void EraseArtifact::applyGs( CGameState *gs )
 {
-	CArtifactInstance *a = al.getArt();
-	assert(a);
-	a->removeFrom(al);
+	al.removeArtifact();
 }
 
 DLL_LINKAGE void MoveArtifact::applyGs( CGameState *gs )