Merge pull request #4025 from IvanSavenko/shared_ptr_lib

[1.6] Use shared_ptr for library entities

AI/VCAI/Goals/GatherArmy.cpp

@@ -152,8 +152,7 @@ TGoalVec GatherArmy::getAllPossibleSubgoals()
 						{
 							for(auto & creatureID : creLevel.second)
 							{
-								auto creature = VLC->creh->objects[creatureID];
-								if(ai->ah->freeResources().canAfford(creature->getFullRecruitCost()))
+								if(ai->ah->freeResources().canAfford(creatureID.toCreature()->getFullRecruitCost()))
 									objs.push_back(obj); //TODO: reserve resources?
 							}
 						}

client/CGameInfo.cpp

@@ -97,11 +97,6 @@ const IGameSettings * CGameInfo::settings() const
 	return globalServices->settings();
 }
 
-void CGameInfo::updateEntity(Metatype metatype, int32_t index, const JsonNode & data)
-{
-	logGlobal->error("CGameInfo::updateEntity call is not expected.");
-}
-
 spells::effects::Registry * CGameInfo::spellEffects()
 {
 	return nullptr;

client/CGameInfo.h

@@ -73,8 +73,6 @@ public:
 	const ObstacleService * obstacles() const override;
 	const IGameSettings * settings() const override;
 
-	void updateEntity(Metatype metatype, int32_t index, const JsonNode & data) override;
-
 	const spells::effects::Registry * spellEffects() const override;
 	spells::effects::Registry * spellEffects() override;
 

client/battle/BattleProjectileController.cpp

@@ -164,7 +164,7 @@ const CCreature & BattleProjectileController::getShooter(const CStack * stack) c
 	if(creature->animation.missleFrameAngles.empty())
 	{
 		logAnim->error("Mod error: Creature '%s' on the Archer's tower is not a shooter. Mod should be fixed. Trying to use archer's data instead...", creature->getNameSingularTranslated());
-		creature = CGI->creh->objects[CreatureID::ARCHER];
+		creature = CreatureID(CreatureID::ARCHER).toCreature();
 	}
 
 	return *creature;

client/battle/BattleSiegeController.cpp

@@ -188,7 +188,7 @@ BattleSiegeController::BattleSiegeController(BattleInterface & owner, const CGTo
 
 const CCreature *BattleSiegeController::getTurretCreature() const
 {
-	return CGI->creh->objects[town->town->clientInfo.siegeShooter];
+	return town->town->clientInfo.siegeShooter.toCreature();
 }
 
 Point BattleSiegeController::getTurretCreaturePosition( BattleHex position ) const

client/lobby/OptionsTab.cpp

@@ -446,7 +446,7 @@ int OptionsTab::SelectionWindow::calcLines(FactionID faction)
 	int count = 0;
 	for(auto & elemh : allowedHeroes)
 	{
-		CHero * type = VLC->heroh->objects[elemh];
+		const CHero * type = elemh.toHeroType();
 		if(type->heroClass->faction == faction)
 			count++;
 	}
@@ -603,7 +603,7 @@ void OptionsTab::SelectionWindow::genContentHeroes()
 
 	for(auto & elem : allowedHeroes)
 	{
-		CHero * type = VLC->heroh->objects[elem];
+		const CHero * type = elem.toHeroType();
 
 		if(type->heroClass->faction == selectedFaction)
 		{

client/lobby/RandomMapTab.cpp

@@ -114,12 +114,12 @@ RandomMapTab::RandomMapTab():
 		GH.windows().createAndPushWindow<TeamAlignments>(*this);
 	});
 	
-	for(auto road : VLC->roadTypeHandler->objects)
+	for(const auto & road : VLC->roadTypeHandler->objects)
 	{
 		std::string cbRoadType = "selectRoad_" + road->getJsonKey();
-		addCallback(cbRoadType, [&, road](bool on)
+		addCallback(cbRoadType, [&, roadID = road->getId()](bool on)
 		{
-			mapGenOptions->setRoadEnabled(road->getId(), on);
+			mapGenOptions->setRoadEnabled(roadID, on);
 			updateMapInfoByHost();
 		});
 	}
@@ -372,7 +372,7 @@ void RandomMapTab::setMapGenOptions(std::shared_ptr<CMapGenOptions> opts)
 		else
 			w->setTextOverlay(readText(variables["randomTemplate"]), EFonts::FONT_SMALL, Colors::WHITE);
 	}
-	for(auto r : VLC->roadTypeHandler->objects)
+	for(const auto & r : VLC->roadTypeHandler->objects)
 	{
 		// Workaround for vcmi-extras bug
 		std::string jsonKey = r->getJsonKey();
@@ -605,4 +605,4 @@ void RandomMapTab::loadOptions()
 	updateMapInfoByHost();
 
 	// TODO: Save & load difficulty?
-}
+}

client/widgets/CComponent.cpp

@@ -222,15 +222,15 @@ std::string CComponent::getDescription() const
 		case ComponentType::CREATURE:
 			return "";
 		case ComponentType::ARTIFACT:
-			return VLC->artifacts()->getById(data.subType.as<ArtifactID>())->getDescriptionTranslated();
+			return CGI->artifacts()->getById(data.subType.as<ArtifactID>())->getDescriptionTranslated();
 		case ComponentType::SPELL_SCROLL:
 		{
-			auto description = VLC->arth->objects[ArtifactID::SPELL_SCROLL]->getDescriptionTranslated();
+			auto description = ArtifactID(ArtifactID::SPELL_SCROLL).toEntity(CGI)->getDescriptionTranslated();
 			ArtifactUtils::insertScrrollSpellName(description, data.subType.as<SpellID>());
 			return description;
 		}
 		case ComponentType::SPELL:
-			return VLC->spells()->getById(data.subType.as<SpellID>())->getDescriptionTranslated(data.value.value_or(0));
+			return CGI->spells()->getById(data.subType.as<SpellID>())->getDescriptionTranslated(data.value.value_or(0));
 		case ComponentType::MORALE:
 			return CGI->generaltexth->heroscrn[ 4 - (data.value.value_or(0)>0) + (data.value.value_or(0)<0)];
 		case ComponentType::LUCK:

client/windows/CCastleInterface.cpp

@@ -272,7 +272,7 @@ CDwellingInfoBox::CDwellingInfoBox(int centerX, int centerY, const CGTownInstanc
 	OBJECT_CONSTRUCTION_CAPTURING(255-DISPOSE);
 	background->colorize(Town->tempOwner);
 
-	const CCreature * creature = CGI->creh->objects.at(Town->creatures.at(level).second.back());
+	const CCreature * creature = Town->creatures.at(level).second.back().toCreature();
 
 	title = std::make_shared<CLabel>(80, 30, FONT_SMALL, ETextAlignment::CENTER, Colors::WHITE, creature->getNamePluralTranslated());
 	animation = std::make_shared<CCreaturePic>(30, 44, creature, true, true);
@@ -1049,10 +1049,9 @@ CCreaInfo::CCreaInfo(Point position, const CGTownInstance * Town, int Level, boo
 	}
 	addUsedEvents(LCLICK | SHOW_POPUP | HOVER);
 
-	ui32 creatureID = town->creatures[level].second.back();
-	creature = CGI->creh->objects[creatureID];
+	CreatureID creatureID = town->creatures[level].second.back();
 
-	picture = std::make_shared<CAnimImage>(AnimationPath::builtin("CPRSMALL"), creature->getIconIndex(), 0, 8, 0);
+	picture = std::make_shared<CAnimImage>(AnimationPath::builtin("CPRSMALL"), creatureID.toEntity(VLC)->getIconIndex(), 0, 8, 0);
 
 	std::string value;
 	if(showAvailable)
@@ -1802,9 +1801,9 @@ CFortScreen::RecruitArea::RecruitArea(int posX, int posY, const CGTownInstance *
 const CCreature * CFortScreen::RecruitArea::getMyCreature()
 {
 	if(!town->creatures.at(level).second.empty()) // built
-		return VLC->creh->objects[town->creatures.at(level).second.back()];
+		return town->creatures.at(level).second.back().toCreature();
 	if(!town->town->creatures.at(level).empty()) // there are creatures on this level
-		return VLC->creh->objects[town->town->creatures.at(level).front()];
+		return town->town->creatures.at(level).front().toCreature();
 	return nullptr;
 }
 
@@ -1888,7 +1887,7 @@ CMageGuildScreen::CMageGuildScreen(CCastleInterface * owner, const ImagePath & i
 		for(size_t j=0; j<spellCount; j++)
 		{
 			if(i<owner->town->mageGuildLevel() && owner->town->spells[i].size()>j)
-				spells.push_back(std::make_shared<Scroll>(positions[i][j], CGI->spellh->objects[owner->town->spells[i][j]]));
+				spells.push_back(std::make_shared<Scroll>(positions[i][j], owner->town->spells[i][j].toSpell()));
 			else
 				emptyScrolls.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("TPMAGES.DEF"), 1, 0, positions[i][j].x, positions[i][j].y));
 		}
@@ -1938,7 +1937,7 @@ CBlacksmithDialog::CBlacksmithDialog(bool possible, CreatureID creMachineID, Art
 	animBG = std::make_shared<CPicture>(ImagePath::builtin("TPSMITBK"), 64, 50);
 	animBG->needRefresh = true;
 
-	const CCreature * creature = CGI->creh->objects[creMachineID];
+	const CCreature * creature = creMachineID.toCreature();
 	anim = std::make_shared<CCreatureAnim>(64, 50, creature->animDefName);
 	anim->clipRect(113,125,200,150);
 

client/windows/CHeroOverview.cpp

@@ -124,7 +124,7 @@ void CHeroOverview::genControls()
     r = Rect(302, 3 * borderOffset + yOffset + 62, 292, 32);
     backgroundRectangles.push_back(std::make_shared<TransparentFilledRectangle>(r.resize(1), rectangleColor, borderColor));
 
-    auto stacksCountChances = VLC->settings()->getVector(EGameSettings::HEROES_STARTING_STACKS_CHANCES);
+    auto stacksCountChances = CGI->settings()->getVector(EGameSettings::HEROES_STARTING_STACKS_CHANCES);
 
     // army
     int space = (260 - 7 * 32) / 6;

client/windows/CSpellWindow.cpp

@@ -290,11 +290,11 @@ void CSpellWindow::processSpells()
 
 	//initializing castable spells
 	mySpells.reserve(CGI->spellh->objects.size());
-	for(const CSpell * spell : CGI->spellh->objects)
+	for(auto const & spell : CGI->spellh->objects)
 	{
 		bool searchTextFound = !searchBox || boost::algorithm::contains(boost::algorithm::to_lower_copy(spell->getNameTranslated()), boost::algorithm::to_lower_copy(searchBox->getText()));
-		if(!spell->isCreatureAbility() && myHero->canCastThisSpell(spell) && searchTextFound)
-			mySpells.push_back(spell);
+		if(!spell->isCreatureAbility() && myHero->canCastThisSpell(spell.get()) && searchTextFound)
+			mySpells.push_back(spell.get());
 	}
 
 	SpellbookSpellSorter spellsorter;

client/windows/GUIClasses.cpp

@@ -261,7 +261,7 @@ void CRecruitmentWindow::availableCreaturesChanged()
 
 		//create new cards
 		for(auto & creature : boost::adaptors::reverse(dwelling->creatures[i].second))
-			cards.push_back(std::make_shared<CCreatureCard>(this, CGI->creh->objects[creature], amount));
+			cards.push_back(std::make_shared<CCreatureCard>(this, creature.toCreature(), amount));
 	}
 
 	const int creatureWidth = 102;

include/vcmi/Entity.h

@@ -30,7 +30,7 @@ public:
 	virtual bool isNativeTerrain(TerrainId terrain) const;
 };
 
-class DLL_LINKAGE Entity
+class DLL_LINKAGE Entity : boost::noncopyable
 {
 public:
 	using IconRegistar = std::function<void(int32_t index, int32_t group, const std::string & listName, const std::string & imageName)>;

include/vcmi/Services.h

@@ -61,8 +61,6 @@ public:
 	virtual const ObstacleService * obstacles() const = 0;
 	virtual const IGameSettings * settings() const = 0;
 
-	virtual void updateEntity(Metatype metatype, int32_t index, const JsonNode & data) = 0;
-
 	virtual const spells::effects::Registry * spellEffects() const = 0;
 	virtual spells::effects::Registry * spellEffects() = 0;
 	//TODO: put map object types registry access here

lib/BattleFieldHandler.cpp

@@ -15,11 +15,11 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-BattleFieldInfo * BattleFieldHandler::loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index)
+std::shared_ptr<BattleFieldInfo> BattleFieldHandler::loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index)
 {
 	assert(identifier.find(':') == std::string::npos);
 
-	auto * info = new BattleFieldInfo(BattleField(index), identifier);
+	auto info = std::make_shared<BattleFieldInfo>(BattleField(index), identifier);
 
 	info->modScope = scope;
 	info->graphics = ImagePath::fromJson(json["graphics"]);

lib/BattleFieldHandler.h

@@ -64,7 +64,7 @@ public:
 class BattleFieldHandler : public CHandlerBase<BattleField, BattleFieldInfo, BattleFieldInfo, BattleFieldService>
 {
 public:
-	virtual BattleFieldInfo * loadFromJson(
+	std::shared_ptr<BattleFieldInfo> loadFromJson(
 		const std::string & scope,
 		const JsonNode & json,
 		const std::string & identifier,

lib/CArtHandler.cpp

@@ -373,7 +373,7 @@ std::vector<JsonNode> CArtHandler::loadLegacyData()
 
 void CArtHandler::loadObject(std::string scope, std::string name, const JsonNode & data)
 {
-	auto * object = loadFromJson(scope, data, name, objects.size());
+	auto object = loadFromJson(scope, data, name, objects.size());
 
 	object->iconIndex = object->getIndex() + 5;
 
@@ -384,7 +384,7 @@ void CArtHandler::loadObject(std::string scope, std::string name, const JsonNode
 
 void CArtHandler::loadObject(std::string scope, std::string name, const JsonNode & data, size_t index)
 {
-	auto * object = loadFromJson(scope, data, name, index);
+	auto object = loadFromJson(scope, data, name, index);
 
 	object->iconIndex = object->getIndex();
 
@@ -400,12 +400,12 @@ const std::vector<std::string> & CArtHandler::getTypeNames() const
 	return typeNames;
 }
 
-CArtifact * CArtHandler::loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index)
+std::shared_ptr<CArtifact> CArtHandler::loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index)
 {
 	assert(identifier.find(':') == std::string::npos);
 	assert(!scope.empty());
 
-	CArtifact * art = new CArtifact();
+	auto art = std::make_shared<CArtifact>();
 	if(!node["growing"].isNull())
 	{
 		for(auto bonus : node["growing"]["bonusesPerLevel"].Vector())
@@ -442,10 +442,10 @@ CArtifact * CArtHandler::loadFromJson(const std::string & scope, const JsonNode
 	art->price = static_cast<ui32>(node["value"].Float());
 	art->onlyOnWaterMap = node["onlyOnWaterMap"].Bool();
 
-	loadSlots(art, node);
-	loadClass(art, node);
-	loadType(art, node);
-	loadComponents(art, node);
+	loadSlots(art.get(), node);
+	loadClass(art.get(), node);
+	loadType(art.get(), node);
+	loadComponents(art.get(), node);
 
 	for(const auto & b : node["bonuses"].Vector())
 	{
@@ -663,7 +663,7 @@ std::set<ArtifactID> CArtHandler::getDefaultAllowed() const
 {
 	std::set<ArtifactID> allowedArtifacts;
 
-	for (auto artifact : objects)
+	for (const auto & artifact : objects)
 	{
 		if (!artifact->isCombined())
 			allowedArtifacts.insert(artifact->getId());

lib/CArtHandler.h

@@ -14,6 +14,7 @@
 
 #include "bonuses/Bonus.h"
 #include "bonuses/CBonusSystemNode.h"
+#include "ConstTransitivePtr.h"
 #include "GameConstants.h"
 #include "IHandlerBase.h"
 #include "serializer/Serializeable.h"
@@ -163,7 +164,7 @@ public:
 
 protected:
 	const std::vector<std::string> & getTypeNames() const override;
-	CArtifact * loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index) override;
+	std::shared_ptr<CArtifact> loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index) override;
 
 private:
 	void addSlot(CArtifact * art, const std::string & slotID) const;

lib/CBonusTypeHandler.cpp

@@ -20,6 +20,7 @@
 #include "CGeneralTextHandler.h"
 #include "json/JsonUtils.h"
 #include "spells/CSpellHandler.h"
+#include "VCMI_Lib.h"
 
 template class std::vector<VCMI_LIB_WRAP_NAMESPACE(CBonusType)>;
 

lib/CCreatureHandler.cpp

@@ -587,12 +587,12 @@ std::vector<JsonNode> CCreatureHandler::loadLegacyData()
 	return h3Data;
 }
 
-CCreature * CCreatureHandler::loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index)
+std::shared_ptr<CCreature> CCreatureHandler::loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index)
 {
 	assert(identifier.find(':') == std::string::npos);
 	assert(!scope.empty());
 
-	auto * cre = new CCreature();
+	auto cre = std::make_shared<CCreature>();
 
 	if(node["hasDoubleWeek"].Bool())
 	{
@@ -635,9 +635,9 @@ CCreature * CCreatureHandler::loadFromJson(const std::string & scope, const Json
 	if(!node["shots"].isNull())
 		cre->addBonus(node["shots"].Integer(), BonusType::SHOTS);
 
-	loadStackExperience(cre, node["stackExperience"]);
-	loadJsonAnimation(cre, node["graphics"]);
-	loadCreatureJson(cre, node);
+	loadStackExperience(cre.get(), node["stackExperience"]);
+	loadJsonAnimation(cre.get(), node["graphics"]);
+	loadCreatureJson(cre.get(), node);
 
 	for(const auto & extraName : node["extraNames"].Vector())
 	{

lib/CCreatureHandler.h

@@ -209,7 +209,7 @@ private:
 
 protected:
 	const std::vector<std::string> & getTypeNames() const override;
-	CCreature * loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index) override;
+	std::shared_ptr<CCreature> loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index) override;
 
 public:
 	std::set<CreatureID> doubledCreatures; //they get double week

lib/CCreatureSet.h

@@ -16,6 +16,7 @@
 #include "CArtHandler.h"
 #include "CArtifactInstance.h"
 #include "CCreatureHandler.h"
+#include "VCMI_Lib.h"
 
 #include <vcmi/Entity.h>
 

lib/CGameInfoCallback.cpp

@@ -370,7 +370,7 @@ bool CGameInfoCallback::getHeroInfo(const CGObjectInstance * hero, InfoAboutHero
 			int maxAIValue = 0;
 			const CCreature * mostStrong = nullptr;
 
-			for(auto creature : VLC->creh->objects)
+			for(const auto & creature : VLC->creh->objects)
 			{
 				if(creature->getFaction() == factionIndex && static_cast<int>(creature->getAIValue()) > maxAIValue)
 				{

lib/CHeroHandler.cpp

@@ -247,14 +247,14 @@ const std::vector<std::string> & CHeroClassHandler::getTypeNames() const
 	return typeNames;
 }
 
-CHeroClass * CHeroClassHandler::loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index)
+std::shared_ptr<CHeroClass> CHeroClassHandler::loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index)
 {
 	assert(identifier.find(':') == std::string::npos);
 	assert(!scope.empty());
 
 	std::string affinityStr[2] = { "might", "magic" };
 
-	auto * heroClass = new CHeroClass();
+	auto heroClass = std::make_shared<CHeroClass>();
 
 	heroClass->id = HeroClassID(index);
 	heroClass->identifier = identifier;
@@ -277,10 +277,10 @@ CHeroClass * CHeroClassHandler::loadFromJson(const std::string & scope, const Js
 		heroClass->affinity = CHeroClass::MIGHT;
 	}
 
-	fillPrimarySkillData(node, heroClass, PrimarySkill::ATTACK);
-	fillPrimarySkillData(node, heroClass, PrimarySkill::DEFENSE);
-	fillPrimarySkillData(node, heroClass, PrimarySkill::SPELL_POWER);
-	fillPrimarySkillData(node, heroClass, PrimarySkill::KNOWLEDGE);
+	fillPrimarySkillData(node, heroClass.get(), PrimarySkill::ATTACK);
+	fillPrimarySkillData(node, heroClass.get(), PrimarySkill::DEFENSE);
+	fillPrimarySkillData(node, heroClass.get(), PrimarySkill::SPELL_POWER);
+	fillPrimarySkillData(node, heroClass.get(), PrimarySkill::KNOWLEDGE);
 
 	auto percentSumm = std::accumulate(heroClass->primarySkillLowLevel.begin(), heroClass->primarySkillLowLevel.end(), 0);
 	if(percentSumm <= 0)
@@ -432,12 +432,12 @@ const std::vector<std::string> & CHeroHandler::getTypeNames() const
 	return typeNames;
 }
 
-CHero * CHeroHandler::loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index)
+std::shared_ptr<CHero> CHeroHandler::loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index)
 {
 	assert(identifier.find(':') == std::string::npos);
 	assert(!scope.empty());
 
-	auto * hero = new CHero();
+	auto hero = std::make_shared<CHero>();
 	hero->ID = HeroTypeID(index);
 	hero->identifier = identifier;
 	hero->modScope = scope;
@@ -459,9 +459,9 @@ CHero * CHeroHandler::loadFromJson(const std::string & scope, const JsonNode & n
 	hero->portraitLarge = node["images"]["large"].String();
 	hero->battleImage = AnimationPath::fromJson(node["battleImage"]);
 
-	loadHeroArmy(hero, node);
-	loadHeroSkills(hero, node);
-	loadHeroSpecialty(hero, node);
+	loadHeroArmy(hero.get(), node);
+	loadHeroSkills(hero.get(), node);
+	loadHeroSpecialty(hero.get(), node);
 
 	VLC->identifiers()->requestIdentifier("heroClass", node["class"],
 	[=](si32 classID)
@@ -753,7 +753,7 @@ void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNod
 {
 	size_t index = objects.size();
 	static const int specialFramesCount = 2; // reserved for 2 special frames
-	auto * object = loadFromJson(scope, data, name, index);
+	auto object = loadFromJson(scope, data, name, index);
 	object->imageIndex = static_cast<si32>(index) + specialFramesCount;
 
 	objects.emplace_back(object);
@@ -766,7 +766,7 @@ void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNod
 
 void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNode & data, size_t index)
 {
-	auto * object = loadFromJson(scope, data, name, index);
+	auto object = loadFromJson(scope, data, name, index);
 	object->imageIndex = static_cast<si32>(index);
 
 	assert(objects[index] == nullptr); // ensure that this id was not loaded before

lib/CHeroHandler.h

@@ -171,7 +171,7 @@ public:
 
 protected:
 	const std::vector<std::string> & getTypeNames() const override;
-	CHeroClass * loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index) override;
+	std::shared_ptr<CHeroClass> loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index) override;
 
 };
 
@@ -209,7 +209,7 @@ public:
 
 protected:
 	const std::vector<std::string> & getTypeNames() const override;
-	CHero * loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index) override;
+	std::shared_ptr<CHero> loadFromJson(const std::string & scope, const JsonNode & node, const std::string & identifier, size_t index) override;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/CSkillHandler.cpp

@@ -22,6 +22,7 @@
 #include "modding/ModUtility.h"
 #include "modding/ModScope.h"
 #include "constants/StringConstants.h"
+#include "VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -190,7 +191,7 @@ const std::vector<std::string> & CSkillHandler::getTypeNames() const
 	return typeNames;
 }
 
-CSkill * CSkillHandler::loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index)
+std::shared_ptr<CSkill> CSkillHandler::loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index)
 {
 	assert(identifier.find(':') == std::string::npos);
 	assert(!scope.empty());
@@ -199,7 +200,7 @@ CSkill * CSkillHandler::loadFromJson(const std::string & scope, const JsonNode &
 
 	major = json["obligatoryMajor"].Bool();
 	minor = json["obligatoryMinor"].Bool();
-	auto * skill = new CSkill(SecondarySkill((si32)index), identifier, major, minor);
+	auto skill = std::make_shared<CSkill>(SecondarySkill((si32)index), identifier, major, minor);
 	skill->modScope = scope;
 
 	skill->onlyOnWaterMap = json["onlyOnWaterMap"].Bool();

lib/CSkillHandler.h

@@ -94,7 +94,7 @@ public:
 
 protected:
 	const std::vector<std::string> & getTypeNames() const override;
-	CSkill * loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index) override;
+	std::shared_ptr<CSkill> loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index) override;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/CTownHandler.cpp

@@ -1036,11 +1036,11 @@ void CTownHandler::loadPuzzle(CFaction &faction, const JsonNode &source) const
 	assert(faction.puzzleMap.size() == GameConstants::PUZZLE_MAP_PIECES);
 }
 
-CFaction * CTownHandler::loadFromJson(const std::string & scope, const JsonNode & source, const std::string & identifier, size_t index)
+std::shared_ptr<CFaction> CTownHandler::loadFromJson(const std::string & scope, const JsonNode & source, const std::string & identifier, size_t index)
 {
 	assert(identifier.find(':') == std::string::npos);
 
-	auto * faction = new CFaction();
+	auto faction = std::make_shared<CFaction>();
 
 	faction->index = static_cast<FactionID>(index);
 	faction->modScope = scope;
@@ -1091,7 +1091,7 @@ CFaction * CTownHandler::loadFromJson(const std::string & scope, const JsonNode
 	if (!source["town"].isNull())
 	{
 		faction->town = new CTown();
-		faction->town->faction = faction;
+		faction->town->faction = faction.get();
 		loadTown(faction->town, source["town"]);
 	}
 	else
@@ -1105,7 +1105,7 @@ CFaction * CTownHandler::loadFromJson(const std::string & scope, const JsonNode
 
 void CTownHandler::loadObject(std::string scope, std::string name, const JsonNode & data)
 {
-	auto * object = loadFromJson(scope, data, name, objects.size());
+	auto object = loadFromJson(scope, data, name, objects.size());
 
 	objects.emplace_back(object);
 
@@ -1144,7 +1144,7 @@ void CTownHandler::loadObject(std::string scope, std::string name, const JsonNod
 
 void CTownHandler::loadObject(std::string scope, std::string name, const JsonNode & data, size_t index)
 {
-	auto * object = loadFromJson(scope, data, name, index);
+	auto object = loadFromJson(scope, data, name, index);
 
 	if (objects.size() > index)
 		assert(objects[index] == nullptr); // ensure that this id was not loaded before
@@ -1260,7 +1260,7 @@ std::set<FactionID> CTownHandler::getDefaultAllowed() const
 {
 	std::set<FactionID> allowedFactions;
 
-	for(auto town : objects)
+	for(const auto & town : objects)
 		if (town->town != nullptr && !town->special)
 			allowedFactions.insert(town->getId());
 
@@ -1273,7 +1273,7 @@ std::set<FactionID> CTownHandler::getAllowedFactions(bool withTown) const
 		return getDefaultAllowed();
 
 	std::set<FactionID> result;
-	for(auto town : objects)
+	for(const auto & town : objects)
 		result.insert(town->getId());
 
 	return result;

lib/CTownHandler.h

@@ -352,7 +352,7 @@ public:
 
 protected:
 	const std::vector<std::string> & getTypeNames() const override;
-	CFaction * loadFromJson(const std::string & scope, const JsonNode & data, const std::string & identifier, size_t index) override;
+	std::shared_ptr<CFaction> loadFromJson(const std::string & scope, const JsonNode & data, const std::string & identifier, size_t index) override;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/IHandlerBase.cpp

@@ -13,6 +13,7 @@
 #include "modding/IdentifierStorage.h"
 #include "modding/ModScope.h"
 #include "modding/CModHandler.h"
+#include "VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

lib/IHandlerBase.h

@@ -9,9 +9,6 @@
  */
 #pragma once
 
-#include "../lib/ConstTransitivePtr.h"
-#include "VCMI_Lib.h"
-
 VCMI_LIB_NAMESPACE_BEGIN
 
 class JsonNode;
@@ -60,13 +57,7 @@ template <class _ObjectID, class _ObjectBase, class _Object, class _ServiceBase>
 	}
 
 public:
-	virtual ~CHandlerBase()
-	{
-		for(auto & o : objects)
-		{
-			o.dellNull();
-		}
-	}
+	using ObjectPtr = std::shared_ptr<_Object>;
 
 	const Entity * getBaseByIndex(const int32_t index) const override
 	{
@@ -95,23 +86,19 @@ public:
 
 	void loadObject(std::string scope, std::string name, const JsonNode & data) override
 	{
-		auto object = loadFromJson(scope, data, name, objects.size());
-
-		objects.push_back(object);
+		objects.push_back(loadFromJson(scope, data, name, objects.size()));
 
 		for(const auto & type_name : getTypeNames())
-			registerObject(scope, type_name, name, object->getIndex());
+			registerObject(scope, type_name, name, objects.back()->getIndex());
 	}
 
 	void loadObject(std::string scope, std::string name, const JsonNode & data, size_t index) override
 	{
-		auto object = loadFromJson(scope, data, name, index);
-
 		assert(objects[index] == nullptr); // ensure that this id was not loaded before
-		objects[index] = object;
+		objects[index] = loadFromJson(scope, data, name, index);
 
 		for(const auto & type_name : getTypeNames())
-			registerObject(scope, type_name, name, object->getIndex());
+			registerObject(scope, type_name, name, objects[index]->getIndex());
 	}
 
 	const _Object * operator[] (const _ObjectID id) const
@@ -124,25 +111,13 @@ public:
 		return getObjectImpl(index);
 	}
 
-	void updateEntity(int32_t index, const JsonNode & data)
-	{
-		if(index < 0 || index >= objects.size())
-		{
-			logMod->error("%s id %d is invalid", getTypeNames()[0], index);
-		}
-		else
-		{
-			objects.at(index)->updateFrom(data);
-		}
-	}
-
 	size_t size() const
 	{
 		return objects.size();
 	}
 
 protected:
-	virtual _Object * loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index) = 0;
+	virtual ObjectPtr loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index) = 0;
 	virtual const std::vector<std::string> & getTypeNames() const = 0;
 
 	template<typename ItemType>
@@ -159,7 +134,7 @@ protected:
 	}
 
 public: //todo: make private
-	std::vector<ConstTransitivePtr<_Object>> objects;
+	std::vector<ObjectPtr> objects;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/ObstacleHandler.cpp

@@ -12,6 +12,7 @@
 #include "BattleFieldHandler.h"
 #include "json/JsonNode.h"
 #include "modding/IdentifierStorage.h"
+#include "VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -84,11 +85,11 @@ bool ObstacleInfo::isAppropriate(const TerrainId terrainType, const BattleField
 	return vstd::contains(allowedTerrains, terrainType);
 }
 
-ObstacleInfo * ObstacleHandler::loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index)
+std::shared_ptr<ObstacleInfo> ObstacleHandler::loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index)
 {
 	assert(identifier.find(':') == std::string::npos);
 
-	auto * info = new ObstacleInfo(Obstacle(index), identifier);
+	auto info = std::make_shared<ObstacleInfo>(Obstacle(index), identifier);
 	
 	info->animation = AnimationPath::fromJson(json["animation"]);
 	info->width = json["width"].Integer();

lib/ObstacleHandler.h

@@ -64,8 +64,8 @@ public:
 
 class ObstacleHandler: public CHandlerBase<Obstacle, ObstacleInfo, ObstacleInfo, ObstacleService>
 {
-public:	
-	ObstacleInfo * loadFromJson(const std::string & scope,
+public:
+	std::shared_ptr<ObstacleInfo> loadFromJson(const std::string & scope,
 										const JsonNode & json,
 										const std::string & identifier,
 										size_t index) override;

lib/RiverHandler.cpp

@@ -13,17 +13,18 @@
 #include "CGeneralTextHandler.h"
 #include "GameSettings.h"
 #include "json/JsonNode.h"
+#include "VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
 RiverTypeHandler::RiverTypeHandler()
 {
-	objects.push_back(new RiverType());
+	objects.emplace_back(new RiverType());
 
 	VLC->generaltexth->registerString("core", objects[0]->getNameTextID(), "");
 }
 
-RiverType * RiverTypeHandler::loadFromJson(
+std::shared_ptr<RiverType> RiverTypeHandler::loadFromJson(
 	const std::string & scope,
 	const JsonNode & json,
 	const std::string & identifier,
@@ -31,7 +32,7 @@ RiverType * RiverTypeHandler::loadFromJson(
 {
 	assert(identifier.find(':') == std::string::npos);
 
-	auto * info = new RiverType;
+	auto info = std::make_shared<RiverType>();
 
 	info->id              = RiverId(index);
 	info->identifier      = identifier;

lib/RiverHandler.h

@@ -61,7 +61,7 @@ public:
 class DLL_LINKAGE RiverTypeHandler : public CHandlerBase<RiverId, RiverType, RiverType, RiverTypeService>
 {
 public:
-	virtual RiverType * loadFromJson(
+	std::shared_ptr<RiverType> loadFromJson(
 		const std::string & scope,
 		const JsonNode & json,
 		const std::string & identifier,

lib/RoadHandler.cpp

@@ -13,17 +13,18 @@
 #include "CGeneralTextHandler.h"
 #include "GameSettings.h"
 #include "json/JsonNode.h"
+#include "VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
 RoadTypeHandler::RoadTypeHandler()
 {
-	objects.push_back(new RoadType());
+	objects.emplace_back(new RoadType());
 
 	VLC->generaltexth->registerString("core", objects[0]->getNameTextID(), "");
 }
 
-RoadType * RoadTypeHandler::loadFromJson(
+std::shared_ptr<RoadType> RoadTypeHandler::loadFromJson(
 	const std::string & scope,
 	const JsonNode & json,
 	const std::string & identifier,
@@ -31,7 +32,7 @@ RoadType * RoadTypeHandler::loadFromJson(
 {
 	assert(identifier.find(':') == std::string::npos);
 
-	auto * info = new RoadType;
+	auto info = std::make_shared<RoadType>();
 
 	info->id              = RoadId(index);
 	info->identifier      = identifier;

lib/RoadHandler.h

@@ -51,7 +51,7 @@ public:
 class DLL_LINKAGE RoadTypeHandler : public CHandlerBase<RoadId, RoadType, RoadType, RoadTypeService>
 {
 public:
-	virtual RoadType * loadFromJson(
+	std::shared_ptr<RoadType> loadFromJson(
 		const std::string & scope,
 		const JsonNode & json,
 		const std::string & identifier,

lib/TerrainHandler.cpp

@@ -14,14 +14,15 @@
 #include "GameSettings.h"
 #include "json/JsonNode.h"
 #include "modding/IdentifierStorage.h"
+#include "VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-TerrainType * TerrainTypeHandler::loadFromJson( const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index)
+std::shared_ptr<TerrainType> TerrainTypeHandler::loadFromJson( const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index)
 {
 	assert(identifier.find(':') == std::string::npos);
 
-	auto * info = new TerrainType;
+	auto info = std::make_shared<TerrainType>();
 
 	info->id = TerrainId(index);
 	info->identifier = identifier;

lib/TerrainHandler.h

@@ -101,7 +101,7 @@ public:
 class DLL_LINKAGE TerrainTypeHandler : public CHandlerBase<TerrainId, TerrainType, TerrainType, TerrainTypeService>
 {
 public:
-	virtual TerrainType * loadFromJson(
+	std::shared_ptr<TerrainType> loadFromJson(
 		const std::string & scope,
 		const JsonNode & json,
 		const std::string & identifier,

lib/VCMI_Lib.cpp

@@ -141,37 +141,6 @@ const IGameSettings * LibClasses::settings() const
 	return settingsHandler.get();
 }
 
-void LibClasses::updateEntity(Metatype metatype, int32_t index, const JsonNode & data)
-{
-	switch(metatype)
-	{
-	case Metatype::ARTIFACT:
-		arth->updateEntity(index, data);
-		break;
-	case Metatype::CREATURE:
-		creh->updateEntity(index, data);
-		break;
-	case Metatype::FACTION:
-		townh->updateEntity(index, data);
-		break;
-	case Metatype::HERO_CLASS:
-		heroclassesh->updateEntity(index, data);
-		break;
-	case Metatype::HERO_TYPE:
-		heroh->updateEntity(index, data);
-		break;
-	case Metatype::SKILL:
-		skillh->updateEntity(index, data);
-		break;
-	case Metatype::SPELL:
-		spellh->updateEntity(index, data);
-		break;
-	default:
-		logGlobal->error("Invalid Metatype id %d", static_cast<int32_t>(metatype));
-		break;
-	}
-}
-
 void LibClasses::loadFilesystem(bool extractArchives)
 {
 	CStopWatch loadTime;

lib/VCMI_Lib.h

@@ -72,8 +72,6 @@ public:
 	const ObstacleService * obstacles() const override;
 	const IGameSettings * settings() const override;
 
-	void updateEntity(Metatype metatype, int32_t index, const JsonNode & data) override;
-
 	const spells::effects::Registry * spellEffects() const override;
 	spells::effects::Registry * spellEffects() override;
 

lib/battle/CObstacleInstance.cpp

@@ -107,7 +107,6 @@ SpellID CObstacleInstance::getTrigger() const
 
 void CObstacleInstance::serializeJson(JsonSerializeFormat & handler)
 {
-	auto obstacleInfo = getInfo();
 	auto hidden = false;
 	auto needAnimationOffsetFix = obstacleType == CObstacleInstance::USUAL;
 	int animationYOffset = 0;
@@ -117,11 +116,7 @@ void CObstacleInstance::serializeJson(JsonSerializeFormat & handler)
 
 	//We need only a subset of obstacle info for correct render
 	handler.serializeInt("position", pos);
-	handler.serializeStruct("appearSound", obstacleInfo.appearSound);
-	handler.serializeStruct("appearAnimation", obstacleInfo.appearAnimation);
-	handler.serializeStruct("animation", obstacleInfo.animation);
 	handler.serializeInt("animationYOffset", animationYOffset);
-
 	handler.serializeBool("hidden", hidden);
 	handler.serializeBool("needAnimationOffsetFix", needAnimationOffsetFix);
 }

lib/gameState/CGameState.cpp

@@ -282,7 +282,7 @@ void CGameState::updateEntity(Metatype metatype, int32_t index, const JsonNode &
 		}
 		break;
 	default:
-		services->updateEntity(metatype, index, data);
+		logGlobal->error("This metatype update is not implemented");
 		break;
 	}
 }

lib/mapObjects/ObstacleSetHandler.cpp

@@ -14,6 +14,7 @@
 #include "../modding/IdentifierStorage.h"
 #include "../constants/StringConstants.h"
 #include "../TerrainHandler.h"
+#include "../VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

lib/mapping/CDrawRoadsOperation.cpp

@@ -15,6 +15,7 @@
 #include "../CRandomGenerator.h"
 #include "../RoadHandler.h"
 #include "../RiverHandler.h"
+#include "../VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

lib/mapping/MapEditUtils.cpp

@@ -13,6 +13,7 @@
 
 #include "../filesystem/Filesystem.h"
 #include "../TerrainHandler.h"
+#include "../VCMI_Lib.h"
 #include "CMap.h"
 #include "CMapOperation.h"
 

lib/mapping/MapFormatJson.cpp

@@ -296,9 +296,9 @@ void CMapFormatJson::serializeAllowedFactions(JsonSerializeFormat & handler, std
 
 	if(handler.saving)
 	{
-		for(auto faction : VLC->townh->objects)
-			if(faction->town && vstd::contains(value, faction->getId()))
-				temp.insert(faction->getId());
+		for(auto const factionID : VLC->townh->getDefaultAllowed())
+			if(vstd::contains(value, factionID))
+				temp.insert(factionID);
 	}
 
 	handler.serializeLIC("allowedFactions", &FactionID::decode, &FactionID::encode, VLC->townh->getDefaultAllowed(), temp);

lib/mapping/ObstacleProxy.cpp

@@ -16,6 +16,7 @@
 #include "../mapObjects/CGObjectInstance.h"
 #include "../mapObjects/ObjectTemplate.h"
 #include "../mapObjects/ObstacleSetHandler.h"
+#include "../VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

lib/modding/CModHandler.cpp

@@ -27,6 +27,7 @@
 #include "../filesystem/Filesystem.h"
 #include "../json/JsonUtils.h"
 #include "../spells/CSpellHandler.h"
+#include "../VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

lib/modding/ContentTypeHandler.cpp

@@ -36,6 +36,7 @@
 #include "../mapObjectConstructors/CObjectClassesHandler.h"
 #include "../rmg/CRmgTemplateStorage.h"
 #include "../spells/CSpellHandler.h"
+#include "../VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

lib/rmg/CMapGenerator.cpp

@@ -100,8 +100,9 @@ const CMapGenOptions& CMapGenerator::getMapGenOptions() const
 void CMapGenerator::initQuestArtsRemaining()
 {
 	//TODO: Move to QuestArtifactPlacer?
-	for (auto art : VLC->arth->objects)
+	for (auto artID : VLC->arth->getDefaultAllowed())
 	{
+		auto art = artID.toArtifact();
 		//Don't use parts of combined artifacts
 		if (art->aClass == CArtifact::ART_TREASURE && VLC->arth->legalArtifact(art->getId()) && art->getPartOf().empty())
 			questArtifacts.push_back(art->getId());

lib/rmg/CRmgTemplate.cpp

@@ -176,7 +176,7 @@ void ZoneOptions::setTerrainTypes(const std::set<TerrainId> & value)
 std::set<TerrainId> ZoneOptions::getDefaultTerrainTypes() const
 {
 	std::set<TerrainId> terrains;
-	for (auto terrain : VLC->terrainTypeHandler->objects)
+	for(const auto & terrain : VLC->terrainTypeHandler->objects)
 	{
 		if (terrain->isLand() && terrain->isPassable())
 		{

lib/rmg/CZonePlacer.cpp

@@ -16,6 +16,7 @@
 #include "../TerrainHandler.h"
 #include "../mapping/CMap.h"
 #include "../mapping/CMapEditManager.h"
+#include "../VCMI_Lib.h"
 #include "CMapGenOptions.h"
 #include "RmgMap.h"
 #include "Zone.h"

lib/rmg/RmgMap.cpp

@@ -16,6 +16,7 @@
 #include "../mapping/CMapEditManager.h"
 #include "../mapping/CMap.h"
 #include "../CTownHandler.h"
+#include "../VCMI_Lib.h"
 #include "modificators/ObjectManager.h"
 #include "modificators/RoadPlacer.h"
 #include "modificators/TreasurePlacer.h"

lib/rmg/modificators/ObjectManager.cpp

@@ -725,7 +725,7 @@ CGCreature * ObjectManager::chooseGuard(si32 strength, bool zoneGuard)
 	CreatureID creId = CreatureID::NONE;
 	int amount = 0;
 	std::vector<CreatureID> possibleCreatures;
-	for(auto cre : VLC->creh->objects)
+	for(auto const & cre : VLC->creh->objects)
 	{
 		if(cre->special)
 			continue;

lib/rmg/modificators/RiverPlacer.cpp

@@ -20,6 +20,7 @@
 #include "../../mapObjects/ObjectTemplate.h"
 #include "../../mapping/CMap.h"
 #include "../../mapping/CMapEditManager.h"
+#include "../../VCMI_Lib.h"
 #include "../RmgPath.h"
 #include "ObjectManager.h"
 #include "ObstaclePlacer.h"

lib/rmg/modificators/RoadPlacer.cpp

@@ -21,6 +21,7 @@
 #include "../../modding/IdentifierStorage.h"
 #include "../../modding/ModScope.h"
 #include "../../TerrainHandler.h"
+#include "../../VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

lib/rmg/modificators/RockPlacer.cpp

@@ -20,6 +20,7 @@
 #include "../../TerrainHandler.h"
 #include "../../CRandomGenerator.h"
 #include "../../mapping/CMapEditManager.h"
+#include "../../VCMI_Lib.h"
 #include "../TileInfo.h"
 
 VCMI_LIB_NAMESPACE_BEGIN

lib/rmg/modificators/TerrainPainter.cpp

@@ -76,7 +76,7 @@ void TerrainPainter::initTerrainType()
 			{
 				//Fill with all terain types by default
 				{
-					for (auto terrain : VLC->terrainTypeHandler->objects)
+					for (const auto & terrain : VLC->terrainTypeHandler->objects)
 					{
 						if (terrain->isLand() && terrain->isPassable())
 						{

lib/rmg/modificators/TownPlacer.cpp

@@ -77,11 +77,9 @@ void TownPlacer::placeTowns(ObjectManager & manager)
 		town->builtBuildings.insert(BuildingID::FORT);
 		town->builtBuildings.insert(BuildingID::DEFAULT);
 		
-		for(auto spell : VLC->spellh->objects) //add all regular spells to town
-		{
-			if(!spell->isSpecial() && !spell->isCreatureAbility())
-				town->possibleSpells.push_back(spell->id);
-		}
+
+		for(auto spellID : VLC->spellh->getDefaultAllowed()) //add all regular spells to town
+			town->possibleSpells.push_back(spellID);
 		
 		auto position = placeMainTown(manager, *town);
 		
@@ -202,11 +200,8 @@ void TownPlacer::addNewTowns(int count, bool hasFort, const PlayerColor & player
 			town->builtBuildings.insert(BuildingID::FORT);
 		town->builtBuildings.insert(BuildingID::DEFAULT);
 		
-		for(auto spell : VLC->spellh->objects) //add all regular spells to town
-		{
-			if(!spell->isSpecial() && !spell->isCreatureAbility())
-				town->possibleSpells.push_back(spell->id);
-		}
+		for(auto spellID : VLC->spellh->getDefaultAllowed()) //add all regular spells to town
+			town->possibleSpells.push_back(spellID);
 		
 		if(totalTowns <= 0)
 		{

lib/rmg/modificators/TreasurePlacer.cpp

@@ -163,7 +163,7 @@ void TreasurePlacer::addAllPossibleObjects()
 	oi.maxPerZone = std::numeric_limits<ui32>::max();
 
 	std::vector<const CCreature *> creatures; //native creatures for this zone
-	for(auto cre : VLC->creh->objects)
+	for(auto const & cre : VLC->creh->objects)
 	{
 		if(!cre->special && cre->getFaction() == zone.getTownType())
 		{
@@ -221,12 +221,10 @@ void TreasurePlacer::addAllPossibleObjects()
 			auto * obj = dynamic_cast<CGArtifact *>(factory->create(map.mapInstance->cb, nullptr));
 			std::vector<SpellID> out;
 			
-			for(auto spell : VLC->spellh->objects) //spellh size appears to be greater (?)
+			for(auto spellID : VLC->spellh->getDefaultAllowed())
 			{
-				if(map.isAllowedSpell(spell->id) && spell->getLevel() == i + 1)
-				{
-					out.push_back(spell->id);
-				}
+				if(map.isAllowedSpell(spellID) && spellID.toSpell()->getLevel() == i + 1)
+					out.push_back(spellID);
 			}
 			auto * a = ArtifactUtils::createScroll(*RandomGeneratorUtil::nextItem(out, zone.getRand()));
 			obj->storedArtifact = a;
@@ -354,10 +352,10 @@ void TreasurePlacer::addAllPossibleObjects()
 			auto * obj = dynamic_cast<CGPandoraBox *>(factory->create(map.mapInstance->cb, nullptr));
 
 			std::vector <const CSpell *> spells;
-			for(auto spell : VLC->spellh->objects)
+			for(auto spellID : VLC->spellh->getDefaultAllowed())
 			{
-				if(map.isAllowedSpell(spell->id) && spell->getLevel() == i)
-					spells.push_back(spell.get());
+				if(map.isAllowedSpell(spellID) && spellID.toSpell()->getLevel() == i)
+					spells.push_back(spellID.toSpell());
 			}
 			
 			RandomGeneratorUtil::randomShuffle(spells, zone.getRand());
@@ -387,10 +385,10 @@ void TreasurePlacer::addAllPossibleObjects()
 			auto * obj = dynamic_cast<CGPandoraBox *>(factory->create(map.mapInstance->cb, nullptr));
 
 			std::vector <const CSpell *> spells;
-			for(auto spell : VLC->spellh->objects)
+			for(auto spellID : VLC->spellh->getDefaultAllowed())
 			{
-				if(map.isAllowedSpell(spell->id) && spell->hasSchool(SpellSchool(i)))
-					spells.push_back(spell.get());
+				if(map.isAllowedSpell(spellID) && spellID.toSpell()->hasSchool(SpellSchool(i)))
+					spells.push_back(spellID.toSpell());
 			}
 			
 			RandomGeneratorUtil::randomShuffle(spells, zone.getRand());
@@ -419,10 +417,10 @@ void TreasurePlacer::addAllPossibleObjects()
 		auto * obj = dynamic_cast<CGPandoraBox *>(factory->create(map.mapInstance->cb, nullptr));
 
 		std::vector <const CSpell *> spells;
-		for(auto spell : VLC->spellh->objects)
+		for(auto spellID : VLC->spellh->getDefaultAllowed())
 		{
-			if(map.isAllowedSpell(spell->id))
-				spells.push_back(spell.get());
+			if(map.isAllowedSpell(spellID))
+				spells.push_back(spellID.toSpell());
 		}
 		
 		RandomGeneratorUtil::randomShuffle(spells, zone.getRand());

lib/rmg/threadpool/MapProxy.cpp

@@ -10,6 +10,7 @@
 
 #include "MapProxy.h"
 #include "../../TerrainHandler.h"
+#include "../../VCMI_Lib.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

lib/spells/CSpellHandler.cpp

@@ -676,7 +676,7 @@ const std::vector<std::string> & CSpellHandler::getTypeNames() const
 	return typeNames;
 }
 
-CSpell * CSpellHandler::loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index)
+std::shared_ptr<CSpell> CSpellHandler::loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index)
 {
 	assert(identifier.find(':') == std::string::npos);
 	assert(!scope.empty());
@@ -685,7 +685,7 @@ CSpell * CSpellHandler::loadFromJson(const std::string & scope, const JsonNode &
 
 	SpellID id(static_cast<si32>(index));
 
-	auto * spell = new CSpell();
+	auto spell = std::make_shared<CSpell>();
 	spell->id = id;
 	spell->identifier = identifier;
 	spell->modScope = scope;

lib/spells/CSpellHandler.h

@@ -355,7 +355,7 @@ public:
 
 protected:
 	const std::vector<std::string> & getTypeNames() const override;
-	CSpell * loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index) override;
+	std::shared_ptr<CSpell> loadFromJson(const std::string & scope, const JsonNode & json, const std::string & identifier, size_t index) override;
 };
 
 VCMI_LIB_NAMESPACE_END

mapeditor/inspector/armywidget.cpp

@@ -36,7 +36,7 @@ ArmyWidget::ArmyWidget(CArmedInstance & a, QWidget *parent) :
 		
 		for(int c = 0; c < VLC->creh->objects.size(); ++c)
 		{
-			auto creature = VLC->creh->objects[c];
+			auto const & creature = VLC->creh->objects[c];
 			uiSlots[i]->insertItem(c + 1, creature->getNamePluralTranslated().c_str());
 			uiSlots[i]->setItemData(c + 1, creature->getIndex());
 		}

mapeditor/inspector/heroskillswidget.cpp

@@ -34,7 +34,7 @@ HeroSkillsWidget::HeroSkillsWidget(CGHeroInstance & h, QWidget *parent) :
 	ui->labelKnowledge->setText(QString::fromStdString(NPrimarySkill::names[3]));
 	
 	auto * delegate = new InspectorDelegate;
-	for(auto s : VLC->skillh->objects)
+	for(auto const & s : VLC->skillh->objects)
 		delegate->options.push_back({QString::fromStdString(s->getNameTranslated()), QVariant::fromValue(s->getId().getNum())});
 	ui->skills->setItemDelegateForColumn(0, delegate);
 	

mapeditor/inspector/herospellwidget.cpp

@@ -44,14 +44,16 @@ void HeroSpellWidget::obtainData()
 void HeroSpellWidget::initSpellLists()
 {
 	QListWidget * spellLists[] = { ui->spellList1, ui->spellList2, ui->spellList3, ui->spellList4, ui->spellList5 };
-	auto spells = VLC->spellh->objects;
+
 	for (int i = 0; i < GameConstants::SPELL_LEVELS; i++)
 	{
-		std::vector<ConstTransitivePtr<CSpell>> spellsByLevel;
-		auto getSpellsByLevel = [i](auto spell) {
-			return spell->getLevel() == i + 1 && !spell->isSpecial() && !spell->isCreatureAbility();
-		};
-		vstd::copy_if(spells, std::back_inserter(spellsByLevel), getSpellsByLevel);
+		std::vector<const CSpell*> spellsByLevel;
+		for (auto const & spellID : VLC->spellh->getDefaultAllowed())
+		{
+			if (spellID.toSpell()->getLevel() == i + 1)
+				spellsByLevel.push_back(spellID.toSpell());
+		}
+
 		spellLists[i]->clear();
 		for (auto spell : spellsByLevel)
 		{
@@ -130,4 +132,4 @@ void HeroSpellDelegate::setModelData(QWidget * editor, QAbstractItemModel * mode
 	{
 		QStyledItemDelegate::setModelData(editor, model, index);
 	}
-}
+}

mapeditor/inspector/inspector.cpp

@@ -139,7 +139,7 @@ void Initializer::initialize(CGHeroInstance * o)
 	
 	if(o->ID == Obj::HERO)
 	{
-		for(auto t : VLC->heroh->objects)
+		for(auto const & t : VLC->heroh->objects)
 		{
 			if(t->heroClass->getId() == HeroClassID(o->subID))
 			{
@@ -171,7 +171,7 @@ void Initializer::initialize(CGTownInstance * o)
 	if(lvl > 2) o->builtBuildings.insert(BuildingID::CASTLE);
 	if(lvl > 3) o->builtBuildings.insert(BuildingID::CAPITOL);
 
-	for(auto spell : VLC->spellh->objects) //add all regular spells to town
+	for(auto const & spell : VLC->spellh->objects) //add all regular spells to town
 	{
 		if(!spell->isSpecial() && !spell->isCreatureAbility())
 			o->possibleSpells.push_back(spell->id);
@@ -185,7 +185,7 @@ void Initializer::initialize(CGArtifact * o)
 	if(o->ID == Obj::SPELL_SCROLL)
 	{
 		std::vector<SpellID> out;
-		for(auto spell : VLC->spellh->objects) //spellh size appears to be greater (?)
+		for(auto const & spell : VLC->spellh->objects) //spellh size appears to be greater (?)
 		{
 			if(VLC->spellh->getDefaultAllowed().count(spell->id) != 0)
 			{
@@ -357,7 +357,7 @@ void Inspector::updateProperties(CGArtifact * o)
 		if(spellId != SpellID::NONE)
 		{
 			auto * delegate = new InspectorDelegate;
-			for(auto spell : VLC->spellh->objects)
+			for(auto const & spell : VLC->spellh->objects)
 			{
 				if(controller.map()->allowedSpells.count(spell->id) != 0)
 					delegate->options.push_back({QObject::tr(spell->getNameTranslated().c_str()), QVariant::fromValue(int(spell->getId()))});
@@ -699,7 +699,7 @@ void Inspector::setProperty(CGHeroInstance * o, const QString & key, const QVari
 	
 	if(key == "Hero type")
 	{
-		for(auto t : VLC->heroh->objects)
+		for(auto const & t : VLC->heroh->objects)
 		{
 			if(t->getId() == value.toInt())
 			{

mapeditor/mainwindow.cpp

@@ -594,7 +594,7 @@ void MainWindow::loadObjectsTree()
 	{
 		auto *b = new QPushButton(QString::fromStdString(terrain->getNameTranslated()));
 		ui->terrainLayout->addWidget(b);
-		connect(b, &QPushButton::clicked, this, [this, terrain]{ terrainButtonClicked(terrain->getId()); });
+		connect(b, &QPushButton::clicked, this, [this, terrainID=terrain->getId()]{ terrainButtonClicked(terrainID); });
 
 		//filter
 		QString displayName = QString::fromStdString(terrain->getNameTranslated());
@@ -609,7 +609,7 @@ void MainWindow::loadObjectsTree()
 	{
 		auto *b = new QPushButton(QString::fromStdString(road->getNameTranslated()));
 		ui->roadLayout->addWidget(b);
-		connect(b, &QPushButton::clicked, this, [this, road]{ roadOrRiverButtonClicked(road->getIndex(), true); });
+		connect(b, &QPushButton::clicked, this, [this, roadID=road->getIndex()]{ roadOrRiverButtonClicked(roadID, true); });
 	}
 	//add spacer to keep terrain button on the top
 	ui->roadLayout->addItem(new QSpacerItem(20, 20, QSizePolicy::Minimum, QSizePolicy::Expanding));
@@ -618,7 +618,7 @@ void MainWindow::loadObjectsTree()
 	{
 		auto *b = new QPushButton(QString::fromStdString(river->getNameTranslated()));
 		ui->riverLayout->addWidget(b);
-		connect(b, &QPushButton::clicked, this, [this, river]{ roadOrRiverButtonClicked(river->getIndex(), false); });
+		connect(b, &QPushButton::clicked, this, [this, riverID=river->getIndex()]{ roadOrRiverButtonClicked(riverID, false); });
 	}
 	//add spacer to keep terrain button on the top
 	ui->riverLayout->addItem(new QSpacerItem(20, 20, QSizePolicy::Minimum, QSizePolicy::Expanding));

mapeditor/mapcontroller.cpp

@@ -137,7 +137,7 @@ void MapController::repairMap(CMap * map) const
 
 			map->allowedHeroes.insert(nih->getHeroType());
 
-			auto type = VLC->heroh->objects[nih->subID];
+			auto const & type = VLC->heroh->objects[nih->subID];
 			assert(type->heroClass);
 			//TODO: find a way to get proper type name
 			if(obj->ID == Obj::HERO)
@@ -187,7 +187,7 @@ void MapController::repairMap(CMap * map) const
 			if(art->ID == Obj::SPELL_SCROLL && !art->storedArtifact)
 			{
 				std::vector<SpellID> out;
-				for(auto spell : VLC->spellh->objects) //spellh size appears to be greater (?)
+				for(auto const & spell : VLC->spellh->objects) //spellh size appears to be greater (?)
 				{
 					//if(map->isAllowedSpell(spell->id))
 					{
@@ -239,7 +239,7 @@ void MapController::setMap(std::unique_ptr<CMap> cmap)
 
 void MapController::initObstaclePainters(CMap * map)
 {
-	for (auto terrain : VLC->terrainTypeHandler->objects)
+	for (auto const & terrain : VLC->terrainTypeHandler->objects)
 	{
 		auto terrainId = terrain->getId();
 		_obstaclePainters[terrainId] = std::make_unique<EditorObstaclePlacer>(map);

mapeditor/mapsettings/mapsettings.cpp

@@ -30,7 +30,7 @@ MapSettings::MapSettings(MapController & ctrl, QWidget *parent) :
 	
 	show();
 
-	for(auto objectPtr : VLC->skillh->objects)
+	for(auto const & objectPtr : VLC->skillh->objects)
 	{
 		auto * item = new QListWidgetItem(QString::fromStdString(objectPtr->getNameTranslated()));
 		item->setData(Qt::UserRole, QVariant::fromValue(objectPtr->getIndex()));
@@ -38,7 +38,7 @@ MapSettings::MapSettings(MapController & ctrl, QWidget *parent) :
 		item->setCheckState(controller.map()->allowedAbilities.count(objectPtr->getId()) ? Qt::Checked : Qt::Unchecked);
 		ui->listAbilities->addItem(item);
 	}
-	for(auto objectPtr : VLC->spellh->objects)
+	for(auto const & objectPtr : VLC->spellh->objects)
 	{
 		auto * item = new QListWidgetItem(QString::fromStdString(objectPtr->getNameTranslated()));
 		item->setData(Qt::UserRole, QVariant::fromValue(objectPtr->getIndex()));
@@ -46,7 +46,7 @@ MapSettings::MapSettings(MapController & ctrl, QWidget *parent) :
 		item->setCheckState(controller.map()->allowedSpells.count(objectPtr->getId()) ? Qt::Checked : Qt::Unchecked);
 		ui->listSpells->addItem(item);
 	}
-	for(auto objectPtr : VLC->arth->objects)
+	for(auto const & objectPtr : VLC->arth->objects)
 	{
 		auto * item = new QListWidgetItem(QString::fromStdString(objectPtr->getNameTranslated()));
 		item->setData(Qt::UserRole, QVariant::fromValue(objectPtr->getIndex()));
@@ -54,7 +54,7 @@ MapSettings::MapSettings(MapController & ctrl, QWidget *parent) :
 		item->setCheckState(controller.map()->allowedArtifact.count(objectPtr->getId()) ? Qt::Checked : Qt::Unchecked);
 		ui->listArts->addItem(item);
 	}
-	for(auto objectPtr : VLC->heroh->objects)
+	for(auto const & objectPtr : VLC->heroh->objects)
 	{
 		auto * item = new QListWidgetItem(QString::fromStdString(objectPtr->getNameTranslated()));
 		item->setData(Qt::UserRole, QVariant::fromValue(objectPtr->getIndex()));

mapeditor/mapview.cpp

@@ -16,6 +16,7 @@
 #include "../lib/mapObjectConstructors/AObjectTypeHandler.h"
 #include "../lib/mapObjectConstructors/CObjectClassesHandler.h"
 #include "../lib/mapping/CMap.h"
+#include "../lib/VCMI_Lib.h"
 
 
 MinimapView::MinimapView(QWidget * parent):

mapeditor/playerparams.cpp

@@ -48,7 +48,7 @@ PlayerParams::PlayerParams(MapController & ctrl, int playerId, QWidget *parent)
 	//load factions
 	for(auto idx : VLC->townh->getAllowedFactions())
 	{
-		const CFaction * faction = VLC->townh->objects.at(idx);
+		const auto & faction = VLC->townh->objects.at(idx);
 		auto * item = new QListWidgetItem(QString::fromStdString(faction->getNameTranslated()));
 		item->setData(Qt::UserRole, QVariant::fromValue(idx.getNum()));
 		item->setFlags(item->flags() | Qt::ItemIsUserCheckable);

scripting/lua/LuaScriptingContext.cpp

@@ -14,6 +14,7 @@
 #include <vstd/StringUtils.h>
 #include <vcmi/events/EventBus.h>
 #include <vcmi/ServerCallback.h>
+#include "vcmi/Services.h"
 
 #include "LuaStack.h"
 

test/scripting/ScriptFixture.cpp

@@ -9,6 +9,7 @@
  */
 #include "StdInc.h"
 #include "lib/modding/ModScope.h"
+#include "lib/VCMI_Lib.h"
 
 #include "ScriptFixture.h"