[Orxonox-commit 853] r3363 - branches/resource/src/core
rgrieder at orxonox.net
rgrieder at orxonox.net
Wed Jul 29 17:24:39 CEST 2009
Author: rgrieder
Date: 2009-07-29 17:24:39 +0200 (Wed, 29 Jul 2009)
New Revision: 3363
Modified:
branches/resource/src/core/Core.cc
branches/resource/src/core/Core.h
branches/resource/src/core/Game.cc
branches/resource/src/core/Game.h
Log:
Exception-safety for the Game and Core c'tors as well as load/unload-Graphics.
Modified: branches/resource/src/core/Core.cc
===================================================================
--- branches/resource/src/core/Core.cc 2009-07-29 15:09:09 UTC (rev 3362)
+++ branches/resource/src/core/Core.cc 2009-07-29 15:24:39 UTC (rev 3363)
@@ -247,8 +247,13 @@
Core::Core(const std::string& cmdLine)
- : bDevRun_(false)
+ // Cleanup guard for identifier destruction (incl. XMLPort, configValues, consoleCommands)
+ : identifierDestroyer_(Identifier::destroyAllIdentifiers)
+ // Cleanup guard for external console commands that don't belong to an Identifier
+ , consoleCommandDestroyer_(CommandExecutor::destroyExternalCommands)
+ , bDevRun_(false)
, bGraphicsLoaded_(false)
+ , configuration_(new CoreConfiguration()) // Don't yet create config values!
{
if (singletonRef_s != 0)
{
@@ -257,9 +262,6 @@
}
Core::singletonRef_s = this;
- // We need the variables very soon. But don't configure them yet!
- this->configuration_ = new CoreConfiguration();
-
// Parse command line arguments first
CommandLine::parseCommandLine(cmdLine);
@@ -275,7 +277,7 @@
// create a signal handler (only active for linux)
// This call is placed as soon as possible, but after the directories are set
- this->signalHandler_ = new SignalHandler();
+ this->signalHandler_.reset(new SignalHandler());
this->signalHandler_->doCatch(configuration_->executablePath_.string(), Core::getLogPathString() + "orxonox_crash.log");
// Set the correct log path. Before this call, /tmp (Unix) or %TEMP% was used
@@ -294,54 +296,40 @@
#endif
// Manage ini files and set the default settings file (usually orxonox.ini)
- this->configFileManager_ = new ConfigFileManager();
+ this->configFileManager_.reset(new ConfigFileManager());
this->configFileManager_->setFilename(ConfigFileType::Settings,
CommandLine::getValue("settingsFile").getString());
// Required as well for the config values
- this->languageInstance_ = new Language();
+ this->languageInstance_.reset(new Language());
// Do this soon after the ConfigFileManager has been created to open up the
// possibility to configure everything below here
this->configuration_->initialise();
// Create the lua interface
- this->luaBind_ = new LuaBind();
+ this->luaBind_.reset(new LuaBind());
// initialise Tcl
- this->tclBind_ = new TclBind(Core::getMediaPathString());
- this->tclThreadManager_ = new TclThreadManager(tclBind_->getTclInterpreter());
+ this->tclBind_.reset(new TclBind(Core::getMediaPathString()));
+ this->tclThreadManager_.reset(new TclThreadManager(tclBind_->getTclInterpreter()));
// create a shell
- this->shell_ = new Shell();
+ this->shell_.reset(new Shell());
// creates the class hierarchy for all classes with factories
Factory::createClassHierarchy();
}
/**
- @brief Sets the bool to true to avoid static functions accessing a deleted object.
+ @brief
+ All destruction code is handled by scoped_ptrs and SimpleScopeGuards.
*/
Core::~Core()
{
- delete this->shell_;
- delete this->tclThreadManager_;
- delete this->tclBind_;
- delete this->luaBind_;
- delete this->configuration_;
- delete this->languageInstance_;
- delete this->configFileManager_;
-
- // Destroy command line arguments
- CommandLine::destroyAllArguments();
- // Also delete external console commands that don't belong to an Identifier
- CommandExecutor::destroyExternalCommands();
- // Clean up class hierarchy stuff (identifiers, XMLPort, configValues, consoleCommand)
- Identifier::destroyAllIdentifiers();
-
- delete this->signalHandler_;
-
// Don't assign singletonRef_s with NULL! Recreation is not supported
+ // The is quite simply because of the pre-main code that uses heap allocation
+ // And we correctly deallocate these resources in this destructor.
}
void Core::loadGraphics()
@@ -350,19 +338,22 @@
return;
// Load OGRE including the render window
- this->graphicsManager_ = new GraphicsManager();
+ scoped_ptr<GraphicsManager> graphicsManager(new GraphicsManager());
// The render window width and height are used to set up the mouse movement.
size_t windowHnd = 0;
- Ogre::RenderWindow* renderWindow = GraphicsManager::getInstance().getRenderWindow();
- renderWindow->getCustomAttribute("WINDOW", &windowHnd);
+ graphicsManager->getRenderWindow()->getCustomAttribute("WINDOW", &windowHnd);
// Calls the InputManager which sets up the input devices.
- inputManager_ = new InputManager(windowHnd);
+ scoped_ptr<InputManager> inputManager(new InputManager(windowHnd));
// load the CEGUI interface
- guiManager_ = new GUIManager(renderWindow);
+ guiManager_.reset(new GUIManager(graphicsManager->getRenderWindow()));
+ // Dismiss scoped pointers
+ graphicsManager_.swap(graphicsManager);
+ inputManager_.swap(inputManager);
+
bGraphicsLoaded_ = true;
}
@@ -371,9 +362,9 @@
if (!bGraphicsLoaded_)
return;
- delete this->guiManager_;
- delete this->inputManager_;
- delete graphicsManager_;
+ this->guiManager_.reset();;
+ this->inputManager_.reset();;
+ this->graphicsManager_.reset();
bGraphicsLoaded_ = false;
}
Modified: branches/resource/src/core/Core.h
===================================================================
--- branches/resource/src/core/Core.h 2009-07-29 15:09:09 UTC (rev 3362)
+++ branches/resource/src/core/Core.h 2009-07-29 15:24:39 UTC (rev 3363)
@@ -42,11 +42,14 @@
#include "CorePrereqs.h"
#include <cassert>
+#include <boost/scoped_ptr.hpp>
#include "util/OutputHandler.h"
+#include "util/ScopeGuard.h"
namespace orxonox
{
class CoreConfiguration;
+ using boost::scoped_ptr;
/**
@brief
@@ -57,6 +60,8 @@
*/
class _CoreExport Core
{
+ typedef Loki::ScopeGuardImpl0<void (*)()> SimpleScopeGuard;
+
public:
/**
@brief
@@ -111,22 +116,24 @@
void createDirectories();
void setThreadAffinity(int limitToCPU);
- // Singletons
- ConfigFileManager* configFileManager_;
- Language* languageInstance_;
- LuaBind* luaBind_;
- Shell* shell_;
- SignalHandler* signalHandler_;
- TclBind* tclBind_;
- TclThreadManager* tclThreadManager_;
+ // Mind the order for the destruction!
+ scoped_ptr<SignalHandler> signalHandler_;
+ SimpleScopeGuard identifierDestroyer_;
+ SimpleScopeGuard consoleCommandDestroyer_;
+ scoped_ptr<ConfigFileManager> configFileManager_;
+ scoped_ptr<Language> languageInstance_;
+ scoped_ptr<CoreConfiguration> configuration_;
+ scoped_ptr<LuaBind> luaBind_;
+ scoped_ptr<TclBind> tclBind_;
+ scoped_ptr<TclThreadManager> tclThreadManager_;
+ scoped_ptr<Shell> shell_;
// graphical
- InputManager* inputManager_; //!< Interface to OIS
- GUIManager* guiManager_; //!< Interface to GUI
- GraphicsManager* graphicsManager_; //!< Interface to OGRE
+ scoped_ptr<GraphicsManager> graphicsManager_; //!< Interface to OGRE
+ scoped_ptr<InputManager> inputManager_; //!< Interface to OIS
+ scoped_ptr<GUIManager> guiManager_; //!< Interface to GUI
- bool bDevRun_; //!< True for runs in the build directory (not installed)
- bool bGraphicsLoaded_;
- CoreConfiguration* configuration_;
+ bool bDevRun_; //!< True for runs in the build directory (not installed)
+ bool bGraphicsLoaded_;
static Core* singletonRef_s;
};
Modified: branches/resource/src/core/Game.cc
===================================================================
--- branches/resource/src/core/Game.cc 2009-07-29 15:09:09 UTC (rev 3362)
+++ branches/resource/src/core/Game.cc 2009-07-29 15:24:39 UTC (rev 3363)
@@ -39,6 +39,7 @@
#include "util/Debug.h"
#include "util/Exception.h"
+#include "util/ScopeGuard.h"
#include "util/Sleep.h"
#include "util/SubString.h"
#include "Clock.h"
@@ -132,10 +133,10 @@
this->declareGameState<GameState>("GameState", "emptyRootGameState", true, false);
// Set up a basic clock to keep time
- this->gameClock_ = new Clock();
+ this->gameClock_.reset(new Clock());
// Create the Core
- this->core_ = new Core(cmdLine);
+ this->core_.reset(new Core(cmdLine));
// After the core has been created, we can safely instantiate the GameStates that don't require graphics
for (std::map<std::string, GameStateInfo>::const_iterator it = gameStateDeclarations_s.begin();
@@ -152,29 +153,15 @@
this->loadedStates_.push_back(this->getState(rootStateNode_->name_));
// Do this after the Core creation!
- this->configuration_ = new GameConfiguration();
+ this->configuration_.reset(new GameConfiguration());
}
/**
@brief
+ All destruction code is handled by scoped_ptrs and SimpleScopeGuards.
*/
Game::~Game()
{
- // Destroy the configuration helper class instance
- delete this->configuration_;
-
- // Destroy the GameStates (note that the nodes still point to them, but doesn't matter)
- for (std::map<std::string, GameState*>::const_iterator it = constructedStates_.begin();
- it != constructedStates_.end(); ++it)
- delete it->second;
-
- // Destroy the Core and with it almost everything
- delete this->core_;
- delete this->gameClock_;
-
- // Take care of the GameStateFactories
- GameStateFactory::destroyFactories();
-
// Don't assign singletonRef_s with NULL! Recreation is not supported
}
@@ -282,7 +269,7 @@
void Game::updateGameStates()
{
// Note: The first element is the empty root state, which doesn't need ticking
- for (std::vector<GameState*>::const_iterator it = this->loadedStates_.begin() + 1;
+ for (GameStateVector::const_iterator it = this->loadedStates_.begin() + 1;
it != this->loadedStates_.end(); ++it)
{
std::string exceptionMessage;
@@ -456,9 +443,9 @@
COUT(2) << "Warning: Can't pop the internal dummy root GameState" << std::endl;
}
- GameState* Game::getState(const std::string& name)
+ shared_ptr<GameState> Game::getState(const std::string& name)
{
- std::map<std::string, GameState*>::const_iterator it = constructedStates_.find(name);
+ GameStateMap::const_iterator it = constructedStates_.find(name);
if (it != constructedStates_.end())
return it->second;
else
@@ -468,7 +455,7 @@
COUT(1) << "Error: GameState '" << name << "' has not yet been loaded." << std::endl;
else
COUT(1) << "Error: Could not find GameState '" << name << "'." << std::endl;
- return 0;
+ return shared_ptr<GameState>();
}
}
@@ -527,6 +514,7 @@
if (!GameMode::bShowsGraphics_s)
{
core_->loadGraphics();
+ Loki::ScopeGuard graphicsUnloader = Loki::MakeObjGuard(*this, &Game::unloadGraphics);
GameMode::bShowsGraphics_s = true;
// Construct all the GameStates that require graphics
@@ -535,11 +523,14 @@
{
if (it->second.bGraphicsMode)
{
+ // Game state loading failure is serious --> don't catch
+ shared_ptr<GameState> gameState = GameStateFactory::fabricate(it->second);
if (!constructedStates_.insert(std::make_pair(
- it->second.stateName, GameStateFactory::fabricate(it->second))).second)
+ it->second.stateName, gameState)).second)
assert(false); // GameState was already created!
}
}
+ graphicsUnloader.Dismiss();
}
}
@@ -548,13 +539,10 @@
if (GameMode::bShowsGraphics_s)
{
// Destroy all the GameStates that require graphics
- for (std::map<std::string, GameState*>::iterator it = constructedStates_.begin(); it != constructedStates_.end();)
+ for (GameStateMap::iterator it = constructedStates_.begin(); it != constructedStates_.end();)
{
if (it->second->getInfo().bGraphicsMode)
- {
- delete it->second;
constructedStates_.erase(it++);
- }
else
++it;
}
@@ -576,57 +564,57 @@
void Game::loadState(const std::string& name)
{
this->bChangingState_ = true;
+ LOKI_ON_BLOCK_EXIT_OBJ(*this, &Game::resetChangingState);
+
// If state requires graphics, load it
- if (gameStateDeclarations_s[name].bGraphicsMode)
+ Loki::ScopeGuard graphicsUnloader = Loki::MakeObjGuard(*this, &Game::unloadGraphics);
+ if (gameStateDeclarations_s[name].bGraphicsMode && !GameMode::showsGraphics())
this->loadGraphics();
- GameState* state = this->getState(name);
+ else
+ graphicsUnloader.Dismiss();
+
+ shared_ptr<GameState> state = this->getState(name);
state->activate();
if (!this->loadedStates_.empty())
this->loadedStates_.back()->activity_.topState = false;
this->loadedStates_.push_back(state);
state->activity_.topState = true;
- this->bChangingState_ = false;
+
+ graphicsUnloader.Dismiss();
}
void Game::unloadState(const std::string& name)
{
- GameState* state = this->getState(name);
this->bChangingState_ = true;
- state->activity_.topState = false;
- this->loadedStates_.pop_back();
- if (!this->loadedStates_.empty())
- this->loadedStates_.back()->activity_.topState = true;
try
{
+ shared_ptr<GameState> state = this->getState(name);
+ state->activity_.topState = false;
+ this->loadedStates_.pop_back();
+ if (!this->loadedStates_.empty())
+ this->loadedStates_.back()->activity_.topState = true;
state->deactivate();
- // Check if graphis is still required
- bool graphicsRequired = false;
- for (unsigned i = 0; i < loadedStates_.size(); ++i)
- graphicsRequired |= loadedStates_[i]->getInfo().bGraphicsMode;
- if (!graphicsRequired)
- this->unloadGraphics();
}
catch (const std::exception& ex)
{
COUT(2) << "Warning: Unloading GameState '" << name << "' threw an exception: " << ex.what() << std::endl;
COUT(2) << " There might be potential resource leaks involved! To avoid this, improve exception-safety." << std::endl;
}
+ // Check if graphics is still required
+ bool graphicsRequired = false;
+ for (unsigned i = 0; i < loadedStates_.size(); ++i)
+ graphicsRequired |= loadedStates_[i]->getInfo().bGraphicsMode;
+ if (!graphicsRequired)
+ this->unloadGraphics();
this->bChangingState_ = false;
}
- std::map<std::string, Game::GameStateFactory*> Game::GameStateFactory::factories_s;
+ std::map<std::string, shared_ptr<Game::GameStateFactory> > Game::GameStateFactory::factories_s;
- /*static*/ GameState* Game::GameStateFactory::fabricate(const GameStateInfo& info)
+ /*static*/ shared_ptr<GameState> Game::GameStateFactory::fabricate(const GameStateInfo& info)
{
- std::map<std::string, GameStateFactory*>::const_iterator it = factories_s.find(info.className);
+ std::map<std::string, shared_ptr<Game::GameStateFactory> >::const_iterator it = factories_s.find(info.className);
assert(it != factories_s.end());
return it->second->fabricateInternal(info);
}
-
- /*static*/ void Game::GameStateFactory::destroyFactories()
- {
- for (std::map<std::string, GameStateFactory*>::const_iterator it = factories_s.begin(); it != factories_s.end(); ++it)
- delete it->second;
- factories_s.clear();
- }
}
Modified: branches/resource/src/core/Game.h
===================================================================
--- branches/resource/src/core/Game.h 2009-07-29 15:09:09 UTC (rev 3362)
+++ branches/resource/src/core/Game.h 2009-07-29 15:24:39 UTC (rev 3363)
@@ -43,6 +43,7 @@
#include <string>
#include <vector>
#include <boost/shared_ptr.hpp>
+#include <boost/scoped_ptr.hpp>
#include <boost/preprocessor/cat.hpp>
#include "util/Debug.h"
@@ -58,6 +59,8 @@
namespace orxonox
{
class GameConfiguration;
+ using boost::scoped_ptr;
+ using boost::shared_ptr;
//! Helper object required before GameStates are being constructed
struct GameStateInfo
@@ -74,13 +77,15 @@
*/
class _CoreExport Game
{
+ typedef std::vector<shared_ptr<GameState> > GameStateVector;
+ typedef std::map<std::string, shared_ptr<GameState> > GameStateMap;
typedef boost::shared_ptr<GameStateTreeNode> GameStateTreeNodePtr;
public:
Game(const std::string& cmdLine);
~Game();
void setStateHierarchy(const std::string& str);
- GameState* getState(const std::string& name);
+ shared_ptr<GameState> getState(const std::string& name);
void run();
void stop();
@@ -105,21 +110,20 @@
{
public:
virtual ~GameStateFactory() { }
- static GameState* fabricate(const GameStateInfo& info);
+ static shared_ptr<GameState> fabricate(const GameStateInfo& info);
template <class T>
static void createFactory(const std::string& className)
- { factories_s[className] = new TemplateGameStateFactory<T>(); }
- static void destroyFactories();
+ { factories_s[className].reset(new TemplateGameStateFactory<T>()); }
private:
- virtual GameState* fabricateInternal(const GameStateInfo& info) = 0;
- static std::map<std::string, GameStateFactory*> factories_s;
+ virtual shared_ptr<GameState> fabricateInternal(const GameStateInfo& info) = 0;
+ static std::map<std::string, shared_ptr<GameStateFactory> > factories_s;
};
template <class T>
class TemplateGameStateFactory : public GameStateFactory
{
public:
- GameState* fabricateInternal(const GameStateInfo& info)
- { return new T(info); }
+ shared_ptr<GameState> fabricateInternal(const GameStateInfo& info)
+ { return shared_ptr<GameState>(new T(info)); }
};
struct StatisticsTickInfo
@@ -143,16 +147,19 @@
void updateStatistics();
void updateFPSLimiter();
- std::map<std::string, GameState*> constructedStates_;
- std::vector<GameState*> loadedStates_;
+ // ScopeGuard helper function
+ void resetChangingState() { this->bChangingState_ = false; }
+
+ scoped_ptr<Clock> gameClock_;
+ scoped_ptr<Core> core_;
+ scoped_ptr<GameConfiguration> configuration_;
+
+ GameStateMap constructedStates_;
+ GameStateVector loadedStates_;
GameStateTreeNodePtr rootStateNode_;
GameStateTreeNodePtr loadedTopStateNode_;
- std::vector<GameStateTreeNodePtr > requestedStateNodes_;
+ std::vector<GameStateTreeNodePtr> requestedStateNodes_;
- Core* core_;
- Clock* gameClock_;
- GameConfiguration* configuration_;
-
bool bChangingState_;
bool bAbort_;
More information about the Orxonox-commit
mailing list