Use unique_ptr for trivial ownership (#16300)

src/client/client.cpp

@@ -118,7 +118,7 @@ Client::Client(
 	m_last_chat_message_sent(time(NULL)),
 	m_password(password),
 	m_chosen_auth_mech(AUTH_MECHANISM_NONE),
-	m_media_downloader(new ClientMediaDownloader()),
+	m_media_downloader(std::make_unique<ClientMediaDownloader>()),
 	m_state(LC_Created),
 	m_modchannel_mgr(new ModChannelMgr())
 {
@@ -131,7 +131,7 @@ Client::Client(
 	m_mod_storage_database->beginSave();
 
 	if (g_settings->getBool("enable_minimap")) {
-		m_minimap = new Minimap(this);
+		m_minimap = std::make_unique<Minimap>(this);
 	}
 
 	m_cache_save_interval = g_settings->getU16("server_map_save_interval");
@@ -212,6 +212,9 @@ void Client::loadMods()
 	// complain about mods with unsatisfied dependencies
 	if (!modconf.isConsistent()) {
 		errorstream << modconf.getUnsatisfiedModsError() << std::endl;
+		delete m_script;
+		m_script = nullptr;
+		m_env.setScript(nullptr);
 		return;
 	}
 
@@ -243,7 +246,7 @@ void Client::loadMods()
 	if (m_camera)
 		m_script->on_camera_ready(m_camera);
 	if (m_minimap)
-		m_script->on_minimap_ready(m_minimap);
+		m_script->on_minimap_ready(m_minimap.get());
 }
 
 void Client::scanModSubfolder(const std::string &mod_name, const std::string &mod_path,
@@ -309,6 +312,7 @@ void Client::Stop()
 	if (m_localdb) {
 		infostream << "Local map saving ended." << std::endl;
 		m_localdb->endSave();
+		m_localdb.reset();
 	}
 
 	if (m_mods_loaded)
@@ -339,8 +343,6 @@ Client::~Client()
 		delete r.mesh;
 	}
 
-	delete m_inventory_from_server;
-
 	// Delete detached inventories
 	for (auto &m_detached_inventorie : m_detached_inventories) {
 		delete m_detached_inventorie.second;
@@ -353,10 +355,8 @@ Client::~Client()
 
 	guiScalingCacheClear();
 
-	delete m_minimap;
+	m_minimap.reset();
-	m_minimap = nullptr;
+	m_media_downloader.reset();
-
-	delete m_media_downloader;
 
 	// Write the changes and delete
 	if (m_mod_storage_database)
@@ -672,8 +672,7 @@ void Client::step(float dtime)
 	if (m_media_downloader && m_media_downloader->isStarted()) {
 		m_media_downloader->step(this);
 		if (m_media_downloader->isDone()) {
-			delete m_media_downloader;
+			m_media_downloader.reset();
-			m_media_downloader = NULL;
 		}
 	}
 	{
@@ -921,7 +920,7 @@ void Client::initLocalMapSaving(const Address &address, const std::string &hostn
 		return;
 	}
 	if (m_localdb) {
-		infostream << "Local map saving already running" << std::endl;
+		infostream << "Local map saving already initialized" << std::endl;
 		return;
 	}
 
@@ -941,7 +940,7 @@ void Client::initLocalMapSaving(const Address &address, const std::string &hostn
 #undef set_world_path
 	fs::CreateAllDirs(world_path);
 
-	m_localdb = new MapDatabaseSQLite3(world_path);
+	m_localdb = std::make_unique<MapDatabaseSQLite3>(world_path);
 	m_localdb->beginSave();
 	actionstream << "Local map saving started, map will be saved at '" << world_path << "'" << std::endl;
 }

src/client/client.h

@@ -363,7 +363,7 @@ public:
 		return getProtoVersion() != 0; // (set in TOCLIENT_HELLO)
 	}
 
-	Minimap* getMinimap() { return m_minimap; }
+	Minimap* getMinimap() { return m_minimap.get(); }
 	void setCamera(Camera* camera) { m_camera = camera; }
 
 	Camera* getCamera () { return m_camera; }
@@ -494,7 +494,7 @@ private:
 	std::string m_address_name;
 	ELoginRegister m_allow_login_or_register = ELoginRegister::Any;
 	Camera *m_camera = nullptr;
-	Minimap *m_minimap = nullptr;
+	std::unique_ptr<Minimap> m_minimap;
 
 	// Server serialization version
 	u8 m_server_ser_ver;
@@ -504,7 +504,7 @@ private:
 	u16 m_proto_ver = 0;
 
 	bool m_update_wielded_item = false;
-	Inventory *m_inventory_from_server = nullptr;
+	std::unique_ptr<Inventory> m_inventory_from_server;
 	float m_inventory_from_server_age = 0.0f;
 	s32 m_mapblock_limit_logged = 0;
 	PacketCounter m_packetcounter;
@@ -543,7 +543,7 @@ private:
 
 	std::vector<std::string> m_remote_media_servers;
 	// Media downloader, only exists during init
-	ClientMediaDownloader *m_media_downloader;
+	std::unique_ptr<ClientMediaDownloader> m_media_downloader;
 	// Pending downloads of dynamic media (key: token)
 	std::vector<std::pair<u32, std::shared_ptr<SingleMediaDownloader>>> m_pending_media_downloads;
 
@@ -574,7 +574,7 @@ private:
 	LocalClientState m_state;
 
 	// Used for saving server map to disk client-side
-	MapDatabase *m_localdb = nullptr;
+	std::unique_ptr<MapDatabase> m_localdb;
 	IntervalLimiter m_localdb_save_interval;
 	u16 m_cache_save_interval;
 

src/gui/guiFormSpecMenu.cpp

@@ -120,7 +120,6 @@ GUIFormSpecMenu::~GUIFormSpecMenu()
 {
 	removeAll();
 
-	delete m_selected_item;
 	delete m_form_src;
 	delete m_text_dst;
 }
@@ -3757,7 +3756,7 @@ void GUIFormSpecMenu::updateSelectedItem()
 
 			} else {
 				// Grab selected item from the crafting result list
-				m_selected_item = new GUIInventoryList::ItemSpec(s);
+				m_selected_item = std::make_unique<GUIInventoryList::ItemSpec>(s);
 				m_selected_amount = item.count;
 				m_selected_dragging = false;
 			}
@@ -3799,8 +3798,7 @@ ItemStack GUIFormSpecMenu::verifySelectedItem()
 		}
 
 		// selection was not valid
-		delete m_selected_item;
+		m_selected_item.reset();
-		m_selected_item = nullptr;
 		m_selected_amount = 0;
 		m_selected_dragging = false;
 	}
@@ -4333,7 +4331,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)
 					shift_move_amount = button == BET_RIGHT ? 1 : count;
 				} else {
 					// No shift: select item
-					m_selected_item = new GUIInventoryList::ItemSpec(s);
+					m_selected_item = std::make_unique<GUIInventoryList::ItemSpec>(s);
 					m_selected_amount = count;
 					m_selected_dragging = button != BET_WHEEL_DOWN;
 				}
