Map objects now use shared_ptr (game)

client/NetPacksClient.cpp

@@ -625,18 +625,18 @@ void ApplyClientNetPackVisitor::visitSetHeroesInTown(SetHeroesInTown & pack)
 
 void ApplyClientNetPackVisitor::visitHeroRecruited(HeroRecruited & pack)
 {
-	CGHeroInstance *h = gs.getMap().heroesOnMap.back();
+	auto & h = gs.getMap().heroesOnMap.back();
 	if(h->getHeroTypeID() != pack.hid)
 	{
 		logNetwork->error("Something wrong with hero recruited!");
 	}
 
-	if(callInterfaceIfPresent(cl, h->tempOwner, &IGameEventsReceiver::heroCreated, h))
+	if(callInterfaceIfPresent(cl, h->tempOwner, &IGameEventsReceiver::heroCreated, h.get()))
 	{
 		if(const CGTownInstance *t = gs.getTown(pack.tid))
 			callInterfaceIfPresent(cl, h->getOwner(), &IGameEventsReceiver::heroInGarrisonChange, t);
 	}
-	GAME->map().onObjectInstantAdd(h, h->getOwner());
+	GAME->map().onObjectInstantAdd(h.get(), h->getOwner());
 }
 
 void ApplyClientNetPackVisitor::visitGiveHero(GiveHero & pack)
@@ -1054,7 +1054,7 @@ void ApplyClientNetPackVisitor::visitNewObject(NewObject & pack)
 {
 	callAllInterfaces(cl, &CGameInterface::invalidatePaths);
 
-	const CGObjectInstance *obj = pack.newObject;
+	const CGObjectInstance * obj = pack.newObject.get();
 	GAME->map().onObjectFadeIn(obj, pack.initiator);
 
 	for(auto i=cl.playerint.begin(); i!=cl.playerint.end(); i++)

client/adventureMap/MapAudioPlayer.cpp

@@ -209,7 +209,7 @@ MapAudioPlayer::MapAudioPlayer()
 	for(const auto & obj : GAME->map().getMap()->objects)
 	{
 		if (obj)
-			addObject(obj);
+			addObject(obj.get());
 	}
 }
 

client/mapView/MapRendererContext.cpp

@@ -103,7 +103,7 @@ const MapRendererBaseContext::MapObjectsList & MapRendererBaseContext::getObject
 
 const CGObjectInstance * MapRendererBaseContext::getObject(ObjectInstanceID objectID) const
 {
-	return GAME->map().getMap()->objects.at(objectID.getNum());
+	return GAME->map().getMap()->objects.at(objectID.getNum()).get();
 }
 
 const CGPath * MapRendererBaseContext::currentPath() const

client/mapView/MapRendererContextState.cpp

@@ -25,7 +25,7 @@
 static bool compareObjectBlitOrder(ObjectInstanceID left, ObjectInstanceID right)
 {
 	//FIXME: remove mh access
-	return GAME->map().compareObjectBlitOrder(GAME->map().getMap()->objects[left.getNum()], GAME->map().getMap()->objects[right.getNum()]);
+	return GAME->map().compareObjectBlitOrder(GAME->map().getMap()->objects.at(left.getNum()).get(), GAME->map().getMap()->objects.at(right.getNum()).get());
 }
 
 MapRendererContextState::MapRendererContextState()
@@ -36,7 +36,7 @@ MapRendererContextState::MapRendererContextState()
 
 	logGlobal->debug("Loading map objects");
 	for(const auto & obj : GAME->map().getMap()->objects)
-		addObject(obj);
+		addObject(obj.get());
 	logGlobal->debug("Done loading map objects");
 }
 

lib/CGameInfoCallback.cpp

@@ -139,7 +139,7 @@ const CGObjectInstance* CGameInfoCallback::getObj(ObjectInstanceID objid, bool v
 		return nullptr;
 	}
 
-	const CGObjectInstance *ret = gs->getMap().objects[oid];
+	const CGObjectInstance *ret = gs->getMap().objects[oid].get();
 	if(!ret)
 	{
 		if(verbose)
@@ -484,8 +484,8 @@ std::vector<const CGObjectInstance *> CGameInfoCallback::getAllVisitableObjs() c
 {
 	std::vector<const CGObjectInstance *> ret;
 	for(auto & obj : gs->getMap().objects)
-		if(obj && obj->isVisitable() && obj->ID != Obj::EVENT && isVisible(obj))
+		if(obj && obj->isVisitable() && obj->ID != Obj::EVENT && isVisible(obj.get()))
-			ret.push_back(obj);
+			ret.push_back(obj.get());
 
 	return ret;
 }
@@ -784,7 +784,7 @@ std::vector < const CGHeroInstance *> CPlayerSpecificInfoCallback::getHeroesInfo
 		if((hero->tempOwner == *getPlayerID()) ||
 			(isVisible(hero->visitablePos(), getPlayerID()) && !onlyOur)	)
 		{
-			ret.push_back(hero);
+			ret.push_back(hero.get());
 		}
 	}
 	return ret;
@@ -956,7 +956,7 @@ const CArtifactInstance * CGameInfoCallback::getArtInstance( ArtifactInstanceID
 
 const CGObjectInstance * CGameInfoCallback::getObjInstance( ObjectInstanceID oid ) const
 {
-	return gs->getMap().objects.at(oid.num);
+	return gs->getMap().objects.at(oid.num).get();
 }
 
 const CArtifactSet * CGameInfoCallback::getArtSet(const ArtifactLocation & loc) const

lib/CMakeLists.txt

@@ -700,7 +700,6 @@ set(lib_MAIN_HEADERS
 	CGameInfoCallback.h
 	CGameInterface.h
 	ConditionalWait.h
-	ConstTransitivePtr.h
 	Color.h
 	CPlayerState.h
 	CRandomGenerator.h

lib/ConstTransitivePtr.h

@@ -1,80 +0,0 @@
-/*
- * ConstTransitivePtr.h, part of VCMI engine
- *
- * Authors: listed in file AUTHORS in main folder
- *
- * License: GNU General Public License v2.0 or later
- * Full text of license available in license.txt file, in main folder
- *
- */
-#pragma once
-
-class CGameHandler;
-
-VCMI_LIB_NAMESPACE_BEGIN
-
-template <typename T>
-class ConstTransitivePtr
-{
-	T *ptr = nullptr;
-	ConstTransitivePtr(const T *Ptr)
-		: ptr(const_cast<T*>(Ptr)) 
-	{}
-public:
-	ConstTransitivePtr(T *Ptr = nullptr)
-		: ptr(Ptr) 
-	{}
-	ConstTransitivePtr(std::nullptr_t)
-	{}
-	const T& operator*() const
-	{
-		return *ptr;
-	}
-	T& operator*()
-	{
-		return *ptr;
-	}
-	operator const T*() const
-	{
-		return ptr;
-	}
-	T* get()
-	{
-		return ptr;
-	}
-	const T* get() const
-	{
-		return ptr;
-	}
-	operator T*() 
-	{
-		return ptr;
-	}
-	T *operator->() 
-	{
-		return ptr;
-	}
-	const T *operator->() const 
-	{
-		return ptr;
-	}
-	const T*operator=(T *t)
-	{
-		return ptr = t;
-	}
-
-	void dellNull()
-	{
-		delete ptr;
-		ptr = nullptr;
-	}
-
-	template <typename Handler> void serialize(Handler &h)
-	{
-		h & ptr;
-	}
-
-	friend class ::CGameHandler;
-};
-
-VCMI_LIB_NAMESPACE_END

lib/IGameCallback.cpp

@@ -256,7 +256,7 @@ CArtifactInstance * CNonConstInfoCallback::getArtInstance(const ArtifactInstance
 
 CGObjectInstance * CNonConstInfoCallback::getObjInstance(const ObjectInstanceID & oid)
 {
-	return gs->getMap().objects.at(oid.num);
+	return gs->getMap().objects.at(oid.num).get();
 }
 
 CArmedInstance * CNonConstInfoCallback::getArmyInstance(const ObjectInstanceID & oid)

lib/campaign/CampaignState.cpp

@@ -327,13 +327,12 @@ std::set<HeroTypeID> CampaignState::getReservedHeroes() const
 	return result;
 }
 
-const CGHeroInstance * CampaignState::strongestHero(CampaignScenarioID scenarioId, const PlayerColor & owner) const
+std::shared_ptr<CGHeroInstance> CampaignState::strongestHero(CampaignScenarioID scenarioId, const PlayerColor & owner) const
 {
 	std::function<bool(const JsonNode & node)> isOwned = [&](const JsonNode & node)
 	{
-		auto * h = CampaignState::crossoverDeserialize(node, nullptr);
+		auto h = CampaignState::crossoverDeserialize(node, nullptr);
 		bool result = h->tempOwner == owner;
-		vstd::clear_pointer(h);
 		return result;
 	};
 	auto ownedHeroes = scenarioHeroPool.at(scenarioId) | boost::adaptors::filtered(isOwned);
@@ -475,10 +474,10 @@ JsonNode CampaignState::crossoverSerialize(CGHeroInstance * hero) const
 	return node;
 }
 
-CGHeroInstance * CampaignState::crossoverDeserialize(const JsonNode & node, CMap * map) const
+std::shared_ptr<CGHeroInstance> CampaignState::crossoverDeserialize(const JsonNode & node, CMap * map) const
 {
 	JsonDeserializer handler(nullptr, const_cast<JsonNode&>(node));
-	auto * hero = new CGHeroInstance(map ? map->cb : nullptr);
+	auto hero = std::make_shared<CGHeroInstance>(map ? map->cb : nullptr);
 	hero->ID = Obj::HERO;
 	hero->serializeJsonOptions(handler);
 	if (map)

lib/campaign/CampaignState.h

@@ -350,7 +350,7 @@ public:
 	std::set<HeroTypeID> getReservedHeroes() const;
 
 	/// Returns strongest hero from specified scenario, or null if none found
-	const CGHeroInstance * strongestHero(CampaignScenarioID scenarioId, const PlayerColor & owner) const;
+	std::shared_ptr<CGHeroInstance> strongestHero(CampaignScenarioID scenarioId, const PlayerColor & owner) const;
 
 	/// Returns heroes that can be instantiated as hero placeholders by power
 	const std::vector<JsonNode> & getHeroesByPower(CampaignScenarioID scenarioId) const;
@@ -360,7 +360,7 @@ public:
 	const JsonNode & getHeroByType(HeroTypeID heroID) const;
 
 	JsonNode crossoverSerialize(CGHeroInstance * hero) const;
-	CGHeroInstance * crossoverDeserialize(const JsonNode & node, CMap * map) const;
+	std::shared_ptr<CGHeroInstance> crossoverDeserialize(const JsonNode & node, CMap * map) const;
 
 	std::string campaignSet;
 

lib/gameState/CGameState.cpp

@@ -233,7 +233,7 @@ void CGameState::updateEntity(Metatype metatype, int32_t index, const JsonNode &
 		//index is hero type
 		if(index >= 0 && index < map->allHeroes.size())
 		{
-			CGHeroInstance * hero = map->allHeroes.at(index);
+			const auto & hero = map->allHeroes.at(index);
 			hero->updateFrom(data);
 		}
 		else
@@ -471,7 +471,7 @@ void CGameState::initRandomFactionsForPlayers()
 void CGameState::randomizeMapObjects()
 {
 	logGlobal->debug("\tRandomizing objects");
-	for(CGObjectInstance *object : map->objects)
+	for(const auto & object : map->objects)
 	{
 		if(!object)
 			continue;
@@ -495,10 +495,10 @@ void CGameState::randomizeMapObjects()
 
 void CGameState::initOwnedObjects()
 {
-	for(CGObjectInstance *object : map->objects)
+	for(const auto & object : map->objects)
 	{
 		if (object && object->getOwner().isValidPlayer())
-			getPlayerState(object->getOwner())->addOwnedObject(object);
+			getPlayerState(object->getOwner())->addOwnedObject(object.get());
 	}
 }
 
@@ -528,8 +528,8 @@ void CGameState::placeStartingHero(const PlayerColor & playerColor, const HeroTy
 	}
 
 	auto handler = LIBRARY->objtypeh->getHandlerFor(Obj::HERO, heroTypeId.toHeroType()->heroClass->getIndex());
-	CGObjectInstance * obj = handler->create(callback, handler->getTemplates().front());
+	auto object = handler->create(callback, handler->getTemplates().front());
-	CGHeroInstance * hero = dynamic_cast<CGHeroInstance *>(obj);
+	auto hero = std::dynamic_pointer_cast<CGHeroInstance>(object);
 
 	hero->ID = Obj::HERO;
 	hero->setHeroType(heroTypeId);
@@ -587,9 +587,7 @@ void CGameState::initHeroes()
 			logGlobal->warn("Hero with uninitialized owner!");
 			continue;
 		}
-
 		hero->initHero(getRandomGenerator());
-		map->allHeroes[hero->getHeroTypeID().getNum()] = hero;
 	}
 
 	// generate boats for all heroes on water
@@ -600,8 +598,8 @@ void CGameState::initHeroes()
 		if (tile.isWater())
 		{
 			auto handler = LIBRARY->objtypeh->getHandlerFor(Obj::BOAT, hero->getBoatType().getNum());
-			auto boat = dynamic_cast<CGBoat*>(handler->create(callback, nullptr));
+			auto boat = std::dynamic_pointer_cast<CGBoat>(handler->create(callback, nullptr));
-			handler->configureObject(boat, gs->getRandomGenerator());
+			handler->configureObject(boat.get(), gs->getRandomGenerator());
 
 			boat->setAnchorPos(hero->anchorPos());
 			boat->appearance = handler->getTemplates().front();
@@ -609,7 +607,7 @@ void CGameState::initHeroes()
 
 			map->objects.emplace_back(boat);
 
-			hero->attachToBoat(boat);
+			hero->attachToBoat(boat.get());
 		}
 	}
 
@@ -619,7 +617,6 @@ void CGameState::initHeroes()
 		{
 			auto * hero = dynamic_cast<CGHeroInstance*>(obj.get());
 			hero->initHero(getRandomGenerator());
-			map->allHeroes[hero->getHeroTypeID().getNum()] = hero;
 		}
 	}
 
@@ -629,20 +626,18 @@ void CGameState::initHeroes()
 		if(!vstd::contains(heroesToCreate, ph->getHeroTypeID()))
 			continue;
 		ph->initHero(getRandomGenerator());
-		heroesPool->addHeroToPool(ph);
+		heroesPool->addHeroToPool(ph.get());
 		heroesToCreate.erase(ph->getHeroTypeID());
-
-		map->allHeroes[ph->getHeroTypeID().getNum()] = ph;
 	}
 
 	for(const HeroTypeID & htype : heroesToCreate) //all not used allowed heroes go with default state into the pool
 	{
-		auto * vhi = new CGHeroInstance(callback);
+		auto vhi = std::make_shared<CGHeroInstance>(callback);
 		vhi->initHero(getRandomGenerator(), htype);
 
 		int typeID = htype.getNum();
 		map->allHeroes[typeID] = vhi;
-		heroesPool->addHeroToPool(vhi);
+		heroesPool->addHeroToPool(vhi.get());
 	}
 
 	for(auto & elem : map->disposedHeroes)
@@ -663,7 +658,7 @@ void CGameState::initFogOfWar()
 		fow.resize(boost::extents[layers][map->width][map->height]);
 		std::fill(fow.data(), fow.data() + fow.num_elements(), 0);
 
-		for(CGObjectInstance *obj : map->objects)
+		for(const auto & obj : map->objects)
 		{
 			if(!obj || !vstd::contains(elem.second.players, obj->tempOwner)) continue; //not a flagged object
 
@@ -918,14 +913,13 @@ void CGameState::initMapObjects()
 {
 	logGlobal->debug("\tObject initialization");
 
-//	objCaller->preInit();
+	for(auto & obj : map->objects)
-	for(CGObjectInstance *obj : map->objects)
 	{
 		if(obj)
 			obj->initObj(getRandomGenerator());
 	}
 	logGlobal->debug("\tObject initialization done");
-	for(CGObjectInstance *obj : map->objects)
+	for(auto & obj : map->objects)
 	{
 		if(!obj)
 			continue;
@@ -935,7 +929,7 @@ void CGameState::initMapObjects()
 			case Obj::QUEST_GUARD:
 			case Obj::SEER_HUT:
 			{
-				auto * q = dynamic_cast<CGSeerHut *>(obj);
+				auto * q = dynamic_cast<CGSeerHut *>(obj.get());
 				assert (q);
 				q->setObjToKill();
 			}
@@ -1001,9 +995,7 @@ void CGameState::initVisitingAndGarrisonedHeroes()
 	for (auto hero : map->heroesOnMap)
 	{
 		if (hero->getVisitedTown())
-		{
+			assert(hero->getVisitedTown()->getVisitingHero() == hero.get());
-			assert (hero->getVisitedTown()->getVisitingHero() == hero);
-		}
 	}
 }
 
@@ -1577,7 +1569,7 @@ void CGameState::buildBonusSystemTree()
 	buildGlobalTeamPlayerTree();
 	attachArmedObjects();
 
-	for(CGTownInstance *t : map->towns)
+	for(auto & t : map->towns)
 	{
 		t->deserializationFix();
 	}
@@ -1607,9 +1599,9 @@ void CGameState::buildGlobalTeamPlayerTree()
 
 void CGameState::attachArmedObjects()
 {
-	for(CGObjectInstance *obj : map->objects)
+	for(auto & obj : map->objects)
 	{
-		if(auto * armed = dynamic_cast<CArmedInstance *>(obj))
+		if(auto * armed = dynamic_cast<CArmedInstance *>(obj.get()))
 		{
 			armed->whatShouldBeAttached().attachTo(armed->whereShouldBeAttached(this));
 		}

lib/gameState/CGameStateCampaign.cpp

@@ -34,7 +34,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-CampaignHeroReplacement::CampaignHeroReplacement(CGHeroInstance * hero, const ObjectInstanceID & heroPlaceholderId):
+CampaignHeroReplacement::CampaignHeroReplacement(std::shared_ptr<CGHeroInstance> hero, const ObjectInstanceID & heroPlaceholderId):
 	hero(hero),
 	heroPlaceholderId(heroPlaceholderId)
 {
@@ -227,7 +227,7 @@ void CGameStateCampaign::placeCampaignHeroes()
 	// 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
 	// with the same hero type id
-	std::vector<CGHeroInstance *> removedHeroes;
+	std::vector<std::shared_ptr<CGObjectInstance>> removedHeroes;
 
 	std::set<HeroTypeID> reservedHeroes = campaignState->getReservedHeroes();
 	std::set<HeroTypeID> heroesToRemove;
@@ -248,10 +248,7 @@ void CGameStateCampaign::placeCampaignHeroes()
 		auto * hero = gameState->getUsedHero(heroID);
 		if(hero)
 		{
-			removedHeroes.push_back(hero);
+			removedHeroes.push_back(gameState->map->eraseObject(hero->id));
-			gameState->map->heroesOnMap -= hero;
-			gameState->map->objects[hero->id.getNum()] = nullptr;
-			gameState->map->removeBlockVisTiles(hero, true);
 		}
 	}
 
@@ -259,8 +256,9 @@ void CGameStateCampaign::placeCampaignHeroes()
 	replaceHeroesPlaceholders();
 
 	// now add removed heroes again with unused type ID
-	for(auto * hero : removedHeroes)
+	for(auto object : removedHeroes)
 	{
+		auto hero = dynamic_cast<CGHeroInstance*>(object.get());
 		HeroTypeID heroTypeId;
 		if(hero->ID == Obj::HERO)
 		{
@@ -285,7 +283,7 @@ void CGameStateCampaign::placeCampaignHeroes()
 		}
 
 		hero->setHeroType(heroTypeId);
-		gameState->map->getEditManager()->insertObject(hero);
+		gameState->map->getEditManager()->insertObject(object);
 	}
 }
 
@@ -363,26 +361,18 @@ void CGameStateCampaign::replaceHeroesPlaceholders()
 		if (!campaignHeroReplacement.heroPlaceholderId.hasValue())
 			continue;
 
-		auto * heroPlaceholder = dynamic_cast<CGHeroPlaceholder *>(gameState->getObjInstance(campaignHeroReplacement.heroPlaceholderId));
+		auto heroPlaceholder = gameState->map->objects.at(campaignHeroReplacement.heroPlaceholderId.getNum());
+		auto heroToPlace = campaignHeroReplacement.hero;
 
-		CGHeroInstance *heroToPlace = campaignHeroReplacement.hero;
-		heroToPlace->id = campaignHeroReplacement.heroPlaceholderId;
 		if(heroPlaceholder->tempOwner.isValidPlayer())
 			heroToPlace->tempOwner = heroPlaceholder->tempOwner;
+
+		// FIXME: consider whether to move these actions to CMap::replaceObject method
 		heroToPlace->setAnchorPos(heroPlaceholder->anchorPos());
 		heroToPlace->setHeroType(heroToPlace->getHeroTypeID());
 		heroToPlace->appearance = heroToPlace->getObjectHandler()->getTemplates().front();
 
-		gameState->map->removeBlockVisTiles(heroPlaceholder, true);
+		gameState->map->replaceObject(campaignHeroReplacement.heroPlaceholderId, heroToPlace);
-		gameState->map->objects[heroPlaceholder->id.getNum()] = nullptr;
-		gameState->map->instanceNames.erase(heroPlaceholder->instanceName);
-
-		gameState->map->heroesOnMap.emplace_back(heroToPlace);
-		gameState->map->objects[heroToPlace->id.getNum()] = heroToPlace;
-		gameState->map->addBlockVisTiles(heroToPlace);
-		gameState->map->instanceNames[heroToPlace->instanceName] = heroToPlace;
-
-		delete heroPlaceholder;
 	}
 }
 
@@ -413,7 +403,7 @@ void CGameStateCampaign::transferMissingArtifacts(const CampaignTravel & travelO
 		if (campaignHeroReplacement.heroPlaceholderId.hasValue())
 			continue;
 
-		auto * donorHero = campaignHeroReplacement.hero;
+		auto donorHero = campaignHeroReplacement.hero;
 
 		if (!donorHero)
 			throw std::runtime_error("Failed to find hero to take artifacts from! Scenario: " + gameState->map->name.toString());
@@ -440,7 +430,8 @@ void CGameStateCampaign::transferMissingArtifacts(const CampaignTravel & travelO
 				logGlobal->error("Cannot transfer artifact - no receiver hero!");
 		}
 
-		delete donorHero;
+		// FIXME: erase entry from array? clear entire campaignHeroReplacements?
+		//campaignHeroReplacement.hero.reset();
 	}
 }
 
@@ -484,7 +475,7 @@ void CGameStateCampaign::generateCampaignHeroesToReplace()
 			continue;
 		}
 
