Replace some raw pointers with unique's or optional

Global.h

@@ -465,14 +465,6 @@ namespace vstd
 		}
 	};
 
-	//deleted pointer and sets it to nullptr
-	template <typename T>
-	void clear_pointer(T* &ptr)
-	{
-		delete ptr;
-		ptr = nullptr;
-	}
-
 	template <typename Container>
 	typename Container::const_reference circularAt(const Container &r, size_t index)
 	{

client/lobby/CCampaignInfoScreen.cpp

@@ -23,8 +23,8 @@
 CCampaignInfoScreen::CCampaignInfoScreen()
 {
 	OBJECT_CONSTRUCTION;
-	localSi = new StartInfo(*GAME->interface()->cb->getStartInfo());
-	localMi = new CMapInfo();
+	localSi = std::make_unique<StartInfo>(*GAME->interface()->cb->getStartInfo());
+	localMi = std::make_unique<CMapInfo>();
 	localMi->mapHeader = std::unique_ptr<CMapHeader>(new CMapHeader(*GAME->interface()->cb->getMapHeader()));
 
 	screenType = ESelectionScreen::scenarioInfo;
@@ -32,17 +32,13 @@ CCampaignInfoScreen::CCampaignInfoScreen()
 	updateAfterStateChange();
 }
 
-CCampaignInfoScreen::~CCampaignInfoScreen()
-{
-	vstd::clear_pointer(localSi);
-	vstd::clear_pointer(localMi);
-}
+CCampaignInfoScreen::~CCampaignInfoScreen() = default;
 
 const CMapInfo * CCampaignInfoScreen::getMapInfo()
 {
-	return localMi;
+	return localMi.get();
 }
 const StartInfo * CCampaignInfoScreen::getStartInfo()
 {
-	return localSi;
+	return localSi.get();
 }

client/lobby/CCampaignInfoScreen.h

@@ -14,8 +14,8 @@
 
 class CCampaignInfoScreen : public CBonusSelection, ISelectionScreenInfo
 {
-	const StartInfo * localSi;
-	CMapInfo * localMi;
+	std::unique_ptr<StartInfo> localSi;
+	std::unique_ptr<CMapInfo> localMi;
 
 public:
 	CCampaignInfoScreen();

client/lobby/CScenarioInfoScreen.cpp

@@ -33,8 +33,8 @@ CScenarioInfoScreen::CScenarioInfoScreen()
 	pos.h = 600;
 	pos = center();
 
-	localSi = new StartInfo(*GAME->interface()->cb->getStartInfo());
-	localMi = new CMapInfo();
+	localSi = std::make_unique<StartInfo>(*GAME->interface()->cb->getStartInfo());
+	localMi = std::make_unique<CMapInfo>();
 	localMi->mapHeader = std::unique_ptr<CMapHeader>(new CMapHeader(*GAME->interface()->cb->getMapHeader()));
 
 	screenType = ESelectionScreen::scenarioInfo;
@@ -49,18 +49,14 @@ CScenarioInfoScreen::CScenarioInfoScreen()
 	buttonBack = std::make_shared<CButton>(Point(584, 535), AnimationPath::builtin("SCNRBACK.DEF"), LIBRARY->generaltexth->zelp[105], [this](){ close();}, EShortcut::GLOBAL_CANCEL);
 }
 
-CScenarioInfoScreen::~CScenarioInfoScreen()
-{
-	vstd::clear_pointer(localSi);
-	vstd::clear_pointer(localMi);
-}
+CScenarioInfoScreen::~CScenarioInfoScreen() = default;
 
 const CMapInfo * CScenarioInfoScreen::getMapInfo()
 {
-	return localMi;
+	return localMi.get();
 }
 
 const StartInfo * CScenarioInfoScreen::getStartInfo()
 {
-	return localSi;
+	return localSi.get();
 }

client/lobby/CScenarioInfoScreen.h

@@ -14,14 +14,13 @@
 /// Scenario information screen shown during the game
 class CScenarioInfoScreen : public WindowBase, public ISelectionScreenInfo
 {
+	std::unique_ptr<StartInfo> localSi;
+	std::unique_ptr<CMapInfo> localMi;
 public:
 	std::shared_ptr<CButton> buttonBack;
 	std::shared_ptr<InfoCard> card;
 	std::shared_ptr<OptionsTab> opt;
 
-	const StartInfo * localSi;
-	CMapInfo * localMi;
-
 	CScenarioInfoScreen();
 	~CScenarioInfoScreen();
 

client/lobby/OptionsTab.cpp

@@ -379,7 +379,7 @@ void OptionsTab::CPlayerOptionTooltipBox::genTownWindow()
 	genHeader();
 	labelAssociatedCreatures = std::make_shared<CLabel>(pos.w / 2 + 8, 122, FONT_MEDIUM, ETextAlignment::CENTER, Colors::YELLOW, LIBRARY->generaltexth->allTexts[79]);
 	std::vector<std::shared_ptr<CComponent>> components;
-	const CTown * town = (*LIBRARY->townh)[factionIndex]->town;
+	const CTown * town = (*LIBRARY->townh)[factionIndex]->town.get();
 
 	for(auto & elem : town->creatures)
 	{

clientapp/EntryPoint.cpp

@@ -416,7 +416,8 @@ int main(int argc, char * argv[])
 	if(!settings["session"]["headless"].Bool())
 	{
 		CMessage::dispose();
-		vstd::clear_pointer(graphics);
+		delete graphics;
+		graphics = nullptr;
 	}
 
 	// must be executed before reset - since unique_ptr resets pointer to null before calling destructor
@@ -424,7 +425,8 @@ int main(int argc, char * argv[])
 
 	ENGINE.reset();
 
-	vstd::clear_pointer(LIBRARY);
+	delete LIBRARY;
+	LIBRARY = nullptr;
 	logConfigurator.deconfigure();
 
 	std::cout << "Ending...\n";

lib/entities/faction/CFaction.cpp

@@ -17,14 +17,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-CFaction::~CFaction()
-{
-	if (town)
-	{
-		delete town;
-		town = nullptr;
-	}
-}
+CFaction::~CFaction() = default;
 
 int32_t CFaction::getIndex() const
 {

lib/entities/faction/CFaction.h

@@ -51,7 +51,7 @@ public:
 	/// and for placing heroes directly on boat (in map editor, water prisons & taverns)
 	BoatId boatType = BoatId::CASTLE;
 
-	CTown * town = nullptr; //NOTE: can be null
+	std::unique_ptr<CTown> town; //NOTE: can be null
 
 	ImagePath creatureBg120;
 	ImagePath creatureBg130;

lib/entities/faction/CTownHandler.cpp

@@ -38,19 +38,15 @@ const int NAMES_PER_TOWN=16; // number of town names per faction in H3 files. Js
 
 CTownHandler::CTownHandler()
 	: buildingsLibrary(JsonPath::builtin("config/buildingsLibrary"))
-	, randomTown(new CTown())
-	, randomFaction(new CFaction())
+	, randomFaction(std::make_unique<CFaction>())
 {
-	randomFaction->town = randomTown;
-	randomTown->faction = randomFaction;
+	randomFaction->town = std::make_unique<CTown>();
+	randomFaction->town->faction = randomFaction.get();
 	randomFaction->identifier = "random";
 	randomFaction->modScope = "core";
 }
 
-CTownHandler::~CTownHandler()
-{
-	delete randomFaction; // will also delete randomTown
-}
+CTownHandler::~CTownHandler() = default;
 
 JsonNode readBuilding(CLegacyConfigParser & parser)
 {
@@ -767,9 +763,9 @@ std::shared_ptr<CFaction> CTownHandler::loadFromJson(const std::string & scope,
 
 	if (!source["town"].isNull())
 	{
-		faction->town = new CTown();
+		faction->town = std::make_unique<CTown>();
 		faction->town->faction = faction.get();
-		loadTown(faction->town, source["town"]);
+		loadTown(faction->town.get(), source["town"]);
 	}
 	else
 		faction->town = nullptr;
@@ -854,7 +850,7 @@ void CTownHandler::loadRandomFaction()
 {
 	JsonNode randomFactionJson(JsonPath::builtin("config/factions/random.json"));
 	randomFactionJson.setModScope(ModScope::scopeBuiltin(), true);
-	loadBuildings(randomTown, randomFactionJson["random"]["town"]["buildings"]);
+	loadBuildings(randomFaction->town.get(), randomFactionJson["random"]["town"]["buildings"]);
 }
 
 void CTownHandler::loadCustom()

lib/entities/faction/CTownHandler.h

@@ -63,8 +63,7 @@ class DLL_LINKAGE CTownHandler : public CHandlerBase<FactionID, Faction, CFactio
 	void loadRandomFaction();
 
 public:
-	CTown * randomTown;
-	CFaction * randomFaction;
+	std::unique_ptr<CFaction> randomFaction;
 
 	CTownHandler();
 	~CTownHandler();

lib/gameState/InfoAboutArmy.cpp

@@ -71,10 +71,10 @@ void InfoAboutArmy::initFromArmy(const CArmedInstance *Army, bool detailed)
 
 void InfoAboutHero::assign(const InfoAboutHero & iah)
 {
-	vstd::clear_pointer(details);
+	details.reset();;
 	InfoAboutArmy::operator = (iah);
 
-	details = (iah.details ? new Details(*iah.details) : nullptr);
+	details = iah.details;
 	hclass = iah.hclass;
 	portraitSource = iah.portraitSource;
 }
@@ -92,11 +92,6 @@ InfoAboutHero::InfoAboutHero(const CGHeroInstance * h, InfoAboutHero::EInfoLevel
 	initFromHero(h, infoLevel);
 }
 
-InfoAboutHero::~InfoAboutHero()
-{
-	vstd::clear_pointer(details);
-}
-
 InfoAboutHero & InfoAboutHero::operator=(const InfoAboutHero & iah)
 {
 	assign(iah);
@@ -110,7 +105,7 @@ int32_t InfoAboutHero::getIconIndex() const
 
 void InfoAboutHero::initFromHero(const CGHeroInstance *h, InfoAboutHero::EInfoLevel infoLevel)
 {
-	vstd::clear_pointer(details);
+	details.reset();
 	if(!h)
 		return;
 
@@ -125,7 +120,7 @@ void InfoAboutHero::initFromHero(const CGHeroInstance *h, InfoAboutHero::EInfoLe
 	if(detailed)
 	{
 		//include details about hero
-		details = new Details();
+		details = Details();
 		details->luck = h->luckVal();
 		details->morale = h->moraleVal();
 		details->mana = h->mana;
@@ -143,7 +138,6 @@ void InfoAboutHero::initFromHero(const CGHeroInstance *h, InfoAboutHero::EInfoLe
 }
 
 InfoAboutTown::InfoAboutTown():
-	details(nullptr),
 	tType(nullptr),
 	built(0),
 	fortLevel(0)
@@ -152,7 +146,6 @@ InfoAboutTown::InfoAboutTown():
 }
 
 InfoAboutTown::InfoAboutTown(const CGTownInstance *t, bool detailed):
-	details(nullptr),
 	tType(nullptr),
 	built(0),
 	fortLevel(0)
@@ -160,11 +153,6 @@ InfoAboutTown::InfoAboutTown(const CGTownInstance *t, bool detailed):
 	initFromTown(t, detailed);
 }
 
-InfoAboutTown::~InfoAboutTown()
-{
-	vstd::clear_pointer(details);
-}
-
 void InfoAboutTown::initFromTown(const CGTownInstance *t, bool detailed)
 {
 	initFromArmy(t, detailed);
@@ -174,12 +162,12 @@ void InfoAboutTown::initFromTown(const CGTownInstance *t, bool detailed)
 	name = t->getNameTranslated();
 	tType = t->getTown();
 
-	vstd::clear_pointer(details);
+	details.reset();
 
 	if(detailed)
 	{
 		//include details about hero
-		details = new Details();
+		details = Details();
 		TResources income = t->dailyIncome();
 		details->goldIncome = income[EGameResID::GOLD];
 		details->customRes = t->hasBuilt(BuildingID::RESOURCE_SILO);

lib/gameState/InfoAboutArmy.h

@@ -52,7 +52,7 @@ public:
 		si32 mana, manaLimit, luck, morale;
 	};
 
-	Details * details = nullptr;
+	std::optional<Details> details;
 
 	const CHeroClass *hclass;
 	HeroTypeID portraitSource;
@@ -67,7 +67,6 @@ public:
 	InfoAboutHero();
 	InfoAboutHero(const InfoAboutHero & iah);
 	InfoAboutHero(const CGHeroInstance *h, EInfoLevel infoLevel);
-	~InfoAboutHero();
 
 	InfoAboutHero & operator=(const InfoAboutHero & iah);
 
@@ -84,7 +83,9 @@ struct DLL_LINKAGE InfoAboutTown : public InfoAboutArmy
 		bool customRes;
 		bool garrisonedHero;
 
-	} *details;
+	};
+
+	std::optional<Details> details;
 
 	const CTown *tType;
 
@@ -93,7 +94,6 @@ struct DLL_LINKAGE InfoAboutTown : public InfoAboutArmy
 
 	InfoAboutTown();
 	InfoAboutTown(const CGTownInstance *t, bool detailed);
-	~InfoAboutTown();
 	void initFromTown(const CGTownInstance *t, bool detailed);
 };
 

lib/mapObjects/CGTownInstance.cpp

@@ -1145,9 +1145,9 @@ const CFaction * CGTownInstance::getFaction() const
 const CTown * CGTownInstance::getTown() const
 {
 	if(ID == Obj::RANDOM_TOWN)
-		return LIBRARY->townh->randomTown;
+		return LIBRARY->townh->randomFaction->town.get();
 
-	return getFaction()->town;
+	return getFaction()->town.get();
 }
 
 FactionID CGTownInstance::getFactionID() const

mapeditor/mapsettings/victoryconditions.cpp

@@ -423,7 +423,7 @@ void VictoryConditions::on_victoryComboBox_currentIndexChanged(int index)
 		case 3: { //EventCondition::HAVE_BUILDING
 			victoryTypeWidget = new QComboBox;
 			ui->victoryParamsLayout->addWidget(victoryTypeWidget);
-			auto * ctown = LIBRARY->townh->randomTown;
+			auto * ctown = LIBRARY->townh->randomFaction->town.get();
 			for(int bId : ctown->getAllBuildings())
 				victoryTypeWidget->addItem(QString::fromStdString(defaultBuildingIdConversion(BuildingID(bId))), QVariant::fromValue(bId));
 

serverapp/EntryPoint.cpp

@@ -100,7 +100,7 @@ int main(int argc, const char * argv[])
 	}
 
 	logConfigurator.deconfigure();
-	vstd::clear_pointer(LIBRARY);
 
+	delete LIBRARY;
 	return 0;
 }

test/entity/CFactionTest.cpp

@@ -34,9 +34,9 @@ protected:
 TEST_F(CFactionTest, HasTown)
 {
 	EXPECT_FALSE(subject->hasTown());
-	subject->town = new CTown();
+	subject->town = std::make_unique<CTown>();
 	EXPECT_TRUE(subject->hasTown());
-	vstd::clear_pointer(subject->town);
+	subject->town.reset();
 	EXPECT_FALSE(subject->hasTown());
 }
 
@@ -56,7 +56,7 @@ TEST_F(CFactionTest, RegistersIcons)
 		registarCb(std::forward<decltype(PH1)>(PH1), std::forward<decltype(PH2)>(PH2), std::forward<decltype(PH3)>(PH3), std::forward<decltype(PH4)>(PH4));
 	};
 
-	subject->town = new CTown();
+	subject->town = std::make_unique<CTown>();
 
 	CTown::ClientInfo & info = subject->town->clientInfo;