Remove manual memory management in CWorld

Accepted By: @vladislavbelov
Comments By: @sera
Differential Revision: https://code.wildfiregames.com/D5143
This was SVN commit r27868.

source/ps/Game.cpp

@@ -68,7 +68,7 @@ const CStr CGame::EventNameSimulationUpdate = "SimulationUpdate";
  *
  **/
 CGame::CGame(bool replayLog):
-	m_World(new CWorld(this)),
+	m_World(new CWorld(*this)),
 	m_Simulation2(new CSimulation2(&m_World->GetUnitManager(), g_ScriptContext, &m_World->GetTerrain())),
 	// TODO: we need to remove that global dependency. Maybe the game view
 	// should be created outside only if needed.

source/ps/World.cpp

@@ -46,16 +46,18 @@ CLightEnv g_LightEnv;
 /**
  * Constructor.
  *
- * @param pGame CGame * pGame pointer to the container game object.
+ * @param game CGame& game reference to the container game object.
  **/
-CWorld::CWorld(CGame *pGame):
+CWorld::CWorld(CGame& game):
-	m_pGame(pGame),
+	m_Game{game},
-	m_Terrain(new CTerrain()),
+	m_Terrain{std::make_unique<CTerrain>()},
-	m_UnitManager(new CUnitManager()),
+	m_UnitManager{std::make_unique<CUnitManager>()},
-	m_MapReader(new CMapReader)
+	m_MapReader{std::make_unique<CMapReader>()}
 {
 }
 
+CWorld::~CWorld() = default;
+
 /**
  * Initializes the game world with the attributes provided.
  **/