-		CGHeroInstance * hero = campaignState->crossoverDeserialize(node, gameState->map.get());
+		auto hero = campaignState->crossoverDeserialize(node, gameState->map.get());
 
 		logGlobal->info("Hero crossover: Loading placeholder for %d (%s)", hero->getHeroType(), hero->getNameTranslated());
 
@@ -509,7 +500,7 @@ void CGameStateCampaign::generateCampaignHeroesToReplace()
 			if (nodeListIter == nodeList.end())
 				break;
 
-			CGHeroInstance * hero = campaignState->crossoverDeserialize(*nodeListIter, gameState->map.get());
+			auto hero = campaignState->crossoverDeserialize(*nodeListIter, gameState->map.get());
 			nodeListIter++;
 
 			logGlobal->info("Hero crossover: Loading placeholder as %d (%s)", hero->getHeroType(), hero->getNameTranslated());
@@ -520,7 +511,7 @@ void CGameStateCampaign::generateCampaignHeroesToReplace()
 		// Add remaining heroes without placeholders - to transfer their artifacts to placed heroes
 		for (;nodeListIter != nodeList.end(); ++nodeListIter)
 		{
-			CGHeroInstance * hero = campaignState->crossoverDeserialize(*nodeListIter, gameState->map.get());
+			auto hero = campaignState->crossoverDeserialize(*nodeListIter, gameState->map.get());
 			campaignHeroReplacements.emplace_back(hero, ObjectInstanceID::NONE);
 		}
 	}
@@ -645,7 +636,7 @@ void CGameStateCampaign::initTowns()
 
 	for (int g=0; g<gameState->map->towns.size(); ++g)
 	{
-		CGTownInstance * town = gameState->map->towns[g];
+		auto town = gameState->map->towns[g];
 
 		PlayerState * owner = gameState->getPlayerState(town->getOwner());
 		if (!owner)

lib/gameState/CGameStateCampaign.h

@@ -23,8 +23,8 @@ class CMap;
 
 struct CampaignHeroReplacement
 {
-	CampaignHeroReplacement(CGHeroInstance * hero, const ObjectInstanceID & heroPlaceholderId);
+	CampaignHeroReplacement(std::shared_ptr<CGHeroInstance> hero, const ObjectInstanceID & heroPlaceholderId);
-	CGHeroInstance * hero;
+	std::shared_ptr<CGHeroInstance> hero;
 	ObjectInstanceID heroPlaceholderId;
 	std::vector<ArtifactPosition> transferrableArtifacts;
 };

lib/gameState/GameStatistics.cpp

@@ -188,34 +188,6 @@ std::string StatisticDataSet::writeCsv()
 	return filePath.string();
 }
 
-std::vector<const CGMine *> Statistic::getMines(const CGameState * gs, const PlayerState * ps)
-{
-	std::vector<const CGMine *> tmp;
-
-	std::vector<const CGObjectInstance *> ownedObjects;
-	for(const CGObjectInstance * obj : gs->getMap().objects)
-	{
-		if(obj && obj->tempOwner == ps->color)
-			ownedObjects.push_back(obj);
-	}
-	/// This is code from CPlayerSpecificInfoCallback::getMyObjects
-	/// I'm really need to find out about callback interface design...
-
-	for(const auto * object : ownedObjects)
-	{
-		//Mines
-		if ( object->ID == Obj::MINE )
-		{
-			const auto * mine = dynamic_cast<const CGMine *>(object);
-			assert(mine);
-
-			tmp.push_back(mine);
-		}
-	}
-
-	return tmp;
-}
-
 //calculates total number of artifacts that belong to given player
 int Statistic::getNumberOfArts(const PlayerState * ps)
 {
@@ -267,15 +239,8 @@ int Statistic::getIncome(const CGameState * gs, const PlayerState * ps)
 	int totalIncome = 0;
 
 	//Heroes can produce gold as well - skill, specialty or arts
-	for(const auto & h : ps->getHeroes())
+	for(const auto & object : ps->getOwnedObjects())
-		totalIncome += h->dailyIncome()[EGameResID::GOLD];
+		totalIncome += object->asOwnable()->dailyIncome()[EGameResID::GOLD];
-
-	//Add town income of all towns
-	for(const auto & t : ps->getTowns())
-		totalIncome += t->dailyIncome()[EGameResID::GOLD];
-
-	for(const CGMine * mine : getMines(gs, ps))
-			totalIncome += mine->dailyIncome()[EGameResID::GOLD];
 
 	return totalIncome;
 }
@@ -366,9 +331,16 @@ std::map<EGameResID, int> Statistic::getNumMines(const CGameState * gs, const Pl
 	for(auto & res : EGameResID::ALL_RESOURCES())
 		tmp[res] = 0;
 
-	for(const CGMine * mine : getMines(gs, ps))
+	for(const auto * object : ps->getOwnedObjects())
+	{
+		//Mines
+		if ( object->ID == Obj::MINE )
+		{
+			const auto * mine = dynamic_cast<const CGMine *>(object);
+			assert(mine);
 			tmp[mine->producedResource]++;
-	
+		}
+	}
 	return tmp;
 }
 

lib/gameState/GameStatistics.h

