- fixed #1643 -> hero placeholder handling

- fixed bug when loading victory/loss conditions of the 3rd scenario in the first ROE campaign
- fixed bug when loading artifacts to hero of the 3rd scenario in the first ROE campaign (due to corrupt H3M map)
- implemented function object to quickly find a object by it's sub ID in a list
- added netbackbase.h to header list in CMake
- removed false message which said that the server loaded the map successfully

client/Client.cpp

@@ -326,10 +326,7 @@ void CClient::newGame( CConnection *con, StartInfo *si )
 		c << ui8(2) << ui8(1); //new game; one client
 		c << *si;
 		c >> pom8;
-		if(pom8) 
-			throw std::runtime_error("Server cannot open the map!");
-		else
-            logNetwork->infoStream() << "Server opened map properly.";
+		if(pom8) throw std::runtime_error("Server cannot open the map!");
 	}
 
 	c >> si;

lib/CGameState.cpp

@@ -432,6 +432,23 @@ CGHeroInstance * CGameState::HeroesPool::pickHeroFor(bool native, PlayerColor pl
 	return ret;
 }
 
+void CGameState::CrossoverHeroesList::addHeroToBothLists(CGHeroInstance * hero)
+{
+	heroesFromPreviousScenario.push_back(hero);
+	heroesFromAnyPreviousScenarios.push_back(hero);
+}
+
+void CGameState::CrossoverHeroesList::removeHeroFromBothLists(CGHeroInstance * hero)
+{
+	heroesFromPreviousScenario -= hero;
+	heroesFromAnyPreviousScenarios -= hero;
+}
+
+CGameState::CampaignHeroReplacement::CampaignHeroReplacement(CGHeroInstance * hero, ObjectInstanceID heroPlaceholderId) : hero(hero), heroPlaceholderId(heroPlaceholderId)
+{
+
+}
+
 int CGameState::pickNextHeroType(PlayerColor owner) const
 {
 	const PlayerSettings &ps = scenarioOps->getIthPlayersSettings(owner);
@@ -1104,15 +1121,15 @@ void CGameState::placeCampaignHeroes()
 		}
 
 		// replace heroes placeholders
-		auto sourceCrossoverHeroes = getCrossoverHeroesFromPreviousScenario();
+		auto crossoverHeroes = getCrossoverHeroesFromPreviousScenarios();
 