@@ -4528,7 +4526,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)
 
 			} else if (m_held_mouse_button == BET_LEFT) {
 				// Start picking up items
-				m_selected_item = new GUIInventoryList::ItemSpec(s);
+				m_selected_item = std::make_unique<GUIInventoryList::ItemSpec>(s);
 				m_selected_amount = s_count;
 				m_selected_dragging = true;
 			}
@@ -4835,8 +4833,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)
 		// and we are not left-dragging, deselect
 		if (m_selected_amount == 0 && !m_left_dragging) {
 			m_selected_swap.clear();
-			delete m_selected_item;
+			m_selected_item.reset();
-			m_selected_item = nullptr;
 			m_selected_amount = 0;
 			m_selected_dragging = false;
 		}

src/gui/guiFormSpecMenu.h

@@ -236,7 +236,7 @@ public:
 
 	const GUIInventoryList::ItemSpec *getSelectedItem() const
 	{
-		return m_selected_item;
+		return m_selected_item.get();
 	}
 
 	u16 getSelectedAmount() const
@@ -346,7 +346,7 @@ protected:
 	std::vector<gui::IGUIElement *> m_clickthrough_elements;
 	std::vector<std::pair<std::string, GUIScrollContainer *>> m_scroll_containers;
 
-	GUIInventoryList::ItemSpec *m_selected_item = nullptr;
+	std::unique_ptr<GUIInventoryList::ItemSpec> m_selected_item;
 	u16 m_selected_amount = 0;
 	bool m_selected_dragging = false;
 	ItemStack m_selected_swap;

src/map_settings_manager.cpp

@@ -21,15 +21,13 @@ MapSettingsManager::MapSettingsManager(const std::string &map_meta_path):
 	 * 1: defaults set by scripts (override_meta = false)
 	 * 2: settings present in map_meta.txt or overridden by scripts
 	 */
-	m_defaults = new Settings("", &m_hierarchy, 1);
+	m_defaults = std::make_unique<Settings>("", &m_hierarchy, 1);
-	m_map_settings = new Settings("[end_of_params]", &m_hierarchy, 2);
+	m_map_settings = std::make_unique<Settings>("[end_of_params]", &m_hierarchy, 2);
 }
 
 
 MapSettingsManager::~MapSettingsManager()
 {
-	delete m_defaults;
-	delete m_map_settings;
 	delete mapgen_params;
 }
 
@@ -109,8 +107,8 @@ bool MapSettingsManager::saveMapMeta()
 		return false;
 	}
 