@@ -68,14 +70,16 @@ void CWorld::RegisterInit(const CStrW& mapFile, const ScriptContext& cx, JS::Han
 
 		try
 		{
-			CTriggerManager* pTriggerManager = NULL;
+			CTriggerManager* pTriggerManager = nullptr;
-			m_MapReader->LoadMap(mapfilename, cx, settings, m_Terrain,
+			m_MapReader->LoadMap(mapfilename, cx, settings, m_Terrain.get(),
-				CRenderer::IsInitialised() ? &g_Renderer.GetSceneRenderer().GetWaterManager() : NULL,
+				CRenderer::IsInitialised() ? &g_Renderer.GetSceneRenderer().GetWaterManager() :
-				CRenderer::IsInitialised() ? &g_Renderer.GetSceneRenderer().GetSkyManager() : NULL,
+					nullptr,
-				&g_LightEnv, m_pGame->GetView(),
+				CRenderer::IsInitialised() ? &g_Renderer.GetSceneRenderer().GetSkyManager() :
-				m_pGame->GetView() ? m_pGame->GetView()->GetCinema() : NULL,
+					nullptr, &g_LightEnv, m_Game.GetView(),
-				pTriggerManager, CRenderer::IsInitialised() ? &g_Renderer.GetPostprocManager() : NULL,
+				m_Game.GetView() ? m_Game.GetView()->GetCinema() : nullptr, pTriggerManager,
-				m_pGame->GetSimulation2(), &m_pGame->GetSimulation2()->GetSimContext(), playerID, false);
+				CRenderer::IsInitialised() ? &g_Renderer.GetPostprocManager() : nullptr,
+				m_Game.GetSimulation2(), &m_Game.GetSimulation2()->GetSimContext(), playerID,
+				false);
 				// fails immediately, or registers for delay loading
 			LDR_Register([this](const double)
 			{
@@ -84,7 +88,7 @@ void CWorld::RegisterInit(const CStrW& mapFile, const ScriptContext& cx, JS::Han
 		}
 		catch (PSERROR_File& err)
 		{
-			SAFE_DELETE(m_MapReader);
+			m_MapReader.reset();
 			LOGERROR("Failed to load map %s: %s", mapfilename.string8(), err.what());
 			throw PSERROR_Game_World_MapLoadFailed("Failed to load map.\nCheck application log for details.");
 		}
@@ -94,14 +98,13 @@ void CWorld::RegisterInit(const CStrW& mapFile, const ScriptContext& cx, JS::Han
 void CWorld::RegisterInitRMS(const CStrW& scriptFile, const ScriptContext& cx, JS::HandleValue settings, int playerID)
 {
 	// If scriptFile is empty, a blank map will be generated using settings (no RMS run)
-	CTriggerManager* pTriggerManager = NULL;
+	CTriggerManager* pTriggerManager = nullptr;
-	m_MapReader->LoadRandomMap(scriptFile, cx, settings, m_Terrain,
+	m_MapReader->LoadRandomMap(scriptFile, cx, settings, m_Terrain.get(),
-		CRenderer::IsInitialised() ? &g_Renderer.GetSceneRenderer().GetWaterManager() : NULL,
+		CRenderer::IsInitialised() ? &g_Renderer.GetSceneRenderer().GetWaterManager() : nullptr,
-		CRenderer::IsInitialised() ? &g_Renderer.GetSceneRenderer().GetSkyManager() : NULL,
+		CRenderer::IsInitialised() ? &g_Renderer.GetSceneRenderer().GetSkyManager() : nullptr,
-		&g_LightEnv, m_pGame->GetView(),
+		&g_LightEnv, m_Game.GetView(), m_Game.GetView() ? m_Game.GetView()->GetCinema() : nullptr,
-		m_pGame->GetView() ? m_pGame->GetView()->GetCinema() : NULL,
+		pTriggerManager, CRenderer::IsInitialised() ? &g_Renderer.GetPostprocManager() : nullptr,
-		pTriggerManager, CRenderer::IsInitialised() ? &g_Renderer.GetPostprocManager() : NULL,
+		m_Game.GetSimulation2(), playerID);
-		m_pGame->GetSimulation2(), playerID);
 		// registers for delay loading
 	LDR_Register([this](const double)
 	{
@@ -111,17 +114,6 @@ void CWorld::RegisterInitRMS(const CStrW& scriptFile, const ScriptContext& cx, J
 
 int CWorld::DeleteMapReader()
 {
-	SAFE_DELETE(m_MapReader);
+	m_MapReader.reset();
 	return 0;
 }
-
-/**
- * Destructor.
- *
- **/
-CWorld::~CWorld()
-{
-	delete m_Terrain;
-	delete m_UnitManager;
-	delete m_MapReader;
-}

source/ps/World.h

@@ -27,6 +27,8 @@
 #include "ps/CStrForward.h"
 #include "ps/Errors.h"
 
+#include <memory>
+
 #ifndef ERROR_GROUP_GAME_DEFINED
 #define ERROR_GROUP_GAME_DEFINED
 ERROR_GROUP(Game);
@@ -46,26 +48,8 @@ class ScriptContext;
  **/
 class CWorld
 {
-	NONCOPYABLE(CWorld);
-	/**
-	 * pointer to the CGame object representing the game.
-	 **/
-	CGame *m_pGame;
-
-	/**
-	 * pointer to the CTerrain object representing the height map.
-	 **/
-	CTerrain *m_Terrain;
-
-	/**
-	 * pointer to the CUnitManager that holds all the units in the world.
-	 **/
-	CUnitManager *m_UnitManager;
-
-	CMapReader* m_MapReader;
-
 public:
-	CWorld(CGame *pGame);
+	CWorld(CGame& game);
 	~CWorld();
 
 	/*
@@ -102,6 +86,27 @@ public:
 	{
 		return *m_UnitManager;
 	}
+
+private:
+	/**
+	 * Reference to the CGame object representing the game.
+	 */
+	CGame& m_Game;
+
+	/**
+	 * The CTerrain object represents the height map.
+	 */
+	const std::unique_ptr<CTerrain> m_Terrain;
+
+	/**
+	 * The CUnitManager that holds all the units in the world.
+	 */
+	const std::unique_ptr<CUnitManager> m_UnitManager;
+
+	/**
+	 * The map reader gets deleted just after the map is read.
+	 */
+	std::unique_ptr<CMapReader> m_MapReader;
 };
 
 // rationale: see definition.