-		if(!sourceCrossoverHeroes.empty())
+		if(!crossoverHeroes.heroesFromAnyPreviousScenarios.empty())
 		{
-			logGlobal->debugStream() << "\tPrepare crossover heroes";
-			auto crossoverHeroes = prepareCrossoverHeroes(sourceCrossoverHeroes, scenarioOps->campState->getCurrentScenario().travelOptions);
-
 			logGlobal->debugStream() << "\tGenerate list of hero placeholders";
-			const auto campaignHeroReplacements = generateCampaignHeroesToReplace(crossoverHeroes);
+			auto campaignHeroReplacements = generateCampaignHeroesToReplace(crossoverHeroes);
+
+			logGlobal->debugStream() << "\tPrepare crossover heroes";
+			prepareCrossoverHeroes(campaignHeroReplacements, scenarioOps->campState->getCurrentScenario().travelOptions);
 
 			// remove same heroes on the map which will be added through crossover heroes
 			// INFO: we will remove heroes because later it may be possible that the API doesn't allow having heroes
@@ -1121,7 +1138,7 @@ void CGameState::placeCampaignHeroes()
 
 			for(auto & campaignHeroReplacement : campaignHeroReplacements)
 			{
-				auto hero = getUsedHero(HeroTypeID(campaignHeroReplacement.first->subID));
+				auto hero = getUsedHero(HeroTypeID(campaignHeroReplacement.hero->subID));
 				if(hero)
 				{
 					removedHeroes.push_back(hero);
@@ -1176,30 +1193,58 @@ void CGameState::placeStartingHero(PlayerColor playerColor, HeroTypeID heroTypeI
 	map->getEditManager()->insertObject(hero, townPos);
 }
 
-std::vector<CGHeroInstance *> CGameState::getCrossoverHeroesFromPreviousScenario() const
+CGameState::CrossoverHeroesList CGameState::getCrossoverHeroesFromPreviousScenarios() const
 {
-	std::vector<CGHeroInstance *> crossoverHeroes;
+	CrossoverHeroesList crossoverHeroes;
 
 	auto campaignState = scenarioOps->campState;
 	auto bonus = campaignState->getBonusForCurrentMap();
 	if (bonus->type == CScenarioTravel::STravelBonus::HEROES_FROM_PREVIOUS_SCENARIO)
 	{
-		crossoverHeroes = campaignState->camp->scenarios[bonus->info2].crossoverHeroes;
+		crossoverHeroes.heroesFromAnyPreviousScenarios = crossoverHeroes.heroesFromPreviousScenario = campaignState->camp->scenarios[bonus->info2].crossoverHeroes;
 	}
 	else
 	{
 		if(!campaignState->mapsConquered.empty())
 		{
-			crossoverHeroes = campaignState->camp->scenarios[campaignState->mapsConquered.back()].crossoverHeroes;
+			crossoverHeroes.heroesFromPreviousScenario = campaignState->camp->scenarios[campaignState->mapsConquered.back()].crossoverHeroes;
+
+			for(auto mapNr : campaignState->mapsConquered)
+			{
+				// create a list of deleted heroes
+				auto & scenario = campaignState->camp->scenarios[mapNr];
+				auto lostCrossoverHeroes = scenario.getLostCrossoverHeroes();
+
+				// remove heroes which didn't reached the end of the scenario, but were available at the start
+				for(auto hero : lostCrossoverHeroes)
+				{
+					range::remove_if(crossoverHeroes.heroesFromAnyPreviousScenarios, CGObjectInstanceBySubIdFinder(hero));
+				}
+
+				// now add heroes which completed the scenario
+				for(auto hero : scenario.crossoverHeroes)
+				{
+					// add new heroes and replace old heroes with newer ones
+					range::remove_if(crossoverHeroes.heroesFromAnyPreviousScenarios, CGObjectInstanceBySubIdFinder(hero));
+					crossoverHeroes.heroesFromAnyPreviousScenarios.push_back(hero);
+				}
+			}
 		}
 	}
 
 	return std::move(crossoverHeroes);
 }
 
-std::vector<CGHeroInstance *> CGameState::prepareCrossoverHeroes(const std::vector<CGHeroInstance *> & sourceCrossoverHeroes, const CScenarioTravel & travelOptions)
+void CGameState::prepareCrossoverHeroes(std::vector<CGameState::CampaignHeroReplacement> & campaignHeroReplacements, const CScenarioTravel & travelOptions) const
 {
-	auto crossoverHeroes = sourceCrossoverHeroes; //TODO deep copy hero instances
+	//TODO deep copy hero instances
+
+	// create heroes list for convenience iterating
+	std::vector<CGHeroInstance *> crossoverHeroes;
+	for(auto & campaignHeroReplacement : campaignHeroReplacements)
+	{
+		crossoverHeroes.push_back(campaignHeroReplacement.hero);
+	}
 
 	if(!(travelOptions.whatHeroKeeps & 1))
 	{
@@ -1280,8 +1325,6 @@ std::vector<CGHeroInstance *> CGameState::prepareCrossoverHeroes(const std::vect
 			return !(travelOptions.monstersKeptByHero[crid / 8] & (1 << (crid % 8)) );
 		});
 	}
-
-	return std::move(crossoverHeroes);
 }
 
 void CGameState::placeStartingHeroes()
@@ -2696,9 +2739,9 @@ std::set<HeroTypeID> CGameState::getUnusedAllowedHeroes(bool alsoIncludeNotAllow
 	return ret;
 }
 
-std::vector<std::pair<CGHeroInstance*, ObjectInstanceID> > CGameState::generateCampaignHeroesToReplace(std::vector<CGHeroInstance *> & crossoverHeroes)
+std::vector<CGameState::CampaignHeroReplacement> CGameState::generateCampaignHeroesToReplace(CrossoverHeroesList & crossoverHeroes)
 {
-	std::vector<std::pair<CGHeroInstance*, ObjectInstanceID> > campaignHeroReplacements;
+	std::vector<CampaignHeroReplacement> campaignHeroReplacements;
 
 	auto removeHeroPlaceholder = [this](CGHeroPlaceholder * heroPlaceholder)
 	{
@@ -2708,56 +2751,50 @@ std::vector<std::pair<CGHeroInstance*, ObjectInstanceID> > CGameState::generateC
 	};
 
 	//selecting heroes by type
-	for(int g = 0; g < map->objects.size(); ++g)
+	for(auto obj : map->objects)
 	{
-		CGObjectInstance * obj = map->objects[g];
-		if (obj && obj->ID == Obj::HERO_PLACEHOLDER)
+		if(obj && obj->ID == Obj::HERO_PLACEHOLDER)
 		{
-			CGHeroPlaceholder * hp = static_cast<CGHeroPlaceholder*>(obj);
-
-			const ObjectInstanceID gid = ObjectInstanceID(g);
-			if(hp->subID != 0xFF) //select by type
+			auto heroPlaceholder = dynamic_cast<CGHeroPlaceholder *>(obj.get());
+			if(heroPlaceholder->subID != 0xFF) //select by type
 			{
-				bool found = false;
-				for(auto ghi : crossoverHeroes)
+				auto it = range::find_if(crossoverHeroes.heroesFromAnyPreviousScenarios, [heroPlaceholder](CGHeroInstance * hero)
 				{
-					if (ghi->subID == hp->subID)
+					return hero->subID == heroPlaceholder->subID;
+				});
+				if(it == crossoverHeroes.heroesFromAnyPreviousScenarios.end())
 				{
-						found = true;
-						campaignHeroReplacements.push_back(std::make_pair(ghi, gid));
-						crossoverHeroes -= ghi;
-						break;
+					removeHeroPlaceholder(heroPlaceholder);
 				}
-				}
-				if(!found)
+				else
 				{
-					removeHeroPlaceholder(hp);
+					campaignHeroReplacements.push_back(CampaignHeroReplacement(*it, obj->id));
+					crossoverHeroes.removeHeroFromBothLists(*it);
 				}
 			}
 		}
 	}
 
 	//selecting heroes by power
-	std::sort(crossoverHeroes.begin(), crossoverHeroes.end(), [](const CGHeroInstance * a, const CGHeroInstance * b)
+	range::sort(crossoverHeroes.heroesFromPreviousScenario, [](const CGHeroInstance * a, const CGHeroInstance * b)
 	{
 		return a->getHeroStrength() > b->getHeroStrength();
 	}); //sort, descending strength
 
 	// sort hero placeholders descending power
 	std::vector<CGHeroPlaceholder *> heroPlaceholders;
-	for(int g = 0; g < map->objects.size(); ++g)
+	for(auto obj : map->objects)
 	{
-		CGObjectInstance * obj = map->objects[g];
 		if(obj && obj->ID == Obj::HERO_PLACEHOLDER)
 		{
-			CGHeroPlaceholder * hp = dynamic_cast<CGHeroPlaceholder*>(obj);
-			if(hp->subID == 0xFF) //select by power
+			auto heroPlaceholder = dynamic_cast<CGHeroPlaceholder *>(obj.get());
+			if(heroPlaceholder->subID == 0xFF) //select by power
 			{
-				heroPlaceholders.push_back(hp);
+				heroPlaceholders.push_back(heroPlaceholder);
 			}
 		}
 	}
-	std::sort(heroPlaceholders.begin(), heroPlaceholders.end(), [](const CGHeroPlaceholder * a, const CGHeroPlaceholder * b)
+	range::sort(heroPlaceholders, [](const CGHeroPlaceholder * a, const CGHeroPlaceholder * b)
 	{
 		return a->power > b->power;
 	});
@@ -2765,9 +2802,9 @@ std::vector<std::pair<CGHeroInstance*, ObjectInstanceID> > CGameState::generateC
 	for(int i = 0; i < heroPlaceholders.size(); ++i)
 	{
 		auto heroPlaceholder = heroPlaceholders[i];
-		if(crossoverHeroes.size() > i)
+		if(crossoverHeroes.heroesFromPreviousScenario.size() > i)
 		{
-			campaignHeroReplacements.push_back(std::make_pair(crossoverHeroes[i], heroPlaceholder->id));
+			campaignHeroReplacements.push_back(CampaignHeroReplacement(crossoverHeroes.heroesFromPreviousScenario[i], heroPlaceholder->id));
 		}
 		else
 		{
@@ -2778,16 +2815,16 @@ std::vector<std::pair<CGHeroInstance*, ObjectInstanceID> > CGameState::generateC
 	return campaignHeroReplacements;
 }
 
-void CGameState::replaceHeroesPlaceholders(const std::vector<std::pair<CGHeroInstance*, ObjectInstanceID> > &campHeroReplacements)
+void CGameState::replaceHeroesPlaceholders(const std::vector<CGameState::CampaignHeroReplacement> & campaignHeroReplacements)
 {
-	for(auto obj : campHeroReplacements)
+	for(auto campaignHeroReplacement : campaignHeroReplacements)
 	{
-		CGHeroPlaceholder *placeholder = dynamic_cast<CGHeroPlaceholder*>(getObjInstance(obj.second));
+		auto heroPlaceholder = dynamic_cast<CGHeroPlaceholder*>(getObjInstance(campaignHeroReplacement.heroPlaceholderId));
 
-		CGHeroInstance *heroToPlace = obj.first;
-		heroToPlace->id = obj.second;
-		heroToPlace->tempOwner = placeholder->tempOwner;
-		heroToPlace->pos = placeholder->pos;
+		CGHeroInstance *heroToPlace = campaignHeroReplacement.hero;
+		heroToPlace->id = campaignHeroReplacement.heroPlaceholderId;
+		heroToPlace->tempOwner = heroPlaceholder->tempOwner;
+		heroToPlace->pos = heroPlaceholder->pos;
 		heroToPlace->type = VLC->heroh->heroes[heroToPlace->subID];
 
 		for(auto &&i : heroToPlace->stacks)
@@ -2808,6 +2845,8 @@ void CGameState::replaceHeroesPlaceholders(const std::vector<std::pair<CGHeroIns
 		map->heroesOnMap.push_back(heroToPlace);
 		map->objects[heroToPlace->id.getNum()] = heroToPlace;
 		map->addBlockVisTiles(heroToPlace);
+
+		scenarioOps->campState->getCurrentScenario().placedCrossoverHeroes.push_back(heroToPlace);
 	}
 }
 

lib/CGameState.h

@@ -460,6 +460,20 @@ public:
 	}
 
 private:
+	struct CrossoverHeroesList
+	{
+		std::vector<CGHeroInstance *> heroesFromPreviousScenario, heroesFromAnyPreviousScenarios;
+		void addHeroToBothLists(CGHeroInstance * hero);
+		void removeHeroFromBothLists(CGHeroInstance * hero);
+	};
+
+	struct CampaignHeroReplacement
+	{
+		CampaignHeroReplacement(CGHeroInstance * hero, ObjectInstanceID heroPlaceholderId);
+		CGHeroInstance * hero;
+		ObjectInstanceID heroPlaceholderId;
+	};
+
 	// ----- initialization -----
 
 	void initNewGame();
@@ -472,15 +486,15 @@ private:
 	void randomizeObject(CGObjectInstance *cur);
 	void initPlayerStates();
 	void placeCampaignHeroes();
-	std::vector<CGHeroInstance *> getCrossoverHeroesFromPreviousScenario() const;
-
-	/// gets prepared and copied hero instances with crossover heroes from prev. scenario and travel options from current scenario
-	std::vector<CGHeroInstance *> prepareCrossoverHeroes(const std::vector<CGHeroInstance *> & sourceCrossoverHeroes, const CScenarioTravel & travelOptions);
+	CrossoverHeroesList getCrossoverHeroesFromPreviousScenarios() const;
 
 	/// returns heroes and placeholders in where heroes will be put
-	std::vector<std::pair<CGHeroInstance*, ObjectInstanceID> > generateCampaignHeroesToReplace(std::vector<CGHeroInstance *> & crossoverHeroes);
+	std::vector<CampaignHeroReplacement> generateCampaignHeroesToReplace(CrossoverHeroesList & crossoverHeroes);
 
-	void replaceHeroesPlaceholders(const std::vector<std::pair<CGHeroInstance*, ObjectInstanceID> > &campHeroReplacements);
+	/// gets prepared and copied hero instances with crossover heroes from prev. scenario and travel options from current scenario
+	void prepareCrossoverHeroes(std::vector<CampaignHeroReplacement> & campaignHeroReplacements, const CScenarioTravel & travelOptions) const;
+
+	void replaceHeroesPlaceholders(const std::vector<CampaignHeroReplacement> & campaignHeroReplacements);
 	void placeStartingHeroes();
 	void placeStartingHero(PlayerColor playerColor, HeroTypeID heroTypeId, int3 townPos);
 	void initStartingResources();

lib/CMakeLists.txt

@@ -94,6 +94,7 @@ set(lib_HEADERS
 		int3.h
 		Interprocess.h
 		NetPacks.h
+		NetPacksBase.h
 		RegisterTypes.h
 		StartInfo.h
 		UnlockGuard.h

lib/CObjectHandler.cpp

@@ -519,6 +519,16 @@ bool CGObjectInstance::passableFor(PlayerColor color) const
 	return getPassableness() & 1<<color.getNum();
 }
 
+CGObjectInstanceBySubIdFinder::CGObjectInstanceBySubIdFinder(CGObjectInstance * obj) : obj(obj)
+{
+
+}
+
+bool CGObjectInstanceBySubIdFinder::operator()(CGObjectInstance * obj) const
+{
+	return this->obj->subID == obj->subID;
+}
+
 static int lowestSpeed(const CGHeroInstance * chi)
 {
 	if(!chi->Slots().size())

lib/CObjectHandler.h

@@ -228,6 +228,18 @@ protected:
 	void getNameVis(std::string &hname) const;
 	void giveDummyBonus(ObjectInstanceID heroID, ui8 duration = Bonus::ONE_DAY) const;
 };
+
+/// function object which can be used to find an object with an specific sub ID
+class CGObjectInstanceBySubIdFinder
+{
+public:
+	CGObjectInstanceBySubIdFinder(CGObjectInstance * obj);
+	bool operator()(CGObjectInstance * obj) const;
+
+private:
+	CGObjectInstance * obj;
+};
+
 class CGHeroPlaceholder : public CGObjectInstance
 {
 public:

lib/mapping/CCampaignHandler.cpp

@@ -337,6 +337,23 @@ const CGHeroInstance * CCampaignScenario::strongestHero( PlayerColor owner ) con
 	return i == ownedHeroes.end() ? nullptr : *i;
 }
 
+std::vector<CGHeroInstance *> CCampaignScenario::getLostCrossoverHeroes() const
+{
+	std::vector<CGHeroInstance *> lostCrossoverHeroes;
+	if(conquered)
+	{
+		for(auto hero : placedCrossoverHeroes)
+		{
+			auto it = range::find_if(crossoverHeroes, CGObjectInstanceBySubIdFinder(hero));
+			if(it == crossoverHeroes.end())
+			{
+				lostCrossoverHeroes.push_back(hero);
+			}
+		}
+	}
+	return std::move(lostCrossoverHeroes);
+}
+
 bool CScenarioTravel::STravelBonus::isBonusForHero() const
 {
 	return type == SPELL || type == MONSTER || type == ARTIFACT || type == SPELL_SCROLL || type == PRIMARY_SKILL
@@ -376,6 +393,11 @@ const CCampaignScenario & CCampaignState::getCurrentScenario() const
 	return camp->scenarios[*currentMap];
 }
 
+CCampaignScenario & CCampaignState::getCurrentScenario()
+{
+	return camp->scenarios[*currentMap];
+}
+
 ui8 CCampaignState::currentBonusID() const
 {
 	return chosenCampaignBonuses.at(*currentMap);

lib/mapping/CCampaignHandler.h

@@ -109,15 +109,17 @@ public:
 
 	CScenarioTravel travelOptions;
 	std::vector<CGHeroInstance *> crossoverHeroes; // contains all heroes with the same state when the campaign scenario was finished
+	std::vector<CGHeroInstance *> placedCrossoverHeroes; // contains all placed crossover heroes defined by hero placeholders when the scenario was started
 
 	const CGHeroInstance * strongestHero(PlayerColor owner) const;
 	void loadPreconditionRegions(ui32 regions);
 	bool isNotVoid() const;
+	std::vector<CGHeroInstance *> getLostCrossoverHeroes() const; /// returns a list of crossover heroes which started the scenario, but didn't complete it
 
 	template <typename Handler> void serialize(Handler &h, const int formatVersion)
 	{
 		h & mapName & scenarioName & packedMapSize & preconditionRegions & regionColor & difficulty & conquered & regionText & 
-			prolog & epilog & travelOptions & crossoverHeroes;
+			prolog & epilog & travelOptions & crossoverHeroes & placedCrossoverHeroes;
 	}
 };
 
@@ -151,7 +153,8 @@ public:
 	//void initNewCampaign(const StartInfo &si);
 	void setCurrentMapAsConquered(const std::vector<CGHeroInstance*> & heroes);
 	boost::optional<CScenarioTravel::STravelBonus> getBonusForCurrentMap() const;
-	const CCampaignScenario &getCurrentScenario() const;
+	const CCampaignScenario & getCurrentScenario() const;
+	CCampaignScenario & getCurrentScenario();
 	ui8 currentBonusID() const;
 
 	CCampaignState();

lib/mapping/MapFormatH3M.cpp

@@ -9,23 +9,21 @@
  */
 
 #include "StdInc.h"
-#include "MapFormatH3M.h"
-
 #include <boost/crc.hpp>
 
-#include "../CStopWatch.h"
-
-#include "../filesystem/Filesystem.h"
+#include "MapFormatH3M.h"
 #include "CMap.h"
 
+#include "../CStopWatch.h"
+#include "../filesystem/Filesystem.h"
 #include "../CSpellHandler.h"
 #include "../CCreatureHandler.h"
 #include "../CGeneralTextHandler.h"
 #include "../CHeroHandler.h"
 #include "../CObjectHandler.h"
 #include "../CDefObjInfoHandler.h"
-
 #include "../VCMI_Lib.h"
+#include "../NetPacksBase.h"
 
 
 const bool CMapLoaderH3M::IS_PROFILING_ENABLED = false;
@@ -442,7 +440,6 @@ void CMapLoaderH3M::readVictoryLossConditions()
 			{
 				EventCondition cond(EventCondition::CONTROL);
 				cond.objectType = Obj::CREATURE_GENERATOR1; // FIXME: generators 2-4?
-				cond.position = readInt3();
 
 				specialVictory.effect.toOtherMessage = VLC->generaltexth->allTexts[289];
 				specialVictory.onFulfill = VLC->generaltexth->allTexts[288];
@@ -473,7 +470,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 		default:
 			assert(0);
 		}
-		//bool allowNormalVictory = reader.readBool();
+
 		// if condition is human-only turn it into following construction: AllOf(human, condition)
 		if (!appliesToAI)
 		{
@@ -869,7 +866,17 @@ bool CMapLoaderH3M::loadArtifactToSlot(CGHeroInstance * hero, int slot)
 			slot = ArtifactPosition::SPELLBOOK;
 		}
 
-		hero->putArtifact(ArtifactPosition(slot), createArtifact(aid));
+		// this is needed, because some H3M maps (last scenario of ROE map) contain invalid data like misplaced artifacts
+		auto artifact = createArtifact(aid);
+		auto artifactPos = ArtifactPosition(slot);
+		if (artifact->canBePutAt(ArtifactLocation(hero, artifactPos)))
+		{
+			hero->putArtifact(artifactPos, artifact);
+		}
+		else
+		{
+			logGlobal->debugStream() << "Artifact can't be put at the specified location."; //TODO add more debugging information
+		}
 	}
 
 	return isArt;