-	mapgen_params->MapgenParams::writeParams(m_map_settings);
+	mapgen_params->MapgenParams::writeParams(m_map_settings.get());
-	mapgen_params->writeParams(m_map_settings);
+	mapgen_params->writeParams(m_map_settings.get());
 
 	if (!m_map_settings->updateConfigFile(m_map_meta_path.c_str())) {
 		errorstream << "saveMapMeta: could not write "
@@ -150,8 +148,8 @@ MapgenParams *MapSettingsManager::makeMapgenParams()
 	params->mgtype = mgtype;
 
 	// Load the rest of the mapgen params from our active settings
-	params->MapgenParams::readParams(m_map_settings);
+	params->MapgenParams::readParams(m_map_settings.get());
-	params->readParams(m_map_settings);
+	params->readParams(m_map_settings.get());
 
 	// Hold onto our params
 	mapgen_params = params;

src/map_settings_manager.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <memory>
 #include <string>
 #include "settings.h"
 
@@ -57,6 +58,6 @@ private:
 	std::string m_map_meta_path;
 
 	SettingsHierarchy m_hierarchy;
-	Settings *m_defaults;
+	std::unique_ptr<Settings> m_defaults;
-	Settings *m_map_settings;
+	std::unique_ptr<Settings> m_map_settings;
 };

src/network/clientpackethandler.cpp

@@ -311,7 +311,7 @@ void Client::handleCommand_BlockData(NetworkPacket* pkt)
 	}
 
 	if (m_localdb) {
-		ServerMap::saveBlock(block, m_localdb);
+		ServerMap::saveBlock(block, m_localdb.get());
 	}
 
 	/*
@@ -335,9 +335,8 @@ void Client::handleCommand_Inventory(NetworkPacket* pkt)
 
 	m_update_wielded_item = true;
 
-	delete m_inventory_from_server;
+	m_inventory_from_server = std::make_unique<Inventory>(player->inventory);
-	m_inventory_from_server = new Inventory(player->inventory);
+	m_inventory_from_server_age = 0.0f;
-	m_inventory_from_server_age = 0.0;
 }
 
 void Client::handleCommand_TimeOfDay(NetworkPacket* pkt)

src/nodemetadata.cpp

@@ -18,13 +18,10 @@
 */
 
 NodeMetadata::NodeMetadata(IItemDefManager *item_def_mgr):
-	m_inventory(new Inventory(item_def_mgr))
+	m_inventory(std::make_unique<Inventory>(item_def_mgr))
 {}
 
-NodeMetadata::~NodeMetadata()
+NodeMetadata::~NodeMetadata() = default;
-{
-	delete m_inventory;
-}
 
 void NodeMetadata::serialize(std::ostream &os, u8 version, bool disk) const
 {

src/nodemetadata.h

@@ -6,6 +6,7 @@
 
 #include <unordered_set>
 #include <map>
+#include <memory>
 #include "metadata.h"
 
 /*
@@ -35,7 +36,7 @@ public:
 	// The inventory
 	Inventory *getInventory()
 	{
-		return m_inventory;
+		return m_inventory.get();
 	}
 
 	inline bool isPrivate(const std::string &name) const
@@ -50,7 +51,7 @@ public:
 private:
 	int countNonPrivate() const;
 
-	Inventory *m_inventory;
+	std::unique_ptr<Inventory> m_inventory;
 	std::unordered_set<std::string> m_privatevars;
 };
 

src/pathfinder.cpp

@@ -169,8 +169,6 @@ public:
 	Pathfinder() = delete;
 	Pathfinder(Map *map, const NodeDefManager *ndef) : m_map(map), m_ndef(ndef) {}
 
-	~Pathfinder();
-
 	/**
 	 * path evaluation function
 	 * @param env environment to look for path
@@ -307,7 +305,7 @@ private:
 	/** contains all map data already collected and analyzed.
 		Access it via the getIndexElement/getIdxElem methods. */
 	friend class GridNodeContainer;
-	GridNodeContainer *m_nodes_container = nullptr;
+	std::unique_ptr<GridNodeContainer> m_nodes_container;
 
 	Map *m_map = nullptr;
 
@@ -639,11 +637,10 @@ std::vector<v3s16> Pathfinder::getPath(v3s16 source,
 	m_max_index_y = diff.Y;
 	m_max_index_z = diff.Z;
 
-	delete m_nodes_container;
 	if (diff.getLength() > 5) {
-		m_nodes_container = new MapGridNodeContainer(this);
+		m_nodes_container = std::make_unique<MapGridNodeContainer>(this);
 	} else {
-		m_nodes_container = new ArrayGridNodeContainer(this, diff);
+		m_nodes_container = std::make_unique<ArrayGridNodeContainer>(this, diff);
 	}
 #ifdef PATHFINDER_DEBUG
 	printType();
@@ -799,10 +796,6 @@ std::vector<v3s16> Pathfinder::getPath(v3s16 source,
 	return retval;
 }
 
-Pathfinder::~Pathfinder()
-{
-	delete m_nodes_container;
-}
 /******************************************************************************/
 v3s16 Pathfinder::getRealPos(v3s16 ipos)
 {