@@ -148,7 +148,6 @@ public:
 
 class DLL_LINKAGE Statistic
 {
-	static std::vector<const CGMine *> getMines(const CGameState * gs, const PlayerState * ps);
 public:
 	static int getNumberOfArts(const PlayerState * ps);
 	static int getNumberOfDwellings(const PlayerState * ps);

lib/mapObjectConstructors/AObjectTypeHandler.h

@@ -116,7 +116,7 @@ public:
 
 	/// Creates object and set up core properties (like ID/subID). Object is NOT initialized
 	/// to allow creating objects before game start (e.g. map loading)
-	virtual CGObjectInstance * create(IGameCallback * cb, std::shared_ptr<const ObjectTemplate> tmpl) const = 0;
+	virtual std::shared_ptr<CGObjectInstance> create(IGameCallback * cb, std::shared_ptr<const ObjectTemplate> tmpl) const = 0;
 
 	/// Configures object properties. Should be re-entrable, resetting state of the object if necessarily
 	/// This should set remaining properties, including randomized or depending on map

lib/mapObjectConstructors/CDefaultObjectTypeHandler.h

@@ -27,16 +27,16 @@ class CDefaultObjectTypeHandler : public AObjectTypeHandler
 		randomizeObject(castedObject, rng);
 	}
 
-	CGObjectInstance * create(IGameCallback * cb, std::shared_ptr<const ObjectTemplate> tmpl) const final
+	std::shared_ptr<CGObjectInstance> create(IGameCallback * cb, std::shared_ptr<const ObjectTemplate> tmpl) const final
 	{
-		ObjectType * result = createObject(cb);
+		auto result = createObject(cb);
 
-		preInitObject(result);
+		preInitObject(result.get());
 
 		if(tmpl)
 			result->appearance = tmpl;
 
-		initializeObject(result);
+		initializeObject(result.get());
 
 		return result;
 	}
@@ -44,9 +44,9 @@ class CDefaultObjectTypeHandler : public AObjectTypeHandler
 protected:
 	virtual void initializeObject(ObjectType * object) const {}
 	virtual void randomizeObject(ObjectType * object, vstd::RNG & rng) const {}
-	virtual ObjectType * createObject(IGameCallback * cb) const
+	virtual std::shared_ptr<ObjectType> createObject(IGameCallback * cb) const
 	{
-		return new ObjectType(cb);
+		return std::make_shared<ObjectType>(cb);
 	}
 };
 

lib/mapObjectConstructors/CRewardableConstructor.cpp

@@ -37,10 +37,10 @@ bool CRewardableConstructor::hasNameTextID() const
 	return !objectInfo.getParameters()["name"].isNull();
 }
 
-CGObjectInstance * CRewardableConstructor::create(IGameCallback * cb, std::shared_ptr<const ObjectTemplate> tmpl) const
+std::shared_ptr<CGObjectInstance> CRewardableConstructor::create(IGameCallback * cb, std::shared_ptr<const ObjectTemplate> tmpl) const
 {
-	auto * ret = new CRewardableObject(cb);
+	auto ret = std::make_shared<CRewardableObject>(cb);
-	preInitObject(ret);
+	preInitObject(ret.get());
 	ret->appearance = tmpl;
 	ret->blockVisit = blockVisit;
 	return ret;

lib/mapObjectConstructors/CRewardableConstructor.h

@@ -25,7 +25,7 @@ class DLL_LINKAGE CRewardableConstructor : public AObjectTypeHandler
 public:
 	bool hasNameTextID() const override;
 
-	CGObjectInstance * create(IGameCallback * cb, std::shared_ptr<const ObjectTemplate> tmpl = nullptr) const override;
+	std::shared_ptr<CGObjectInstance> create(IGameCallback * cb, std::shared_ptr<const ObjectTemplate> tmpl = nullptr) const override;
 
 	void configureObject(CGObjectInstance * object, vstd::RNG & rng) const override;
 

lib/mapObjectConstructors/CommonConstructors.cpp

@@ -320,7 +320,7 @@ bool MarketInstanceConstructor::hasDescription() const
 	return !descriptionTextID.empty();
 }
 
-CGMarket * MarketInstanceConstructor::createObject(IGameCallback * cb) const
+std::shared_ptr<CGMarket> MarketInstanceConstructor::createObject(IGameCallback * cb) const
 {
 	if(marketModes.size() == 1)
 	{
@@ -328,13 +328,13 @@ CGMarket * MarketInstanceConstructor::createObject(IGameCallback * cb) const
 		{
 			case EMarketMode::ARTIFACT_RESOURCE:
 			case EMarketMode::RESOURCE_ARTIFACT:
-				return new CGBlackMarket(cb);
+				return std::make_shared<CGBlackMarket>(cb);
 
 			case EMarketMode::RESOURCE_SKILL:
-				return new CGUniversity(cb);
+				return std::make_shared<CGUniversity>(cb);
 		}
 	}
-	return new CGMarket(cb);
+	return std::make_shared<CGMarket>(cb);
 }
 
 const std::set<EMarketMode> & MarketInstanceConstructor::availableModes() const

lib/mapObjectConstructors/CommonConstructors.h

@@ -137,7 +137,7 @@ class MarketInstanceConstructor : public CDefaultObjectTypeHandler<CGMarket>
 
 	void initTypeData(const JsonNode & config) override;
 public:
-	CGMarket * createObject(IGameCallback * cb) const override;
+	std::shared_ptr<CGMarket> createObject(IGameCallback * cb) const override;
 	void randomizeObject(CGMarket * object, vstd::RNG & rng) const override;
 
 	const std::set<EMarketMode> & availableModes() const;

lib/mapObjects/CGHeroInstance.cpp

@@ -1604,13 +1604,12 @@ std::string CGHeroInstance::getHeroTypeName() const
 
 void CGHeroInstance::afterAddToMap(CMap * map)
 {
-	if(ID != Obj::PRISON)
+	map->heroAddedToMap(this);
-		map->heroesOnMap.emplace_back(this);
 }
+
 void CGHeroInstance::afterRemoveFromMap(CMap* map)
 {
-	if (ID == Obj::PRISON)
+	map->heroRemovedFromMap(this);
-		vstd::erase_if_present(map->heroesOnMap, this);
 }
 
 void CGHeroInstance::setHeroTypeName(const std::string & identifier)

lib/mapObjects/CGTownInstance.cpp

@@ -1058,12 +1058,12 @@ void CGTownInstance::onTownCaptured(const PlayerColor & winner) const
 
 void CGTownInstance::afterAddToMap(CMap * map)
 {
-	map->towns.emplace_back(this);
+	map->townAddedToMap(this);
 }
 
 void CGTownInstance::afterRemoveFromMap(CMap * map)
 {
-	vstd::erase_if_present(map->towns, this);
+	map->townRemovedFromMap(this);
 }
 
 void CGTownInstance::serializeJsonOptions(JsonSerializeFormat & handler)

lib/mapObjects/MiscObjects.cpp

@@ -948,10 +948,10 @@ void CGMagi::onHeroVisit(const CGHeroInstance * h) const
 
 		std::vector<const CGObjectInstance *> eyes;
 
-		for (auto object : cb->gameState()->getMap().objects)
+		for (const auto & object : cb->gameState()->getMap().objects)
 		{
 			if (object && object->ID == Obj::EYE_OF_MAGI && object->subID == this->subID)
-				eyes.push_back(object);
+				eyes.push_back(object.get());
 		}
 
 		if (!eyes.empty())

lib/mapping/CMap.cpp

@@ -191,9 +191,6 @@ CMap::~CMap()
 {
 	getEditManager()->getUndoManager().clearAll();
 
-	for(auto obj : objects)
-		obj.dellNull();
-
 	resetStaticData();
 }
 
@@ -260,7 +257,7 @@ CGHeroInstance * CMap::getHero(HeroTypeID heroID)
 {
 	for(auto & elem : heroesOnMap)
 		if(elem->getHeroTypeID() == heroID)
-			return elem;
+			return elem.get();
 	return nullptr;
 }
 
@@ -377,16 +374,16 @@ const CGObjectInstance * CMap::getObjectiveObjectFrom(const int3 & pos, Obj type
 	logGlobal->error("Will try to find closest matching object");
 
 	CGObjectInstance * bestMatch = nullptr;
-	for (CGObjectInstance * object : objects)
+	for (const auto & object : objects)
 	{
 		if (object && object->ID == type)
 		{
 			if (bestMatch == nullptr)
-				bestMatch = object;
+				bestMatch = object.get();
 			else
 			{
 				if (object->anchorPos().dist2dSQ(pos) < bestMatch->anchorPos().dist2dSQ(pos))
-					bestMatch = object;// closer than one we already found
+					bestMatch = object.get();// closer than one we already found
 			}
 		}
 	}
@@ -424,32 +421,32 @@ void CMap::checkForObjectives()
 						cond.objectID = getObjectiveObjectFrom(cond.position, Obj::TOWN)->id;
 					break;
 
-				case EventCondition::CONTROL:
+//				case EventCondition::CONTROL:
-					if (isInTheMap(cond.position))
+//					if (isInTheMap(cond.position))
-						cond.objectID = getObjectiveObjectFrom(cond.position, cond.objectType.as<MapObjectID>())->id;
+//						cond.objectID = getObjectiveObjectFrom(cond.position, cond.objectType.as<MapObjectID>())->id;
-
+//
-					if (cond.objectID != ObjectInstanceID::NONE)
+//					if (cond.objectID != ObjectInstanceID::NONE)
-					{
+//					{
-						const auto * town = dynamic_cast<const CGTownInstance *>(objects[cond.objectID].get());
+//						const auto * town = dynamic_cast<const CGTownInstance *>(objects[cond.objectID].get());
-						if (town)
+//						if (town)
-							event.onFulfill.replaceRawString(town->getNameTranslated());
+//							event.onFulfill.replaceRawString(town->getNameTranslated());
-						const auto * hero = dynamic_cast<const CGHeroInstance *>(objects[cond.objectID].get());
+//						const auto * hero = dynamic_cast<const CGHeroInstance *>(objects[cond.objectID].get());
-						if (hero)
+//						if (hero)
-							event.onFulfill.replaceRawString(hero->getNameTranslated());
+//							event.onFulfill.replaceRawString(hero->getNameTranslated());
-					}
+//					}
-					break;
+//					break;
-
+//
-				case EventCondition::DESTROY:
+//				case EventCondition::DESTROY:
-					if (isInTheMap(cond.position))
+//					if (isInTheMap(cond.position))
-						cond.objectID = getObjectiveObjectFrom(cond.position, cond.objectType.as<MapObjectID>())->id;
+//						cond.objectID = getObjectiveObjectFrom(cond.position, cond.objectType.as<MapObjectID>())->id;
-
+//
-					if (cond.objectID != ObjectInstanceID::NONE)
+//					if (cond.objectID != ObjectInstanceID::NONE)
-					{
+//					{
-						const auto * hero = dynamic_cast<const CGHeroInstance *>(objects[cond.objectID].get());
+//						const auto * hero = dynamic_cast<const CGHeroInstance *>(objects[cond.objectID].get());
-						if (hero)
+//						if (hero)
-							event.onFulfill.replaceRawString(hero->getNameTranslated());
+//							event.onFulfill.replaceRawString(hero->getNameTranslated());
-					}
+//					}
-					break;
+//					break;
 				case EventCondition::TRANSPORT:
 					cond.objectID = getObjectiveObjectFrom(cond.position, Obj::TOWN)->id;
 					break;
@@ -527,63 +524,89 @@ void CMap::clearQuestInstance(const CQuest * quest)
 	quests.at(quest->qid) = nullptr;
 }
 
