From c0fb1d1b3b1f71c0a0b352b112ecdf951d0724c6 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Sun, 27 Apr 2025 19:58:21 +0300 Subject: [PATCH] Replace some raw pointers with unique's or optional --- Global.h | 8 ------- client/lobby/CCampaignInfoScreen.cpp | 14 +++++------- client/lobby/CCampaignInfoScreen.h | 6 +++--- client/lobby/CScenarioInfoScreen.cpp | 14 +++++------- client/lobby/CScenarioInfoScreen.h | 5 ++--- client/lobby/OptionsTab.cpp | 2 +- clientapp/EntryPoint.cpp | 6 ++++-- lib/entities/faction/CFaction.cpp | 9 +------- lib/entities/faction/CFaction.h | 2 +- lib/entities/faction/CTownHandler.cpp | 18 ++++++---------- lib/entities/faction/CTownHandler.h | 3 +-- lib/gameState/InfoAboutArmy.cpp | 24 ++++++--------------- lib/gameState/InfoAboutArmy.h | 8 +++---- lib/mapObjects/CGTownInstance.cpp | 4 ++-- mapeditor/mapsettings/victoryconditions.cpp | 2 +- serverapp/EntryPoint.cpp | 2 +- test/entity/CFactionTest.cpp | 6 +++--- 17 files changed, 47 insertions(+), 86 deletions(-) diff --git a/Global.h b/Global.h index 10e57cb5a..8312a3184 100644 --- a/Global.h +++ b/Global.h @@ -465,14 +465,6 @@ namespace vstd } }; - //deleted pointer and sets it to nullptr - template - void clear_pointer(T* &ptr) - { - delete ptr; - ptr = nullptr; - } - template typename Container::const_reference circularAt(const Container &r, size_t index) { diff --git a/client/lobby/CCampaignInfoScreen.cpp b/client/lobby/CCampaignInfoScreen.cpp index dfc279329..b6edce298 100644 --- a/client/lobby/CCampaignInfoScreen.cpp +++ b/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(*GAME->interface()->cb->getStartInfo()); + localMi = std::make_unique(); localMi->mapHeader = std::unique_ptr(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(); } diff --git a/client/lobby/CCampaignInfoScreen.h b/client/lobby/CCampaignInfoScreen.h index bceab47a6..ea933bc3a 100644 --- a/client/lobby/CCampaignInfoScreen.h +++ b/client/lobby/CCampaignInfoScreen.h @@ -14,8 +14,8 @@ class CCampaignInfoScreen : public CBonusSelection, ISelectionScreenInfo { - const StartInfo * localSi; - CMapInfo * localMi; + std::unique_ptr localSi; + std::unique_ptr localMi; public: CCampaignInfoScreen(); @@ -23,4 +23,4 @@ public: const CMapInfo * getMapInfo() override; const StartInfo * getStartInfo() override; -}; \ No newline at end of file +}; diff --git a/client/lobby/CScenarioInfoScreen.cpp b/client/lobby/CScenarioInfoScreen.cpp index f5f893f51..5bd25b3da 100644 --- a/client/lobby/CScenarioInfoScreen.cpp +++ b/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(*GAME->interface()->cb->getStartInfo()); + localMi = std::make_unique(); localMi->mapHeader = std::unique_ptr(new CMapHeader(*GAME->interface()->cb->getMapHeader())); screenType = ESelectionScreen::scenarioInfo; @@ -49,18 +49,14 @@ CScenarioInfoScreen::CScenarioInfoScreen() buttonBack = std::make_shared(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(); } diff --git a/client/lobby/CScenarioInfoScreen.h b/client/lobby/CScenarioInfoScreen.h index e2a895569..c1d19e668 100644 --- a/client/lobby/CScenarioInfoScreen.h +++ b/client/lobby/CScenarioInfoScreen.h @@ -14,14 +14,13 @@ /// Scenario information screen shown during the game class CScenarioInfoScreen : public WindowBase, public ISelectionScreenInfo { + std::unique_ptr localSi; + std::unique_ptr localMi; public: std::shared_ptr buttonBack; std::shared_ptr card; std::shared_ptr opt; - const StartInfo * localSi; - CMapInfo * localMi; - CScenarioInfoScreen(); ~CScenarioInfoScreen(); diff --git a/client/lobby/OptionsTab.cpp b/client/lobby/OptionsTab.cpp index 544d3334c..a5645b55b 100644 --- a/client/lobby/OptionsTab.cpp +++ b/client/lobby/OptionsTab.cpp @@ -379,7 +379,7 @@ void OptionsTab::CPlayerOptionTooltipBox::genTownWindow() genHeader(); labelAssociatedCreatures = std::make_shared(pos.w / 2 + 8, 122, FONT_MEDIUM, ETextAlignment::CENTER, Colors::YELLOW, LIBRARY->generaltexth->allTexts[79]); std::vector> components; - const CTown * town = (*LIBRARY->townh)[factionIndex]->town; + const CTown * town = (*LIBRARY->townh)[factionIndex]->town.get(); for(auto & elem : town->creatures) { diff --git a/clientapp/EntryPoint.cpp b/clientapp/EntryPoint.cpp index 1485ed4dc..faa54b499 100644 --- a/clientapp/EntryPoint.cpp +++ b/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"; diff --git a/lib/entities/faction/CFaction.cpp b/lib/entities/faction/CFaction.cpp index cab225089..07cb6673d 100644 --- a/lib/entities/faction/CFaction.cpp +++ b/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 { diff --git a/lib/entities/faction/CFaction.h b/lib/entities/faction/CFaction.h index 27d6a2239..73c41ae1d 100644 --- a/lib/entities/faction/CFaction.h +++ b/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 town; //NOTE: can be null ImagePath creatureBg120; ImagePath creatureBg130; diff --git a/lib/entities/faction/CTownHandler.cpp b/lib/entities/faction/CTownHandler.cpp index a2dcbdaad..26c1c19a0 100644 --- a/lib/entities/faction/CTownHandler.cpp +++ b/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()) { - randomFaction->town = randomTown; - randomTown->faction = randomFaction; + randomFaction->town = std::make_unique(); + 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 CTownHandler::loadFromJson(const std::string & scope, if (!source["town"].isNull()) { - faction->town = new CTown(); + faction->town = std::make_unique(); 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() diff --git a/lib/entities/faction/CTownHandler.h b/lib/entities/faction/CTownHandler.h index f4066934f..41a861672 100644 --- a/lib/entities/faction/CTownHandler.h +++ b/lib/entities/faction/CTownHandler.h @@ -63,8 +63,7 @@ class DLL_LINKAGE CTownHandler : public CHandlerBase randomFaction; CTownHandler(); ~CTownHandler(); diff --git a/lib/gameState/InfoAboutArmy.cpp b/lib/gameState/InfoAboutArmy.cpp index 6002a5040..cc56290ff 100644 --- a/lib/gameState/InfoAboutArmy.cpp +++ b/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); diff --git a/lib/gameState/InfoAboutArmy.h b/lib/gameState/InfoAboutArmy.h index 569084c83..bdbd47e9e 100644 --- a/lib/gameState/InfoAboutArmy.h +++ b/lib/gameState/InfoAboutArmy.h @@ -52,7 +52,7 @@ public: si32 mana, manaLimit, luck, morale; }; - Details * details = nullptr; + std::optional
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; 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); }; diff --git a/lib/mapObjects/CGTownInstance.cpp b/lib/mapObjects/CGTownInstance.cpp index fbd488c25..615688279 100644 --- a/lib/mapObjects/CGTownInstance.cpp +++ b/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 diff --git a/mapeditor/mapsettings/victoryconditions.cpp b/mapeditor/mapsettings/victoryconditions.cpp index e2dae21bf..ac355859a 100644 --- a/mapeditor/mapsettings/victoryconditions.cpp +++ b/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)); diff --git a/serverapp/EntryPoint.cpp b/serverapp/EntryPoint.cpp index 9bcce94b3..d32643137 100644 --- a/serverapp/EntryPoint.cpp +++ b/serverapp/EntryPoint.cpp @@ -100,7 +100,7 @@ int main(int argc, const char * argv[]) } logConfigurator.deconfigure(); - vstd::clear_pointer(LIBRARY); + delete LIBRARY; return 0; } diff --git a/test/entity/CFactionTest.cpp b/test/entity/CFactionTest.cpp index 3e1b9e31f..01bb815a7 100644 --- a/test/entity/CFactionTest.cpp +++ b/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(); 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(PH1), std::forward(PH2), std::forward(PH3), std::forward(PH4)); }; - subject->town = new CTown(); + subject->town = std::make_unique(); CTown::ClientInfo & info = subject->town->clientInfo;