Access to heroes storage in CMap is now done via public methods

AI/Nullkiller/Pathfinding/Actions/QuestAction.cpp

@@ -12,7 +12,7 @@
 #include "QuestAction.h"
 #include "../../AIGateway.h"
 #include "../../Goals/CompleteQuest.h"
-#include "../../../lib/mapObjects/CQuest.h"
+#include "../../../../lib/mapObjects/CQuest.h"
 
 namespace NKAI
 {

client/NetPacksClient.cpp

@@ -744,12 +744,12 @@ void ApplyClientNetPackVisitor::visitMapObjectSelectDialog(MapObjectSelectDialog
 void ApplyFirstClientNetPackVisitor::visitBattleStart(BattleStart & pack)
 {
 	// Cannot use the usual code because curB is not set yet
-	callOnlyThatBattleInterface(cl, pack.info->getSide(BattleSide::ATTACKER).color, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->getSide(BattleSide::ATTACKER).armyObject, pack.info->getSide(BattleSide::DEFENDER).armyObject,
-		pack.info->tile, pack.info->getSide(BattleSide::ATTACKER).hero, pack.info->getSide(BattleSide::DEFENDER).hero);
-	callOnlyThatBattleInterface(cl, pack.info->getSide(BattleSide::DEFENDER).color, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->getSide(BattleSide::ATTACKER).armyObject, pack.info->getSide(BattleSide::DEFENDER).armyObject,
-		pack.info->tile, pack.info->getSide(BattleSide::ATTACKER).hero, pack.info->getSide(BattleSide::DEFENDER).hero);
-	callOnlyThatBattleInterface(cl, PlayerColor::SPECTATOR, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->getSide(BattleSide::ATTACKER).armyObject, pack.info->getSide(BattleSide::DEFENDER).armyObject,
-		pack.info->tile, pack.info->getSide(BattleSide::ATTACKER).hero, pack.info->getSide(BattleSide::DEFENDER).hero);
+	callOnlyThatBattleInterface(cl, pack.info->getSide(BattleSide::ATTACKER).color, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->getSideArmy(BattleSide::ATTACKER), pack.info->getSideArmy(BattleSide::DEFENDER),
+		pack.info->tile, pack.info->getSide(BattleSide::ATTACKER).hero, pack.info->getSideHero(BattleSide::DEFENDER));
+	callOnlyThatBattleInterface(cl, pack.info->getSide(BattleSide::DEFENDER).color, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->getSideArmy(BattleSide::ATTACKER), pack.info->getSideArmy(BattleSide::DEFENDER),
+		pack.info->tile, pack.info->getSide(BattleSide::ATTACKER).hero, pack.info->getSideHero(BattleSide::DEFENDER));
+	callOnlyThatBattleInterface(cl, PlayerColor::SPECTATOR, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->getSideArmy(BattleSide::ATTACKER), pack.info->getSideArmy(BattleSide::DEFENDER),
+		pack.info->tile, pack.info->getSide(BattleSide::ATTACKER).hero, pack.info->getSideHero(BattleSide::DEFENDER));
 }
 
 void ApplyClientNetPackVisitor::visitBattleStart(BattleStart & pack)

client/mapView/MapRenderer.cpp

@@ -439,8 +439,8 @@ std::shared_ptr<CAnimation> MapRendererObjects::getFlagAnimation(const CGObjectI
 	if(obj->ID == Obj::BOAT)
 	{
 		const auto * boat = dynamic_cast<const CGBoat *>(obj);
-		if(boat && boat->hero && !boat->flagAnimations[boat->hero->tempOwner.getNum()].empty())
-			return getAnimation(boat->flagAnimations[boat->hero->tempOwner.getNum()], true, false);
+		if(boat && boat->getBoardedHero() && !boat->flagAnimations[boat->getBoardedHero()->tempOwner.getNum()].empty())
+			return getAnimation(boat->flagAnimations[boat->getBoardedHero()->tempOwner.getNum()], true, false);
 	}
 
 	return nullptr;
@@ -452,7 +452,7 @@ std::shared_ptr<CAnimation> MapRendererObjects::getOverlayAnimation(const CGObje
 	{
 		// Boats have additional animation with waves around boat
 		const auto * boat = dynamic_cast<const CGBoat *>(obj);
-		if(boat && boat->hero && !boat->overlayAnimation.empty())
+		if(boat && boat->getBoardedHero() && !boat->overlayAnimation.empty())
 			return getAnimation(boat->overlayAnimation, true, false);
 	}
 	return nullptr;

client/mapView/MapRendererContext.cpp

@@ -45,8 +45,8 @@ uint32_t MapRendererBaseContext::getObjectRotation(ObjectInstanceID objectID) co
 	{
 		const auto * boat = dynamic_cast<const CGBoat *>(obj);
 
-		if(boat->hero)
-			return boat->hero->moveDir;
+		if(boat->getBoardedHero())
+			return boat->getBoardedHero()->moveDir;
 		return boat->direction;
 	}
 	return 0;

client/mapView/MapViewController.cpp

@@ -182,7 +182,7 @@ void MapViewController::tick(uint32_t timeDelta)
 		assert(boat || hero);
 
 		if(!hero)
-			hero = boat->hero;
+			hero = boat->getBoardedHero();
 
 		double heroMoveTime = GAME->interface()->playerID == hero->getOwner() ?
 			settings["adventure"]["heroMoveTime"].Float() :
@@ -249,7 +249,7 @@ void MapViewController::afterRender()
 		assert(boat || hero);
 
 		if(!hero)
-			hero = boat->hero;
+			hero = boat->getBoardedHero();
 
 		if(movementContext->progress >= 0.999)
 		{
@@ -406,10 +406,10 @@ void MapViewController::removeObject(const CGObjectInstance * obj)
 	if (obj->ID == Obj::BOAT)
 	{
 		auto * boat = dynamic_cast<const CGBoat*>(obj);
-		if (boat->hero)
+		if (boat->getBoardedHero())
 		{
-			view->invalidate(context, boat->hero->id);
-			state->removeObject(boat->hero);
+			view->invalidate(context, boat->getBoardedHero()->id);
+			state->removeObject(boat->getBoardedHero());
 		}
 	}
 

lib/CGameInfoCallback.cpp

@@ -914,16 +914,6 @@ const TeamState * CGameInfoCallback::getPlayerTeam( PlayerColor color ) const
 	}
 }
 
-const CGHeroInstance * CGameInfoCallback::getHeroWithSubid( int subid ) const
-{
-	if(subid<0)
-		return nullptr;
-	if(subid>= gs->getMap().allHeroes.size())
-		return nullptr;
-
-	return gs->getMap().allHeroes.at(subid).get();
-}
-
 bool CGameInfoCallback::isInTheMap(const int3 &pos) const
 {
 	return gs->getMap().isInTheMap(pos);

lib/CGameInfoCallback.h

@@ -80,7 +80,6 @@ public:
 
 	//hero
 	virtual const CGHeroInstance * getHero(ObjectInstanceID objid) const = 0;
-	virtual const CGHeroInstance * getHeroWithSubid(int subid) const = 0;
 //	int getHeroCount(PlayerColor player, bool includeGarrisoned) const;
 //	bool getHeroInfo(const CGObjectInstance * hero, InfoAboutHero & dest, const CGObjectInstance * selectedObject = nullptr) const;
 //	int32_t getSpellCost(const spells::Spell * sp, const CGHeroInstance * caster) const; //when called during battle, takes into account creatures' spell cost reduction
@@ -177,7 +176,6 @@ public:
 
 	//hero
 	const CGHeroInstance * getHero(ObjectInstanceID objid) const override;
-	const CGHeroInstance * getHeroWithSubid(int subid) const override;
 	virtual int getHeroCount(PlayerColor player, bool includeGarrisoned) const;
 	virtual bool getHeroInfo(const CGObjectInstance * hero, InfoAboutHero & dest, const CGObjectInstance * selectedObject = nullptr) const;
 	virtual int32_t getSpellCost(const spells::Spell * sp, const CGHeroInstance * caster) const; //when called during battle, takes into account creatures' spell cost reduction

lib/gameState/CGameState.cpp

@@ -230,17 +230,26 @@ void CGameState::updateEntity(Metatype metatype, int32_t index, const JsonNode &
 		logGlobal->error("Creature instance update is not implemented");
 		break;
 	case Metatype::HERO_INSTANCE:
+	{
 		//index is hero type
-		if(index >= 0 && index < map->allHeroes.size())
+		HeroTypeID heroID(index);
+		auto heroOnMap = map->getHero(heroID);
+		auto heroInPool = map->tryGetFromHeroPool(heroID);
+
+		if(heroOnMap)
+		{
+			heroOnMap->updateFrom(data);
+		}
+		else if (heroInPool)
 		{
-			const auto & hero = map->allHeroes.at(index);
-			hero->updateFrom(data);
+			heroInPool->updateFrom(data);
 		}
 		else
 		{
-			logGlobal->error("Update entity: hero index %s is out of range [%d,%d]", index, 0,  map->allHeroes.size());
+			logGlobal->error("Update entity: hero index %s is out of range", index);
 		}
 		break;
+	}
 	case Metatype::MAP_OBJECT_INSTANCE:
 		if(index >= 0 && index < map->objects.size())
 		{
@@ -625,22 +634,22 @@ void CGameState::initHeroes()
 	}
 
 	std::set<HeroTypeID> heroesToCreate = getUnusedAllowedHeroes(); //ids of heroes to be created and put into the pool
-	for(auto ph : map->predefinedHeroes)
-	{
-		if(!vstd::contains(heroesToCreate, ph->getHeroTypeID()))
-			continue;
-		ph->initHero(getRandomGenerator());
-		heroesPool->addHeroToPool(ph);
-		heroesToCreate.erase(ph->getHeroTypeID());
-	}
 
 	for(const HeroTypeID & htype : heroesToCreate) //all not used allowed heroes go with default state into the pool
 	{
-		auto vhi = std::make_shared<CGHeroInstance>(callback);
-		vhi->initHero(getRandomGenerator(), htype);
+		auto vhi = map->tryTakeFromHeroPool(htype);
+
+		if (vhi)
+		{
+			vhi->initHero(getRandomGenerator());
+		}
+		else
+		{
+			auto vhi = std::make_shared<CGHeroInstance>(callback);
+			vhi->initHero(getRandomGenerator(), htype);
+		}
 
-		int typeID = htype.getNum();
-		map->allHeroes[typeID] = vhi;
+		map->addToHeroPool(vhi);
 		heroesPool->addHeroToPool(vhi);
 	}
 

lib/mapObjects/CGHeroInstance.cpp

@@ -1299,11 +1299,10 @@ void CGHeroInstance::attachToBoat(CGBoat* newBoat)
 {
 	assert(newBoat);
 	boat = newBoat;
-	attachTo(const_cast<CGBoat&>(*boat));
-	const_cast<CGBoat*>(boat)->hero = this;
+	attachTo(*newBoat);
+	newBoat->setBoardedHero(this);
 }
 
-
 void CGHeroInstance::deserializationFix()
 {
 	artDeserializationFix(this);

lib/mapObjects/MiscObjects.cpp

@@ -987,7 +987,6 @@ void CGMagi::onHeroVisit(const CGHeroInstance * h) const
 CGBoat::CGBoat(IGameCallback * cb)
 	: CGObjectInstance(cb)
 {
-	hero = nullptr;
 	direction = 4;
 	layer = EPathfindingLayer::SAIL;
 }
@@ -997,6 +996,22 @@ bool CGBoat::isCoastVisitable() const
 	return true;
 }
 
+void CGBoat::setBoardedHero(const CGHeroInstance * hero)
+{
+	if (hero)
+		boardedHeroID = hero->id;
+	else
+		boardedHeroID = ObjectInstanceID();
+}
+
+const CGHeroInstance * CGBoat::getBoardedHero() const
+{
+	if (boardedHeroID.hasValue())
+		return cb->getHero(boardedHeroID);
+	else
+		return nullptr;
+}
+
 void CGSirens::initObj(vstd::RNG & rand)
 {
 	blockVisit = true;

lib/mapObjects/MiscObjects.h

@@ -284,11 +284,12 @@ public:
 
 class DLL_LINKAGE CGBoat : public CGObjectInstance, public CBonusSystemNode
 {
+	ObjectInstanceID boardedHeroID;
+
 public:
 	using CGObjectInstance::CGObjectInstance;
 
 	ui8 direction;
-	const CGHeroInstance *hero;  //hero on board
 	bool onboardAssaultAllowed; //if true, hero can attack units from transport
 	bool onboardVisitAllowed; //if true, hero can visit objects from transport
 	EPathfindingLayer layer;
@@ -301,12 +302,15 @@ public:
 	CGBoat(IGameCallback * cb);
 	bool isCoastVisitable() const override;
 
+	void setBoardedHero(const CGHeroInstance * hero);
+	const CGHeroInstance * getBoardedHero() const;
+
 	template <typename Handler> void serialize(Handler &h)
 	{
 		h & static_cast<CGObjectInstance&>(*this);
 		h & static_cast<CBonusSystemNode&>(*this);
 		h & direction;
-		h & hero;
+		h & boardedHeroID;
 		h & layer;
 		h & onboardAssaultAllowed;
 		h & onboardVisitAllowed;

lib/mapping/CMap.cpp

@@ -178,7 +178,7 @@ CMap::CMap(IGameCallback * cb)
 	, waterMap(false)
 	, uidCounter(0)
 {
-	allHeroes.resize(LIBRARY->heroh->size());
+	heroesPool.resize(LIBRARY->heroh->size());
 	allowedAbilities = LIBRARY->skillh->getDefaultAllowed();
 	allowedArtifact = LIBRARY->arth->getDefaultAllowed();
 	allowedSpells = LIBRARY->spellh->getDefaultAllowed();
@@ -187,12 +187,7 @@ CMap::CMap(IGameCallback * cb)
 	gameSettings->loadBase(LIBRARY->settingsHandler->getFullConfig());
 }
 
-CMap::~CMap()
-{
-	getEditManager()->getUndoManager().clearAll();
-
-	resetStaticData();
-}
+CMap::~CMap() = default;
 
 void CMap::removeBlockVisTiles(CGObjectInstance * obj, bool total)
 {
@@ -255,8 +250,13 @@ void CMap::calculateGuardingGreaturePositions()
 
 CGHeroInstance * CMap::getHero(HeroTypeID heroID)
 {
-	if (vstd::contains(heroesOnMap, heroID))
-		return allHeroes.at(heroID.getNum()).get();
+	for (const auto & objectID : heroesOnMap)
+	{
+		const auto hero = std::dynamic_pointer_cast<CGHeroInstance>(objects.at(objectID.getNum()));
+
+		if (hero->getHeroTypeID() == heroID)
+			return hero.get();
+	}
 	return nullptr;
 }
 
@@ -727,14 +727,6 @@ CMapEditManager * CMap::getEditManager()
 	return editManager.get();
 }
 
-void CMap::resetStaticData()
-{
-	obeliskCount = 0;
-	obelisksVisited.clear();
-	townMerchantArtifacts.clear();
-	townUniversitySkills.clear();
-}
-
 void CMap::resolveQuestIdentifiers()
 {
 	//FIXME: move to CMapLoaderH3M
@@ -865,5 +857,35 @@ void CMap::postInitialize()
 	});
 }
 
+void CMap::addToHeroPool(std::shared_ptr<CGHeroInstance> hero)
+{
+	assert(hero->getHeroTypeID().isValid());
+	assert(!vstd::contains(heroesOnMap, hero->getHeroTypeID()));
+	assert(heroesPool.at(hero->getHeroTypeID().getNum()) == nullptr);
+
+	heroesPool.at(hero->getHeroTypeID().getNum()) = hero;
+}
+
+CGHeroInstance * CMap::tryGetFromHeroPool(HeroTypeID hero)
+{
+	return heroesPool.at(hero.getNum()).get();
+}
+
+std::shared_ptr<CGHeroInstance> CMap::tryTakeFromHeroPool(HeroTypeID hero)
+{
+	auto result = heroesPool.at(hero.getNum());
+	heroesPool.at(hero.getNum()) = nullptr;
+	return result;
+}
+
+std::vector<HeroTypeID> CMap::getHeroesInPool() const
+{
+	std::vector<HeroTypeID> result;
+	for (const auto & hero : heroesPool)
+		if (hero)
+			result.push_back(hero->getHeroTypeID());
+
+	return result;
+}
 
 VCMI_LIB_NAMESPACE_END

lib/mapping/CMap.h

@@ -62,11 +62,17 @@ class DLL_LINKAGE CMap : public CMapHeader, public GameCallbackHolder
 
 	std::unique_ptr<GameSettings> gameSettings;
 
+	/// All quests that are currently present on map
 	std::vector<std::shared_ptr<CQuest>> quests;
+	/// All artifacts that exists on map, whether on map, in hero inventory, or stored in some object
 	std::vector<std::shared_ptr<CArtifactInstance>> artInstances;
+	/// All heroes that are currently free for recruitment in taverns and are not present on map
+	std::vector<std::shared_ptr<CGHeroInstance> > heroesPool;
 
+	/// Precomputed indices of all towns on map
 	std::vector<ObjectInstanceID> towns;
 
+	/// Precomputed indices of all towns on map. Does not includes heroes in prisons
 	std::vector<ObjectInstanceID> heroesOnMap;
 
 public:
@@ -127,6 +133,11 @@ public:
 	void townAddedToMap(const CGTownInstance * town);
 	void townRemovedFromMap(const CGTownInstance * town);
 
+	void addToHeroPool(std::shared_ptr<CGHeroInstance> hero);
+	std::shared_ptr<CGHeroInstance> tryTakeFromHeroPool(HeroTypeID hero);
+	CGHeroInstance * tryGetFromHeroPool(HeroTypeID hero);
+	std::vector<HeroTypeID> getHeroesInPool() const;
+
 	bool isWaterMap() const;
 	bool calculateWaterContent();
 	void banWaterArtifacts();
@@ -139,6 +150,8 @@ public:
 
 	/// Gets object of specified type on requested position
 	const CGObjectInstance * getObjectiveObjectFrom(const int3 & pos, Obj type);
+
+	/// Returns pointer to hero of specified type if hero is present on map
 	CGHeroInstance * getHero(HeroTypeID heroId);
 
 	/// Returns ID's of all heroes that are currently present on map
@@ -152,7 +165,6 @@ public:
 	/// Sets the victory/loss condition objectives ??
 	void checkForObjectives();
 
-	void resetStaticData();
 	void resolveQuestIdentifiers();
 
 	void reindexObjects();
@@ -167,11 +179,9 @@ public:
 
 	//Central lists of items in game. Position of item in the vectors below is their (instance) id.
 	std::vector< std::shared_ptr<CGObjectInstance> > objects;
-	std::vector< std::shared_ptr<CGHeroInstance> > allHeroes; //indexed by [hero_type_id]; on map, disposed, prisons, etc.
 
 	//Helper lists
 	std::map<TeleportChannelID, std::shared_ptr<TeleportChannel> > teleportChannels;
-	std::vector<std::shared_ptr<CGHeroInstance> > predefinedHeroes;
 
 	/// associative list to identify which hero/creature id belongs to which object id(index for objects)
 	std::map<si32, ObjectInstanceID> questIdentifierToId;
@@ -215,7 +225,6 @@ public:
 		h & grailPos;
 		h & artInstances;
 		h & quests;
-		h & allHeroes;
 
 		//TODO: viccondetails
 		h & terrain;

lib/mapping/MapFormatH3M.cpp

@@ -871,7 +871,7 @@ void CMapLoaderH3M::readPredefinedHeroes()
 		if(!custom)
 			continue;
 
-		auto * hero = new CGHeroInstance(map->cb);
+		auto hero = std::make_shared<CGHeroInstance>(map->cb);
 		hero->ID = Obj::HERO;
 		hero->subID = heroID;
 
@@ -897,7 +897,7 @@ void CMapLoaderH3M::readPredefinedHeroes()
 			}
 		}
 
-		loadArtifactsOfHero(hero);
+		loadArtifactsOfHero(hero.get());
 
 		bool hasCustomBio = reader->readBool();
 		if(hasCustomBio)
@@ -919,7 +919,7 @@ void CMapLoaderH3M::readPredefinedHeroes()
 				hero->pushPrimSkill(static_cast<PrimarySkill>(skillID), reader->readUInt8());
 			}
 		}
-		map->predefinedHeroes.emplace_back(hero);
+		map->addToHeroPool(hero);
 
 		logGlobal->debug("Map '%s': Hero predefined in map: %s", mapName, hero->getHeroType()->getJsonKey());
 	}
@@ -2032,8 +2032,6 @@ void CMapLoaderH3M::setOwnerAndValidate(const int3 & mapPosition, CGObjectInstan
 
 std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readHero(const int3 & mapPosition, const ObjectInstanceID & objectInstanceID)
 {
-	auto object = std::make_shared<CGHeroInstance>(map->cb);
-
 	if(features.levelAB)
 	{
 		unsigned int identifier = reader->readUInt32();
@@ -2041,18 +2039,15 @@ std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readHero(const int3 & mapPositi
 	}
 
 	PlayerColor owner = reader->readPlayer();
-	object->subID = reader->readHero().getNum();
+	HeroTypeID heroType = reader->readHero();
 
 	//If hero of this type has been predefined, use that as a base.
 	//Instance data will overwrite the predefined values where appropriate.
-	for(auto & elem : map->predefinedHeroes)
+	auto object = map->tryTakeFromHeroPool(heroType);
+	if (!object)
 	{
-		if(elem->subID == object->subID)
-		{
-			logGlobal->debug("Hero %d will be taken from the predefined heroes list.", object->subID);
-			object = elem;
-			break;
-		}
+		object = std::make_shared<CGHeroInstance>(map->cb);
+		object->subID = heroType.getNum();
 	}
 
 	setOwnerAndValidate(mapPosition, object.get(), owner);

lib/mapping/MapFormatJson.cpp

@@ -679,19 +679,19 @@ void CMapFormatJson::serializeTimedEvents(JsonSerializeFormat & handler)
 
 void CMapFormatJson::serializePredefinedHeroes(JsonSerializeFormat & handler)
 {
-	//todo:serializePredefinedHeroes
-
 	if(handler.saving)
 	{
-		if(!map->predefinedHeroes.empty())
+		auto heroPool = map->getHeroesInPool();
+		if(!heroPool.empty())
 		{
 			auto predefinedHeroes = handler.enterStruct("predefinedHeroes");
 
-			for(auto & hero : map->predefinedHeroes)
+			for(auto & heroID : heroPool)
 			{
-				auto predefinedHero = handler.enterStruct(hero->getHeroTypeName());
+				auto heroPtr = map->tryGetFromHeroPool(heroID);
+				auto predefinedHero = handler.enterStruct(heroPtr->getHeroTypeName());
 
-				hero->serializeJsonDefinition(handler);
+				heroPtr->serializeJsonDefinition(handler);
 			}
 		}
 	}
@@ -705,12 +705,12 @@ void CMapFormatJson::serializePredefinedHeroes(JsonSerializeFormat & handler)
 		{
 			auto predefinedHero = handler.enterStruct(p.first);
 
-			auto * hero = new CGHeroInstance(map->cb);
+			auto hero = std::make_shared<CGHeroInstance>(map->cb);
 			hero->ID = Obj::HERO;
 			hero->setHeroTypeName(p.first);
 			hero->serializeJsonDefinition(handler);
 
-			map->predefinedHeroes.emplace_back(hero);
+			map->addToHeroPool(hero);
 		}
 	}
 }

lib/networkPacks/NetPacksLib.cpp

@@ -1073,9 +1073,11 @@ void ChangeObjectVisitors::applyGs(CGameState *gs)
 			break;
 		case VISITOR_CLEAR:
 			// remove visit info from all heroes, including those that are not present on map
-			for (auto hero : gs->getMap().allHeroes)
-				if (hero)
-					hero->visitedObjects.erase(object);
+			for (auto heroID : gs->getMap().getHeroesOnMap())
+				gs->getHero(heroID)->visitedObjects.erase(object);
+
+			for (auto heroID : gs->getMap().getHeroesInPool())
+				gs->getMap().tryGetFromHeroPool(heroID)->visitedObjects.erase(object);
 
 			for(auto &elem : gs->players)
 				elem.second.visitedObjects.erase(object);
@@ -1329,14 +1331,14 @@ void TryMoveHero::applyGs(CGameState *gs)
 		gs->getMap().removeBlockVisTiles(boat); //hero blockvis mask will be used, we don't need to duplicate it with boat
 		h->boat = boat;
 		h->attachTo(*boat);
-		boat->hero = h;
+		boat->setBoardedHero(h);
 	}
 	else if(result == DISEMBARK) //hero leaves boat to destination tile
 	{
 		auto * b = const_cast<CGBoat *>(h->boat);
 		b->direction = h->moveDir;
 		b->pos = start;
-		b->hero = nullptr;
+		b->setBoardedHero(nullptr);
 		gs->getMap().addBlockVisTiles(b);
 		h->detachFrom(*b);
 		h->boat = nullptr;
@@ -2093,17 +2095,18 @@ void BattleCancelled::applyGs(CGameState *gs)
 void BattleResultAccepted::applyGs(CGameState *gs)
 {
 	// Remove any "until next battle" bonuses
-	if(const auto attackerHero = gs->getHero(heroResult[BattleSide::ATTACKER].heroId))
+	if(const auto attackerHero = gs->getHero(heroResult[BattleSide::ATTACKER].heroID))
 		attackerHero->removeBonusesRecursive(Bonus::OneBattle);
-	if(const auto defenderHero = gs->getHero(heroResult[BattleSide::DEFENDER].heroId))
+	if(const auto defenderHero = gs->getHero(heroResult[BattleSide::DEFENDER].heroID))
 		defenderHero->removeBonusesRecursive(Bonus::OneBattle);
 
 	if(winnerSide != BattleSide::NONE)
 	{
 		// Grow up growing artifacts
-		if(const auto winnerHero = gs->getHero(heroResult[winnerSide].heroId))
+		if(const auto winnerHero = gs->getHero(heroResult[winnerSide].heroID))
 		{
 			if(winnerHero->getCommander() && winnerHero->getCommander()->alive)
+
 			{
 				for(auto & art : winnerHero->getCommander()->artifactsWorn)
 					gs->getArtInstance(art.second.getID())->growingUp();
@@ -2115,12 +2118,12 @@ void BattleResultAccepted::applyGs(CGameState *gs)
 
 	if(gs->getSettings().getBoolean(EGameSettings::MODULE_STACK_EXPERIENCE))
 	{
-		if(const auto attackerArmy = gs->getArmyInstance(heroResult[BattleSide::ATTACKER].armyId))
+		if(const auto attackerArmy = gs->getArmyInstance(heroResult[BattleSide::ATTACKER].armyID))
 		{
 			attackerArmy->giveStackExp(heroResult[BattleSide::ATTACKER].exp);
 			attackerArmy->nodeHasChanged();
 		}
-		if(const auto defenderArmy = gs->getArmyInstance(heroResult[BattleSide::DEFENDER].armyId))
+		if(const auto defenderArmy = gs->getArmyInstance(heroResult[BattleSide::DEFENDER].armyID))
 		{
 			defenderArmy->giveStackExp(heroResult[BattleSide::DEFENDER].exp);
 			defenderArmy->nodeHasChanged();

lib/networkPacks/PacksForClientBattle.h

@@ -95,19 +95,14 @@ struct DLL_LINKAGE BattleResultAccepted : public CPackForClient
 
 	struct HeroBattleResults
 	{
-		HeroBattleResults()
-			: heroId(ObjectInstanceID::NONE)
-			, armyId(ObjectInstanceID::NONE)
-			, exp(0) {}
-
-		ObjectInstanceID heroId;
-		ObjectInstanceID armyId;
-		TExpType exp;
+		ObjectInstanceID heroID;
+		ObjectInstanceID armyID;
+		TExpType exp = 0;
 
 		template <typename Handler> void serialize(Handler & h)
 		{
-			h & armyId;
-			h & heroId;
+			h & heroID;
+			h & armyID;
 			h & exp;
 		}
 	};

lib/serializer/CSerializer.cpp

@@ -23,10 +23,8 @@ CSerializer::~CSerializer() = default;
 
 void CSerializer::addStdVecItems(CGameState *gs, GameLibrary *lib)
 {
-//	registerVectoredType<CGObjectInstance, ObjectInstanceID>(&gs->getMap().objects,
-//		[](const CGObjectInstance &obj){ return obj.id; });
-	registerVectoredType<CGHeroInstance, HeroTypeID>(&gs->getMap().allHeroes,
-		[](const CGHeroInstance &h){ return h.getHeroType()->getId(); });
+	registerVectoredType<CGObjectInstance, ObjectInstanceID>(&gs->getMap().objects,
+		[](const CGObjectInstance &obj){ return obj.id; });
 	registerVectoredType<CArtifactInstance, ArtifactInstanceID>(&gs->getMap().artInstances,
 		[](const CArtifactInstance &artInst){ return artInst.getId(); });
 	registerVectoredType<CQuest, si32>(&gs->getMap().quests,

lib/serializer/CSerializer.h

@@ -146,12 +146,6 @@ struct VectorizedTypeFor
 	using type = std::conditional_t<std::is_base_of_v<CGObjectInstance, T>, CGObjectInstance, T>;
 };
 
-template <>
-struct VectorizedTypeFor<CGHeroInstance>
-{
-	using type = CGHeroInstance;
-};
-
 template <typename T>
 struct VectorizedIDType
 {
@@ -164,12 +158,6 @@ struct VectorizedIDType<CArtifactInstance>
 	using type = ArtifactInstanceID;
 };
 
-template <>
-struct VectorizedIDType<CGHeroInstance>
-{
-	using type = HeroTypeID;
-};
-
 /// Base class for deserializers
 class DLL_LINKAGE IBinaryReader : public virtual CSerializer
 {

lib/spells/AdventureSpellMechanics.cpp

@@ -206,7 +206,7 @@ ESpellCastResult SummonBoatMechanics::applyAdventureEffects(SpellCastEnvironment
 		if(obj && obj->ID == Obj::BOAT)
 		{
 			const auto * b = dynamic_cast<const CGBoat *>(obj.get());
-			if(b->hero || b->layer != EPathfindingLayer::SAIL)
+			if(b->getBoardedHero() || b->layer != EPathfindingLayer::SAIL)
 				continue; //we're looking for unoccupied boat
 
 			double nDist = b->visitablePos().dist2d(parameters.caster->getHeroCaster()->visitablePos());

mapeditor/campaigneditor/startingbonus.cpp

@@ -53,9 +53,12 @@ StartingBonus::~StartingBonus()
 void StartingBonus::initControls()
 {
 	std::map<int, std::string> heroSelection;
-	for(auto & hero : map->heroesOnMap)
+	for(ObjectInstanceID heroID : map->getHeroesOnMap())
+	{
+		const auto * hero = dynamic_cast<const CGHeroInstance*>(map->objects.at(heroID.getNum()).get());
 		if(hero->getOwner() == color || color == PlayerColor::CANNOT_DETERMINE)
 			heroSelection.emplace(hero->getHeroTypeID(), hero->getNameTranslated());
+	}
 	heroSelection.emplace(HeroTypeID::CAMP_STRONGEST, tr("Strongest").toStdString());
 	heroSelection.emplace(HeroTypeID::CAMP_GENERATED, tr("Generated").toStdString());
 	heroSelection.emplace(HeroTypeID::CAMP_RANDOM, tr("Random").toStdString());
@@ -80,13 +83,19 @@ void StartingBonus::initControls()
 		ui->comboBoxCreatureCreatureType->addItem(QString::fromStdString(objectPtr->getNameSingularTranslated()), QVariant(objectPtr->getId()));
 	
 	if(map->players[color].hasMainTown)
-		for(auto & town : map->towns)
+	{
+		for(ObjectInstanceID townID : map->getAllTowns())
+		{
+			const auto * town = dynamic_cast<const CGTownInstance*>(map->objects.at(townID.getNum()).get());
+
 			if(town->pos == map->players[color].posOfMainTown)
 			{
 				for(auto & building : town->getTown()->buildings)
 					ui->comboBoxBuildingBuilding->addItem(QString::fromStdString(building.second->getNameTranslated()), QVariant(building.first.getNum()));
 				break;
 			}
+		}
+	}
 
 	for(auto const & objectPtr : LIBRARY->arth->objects)
 		if(!vstd::contains(std::vector<ArtifactID>({

mapeditor/mapcontroller.cpp

@@ -109,10 +109,13 @@ void MapController::repairMap(CMap * map)
 			e.name = "rumor_" + std::to_string(emptyNameId++);
 	
 	//fix owners for objects
-	auto allImpactedObjects(map->objects);
+	std::vector<CGObjectInstance*> allImpactedObjects;
 
-	for (const auto & hero : map->predefinedHeroes)
-		allImpactedObjects.push_back(hero);
+	for (const auto & object : map->objects)
+		allImpactedObjects.push_back(object.get());
+
+	for (const auto & hero : map->getHeroesInPool())
+		allImpactedObjects.push_back(map->tryGetFromHeroPool(hero));
 
 	for(auto obj : allImpactedObjects)
 	{
@@ -123,7 +126,7 @@ void MapController::repairMap(CMap * map)
 				obj->tempOwner = PlayerColor::NEUTRAL;
 		}
 		//fix hero instance
-		if(auto * nih = dynamic_cast<CGHeroInstance*>(obj.get()))
+		if(auto * nih = dynamic_cast<CGHeroInstance*>(obj))
 		{
 			// All heroes present on map or in prisons need to be allowed to rehire them after they are defeated
 
@@ -146,7 +149,7 @@ void MapController::repairMap(CMap * map)
 			
 		}
 		//fix town instance
-		if(auto * tnh = dynamic_cast<CGTownInstance*>(obj.get()))
+		if(auto * tnh = dynamic_cast<CGTownInstance*>(obj))
 		{
 			if(tnh->getTown())
 			{
@@ -162,7 +165,7 @@ void MapController::repairMap(CMap * map)
 			}
 		}
 		//fix spell scrolls
-		if(auto * art = dynamic_cast<CGArtifact*>(obj.get()))
+		if(auto * art = dynamic_cast<CGArtifact*>(obj))
 		{
 			if(art->ID == Obj::SPELL_SCROLL && !art->storedArtifact)
 			{
@@ -179,7 +182,7 @@ void MapController::repairMap(CMap * map)
 			}
 		}
 		//fix mines 
-		if(auto * mine = dynamic_cast<CGMine*>(obj.get()))
+		if(auto * mine = dynamic_cast<CGMine*>(obj))
 		{
 			if(!mine->isAbandoned())
 			{

mapeditor/maphandler.cpp

@@ -174,8 +174,8 @@ void setPlayerColor(QImage * sur, PlayerColor player)
 std::shared_ptr<QImage> MapHandler::getObjectImage(const CGObjectInstance * obj)
 {
 	if(	!obj
-	   || (obj->ID==Obj::HERO && static_cast<const CGHeroInstance*>(obj)->isGarrisoned()) //garrisoned hero
-	   || (obj->ID==Obj::BOAT && static_cast<const CGBoat*>(obj)->hero)) //boat with hero (hero graphics is used)
+	   || (obj->ID==Obj::HERO && dynamic_cast<const CGHeroInstance*>(obj)->isGarrisoned()) //garrisoned hero
+	   || (obj->ID==Obj::BOAT && dynamic_cast<const CGBoat*>(obj)->getBoardedHero())) //boat with hero (hero graphics is used)
 	{
 		return nullptr;
 	}

server/CGameHandler.cpp

@@ -555,11 +555,11 @@ void CGameHandler::init(StartInfo *si, Load::ProgressAccumulator & progressTrack
 	for (auto & elem : gs->players)
 		turnOrder->addPlayer(elem.first);
 
-	for (auto & elem : gs->getMap().allHeroes)
-	{
-		if(elem)
-			heroPool->getHeroSkillsRandomGenerator(elem->getHeroTypeID()); // init RMG seed
-	}
+//	for (auto & elem : gs->getMap().allHeroes)
+//	{
+//		if(elem)
+//			heroPool->getHeroSkillsRandomGenerator(elem->getHeroTypeID()); // init RMG seed
+//	}
 
 	reinitScripting();
 }

server/battles/BattleProcessor.cpp

@@ -84,8 +84,8 @@ void BattleProcessor::restartBattle(const BattleID & battleID, const CArmedInsta
 
 		lastBattleQuery->result = std::nullopt;
 
-		assert(lastBattleQuery->belligerents[BattleSide::ATTACKER] == battle->getSide(BattleSide::ATTACKER).armyObject);
-		assert(lastBattleQuery->belligerents[BattleSide::DEFENDER] == battle->getSide(BattleSide::DEFENDER).armyObject);
+		assert(lastBattleQuery->belligerents[BattleSide::ATTACKER] == battle->getSideArmy(BattleSide::ATTACKER));
+		assert(lastBattleQuery->belligerents[BattleSide::DEFENDER] == battle->getSideArmy(BattleSide::DEFENDER));
 	}
 
 	BattleCancelled bc;

server/battles/BattleResultProcessor.cpp

@@ -360,10 +360,10 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 
 	BattleResultAccepted raccepted;
 	raccepted.battleID = battle.getBattle()->getBattleID();
-	raccepted.heroResult[finishingBattle->winnerSide].heroId = winnerHero ? winnerHero->id : ObjectInstanceID::NONE;
-	raccepted.heroResult[CBattleInfoEssentials::otherSide(finishingBattle->winnerSide)].heroId = loserHero ? loserHero->id : ObjectInstanceID::NONE;
-	raccepted.heroResult[BattleSide::ATTACKER].armyId = battle.battleGetArmyObject(BattleSide::ATTACKER)->id;
-	raccepted.heroResult[BattleSide::DEFENDER].armyId = battle.battleGetArmyObject(BattleSide::DEFENDER)->id;
+	raccepted.heroResult[finishingBattle->winnerSide].heroID = winnerHero ? winnerHero->id : ObjectInstanceID::NONE;
+	raccepted.heroResult[CBattleInfoEssentials::otherSide(finishingBattle->winnerSide)].heroID = loserHero ? loserHero->id : ObjectInstanceID::NONE;
+	raccepted.heroResult[BattleSide::ATTACKER].armyID = battle.battleGetArmyObject(BattleSide::ATTACKER)->id;
+	raccepted.heroResult[BattleSide::DEFENDER].armyID = battle.battleGetArmyObject(BattleSide::DEFENDER)->id;
 	raccepted.heroResult[BattleSide::ATTACKER].exp = battleResult->exp[BattleSide::ATTACKER];
 	raccepted.heroResult[BattleSide::DEFENDER].exp = battleResult->exp[BattleSide::DEFENDER];
 	raccepted.winnerSide = finishingBattle->winnerSide;