-void CMap::setUniqueInstanceName(CGObjectInstance * obj)
+//void CMap::setUniqueInstanceName(CGObjectInstance * obj)
-{
+//{
-	//this gives object unique name even if objects are removed later
+//	//this gives object unique name even if objects are removed later
+//
+//	auto uid = uidCounter++;
+//
+//	boost::format fmt("%s_%d");
+//	fmt % obj->getTypeName() % uid;
+//	obj->instanceName = fmt.str();
+//}
 
-	auto uid = uidCounter++;
+//void CMap::addNewObject(CGObjectInstance * obj)
+//{
+//	if(obj->id != ObjectInstanceID(static_cast<si32>(objects.size())))
+//		throw std::runtime_error("Invalid object instance id");
+//
+//	if(obj->instanceName.empty())
+//		throw std::runtime_error("Object instance name missing");
+//
+//	if (vstd::contains(instanceNames, obj->instanceName))
+//		throw std::runtime_error("Object instance name duplicated: "+obj->instanceName);
+//
+//	objects.emplace_back(obj);
+//	instanceNames[obj->instanceName] = obj;
+//	addBlockVisTiles(obj);
+//
+//	//TODO: how about defeated heroes recruited again?
+//
+//	obj->afterAddToMap(this);
+//}
+//
+//void CMap::moveObject(CGObjectInstance * obj, const int3 & pos)
+//{
+//	removeBlockVisTiles(obj);
+//	obj->setAnchorPos(pos);
+//	addBlockVisTiles(obj);
+//}
+//
+//void CMap::removeObject(CGObjectInstance * obj)
+//{
+//	removeBlockVisTiles(obj);
+//	instanceNames.erase(obj->instanceName);
+//
+//	//update indices
+//
+//	auto iter = std::next(objects.begin(), obj->id.getNum());
+//	iter = objects.erase(iter);
+//	for(int i = obj->id.getNum(); iter != objects.end(); ++i, ++iter)
+//	{
+//		(*iter)->id = ObjectInstanceID(i);
+//	}
+//
+//	obj->afterRemoveFromMap(this);
+//
+//	//TODO: Clean artifact instances (mostly worn by hero?) and quests related to this object
+//	//This causes crash with undo/redo in editor
+//}
 
-	boost::format fmt("%s_%d");
+//void CMap::replaceObject(ObjectInstanceID oldObjectID, const std::shared_ptr<CGObjectInstance> & newObject)
-	fmt % obj->getTypeName() % uid;
+//{
-	obj->instanceName = fmt.str();
+//	auto oldObject = objects.at(oldObjectID.getNum());
-}
+//
-
+//	newObject->id = oldObjectID;
-void CMap::addNewObject(CGObjectInstance * obj)
+//
-{
+//	removeBlockVisTiles(oldObject.get(), true);
-	if(obj->id != ObjectInstanceID(static_cast<si32>(objects.size())))
+//	instanceNames.erase(oldObject->instanceName);
-		throw std::runtime_error("Invalid object instance id");
+//
-
+//	objects.at(oldObjectID.getNum()) = newObject;
-	if(obj->instanceName.empty())
+//	addBlockVisTiles(newObject.get());
-		throw std::runtime_error("Object instance name missing");
+//	instanceNames[newObject->instanceName] = newObject;
-
+//
-	if (vstd::contains(instanceNames, obj->instanceName))
+//	oldObject->afterRemoveFromMap(this);
-		throw std::runtime_error("Object instance name duplicated: "+obj->instanceName);
+//	newObject->afterAddToMap(this);
-
+//}
-	objects.emplace_back(obj);
+//
-	instanceNames[obj->instanceName] = obj;
+//void CMap::eraseObject(ObjectInstanceID oldObjectID)
-	addBlockVisTiles(obj);
+//{
-
+//	auto oldObject = objects.at(oldObjectID.getNum());
-	//TODO: how about defeated heroes recruited again?
+//
-
+//	objects.at(oldObjectID) = nullptr;
-	obj->afterAddToMap(this);
+//	removeBlockVisTiles(oldObject.get(), true);
-}
+//	oldObject->afterRemoveFromMap(this);
-
+//}
-void CMap::moveObject(CGObjectInstance * obj, const int3 & pos)
-{
-	removeBlockVisTiles(obj);
-	obj->setAnchorPos(pos);
-	addBlockVisTiles(obj);
-}
-
-void CMap::removeObject(CGObjectInstance * obj)
-{
-	removeBlockVisTiles(obj);
-	instanceNames.erase(obj->instanceName);
-
-	//update indices
-
-	auto iter = std::next(objects.begin(), obj->id.getNum());
-	iter = objects.erase(iter);
-	for(int i = obj->id.getNum(); iter != objects.end(); ++i, ++iter)
-	{
-		(*iter)->id = ObjectInstanceID(i);
-	}
-
-	obj->afterRemoveFromMap(this);
-
-	//TODO: Clean artifact instances (mostly worn by hero?) and quests related to this object
-	//This causes crash with undo/redo in editor
-}
 
 bool CMap::isWaterMap() const
 {
@@ -709,7 +732,7 @@ void CMap::reindexObjects()
 {
 	// Only reindex at editor / RMG operations
 
-	std::sort(objects.begin(), objects.end(), [](const CGObjectInstance * lhs, const CGObjectInstance * rhs)
+	std::sort(objects.begin(), objects.end(), [](const auto & lhs, const auto & rhs)
 	{
 		// Obstacles first, then visitable, at the end - removable
 

lib/mapping/CMap.h

@@ -13,7 +13,6 @@
 #include "CMapDefines.h"
 #include "CMapHeader.h"
 
-#include "../ConstTransitivePtr.h"
 #include "../GameCallbackHolder.h"
 #include "../networkPacks/TradeItem.h"
 
@@ -101,11 +100,28 @@ public:
 	void removeQuestInstance(std::shared_ptr<CQuest> quest);
 	void clearQuestInstance(const CQuest * quest);
 
-	void setUniqueInstanceName(CGObjectInstance * obj);
+	void setUniqueInstanceName(ObjectInstanceID target){};
 	///Use only this method when creating new map object instances
-	void addNewObject(CGObjectInstance * obj);
+	void addNewObject(std::shared_ptr<CGObjectInstance> obj){}
-	void moveObject(CGObjectInstance * obj, const int3 & dst);
+	void moveObject(ObjectInstanceID target, const int3 & dst){}
-	void removeObject(CGObjectInstance * obj);
+
+	/// Remove objects and shifts object indicies.
+	/// Only for use in map editor / RMG
+	std::shared_ptr<CGObjectInstance> removeObject(ObjectInstanceID oldObject){return nullptr;}
+
+	/// Replaced map object with specified ID with new object
+	/// Old object must exist and will be removed from map
+	/// Returns pointer to old object, which can be manipulated or dropped
+	std::shared_ptr<CGObjectInstance> replaceObject(ObjectInstanceID oldObject, const std::shared_ptr<CGObjectInstance> & newObject){return nullptr;}
+
+	/// Erases object from map without shifting indices
+	/// Returns pointer to old object, which can be manipulated or dropped
+	std::shared_ptr<CGObjectInstance> eraseObject(ObjectInstanceID oldObject){return nullptr;}
+
+	void heroAddedToMap(const CGHeroInstance * hero) {}
+	void heroRemovedFromMap(const CGHeroInstance * hero) {}
+	void townAddedToMap(const CGTownInstance * town) {}
+	void townRemovedFromMap(const CGTownInstance * town) {}
 
 	bool isWaterMap() const;
 	bool calculateWaterContent();
@@ -130,7 +146,6 @@ public:
 	void reindexObjects();
 
 	std::vector<Rumor> rumors;
-	std::vector<ConstTransitivePtr<CGHeroInstance> > predefinedHeroes;
 	std::set<SpellID> allowedSpells;
 	std::set<ArtifactID> allowedArtifact;
 	std::set<SecondarySkill> allowedAbilities;
@@ -139,13 +154,14 @@ public:
 	int grailRadius;
 
 	//Central lists of items in game. Position of item in the vectors below is their (instance) id.
-	std::vector< ConstTransitivePtr<CGObjectInstance> > objects;
+	std::vector< std::shared_ptr<CGObjectInstance> > objects;
-	std::vector< ConstTransitivePtr<CGTownInstance> > towns;
+	std::vector< std::shared_ptr<CGTownInstance> > towns;
-	std::vector< ConstTransitivePtr<CGHeroInstance> > allHeroes; //indexed by [hero_type_id]; on map, disposed, prisons, etc.
+	std::vector< std::shared_ptr<CGHeroInstance> > allHeroes; //indexed by [hero_type_id]; on map, disposed, prisons, etc.
 
 	//Helper lists
-	std::vector< ConstTransitivePtr<CGHeroInstance> > heroesOnMap;
+	std::vector< std::shared_ptr<CGHeroInstance> > heroesOnMap;
 	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;
@@ -153,7 +169,7 @@ public:
 	std::unique_ptr<CMapEditManager> editManager;
 	boost::multi_array<int3, 3> guardingCreaturePositions;
 
-	std::map<std::string, ConstTransitivePtr<CGObjectInstance> > instanceNames;
+	std::map<std::string, std::shared_ptr<CGObjectInstance> > instanceNames;
 
 	bool waterMap;
 

lib/mapping/CMapEditManager.cpp

@@ -147,15 +147,15 @@ void CMapEditManager::drawRiver(RiverId riverType, vstd::RNG* customGen)
 	terrainSel.clearSelection();
 }
 
-void CMapEditManager::insertObject(CGObjectInstance * obj)
+void CMapEditManager::insertObject(std::shared_ptr<CGObjectInstance> obj)
 {
 	execute(std::make_unique<CInsertObjectOperation>(map, obj));
 }
 
-void CMapEditManager::insertObjects(std::set<CGObjectInstance*>& objects)
+void CMapEditManager::insertObjects(std::set<std::shared_ptr<CGObjectInstance>>& objects)
 {
 	auto composedOperation = std::make_unique<CComposedOperation>(map);
-	for(auto * obj : objects)
+	for(auto obj : objects)
 	{
 		composedOperation->addOperation(std::make_unique<CInsertObjectOperation>(map, obj));
 	}
@@ -175,7 +175,7 @@ void CMapEditManager::removeObject(CGObjectInstance * obj)
 void CMapEditManager::removeObjects(std::set<CGObjectInstance *> & objects)
 {
 	auto composedOperation = std::make_unique<CComposedOperation>(map);
-	for(auto * obj : objects)
+	for(auto obj : objects)
 	{
 		composedOperation->addOperation(std::make_unique<CRemoveObjectOperation>(map, obj));
 	}

lib/mapping/CMapEditManager.h

@@ -83,8 +83,8 @@ public:
 	/// Draws rivers at the current terrain selection. The selection will be cleared automatically.
 	void drawRiver(RiverId riverType, vstd::RNG * gen);
 
-	void insertObject(CGObjectInstance * obj);
+	void insertObject(std::shared_ptr<CGObjectInstance> obj);
-	void insertObjects(std::set<CGObjectInstance *> & objects);
+	void insertObjects(std::set<std::shared_ptr<CGObjectInstance>> & objects);
 	void moveObject(CGObjectInstance* obj, const int3 & pos);
 	void removeObject(CGObjectInstance* obj);
 	void removeObjects(std::set<CGObjectInstance*> & objects);

lib/mapping/CMapOperation.cpp

@@ -579,7 +579,7 @@ std::string CClearTerrainOperation::getLabel() const
 	return "Clear Terrain";
 }
 
-CInsertObjectOperation::CInsertObjectOperation(CMap* map, CGObjectInstance* obj)
+CInsertObjectOperation::CInsertObjectOperation(CMap* map, std::shared_ptr<CGObjectInstance> obj)
 	: CMapOperation(map), obj(obj)
 {
 
@@ -591,7 +591,7 @@ void CInsertObjectOperation::execute()
 
 	do
 	{
-		map->setUniqueInstanceName(obj);
+		map->setUniqueInstanceName(obj->id);
 	} while(vstd::contains(map->instanceNames, obj->instanceName));
 
 	map->addNewObject(obj);
@@ -599,7 +599,7 @@ void CInsertObjectOperation::execute()
 
 void CInsertObjectOperation::undo()
 {
-	map->removeObject(obj);
+	map->removeObject(obj->id);
 }
 
 void CInsertObjectOperation::redo()
@@ -622,12 +622,12 @@ CMoveObjectOperation::CMoveObjectOperation(CMap* map, CGObjectInstance* obj, con
 
 void CMoveObjectOperation::execute()
 {
-	map->moveObject(obj, targetPos);
+	map->moveObject(obj->id, targetPos);
 }
 
 void CMoveObjectOperation::undo()
 {
-	map->moveObject(obj, initialPos);
+	map->moveObject(obj->id, initialPos);
 }
 
 void CMoveObjectOperation::redo()
@@ -641,45 +641,22 @@ std::string CMoveObjectOperation::getLabel() const
 }
 
 CRemoveObjectOperation::CRemoveObjectOperation(CMap* map, CGObjectInstance * obj)
-	: CMapOperation(map), obj(obj)
+	: CMapOperation(map), targetedObject(obj)
 {
 
 }
 
-CRemoveObjectOperation::~CRemoveObjectOperation()
-{
-	//when operation is destroyed and wasn't undone, the object is lost forever
-
-	if(!obj)
-	{
-		return;
-	}
-
-	//do not destroy an object that belongs to map
-	if(!vstd::contains(map->instanceNames, obj->instanceName))
-	{
-		delete obj;
-		obj = nullptr;
-	}
-}
-
 void CRemoveObjectOperation::execute()
 {
-	map->removeObject(obj);
+	removedObject = map->removeObject(targetedObject->id);
 }
 
 void CRemoveObjectOperation::undo()
 {
-	try
+	assert(removedObject != nullptr);
-	{
 	//set new id, but do not rename object
-		obj->id = ObjectInstanceID(static_cast<si32>(map->objects.size()));
+	removedObject->id = ObjectInstanceID(static_cast<si32>(map->objects.size()));
-		map->addNewObject(obj);
+	map->addNewObject(removedObject);
-	}
-	catch(const std::exception& e)
-	{
-		logGlobal->error(e.what());
-	}
 }
 
 void CRemoveObjectOperation::redo()

lib/mapping/CMapOperation.h

@@ -124,7 +124,7 @@ public:
 class CInsertObjectOperation : public CMapOperation
 {
 public:
-	CInsertObjectOperation(CMap * map, CGObjectInstance * obj);
+	CInsertObjectOperation(CMap * map, std::shared_ptr<CGObjectInstance> obj);
 
 	void execute() override;
 	void undo() override;
@@ -132,7 +132,7 @@ public:
 	std::string getLabel() const override;
 
 private:
-	CGObjectInstance * obj;
+	std::shared_ptr<CGObjectInstance> obj;
 };
 
 /// The CMoveObjectOperation class moves object to another position
@@ -157,7 +157,6 @@ class CRemoveObjectOperation : public CMapOperation
 {
 public:
 	CRemoveObjectOperation(CMap* map, CGObjectInstance * obj);
-	~CRemoveObjectOperation();
 
 	void execute() override;
 	void undo() override;
@@ -165,7 +164,8 @@ public:
 	std::string getLabel() const override;
 
 private:
-	CGObjectInstance* obj;
+	CGObjectInstance * targetedObject;
+	std::shared_ptr<CGObjectInstance> removedObject;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/mapping/MapFormatH3M.cpp

@@ -1067,11 +1067,11 @@ void CMapLoaderH3M::readObjectTemplates()
 	}
 }
 
-CGObjectInstance * CMapLoaderH3M::readEvent(const int3 & mapPosition, const ObjectInstanceID & idToBeGiven)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readEvent(const int3 & mapPosition, const ObjectInstanceID & idToBeGiven)
 {
-	auto * object = new CGEvent(map->cb);
+	auto object = std::make_shared<CGEvent>(map->cb);
 
-	readBoxContent(object, mapPosition, idToBeGiven);
+	readBoxContent(object.get(), mapPosition, idToBeGiven);
 
 	reader->readBitmaskPlayers(object->availableFor, false);
 	object->computerActivate = reader->readBool();
@@ -1084,20 +1084,20 @@ CGObjectInstance * CMapLoaderH3M::readEvent(const int3 & mapPosition, const Obje
 	else
 		object->humanActivate = true;
 
-	readBoxHotaContent(object, mapPosition, idToBeGiven);
+	readBoxHotaContent(object.get(), mapPosition, idToBeGiven);
 
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readPandora(const int3 & mapPosition, const ObjectInstanceID & idToBeGiven)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readPandora(const int3 & mapPosition, const ObjectInstanceID & idToBeGiven)
 {
-	auto * object = new CGPandoraBox(map->cb);
+	auto object = std::make_shared<CGPandoraBox>(map->cb);
-	readBoxContent(object, mapPosition, idToBeGiven);
+	readBoxContent(object.get(), mapPosition, idToBeGiven);
 
 	if(features.levelHOTA5)
 		reader->skipZero(1); // Unknown value, always 0 so far
 
-	readBoxHotaContent(object, mapPosition, idToBeGiven);
+	readBoxHotaContent(object.get(), mapPosition, idToBeGiven);
 
 	return object;
 }
@@ -1179,9 +1179,9 @@ void CMapLoaderH3M::readBoxHotaContent(CGPandoraBox * object, const int3 & mapPo
 	}
 }
 
-CGObjectInstance * CMapLoaderH3M::readMonster(const int3 & mapPosition, const ObjectInstanceID & objectInstanceID)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readMonster(const int3 & mapPosition, const ObjectInstanceID & objectInstanceID)
 {
-	auto * object = new CGCreature(map->cb);
+	auto object = std::make_shared<CGCreature>(map->cb);
 
 	if(features.levelAB)
 	{
@@ -1243,18 +1243,18 @@ CGObjectInstance * CMapLoaderH3M::readMonster(const int3 & mapPosition, const Ob
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readSign(const int3 & mapPosition)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readSign(const int3 & mapPosition)
 {
-	auto * object = new CGSignBottle(map->cb);
+	auto object = std::make_shared<CGSignBottle>(map->cb);
 	object->message.appendTextID(readLocalizedString(TextIdentifier("sign", mapPosition.x, mapPosition.y, mapPosition.z, "message")));
 	reader->skipZero(4);
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readWitchHut(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readWitchHut(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
-	auto * object = readGeneric(position, objectTemplate);
+	auto object = readGeneric(position, objectTemplate);
-	auto * rewardable = dynamic_cast<CRewardableObject*>(object);
+	auto rewardable = std::dynamic_pointer_cast<CRewardableObject>(object);
 
 	// AB and later maps have allowed abilities defined in H3M
 	if(features.levelAB)
@@ -1301,7 +1301,7 @@ CGObjectInstance * CMapLoaderH3M::readWitchHut(const int3 & position, std::share
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readScholar(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readScholar(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	enum class ScholarBonusType : int8_t {
 		RANDOM = -1,
@@ -1310,8 +1310,8 @@ CGObjectInstance * CMapLoaderH3M::readScholar(const int3 & position, std::shared
 		SPELL = 2,
 	};
 
-	auto * object = readGeneric(position, objectTemplate);
+	auto object = readGeneric(position, objectTemplate);
-	auto * rewardable = dynamic_cast<CRewardableObject*>(object);
+	auto rewardable = std::dynamic_pointer_cast<CRewardableObject>(object);
 
 	uint8_t bonusTypeRaw = reader->readInt8Checked(-1, 2);
 	auto bonusType = static_cast<ScholarBonusType>(bonusTypeRaw);
@@ -1369,12 +1369,12 @@ CGObjectInstance * CMapLoaderH3M::readScholar(const int3 & position, std::shared
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readGarrison(const int3 & mapPosition)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readGarrison(const int3 & mapPosition)
 {
-	auto * object = new CGGarrison(map->cb);
+	auto object = std::make_shared<CGGarrison>(map->cb);
 
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
+	setOwnerAndValidate(mapPosition, object.get(), reader->readPlayer32());
-	readCreatureSet(object, 7);
+	readCreatureSet(object.get(), 7);
 	if(features.levelAB)
 		object->removableUnits = reader->readBool();
 	else
@@ -1384,12 +1384,12 @@ CGObjectInstance * CMapLoaderH3M::readGarrison(const int3 & mapPosition)
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readArtifact(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readArtifact(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	ArtifactID artID = ArtifactID::NONE; //random, set later
-	auto * object = new CGArtifact(map->cb);
+	auto object = std::make_shared<CGArtifact>(map->cb);
 
-	readMessageAndGuards(object->message, object, mapPosition);
+	readMessageAndGuards(object->message, object.get(), mapPosition);
 
 	//specific artifact
 	if(objectTemplate->id == Obj::ARTIFACT)
@@ -1410,21 +1410,21 @@ CGObjectInstance * CMapLoaderH3M::readArtifact(const int3 & mapPosition, std::sh
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readScroll(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readScroll(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
-	auto * object = new CGArtifact(map->cb);
+	auto object = std::make_shared<CGArtifact>(map->cb);
-	readMessageAndGuards(object->message, object, mapPosition);
+	readMessageAndGuards(object->message, object.get(), mapPosition);
 	SpellID spellID = reader->readSpell32();
 
 	object->storedArtifact = map->createArtifact(ArtifactID::SPELL_SCROLL, spellID.getNum());
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readResource(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readResource(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
-	auto * object = new CGResource(map->cb);
+	auto object = std::make_shared<CGResource>(map->cb);
 
-	readMessageAndGuards(object->message, object, mapPosition);
+	readMessageAndGuards(object->message, object.get(), mapPosition);
 
 	object->amount = reader->readUInt32();
 
@@ -1440,16 +1440,16 @@ CGObjectInstance * CMapLoaderH3M::readResource(const int3 & mapPosition, std::sh
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readMine(const int3 & mapPosition)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readMine(const int3 & mapPosition)
 {
-	auto * object = new CGMine(map->cb);
+	auto object = std::make_shared<CGMine>(map->cb);
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
+	setOwnerAndValidate(mapPosition, object.get(), reader->readPlayer32());
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readAbandonedMine(const int3 & mapPosition)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readAbandonedMine(const int3 & mapPosition)
 {
-	auto * object = new CGMine(map->cb);
+	auto object = std::make_shared<CGMine>(map->cb);
 	object->setOwner(PlayerColor::NEUTRAL);
 	reader->readBitmaskResources(object->abandonedMineResources, false);
 
@@ -1470,18 +1470,18 @@ CGObjectInstance * CMapLoaderH3M::readAbandonedMine(const int3 & mapPosition)
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readDwelling(const int3 & position)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readDwelling(const int3 & position)
 {
-	auto * object = new CGDwelling(map->cb);
+	auto object = std::make_shared<CGDwelling>(map->cb);
-	setOwnerAndValidate(position, object, reader->readPlayer32());
+	setOwnerAndValidate(position, object.get(), reader->readPlayer32());
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readDwellingRandom(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readDwellingRandom(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
-	auto * object = new CGDwelling(map->cb);
+	auto object = std::make_shared<CGDwelling>(map->cb);
 
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
+	setOwnerAndValidate(mapPosition, object.get(), reader->readPlayer32());
 
 	object->randomizationInfo = CGDwellingRandomizationInfo();
 
@@ -1512,10 +1512,10 @@ CGObjectInstance * CMapLoaderH3M::readDwellingRandom(const int3 & mapPosition, s
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readShrine(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readShrine(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
-	auto * object = readGeneric(position, objectTemplate);
+	auto object = readGeneric(position, objectTemplate);
-	auto * rewardable = dynamic_cast<CRewardableObject*>(object);
+	auto rewardable = std::dynamic_pointer_cast<CRewardableObject>(object);
 
 	SpellID spell = reader->readSpell32();
 
@@ -1536,11 +1536,11 @@ CGObjectInstance * CMapLoaderH3M::readShrine(const int3 & position, std::shared_
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readHeroPlaceholder(const int3 & mapPosition)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readHeroPlaceholder(const int3 & mapPosition)
 {
-	auto * object = new CGHeroPlaceholder(map->cb);
+	auto object = std::make_shared<CGHeroPlaceholder>(map->cb);
 
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer());
+	setOwnerAndValidate(mapPosition, object.get(), reader->readPlayer());
 
 	HeroTypeID htid = reader->readHero(); //hero type id
 
@@ -1586,51 +1586,51 @@ CGObjectInstance * CMapLoaderH3M::readHeroPlaceholder(const int3 & mapPosition)
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readGrail(const int3 & mapPosition)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readGrail(const int3 & mapPosition)
 {
 	map->grailPos = mapPosition;
 	map->grailRadius = reader->readInt32();
 	return nullptr;
 }
 
-CGObjectInstance * CMapLoaderH3M::readHotaBattleLocation(const int3 & mapPosition)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readHotaBattleLocation(const int3 & mapPosition)
 {
 	// Battle location for arena mode in HotA
 	logGlobal->warn("Map '%s': Arena mode is not supported!", mapName);
 	return nullptr;
 }
 
-CGObjectInstance * CMapLoaderH3M::readGeneric(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readGeneric(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	if(LIBRARY->objtypeh->knownSubObjects(objectTemplate->id).count(objectTemplate->subid))
 		return LIBRARY->objtypeh->getHandlerFor(objectTemplate->id, objectTemplate->subid)->create(map->cb, objectTemplate);
 
 	logGlobal->warn("Map '%s': Unrecognized object %d:%d ('%s') at %s found!", mapName, objectTemplate->id.toEnum(), objectTemplate->subid, objectTemplate->animationFile.getOriginalName(), mapPosition.toString());
-	return new CGObjectInstance(map->cb);
+	return std::make_shared<CGObjectInstance>(map->cb);
 }
 
-CGObjectInstance * CMapLoaderH3M::readQuestGuard(const int3 & mapPosition)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readQuestGuard(const int3 & mapPosition)
 {
-	auto * guard = new CGQuestGuard(map->cb);
+	auto guard = std::make_shared<CGQuestGuard>(map->cb);
-	readQuest(guard, mapPosition);
+	readQuest(guard.get(), mapPosition);
 	return guard;
 }
 
-CGObjectInstance * CMapLoaderH3M::readShipyard(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readShipyard(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
-	auto * object = readGeneric(mapPosition, objectTemplate);
+	auto object = readGeneric(mapPosition, objectTemplate);
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
+	setOwnerAndValidate(mapPosition, object.get(), reader->readPlayer32());
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readLighthouse(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readLighthouse(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
-	auto * object = readGeneric(mapPosition, objectTemplate);
+	auto object = readGeneric(mapPosition, objectTemplate);
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
+	setOwnerAndValidate(mapPosition, object.get(), reader->readPlayer32());
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readBank(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readBank(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	if(features.levelHOTA3)
 	{
@@ -1669,7 +1669,7 @@ CGObjectInstance * CMapLoaderH3M::readBank(const int3 & mapPosition, std::shared
 	return readGeneric(mapPosition, objectTemplate);
 }
 
-CGObjectInstance * CMapLoaderH3M::readRewardWithArtifact(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readRewardWithArtifact(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	if(features.levelHOTA5)
 	{
@@ -1697,7 +1697,7 @@ CGObjectInstance * CMapLoaderH3M::readRewardWithArtifact(const int3 & mapPositio
 	return readGeneric(mapPosition, objectTemplate);
 }
 
-CGObjectInstance * CMapLoaderH3M::readBlackMarket(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readBlackMarket(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	if(features.levelHOTA5)
 	{
@@ -1718,7 +1718,7 @@ CGObjectInstance * CMapLoaderH3M::readBlackMarket(const int3 & mapPosition, std:
 	return readGeneric(mapPosition, objectTemplate);
 }
 
-CGObjectInstance * CMapLoaderH3M::readUniversity(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readUniversity(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	if(features.levelHOTA5)
 	{
@@ -1735,7 +1735,7 @@ CGObjectInstance * CMapLoaderH3M::readUniversity(const int3 & mapPosition, std::
 	return readGeneric(mapPosition, objectTemplate);
 }
 
-CGObjectInstance * CMapLoaderH3M::readRewardWithArtifactAndResources(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readRewardWithArtifactAndResources(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	if(features.levelHOTA5)
 	{
@@ -1761,7 +1761,7 @@ CGObjectInstance * CMapLoaderH3M::readRewardWithArtifactAndResources(const int3
 	return readGeneric(mapPosition, objectTemplate);
 }
 
-CGObjectInstance * CMapLoaderH3M::readObject(MapObjectID id, MapObjectSubID subid, std::shared_ptr<const ObjectTemplate> objectTemplate, const int3 & mapPosition, const ObjectInstanceID & objectInstanceID)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readObject(MapObjectID id, MapObjectSubID subid, std::shared_ptr<const ObjectTemplate> objectTemplate, const int3 & mapPosition, const ObjectInstanceID & objectInstanceID)
 {
 	switch(id.toEnum())
 	{
@@ -1935,7 +1935,7 @@ void CMapLoaderH3M::readObjects()
 		auto originalSubID = originalTemplate->subid;
 		reader->skipZero(5);
 
-		CGObjectInstance * newObject = readObject(originalID, originalSubID, remappedTemplate, mapPosition, objectInstanceID);
+		auto newObject = readObject(originalID, originalSubID, remappedTemplate, mapPosition, objectInstanceID);
 
 		if(!newObject)
 			continue;
@@ -1967,7 +1967,7 @@ void CMapLoaderH3M::readObjects()
 	std::sort(
 		map->heroesOnMap.begin(),
 		map->heroesOnMap.end(),
-		[](const ConstTransitivePtr<CGHeroInstance> & a, const ConstTransitivePtr<CGHeroInstance> & b)
+		[](const std::shared_ptr<CGHeroInstance> & a, const std::shared_ptr<CGHeroInstance> & b)
 		{
 			return a->subID < b->subID;
 		}
@@ -2037,9 +2037,9 @@ void CMapLoaderH3M::setOwnerAndValidate(const int3 & mapPosition, CGObjectInstan
 	object->setOwner(owner);
 }
 
-CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const ObjectInstanceID & objectInstanceID)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readHero(const int3 & mapPosition, const ObjectInstanceID & objectInstanceID)
 {
-	auto * object = new CGHeroInstance(map->cb);
+	auto object = std::make_shared<CGHeroInstance>(map->cb);
 
 	if(features.levelAB)
 	{
@@ -2057,13 +2057,12 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 		if(elem->subID == object->subID)
 		{
 			logGlobal->debug("Hero %d will be taken from the predefined heroes list.", object->subID);
-			delete object;
 			object = elem;
 			break;
 		}
 	}
 
-	setOwnerAndValidate(mapPosition, object, owner);
+	setOwnerAndValidate(mapPosition, object.get(), owner);
 
 	for(auto & elem : map->disposedHeroes)
 	{
@@ -2119,12 +2118,12 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 
 	bool hasGarison = reader->readBool();
 	if(hasGarison)
-		readCreatureSet(object, 7);
+		readCreatureSet(object.get(), 7);
 
 	object->formation = static_cast<EArmyFormation>(reader->readInt8Checked(0, 1));
 	assert(object->formation == EArmyFormation::LOOSE || object->formation == EArmyFormation::TIGHT);
 
-	loadArtifactsOfHero(object);
+	loadArtifactsOfHero(object.get());
 	object->patrol.patrolRadius = reader->readUInt8();
 	object->patrol.patrolling = (object->patrol.patrolRadius != 0xff);
 
@@ -2218,9 +2217,9 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readSeerHut(const int3 & position, const ObjectInstanceID & idToBeGiven)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readSeerHut(const int3 & position, const ObjectInstanceID & idToBeGiven)
 {
-	auto * hut = new CGSeerHut(map->cb);
+	auto hut = std::make_shared<CGSeerHut>(map->cb);
 
 	uint32_t questsCount = 1;
 
@@ -2232,7 +2231,7 @@ CGObjectInstance * CMapLoaderH3M::readSeerHut(const int3 & position, const Objec
 		logGlobal->warn("Map '%s': Seer Hut at %s - %d quests are not implemented!", mapName, position.toString(), questsCount);
 
 	for(size_t i = 0; i < questsCount; ++i)
-		readSeerHutQuest(hut, position, idToBeGiven);
+		readSeerHutQuest(hut.get(), position, idToBeGiven);
 
 	if(features.levelHOTA3)
 	{
@@ -2243,7 +2242,7 @@ CGObjectInstance * CMapLoaderH3M::readSeerHut(const int3 & position, const Objec
 			logGlobal->warn("Map '%s': Seer Hut at %s - %d repeatable quests are not implemented!", mapName, position.toString(), repeateableQuestsCount);
 
 		for(size_t i = 0; i < repeateableQuestsCount; ++i)
-			readSeerHutQuest(hut, position, idToBeGiven);
+			readSeerHutQuest(hut.get(), position, idToBeGiven);
 	}
 
 	reader->skipZero(2);
@@ -2506,13 +2505,13 @@ EQuestMission CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & positi
 	return missionId;
 }
 
-CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate)
+std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readTown(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
-	auto * object = new CGTownInstance(map->cb);
+	auto object = std::make_shared<CGTownInstance>(map->cb);
 	if(features.levelAB)
 		object->identifier = reader->readUInt32();
 
-	setOwnerAndValidate(position, object, reader->readPlayer());
+	setOwnerAndValidate(position, object.get(), reader->readPlayer());
 
 	std::optional<FactionID> faction;
 	if (objectTemplate->id == Obj::TOWN)
@@ -2524,7 +2523,7 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt
 
 	bool hasGarrison = reader->readBool();
 	if(hasGarrison)
-		readCreatureSet(object, 7);
+		readCreatureSet(object.get(), 7);
 
 	object->formation = static_cast<EArmyFormation>(reader->readInt8Checked(0, 1));
 	assert(object->formation == EArmyFormation::LOOSE || object->formation == EArmyFormation::TIGHT);

lib/mapping/MapFormatH3M.h

@@ -187,38 +187,37 @@ private:
 	void readObjects();
 
 	/// Reads single object from input stream based on template
-	CGObjectInstance * readObject(MapObjectID id, MapObjectSubID subid, std::shared_ptr<const ObjectTemplate> objectTemplate, const int3 & objectPosition, const ObjectInstanceID & idToBeGiven);
+	std::shared_ptr<CGObjectInstance> readObject(MapObjectID id, MapObjectSubID subid, std::shared_ptr<const ObjectTemplate> objectTemplate, const int3 & objectPosition, const ObjectInstanceID & idToBeGiven);
-
+	std::shared_ptr<CGObjectInstance> readEvent(const int3 & objectPosition, const ObjectInstanceID & idToBeGiven);
-	CGObjectInstance * readEvent(const int3 & objectPosition, const ObjectInstanceID & idToBeGiven);
+	std::shared_ptr<CGObjectInstance> readMonster(const int3 & objectPosition, const ObjectInstanceID & idToBeGiven);
-	CGObjectInstance * readMonster(const int3 & objectPosition, const ObjectInstanceID & idToBeGiven);
+	std::shared_ptr<CGObjectInstance> readHero(const int3 & initialPos, const ObjectInstanceID & idToBeGiven);
-	CGObjectInstance * readHero(const int3 & initialPos, const ObjectInstanceID & idToBeGiven);
+	std::shared_ptr<CGObjectInstance> readSeerHut(const int3 & initialPos, const ObjectInstanceID & idToBeGiven);
-	CGObjectInstance * readSeerHut(const int3 & initialPos, const ObjectInstanceID & idToBeGiven);
+	std::shared_ptr<CGObjectInstance> readTown(const int3 & position, std::shared_ptr<const ObjectTemplate> objTempl);
-	CGObjectInstance * readTown(const int3 & position, std::shared_ptr<const ObjectTemplate> objTempl);
+	std::shared_ptr<CGObjectInstance> readSign(const int3 & position);
-	CGObjectInstance * readSign(const int3 & position);
+	std::shared_ptr<CGObjectInstance> readWitchHut(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readWitchHut(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
+	std::shared_ptr<CGObjectInstance> readScholar(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readScholar(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
+	std::shared_ptr<CGObjectInstance> readGarrison(const int3 & mapPosition);
-	CGObjectInstance * readGarrison(const int3 & mapPosition);
+	std::shared_ptr<CGObjectInstance> readArtifact(const int3 & position, std::shared_ptr<const ObjectTemplate> objTempl);
-	CGObjectInstance * readArtifact(const int3 & position, std::shared_ptr<const ObjectTemplate> objTempl);
+	std::shared_ptr<CGObjectInstance> readScroll(const int3 & position, std::shared_ptr<const ObjectTemplate> objTempl);
-	CGObjectInstance * readScroll(const int3 & position, std::shared_ptr<const ObjectTemplate> objTempl);
+	std::shared_ptr<CGObjectInstance> readResource(const int3 & position, std::shared_ptr<const ObjectTemplate> objTempl);
-	CGObjectInstance * readResource(const int3 & position, std::shared_ptr<const ObjectTemplate> objTempl);
+	std::shared_ptr<CGObjectInstance> readMine(const int3 & position);
-	CGObjectInstance * readMine(const int3 & position);
+	std::shared_ptr<CGObjectInstance> readAbandonedMine(const int3 & position);
-	CGObjectInstance * readAbandonedMine(const int3 & position);
+	std::shared_ptr<CGObjectInstance> readPandora(const int3 & position, const ObjectInstanceID & idToBeGiven);
-	CGObjectInstance * readPandora(const int3 & position, const ObjectInstanceID & idToBeGiven);
+	std::shared_ptr<CGObjectInstance> readDwelling(const int3 & position);
-	CGObjectInstance * readDwelling(const int3 & position);
+	std::shared_ptr<CGObjectInstance> readDwellingRandom(const int3 & position, std::shared_ptr<const ObjectTemplate> objTempl);
-	CGObjectInstance * readDwellingRandom(const int3 & position, std::shared_ptr<const ObjectTemplate> objTempl);
+	std::shared_ptr<CGObjectInstance> readShrine(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readShrine(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
+	std::shared_ptr<CGObjectInstance> readHeroPlaceholder(const int3 & position);
-	CGObjectInstance * readHeroPlaceholder(const int3 & position);
+	std::shared_ptr<CGObjectInstance> readGrail(const int3 & position);
-	CGObjectInstance * readGrail(const int3 & position);
+	std::shared_ptr<CGObjectInstance> readHotaBattleLocation(const int3 & position);
-	CGObjectInstance * readHotaBattleLocation(const int3 & position);
+	std::shared_ptr<CGObjectInstance> readQuestGuard(const int3 & position);
-	CGObjectInstance * readQuestGuard(const int3 & position);
+	std::shared_ptr<CGObjectInstance> readShipyard(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readShipyard(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate);
+	std::shared_ptr<CGObjectInstance> readLighthouse(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readLighthouse(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate);
+	std::shared_ptr<CGObjectInstance> readGeneric(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readGeneric(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
+	std::shared_ptr<CGObjectInstance> readBank(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readBank(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
+	std::shared_ptr<CGObjectInstance> readRewardWithArtifact(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readRewardWithArtifact(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
+	std::shared_ptr<CGObjectInstance> readRewardWithArtifactAndResources(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readRewardWithArtifactAndResources(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
+	std::shared_ptr<CGObjectInstance> readBlackMarket(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readBlackMarket(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
+	std::shared_ptr<CGObjectInstance> readUniversity(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	CGObjectInstance * readUniversity(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
 
 	/**
 	 * Reads a creature set.

lib/mapping/MapFormatJson.cpp

@@ -1084,7 +1084,7 @@ void CMapLoaderJson::MapObjectLoader::configure()
 	//artifact instance serialization requires access to Map object, handle it here for now
 	//todo: find better solution for artifact instance serialization
 
-	if(auto * art = dynamic_cast<CGArtifact *>(instance))
+	if(auto art = std::dynamic_pointer_cast<CGArtifact>(instance))
 	{
 		ArtifactID artID = ArtifactID::NONE;
 		SpellID spellID = SpellID::NONE;
@@ -1108,7 +1108,7 @@ void CMapLoaderJson::MapObjectLoader::configure()
 		art->storedArtifact = owner->map->createArtifact(artID, spellID.getNum());
 	}
 
-	if(auto * hero = dynamic_cast<CGHeroInstance *>(instance))
+	if(auto hero = std::dynamic_pointer_cast<CGHeroInstance>(instance))
 	{
 		auto o = handler.enterStruct("options");
 		hero->serializeJsonArtifacts(handler, "artifacts", owner->map);
@@ -1134,7 +1134,7 @@ void CMapLoaderJson::readObjects()
 	for(auto & ptr : loaders)
 		ptr->configure();
 
-	std::sort(map->heroesOnMap.begin(), map->heroesOnMap.end(), [](const ConstTransitivePtr<CGHeroInstance> & a, const ConstTransitivePtr<CGHeroInstance> & b)
+	std::sort(map->heroesOnMap.begin(), map->heroesOnMap.end(), [](const std::shared_ptr<CGHeroInstance> & a, const std::shared_ptr<CGHeroInstance> & b)
 	{
 		return a->getObjTypeIndex() < b->getObjTypeIndex();
 	});
@@ -1305,7 +1305,7 @@ void CMapSaverJson::writeObjects()
 
 	JsonSerializer handler(mapObjectResolver.get(), data);
 
-	for(CGObjectInstance * obj : map->objects)
+	for(const auto & obj : map->objects)
 	{
 		//logGlobal->trace("\t%s", obj->instanceName);
 		auto temp = handler.enterStruct(obj->instanceName);

lib/mapping/MapFormatJson.h

@@ -180,7 +180,7 @@ public:
 	{
 		MapObjectLoader(CMapLoaderJson * _owner, JsonMap::value_type & json);
 		CMapLoaderJson * owner;
-		CGObjectInstance * instance;
+		std::shared_ptr<CGObjectInstance> instance;
 		ObjectInstanceID id;
 		std::string jsonKey;//full id defined by map creator
 		JsonNode & configuration;

lib/mapping/ObstacleProxy.cpp

@@ -244,8 +244,8 @@ int ObstacleProxy::getWeightedObjects(const int3 & tile, vstd::RNG & rand, IGame
 		for(const auto & temp : shuffledObstacles)
 		{
 			auto handler = LIBRARY->objtypeh->getHandlerFor(temp->id, temp->subid);
-			auto * obj = handler->create(nullptr, temp);
+			auto obj = handler->create(nullptr, temp);
-			allObjects.emplace_back(*obj);
+			allObjects.emplace_back(obj);
 			rmg::Object * rmgObject = &allObjects.back();
 			for(const auto & offset : obj->getBlockedOffsets())
 			{
@@ -311,12 +311,12 @@ int ObstacleProxy::getWeightedObjects(const int3 & tile, vstd::RNG & rand, IGame
 	return maxWeight;
 }
 
-std::set<CGObjectInstance*> ObstacleProxy::createObstacles(vstd::RNG & rand, IGameCallback * cb)
+std::set<std::shared_ptr<CGObjectInstance>> ObstacleProxy::createObstacles(vstd::RNG & rand, IGameCallback * cb)
 {
 	//reverse order, since obstacles begin in bottom-right corner, while the map coordinates begin in top-left
 	auto blockedTiles = blockedArea.getTilesVector();
 	int tilePos = 0;
-	std::set<CGObjectInstance*> objs;
+	std::set<std::shared_ptr<CGObjectInstance>> objs;
 
 	while(!blockedArea.empty() && tilePos < blockedArea.getTilesVector().size())
 	{
@@ -356,7 +356,7 @@ std::set<CGObjectInstance*> ObstacleProxy::createObstacles(vstd::RNG & rand, IGa
 
 //FIXME: Only editor placer obstacles directly
 
-void ObstacleProxy::finalInsertion(CMapEditManager * manager, std::set<CGObjectInstance*> & instances)
+void ObstacleProxy::finalInsertion(CMapEditManager * manager, std::set<std::shared_ptr<CGObjectInstance>> & instances)
 {
 	manager->insertObjects(instances); //insert as one operation - for undo purposes
 }
@@ -366,11 +366,11 @@ std::pair<bool, bool> ObstacleProxy::verifyCoverage(const int3 & t) const
 	return {blockedArea.contains(t), false};
 }
 
-void ObstacleProxy::placeObject(rmg::Object & object, std::set<CGObjectInstance*> & instances)
+void ObstacleProxy::placeObject(rmg::Object & object, std::set<std::shared_ptr<CGObjectInstance>> & instances)
 {
 	for (auto * instance : object.instances())
 	{
-		instances.insert(&instance->object());
+		instances.insert(instance->object());
 	}
 }
 
@@ -384,7 +384,7 @@ bool EditorObstaclePlacer::isInTheMap(const int3& tile)
 	return map->isInTheMap(tile);
 }
 
-std::set<CGObjectInstance*> EditorObstaclePlacer::placeObstacles(vstd::RNG & rand)
+std::set<std::shared_ptr<CGObjectInstance>> EditorObstaclePlacer::placeObstacles(vstd::RNG & rand)
 {
 	auto obstacles = createObstacles(rand, map->cb);
 	finalInsertion(map->getEditManager(), obstacles);

lib/mapping/ObstacleProxy.h

@@ -41,13 +41,13 @@ public:
 
 	virtual std::pair<bool, bool> verifyCoverage(const int3 & t) const;
 
-	virtual void placeObject(rmg::Object & object, std::set<CGObjectInstance*> & instances);
+	virtual void placeObject(rmg::Object & object, std::set<std::shared_ptr<CGObjectInstance>> & instances);
 
-	virtual std::set<CGObjectInstance*> createObstacles(vstd::RNG & rand, IGameCallback * cb);
+	virtual std::set<std::shared_ptr<CGObjectInstance>> createObstacles(vstd::RNG & rand, IGameCallback * cb);
 
 	virtual bool isInTheMap(const int3& tile) = 0;
 	
-	void finalInsertion(CMapEditManager * manager, std::set<CGObjectInstance*> & instances);
+	void finalInsertion(CMapEditManager * manager, std::set<std::shared_ptr<CGObjectInstance>> & instances);
 
 	virtual void postProcess(const rmg::Object& object) {};
 
@@ -70,7 +70,7 @@ public:
 
 	bool isInTheMap(const int3& tile) override;
 
-	std::set<CGObjectInstance*> placeObstacles(vstd::RNG& rand);
+	std::set<std::shared_ptr<CGObjectInstance>> placeObstacles(vstd::RNG& rand);
 
 private:
 	CMap* map;

lib/networkPacks/NetPacksLib.cpp

@@ -992,9 +992,8 @@ void FoWChange::applyGs(CGameState *gs)
 	if (mode == ETileVisibility::HIDDEN) //do not hide too much
 	{
 		std::unordered_set<int3> tilesRevealed;
-		for (auto & elem : gs->getMap().objects)
+		for (auto & o : gs->getMap().objects)
 		{
-			const CGObjectInstance *o = elem;
 			if (o)
 			{
 				switch(o->ID.toEnum())
@@ -1074,7 +1073,7 @@ void ChangeObjectVisitors::applyGs(CGameState *gs)
 			break;
 		case VISITOR_CLEAR:
 			// remove visit info from all heroes, including those that are not present on map
-			for (CGHeroInstance * hero : gs->getMap().allHeroes)
+			for (auto hero : gs->getMap().allHeroes)
 				if (hero)
 					hero->visitedObjects.erase(object);
 
@@ -1119,9 +1118,9 @@ void PlayerEndsGame::applyGs(CGameState *gs)
 		if(p->human && gs->getStartInfo()->campState)
 		{
 			std::vector<CGHeroInstance *> crossoverHeroes;
-			for (CGHeroInstance * hero : gs->getMap().heroesOnMap)
+			for (auto hero : gs->getMap().heroesOnMap)
 				if (hero->tempOwner == player)
-					crossoverHeroes.push_back(hero);
+					crossoverHeroes.push_back(hero.get());
 
 			gs->getStartInfo()->campState->setCurrentMapAsConquered(crossoverHeroes);
 		}
@@ -1203,7 +1202,7 @@ void RemoveObject::applyGs(CGameState *gs)
 	{
 		auto * beatenHero = dynamic_cast<CGHeroInstance *>(obj);
 		assert(beatenHero);
-		gs->getMap().heroesOnMap -= beatenHero;
+		gs->getMap().eraseObject(beatenHero->id);
 
 		auto * siegeNode = beatenHero->whereShouldBeAttachedOnSiege(gs);
 
@@ -1237,8 +1236,7 @@ void RemoveObject::applyGs(CGameState *gs)
 		if(beatenHero->boat)
 		{
 			beatenHero->detachFrom(const_cast<CGBoat&>(*beatenHero->boat));
-			gs->getMap().instanceNames.erase(beatenHero->boat->instanceName);
+			gs->getMap().eraseObject(beatenHero->boat->id);
-			gs->getMap().objects[beatenHero->boat->id.getNum()].dellNull();
 			beatenHero->boat = nullptr;
 		}
 		return;
@@ -1257,7 +1255,7 @@ void RemoveObject::applyGs(CGameState *gs)
 	}
 
 	gs->getMap().instanceNames.erase(obj->instanceName);
-	gs->getMap().objects[objectID.getNum()].dellNull();
+	gs->getMap().eraseObject(objectID);
 	gs->getMap().calculateGuardingGreaturePositions();//FIXME: excessive, update only affected tiles
 }
 
@@ -1445,13 +1443,14 @@ void HeroRecruited::applyGs(CGameState *gs)
 	h->pos = tile;
 	h->updateAppearance();
 
+	assert(h->id == ObjectInstanceID());
 	if(h->id == ObjectInstanceID())
 	{
 		h->id = ObjectInstanceID(static_cast<si32>(gs->getMap().objects.size()));
 		gs->getMap().objects.emplace_back(h);
 	}
-	else
+//	else
-		gs->getMap().objects[h->id.getNum()] = h;
+//		gs->getMap().replaceObject(h->id, h);
 
 	gs->getMap().heroesOnMap.emplace_back(h);
 	p->addOwnedObject(h);
@@ -1499,12 +1498,11 @@ void NewObject::applyGs(CGameState *gs)
 {
 	newObject->id = ObjectInstanceID(static_cast<si32>(gs->getMap().objects.size()));
 
-	gs->getMap().objects.emplace_back(newObject);
+	gs->getMap().addNewObject(newObject);
-	gs->getMap().addBlockVisTiles(newObject);
 	gs->getMap().calculateGuardingGreaturePositions();
 
 	// attach newly spawned wandering monster to global bonus system node
-	auto newArmy = dynamic_cast<CArmedInstance*>(newObject);
+	auto newArmy = std::dynamic_pointer_cast<CArmedInstance>(newObject);
 	if (newArmy)
 		newArmy->whatShouldBeAttached().attachTo(newArmy->whereShouldBeAttached(gs));
 
@@ -1518,27 +1516,6 @@ void NewArtifact::applyGs(CGameState *gs)
 	pa.applyGs(gs);
 }
 
-struct ObjectRetriever
-{
-	const CArmedInstance * operator()(const ConstTransitivePtr<CGHeroInstance> &h) const
-	{
-		return h;
-	}
-	const CArmedInstance * operator()(const ConstTransitivePtr<CStackInstance> &s) const
-	{
-		return s->armyObj;
-	}
-};
-template<typename T>
-struct GetBase
-{
-	template <typename TArg>
-	T * operator()(TArg &arg) const
-	{
-		return arg;
-	}
-};
-
 void ChangeStackCount::applyGs(CGameState *gs)
 {
 	auto * srcObj = gs->getArmyInstance(army);
@@ -1953,7 +1930,7 @@ void NewTurn::applyGs(CGameState *gs)
 	for(auto & creatureSet : availableCreatures) //set available creatures in towns
 		creatureSet.applyGs(gs);
 
-	for(CGTownInstance* t : gs->getMap().towns)
+	for(auto & t : gs->getMap().towns)
 	{
 		t->built = 0;
 		t->spellResearchCounterDay = 0;

lib/networkPacks/PacksForClient.h

@@ -806,7 +806,7 @@ struct DLL_LINKAGE NewObject : public CPackForClient
 	void applyGs(CGameState * gs) override;
 
 	/// Object ID to create
-	CGObjectInstance * newObject;
+	std::shared_ptr<CGObjectInstance> newObject;
 	/// Which player initiated creation of this object
 	PlayerColor initiator;
 

lib/serializer/CMemorySerializer.h

@@ -40,6 +40,17 @@ public:
 		mem.iser & ret;
 		return ret;
 	}
+
+	template <typename T>
+	static std::shared_ptr<T> deepCopyShared(const T &data)
+	{
+		CMemorySerializer mem;
+		mem.oser & &data;
+
+		std::shared_ptr<T> ret;
+		mem.iser & ret;
+		return ret;
+	}
 };
 
 VCMI_LIB_NAMESPACE_END

lib/serializer/CSerializer.cpp

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

lib/serializer/CSerializer.h

@@ -9,7 +9,6 @@
  */
 #pragma once
 
-#include "../ConstTransitivePtr.h"
 #include "../GameConstants.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
@@ -39,10 +38,10 @@ struct TypeComparer
 template <typename ObjType, typename IdType>
 struct VectorizedObjectInfo
 {
-	const std::vector<ConstTransitivePtr<ObjType> > *vector;	//pointer to the appropriate vector
+	const std::vector<std::shared_ptr<ObjType> > *vector;	//pointer to the appropriate vector
 	std::function<IdType(const ObjType &)> idRetriever;
 
-	VectorizedObjectInfo(const std::vector< ConstTransitivePtr<ObjType> > *Vector, std::function<IdType(const ObjType &)> IdGetter)
+	VectorizedObjectInfo(const std::vector< std::shared_ptr<ObjType> > *Vector, std::function<IdType(const ObjType &)> IdGetter)
 		:vector(Vector), idRetriever(IdGetter)
 	{
 	}
@@ -69,7 +68,7 @@ class DLL_LINKAGE CSerializer : boost::noncopyable
 		vectors[&typeid(T)] = VectorizedObjectInfo<T, U>(Vector, idRetriever);
 	}
 	template <typename T, typename U>
-	void registerVectoredType(const std::vector<ConstTransitivePtr<T> > *Vector, const std::function<U(const T&)> &idRetriever)
+	void registerVectoredType(const std::vector<std::shared_ptr<T> > *Vector, const std::function<U(const T&)> &idRetriever)
 	{
 		vectors[&typeid(T)] = VectorizedObjectInfo<T, U>(Vector, idRetriever);
 	}

lib/spells/AdventureSpellMechanics.cpp

@@ -201,11 +201,11 @@ ESpellCastResult SummonBoatMechanics::applyAdventureEffects(SpellCastEnvironment
 	//try to find unoccupied boat to summon
 	const CGBoat * nearest = nullptr;
 	double dist = 0;
-	for(const CGObjectInstance * obj : env->getMap()->objects)
+	for(const auto & obj : env->getMap()->objects)
 	{
 		if(obj && obj->ID == Obj::BOAT)
 		{
-			const auto * b = dynamic_cast<const CGBoat *>(obj);
+			const auto * b = dynamic_cast<const CGBoat *>(obj.get());
 			if(b->hero || b->layer != EPathfindingLayer::SAIL)
 				continue; //we're looking for unoccupied boat
 
@@ -726,12 +726,12 @@ ESpellCastResult ViewMechanics::applyAdventureEffects(SpellCastEnvironment * env
 
 	const auto & fowMap = env->getCb()->getPlayerTeam(parameters.caster->getCasterOwner())->fogOfWarMap;
 
-	for(const CGObjectInstance * obj : env->getMap()->objects)
+	for(const auto & obj : env->getMap()->objects)
 	{
 		//deleted object remain as empty pointer
-		if(obj && filterObject(obj, spellLevel))
+		if(obj && filterObject(obj.get(), spellLevel))
 		{
-			ObjectPosInfo posInfo(obj);
+			ObjectPosInfo posInfo(obj.get());
 
 			if(fowMap[posInfo.pos.z][posInfo.pos.x][posInfo.pos.y] == 0)
 				pack.objectPositions.push_back(posInfo);

server/CGameHandler.cpp

@@ -657,7 +657,7 @@ void CGameHandler::onNewTurn()
 		addStatistics(gameState()->statistic); // write at end of turn
 	}
 
-	for (CGTownInstance *t : gs->getMap().towns)
+	for (const auto & t : gs->getMap().towns)
 	{
 		PlayerColor player = t->tempOwner;
 
@@ -671,7 +671,7 @@ void CGameHandler::onNewTurn()
 		}
 	}
 
-	for (CGTownInstance *t : gs->getMap().towns)
+	for (const auto & t : gs->getMap().towns)
 	{
 		if (t->hasBonusOfType (BonusType::DARKNESS))
 		{
@@ -2682,7 +2682,7 @@ bool CGameHandler::bulkMoveArtifacts(const PlayerColor & player, ObjectInstanceI
 		auto dstSlot = ArtifactUtils::getArtAnyPosition(&artFittingSet, artifact->getTypeId());
 		if(dstSlot != ArtifactPosition::PRE_FIRST)
 		{
-			artFittingSet.putArtifact(dstSlot, static_cast<ConstTransitivePtr<CArtifactInstance>>(artifact));
+			artFittingSet.putArtifact(dstSlot, artifact);
 			slots.emplace_back(srcSlot, dstSlot);
 
 			// TODO Shouldn't be here. Possibly in callback after equipping the artifact
@@ -3570,8 +3570,8 @@ void CGameHandler::checkVictoryLossConditionsForPlayer(PlayerColor player)
 			//player lost -> all his objects become unflagged (neutral)
 			for (auto obj : gs->getMap().objects) //unflag objs
 			{
-				if (obj.get() && obj->tempOwner == player)
+				if (obj && obj->tempOwner == player)
-					setOwner(obj, PlayerColor::NEUTRAL);
+					setOwner(obj.get(), PlayerColor::NEUTRAL);
 			}
 
 			//eliminating one player may cause victory of another:
@@ -4059,11 +4059,6 @@ void CGameHandler::synchronizeArtifactHandlerLists()
 	sendAndApply(uahl);
 }
 
-bool CGameHandler::isValidObject(const CGObjectInstance *obj) const
-{
-	return vstd::contains(gs->getMap().objects, obj);
-}
-
 bool CGameHandler::isBlockedByQueries(const CPackForServer *pack, PlayerColor player)
 {
 	if (dynamic_cast<const PlayerMessage *>(pack) != nullptr)
@@ -4093,7 +4088,7 @@ void CGameHandler::removeAfterVisit(const ObjectInstanceID & id)
 	{
 		if (auto someVistQuery = std::dynamic_pointer_cast<MapObjectVisitQuery>(query))
 		{
-			if(someVistQuery->visitedObject->id == id)
+			if (someVistQuery->visitedObject == id)
 			{
 				someVistQuery->removeObjectAfterVisit = true;
 				return;
@@ -4154,8 +4149,8 @@ const CGHeroInstance * CGameHandler::getVisitingHero(const CGObjectInstance *obj
 	for(const auto & query : queries->allQueries())
 	{
 		auto visit = std::dynamic_pointer_cast<const VisitQuery>(query);
-		if (visit && visit->visitedObject == obj)
+		if (visit && visit->visitedObject == obj->id)
-			return visit->visitingHero;
+			return getHero(visit->visitingHero);
 	}
 	return nullptr;
 }
@@ -4167,8 +4162,8 @@ const CGObjectInstance * CGameHandler::getVisitingObject(const CGHeroInstance *h
 	for(const auto & query : queries->allQueries())
 	{
 		auto visit = std::dynamic_pointer_cast<const VisitQuery>(query);
-		if (visit && visit->visitingHero == hero)
+		if (visit && visit->visitingHero == hero->id)
-			return visit->visitedObject;
+			return getObjInstance(visit->visitedObject);
 	}
 	return nullptr;
 }
@@ -4184,7 +4179,7 @@ bool CGameHandler::isVisitCoveredByAnotherQuery(const CGObjectInstance *obj, con
 
 	if (auto topQuery = queries->topQuery(hero->getOwner()))
 		if (auto visit = std::dynamic_pointer_cast<const VisitQuery>(topQuery))
-			return !(visit->visitedObject == obj && visit->visitingHero == hero);
+			return !(visit->visitedObject == obj->id && visit->visitingHero == hero->id);
 
 	return true;
 }
@@ -4247,7 +4242,7 @@ scripting::Pool * CGameHandler::getGlobalContextPool() const
 #endif
 
 
-CGObjectInstance * CGameHandler::createNewObject(const int3 & visitablePosition, MapObjectID objectID, MapObjectSubID subID)
+std::shared_ptr<CGObjectInstance> CGameHandler::createNewObject(const int3 & visitablePosition, MapObjectID objectID, MapObjectSubID subID)
 {
 	TerrainId terrainType = ETerrainId::NONE;
 
@@ -4259,8 +4254,8 @@ CGObjectInstance * CGameHandler::createNewObject(const int3 & visitablePosition,
 
 	auto handler = LIBRARY->objtypeh->getHandlerFor(objectID, subID);
 
-	CGObjectInstance * o = handler->create(gs->callback, nullptr);
+	auto o = handler->create(gs->callback, nullptr);
-	handler->configureObject(o, getRandomGenerator());
+	handler->configureObject(o.get(), getRandomGenerator());
 	assert(o->ID == objectID);
 
 	assert(!handler->getTemplates(terrainType).empty());
@@ -4284,7 +4279,7 @@ void CGameHandler::createWanderingMonster(const int3 & visitablePosition, Creatu
 {
 	auto createdObject = createNewObject(visitablePosition, Obj::MONSTER, creature);
 
-	auto * cre = dynamic_cast<CGCreature *>(createdObject);
+	auto cre = std::dynamic_pointer_cast<CGCreature>(createdObject);
 	assert(cre);
 	cre->notGrowingTeam = cre->neverFlees = false;
 	cre->character = 2;
@@ -4307,7 +4302,7 @@ void CGameHandler::createHole(const int3 & visitablePosition, PlayerColor initia
 	newObject(createdObject, initiator);
 }
 
-void CGameHandler::newObject(CGObjectInstance * object, PlayerColor initiator)
+void CGameHandler::newObject(std::shared_ptr<CGObjectInstance> object, PlayerColor initiator)
 {
 	object->initObj(gs->getRandomGenerator());
 

server/CGameHandler.h

@@ -87,18 +87,17 @@ public:
 	events::EventBus * eventBus() const override;
 	CVCMIServer & gameLobby() const;
 
-	bool isValidObject(const CGObjectInstance *obj) const;
 	bool isBlockedByQueries(const CPackForServer *pack, PlayerColor player);
 	bool isAllowedExchange(ObjectInstanceID id1, ObjectInstanceID id2);
 	void giveSpells(const CGTownInstance *t, const CGHeroInstance *h);
 
 	// Helpers to create new object of specified type
 
-	CGObjectInstance * createNewObject(const int3 & visitablePosition, MapObjectID objectID, MapObjectSubID subID);
+	std::shared_ptr<CGObjectInstance> createNewObject(const int3 & visitablePosition, MapObjectID objectID, MapObjectSubID subID);
 	void createWanderingMonster(const int3 & visitablePosition, CreatureID creature);
 	void createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator) override;
 	void createHole(const int3 & visitablePosition, PlayerColor initiator);
-	void newObject(CGObjectInstance * object, PlayerColor initiator);
+	void newObject(std::shared_ptr<CGObjectInstance> object, PlayerColor initiator);
 
 	explicit CGameHandler(CVCMIServer * lobby);
 	~CGameHandler();

server/processors/NewTurnProcessor.cpp

@@ -507,7 +507,7 @@ RumorState NewTurnProcessor::pickNewRumor()
 
 std::tuple<EWeekType, CreatureID> NewTurnProcessor::pickWeekType(bool newMonth)
 {
-	for (const CGTownInstance *t : gameHandler->gameState()->getMap().towns)
+	for (const auto & t : gameHandler->gameState()->getMap().towns)
 	{
 		if (t->hasBuilt(BuildingID::GRAIL, ETownType::INFERNO))
 			return { EWeekType::DEITYOFFIRE, CreatureID::IMP };
@@ -666,8 +666,8 @@ NewTurn NewTurnProcessor::generateNewTurnPack()
 
 	if (newWeek)
 	{
-		for (CGTownInstance *t : gameHandler->gameState()->getMap().towns)
+		for (const auto & t : gameHandler->gameState()->getMap().towns)
-			n.availableCreatures.push_back(generateTownGrowth(t, n.specialWeek, n.creatureid, firstTurn));
+			n.availableCreatures.push_back(generateTownGrowth(t.get(), n.specialWeek, n.creatureid, firstTurn));
 	}
 
 	if (newWeek)
@@ -695,17 +695,17 @@ void NewTurnProcessor::onNewTurn()
 
 	if (newWeek)
 	{
-		for (CGTownInstance *t : gameHandler->gameState()->getMap().towns)
+		for (const auto & t : gameHandler->gameState()->getMap().towns)
 			if (t->hasBuilt(BuildingSubID::PORTAL_OF_SUMMONING))
-				gameHandler->setPortalDwelling(t, true, (n.specialWeek == EWeekType::PLAGUE ? true : false)); //set creatures for Portal of Summoning
+				gameHandler->setPortalDwelling(t.get(), true, (n.specialWeek == EWeekType::PLAGUE ? true : false)); //set creatures for Portal of Summoning
 	}
 
 	if (newWeek && !firstTurn)
 	{
-		for (CGTownInstance *t : gameHandler->gameState()->getMap().towns)
+		for (const auto & t : gameHandler->gameState()->getMap().towns)
 		{
 			if (!t->getOwner().isValidPlayer())
-				updateNeutralTownGarrison(t, 1 + gameHandler->getDate(Date::DAY) / 7);
+				updateNeutralTownGarrison(t.get(), 1 + gameHandler->getDate(Date::DAY) / 7);
 		}
 	}
 

server/queries/VisitQueries.cpp

@@ -10,6 +10,7 @@
 #include "StdInc.h"
 #include "VisitQueries.h"
 
+#include "../../lib/gameState/CGameState.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
 #include "../../lib/mapObjects/CGTownInstance.h"
 #include "../../lib/mapObjects/TownBuildingInstance.h"
@@ -18,8 +19,8 @@
 
 VisitQuery::VisitQuery(CGameHandler * owner, const CGObjectInstance * Obj, const CGHeroInstance * Hero)
 	: CQuery(owner)
-	, visitedObject(Obj)
+	, visitedObject(Obj->id)
-	, visitingHero(Hero)
+	, visitingHero(Hero->id)
 {
 	addPlayer(Hero->tempOwner);
 }
@@ -33,9 +34,12 @@ bool VisitQuery::blocksPack(const CPackForServer * pack) const
 
 void MapObjectVisitQuery::onExposure(QueryPtr topQuery)
 {
+	auto object = gh->gameState()->getObjInstance(visitedObject);
+	auto hero = gh->gameState()->getHero(visitingHero);
+
 	//Object may have been removed and deleted.
-	if(gh->isValidObject(visitedObject))
+	if (object)
-		topQuery->notifyObjectAboutRemoval(visitedObject, visitingHero);
+		topQuery->notifyObjectAboutRemoval(object, hero);
 
 	owner->popIfTop(*this);
 }
@@ -48,11 +52,13 @@ MapObjectVisitQuery::MapObjectVisitQuery(CGameHandler * owner, const CGObjectIns
 
 void MapObjectVisitQuery::onRemoval(PlayerColor color)
 {
-	gh->objectVisitEnded(visitingHero, players.front());
+	auto hero = gh->gameState()->getHero(visitingHero);
+
+	gh->objectVisitEnded(hero, players.front());
 
 	//Can object visit affect 2 players and what would be desired behavior?
 	if(removeObjectAfterVisit)
-		gh->removeObject(visitedObject, color);
+		gh->removeObject(hero, color);
 }
 
 TownBuildingVisitQuery::TownBuildingVisitQuery(CGameHandler * owner, const CGTownInstance * Obj, std::vector<const CGHeroInstance *> heroes, std::vector<BuildingID> buildingToVisit)
@@ -67,7 +73,10 @@ TownBuildingVisitQuery::TownBuildingVisitQuery(CGameHandler * owner, const CGTow
 
 void TownBuildingVisitQuery::onExposure(QueryPtr topQuery)
 {
-	topQuery->notifyObjectAboutRemoval(visitedObject, visitingHero);
+	auto object = gh->gameState()->getObjInstance(visitedObject);
+	auto hero = gh->gameState()->getHero(visitingHero);
+
+	topQuery->notifyObjectAboutRemoval(object, hero);
 
 	onAdded(players.front());
 }
@@ -76,9 +85,9 @@ void TownBuildingVisitQuery::onAdded(PlayerColor color)
 {
 	while (!visitedBuilding.empty() && owner->topQuery(color).get() == this)
 	{
-		visitingHero = visitedBuilding.back().hero;
+		visitingHero = visitedBuilding.back().hero->id;
 		const auto * building = visitedTown->rewardableBuildings.at(visitedBuilding.back().building);
-		building->onHeroVisit(visitingHero);
+		building->onHeroVisit(visitedBuilding.back().hero);
 		visitedBuilding.pop_back();
 	}
 

server/queries/VisitQueries.h

@@ -23,8 +23,8 @@ protected:
 	VisitQuery(CGameHandler * owner, const CGObjectInstance * Obj, const CGHeroInstance * Hero);
 
 public:
-	const CGObjectInstance * visitedObject;
+	ObjectInstanceID visitedObject;
-	const CGHeroInstance * visitingHero;
+	ObjectInstanceID visitingHero;
 
 	bool blocksPack(const CPackForServer * pack) const final;
 };

test/game/CGameStateTest.cpp

@@ -226,8 +226,8 @@ TEST_F(CGameStateTest, DISABLED_issue2765)
 {
 	startTestGame();
 
-	CGHeroInstance * attacker = map->heroesOnMap[0];
+	auto attacker = map->heroesOnMap[0];
-	CGHeroInstance * defender = map->heroesOnMap[1];
+	auto defender = map->heroesOnMap[1];
 
 	ASSERT_NE(attacker->tempOwner, defender->tempOwner);
 
@@ -239,7 +239,7 @@ TEST_F(CGameStateTest, DISABLED_issue2765)
 		gameCallback->sendAndApply(na);
 	}
 
-	startTestBattle(attacker, defender);
+	startTestBattle(attacker.get(), defender.get());
 
 	{
 		battle::UnitInfo info;
@@ -270,11 +270,11 @@ TEST_F(CGameStateTest, DISABLED_issue2765)
 	ASSERT_NE(def, nullptr);
 	ASSERT_NE(att, def);
 
-	EXPECT_NE(att->getMyHero(), defender);
+	EXPECT_NE(att->getMyHero(), defender.get());
-	EXPECT_NE(def->getMyHero(), attacker);
+	EXPECT_NE(def->getMyHero(), attacker.get());
 
-	EXPECT_EQ(att->getMyHero(), attacker) << att->nodeName();
+	EXPECT_EQ(att->getMyHero(), attacker.get()) << att->nodeName();
-	EXPECT_EQ(def->getMyHero(), defender) << def->nodeName();
+	EXPECT_EQ(def->getMyHero(), defender.get()) << def->nodeName();
 
 	{
 		using namespace ::testing;
@@ -308,8 +308,8 @@ TEST_F(CGameStateTest, DISABLED_battleResurrection)
 {
 	startTestGame();
 
-	CGHeroInstance * attacker = map->heroesOnMap[0];
+	auto attacker = map->heroesOnMap[0];
-	CGHeroInstance * defender = map->heroesOnMap[1];
+	auto defender = map->heroesOnMap[1];
 
 	ASSERT_NE(attacker->tempOwner, defender->tempOwner);
 
@@ -327,7 +327,7 @@ TEST_F(CGameStateTest, DISABLED_battleResurrection)
 		gameCallback->sendAndApply(na);
 	}
 
-	startTestBattle(attacker, defender);
+	startTestBattle(attacker.get(), defender.get());
 
 	uint32_t unitId = gameState->currentBattles.front()->battleNextUnitId();
 
@@ -380,7 +380,7 @@ TEST_F(CGameStateTest, DISABLED_battleResurrection)
 		const CSpell * spell = SpellID(SpellID::RESURRECTION).toSpell();
 		ASSERT_NE(spell, nullptr);
 
-			spells::BattleCast cast(gameState->currentBattles.front().get(), attacker, spells::Mode::HERO, spell);
+			spells::BattleCast cast(gameState->currentBattles.front().get(), attacker.get(), spells::Mode::HERO, spell);
 
 		spells::Target target;
 		target.emplace_back(unit);

test/map/MapComparer.cpp

@@ -232,7 +232,7 @@ void MapComparer::compareObjects()
 
 			auto actualObject = it->second;
 
-			compareObject(actualObject, expectedObject);
+			compareObject(actualObject.get(), expectedObject.get());
 		}
 	}
 }