Changeset 3238 for code

Timestamp:

Jun 28, 2009, 1:47:57 PM (15 years ago)

Author:

rgrieder

Message:

Improved exception-safety in the Game class and fixed some issues and bugs resulting from the changes.

Location:

code/branches/core4/src

Files:

• core/Game.cc (modified) (14 diffs)
• core/Game.h (modified) (1 diff)
• core/GameState.cc (modified) (1 diff)
• core/GameState.h (modified) (4 diffs)
• orxonox/Main.cc (modified) (1 diff)
• orxonox/gamestates/GSGraphics.cc (modified) (2 diffs)
• orxonox/gamestates/GSRoot.cc (modified) (1 diff)
• util/Exception.cc (modified) (2 diffs)
• util/Exception.h (modified) (3 diffs)

code/branches/core4/src/core/Game.cc

@@ -54 +54 @@
     using boost::weak_ptr;
 
+    std::map<std::string, GameState*> Game::allStates_s;
+    Game* Game::singletonRef_s = 0;
+
     static void stop_game()
         { Game::getInstance().stop(); }
     SetConsoleCommandShortcutExternAlias(stop_game, "exit");
+    // Add an empty gamestate that serves as internal root state
+    AddGameState(GameState, "emptyRootGameState");
 
     struct _CoreExport GameStateTreeNode
@@ -64 +69 @@
         std::vector<shared_ptr<GameStateTreeNode> > children_;
     };
-
-    std::map<std::string, GameState*> Game::allStates_s;
-    Game* Game::singletonRef_s = 0;
 
     /**
@@ -77 +79 @@
         singletonRef_s = this;
 
-        this->abort_ = false;
+        this->bAbort_ = false;
+        bChangingState_ = false;
+        // The empty root state is ALWAYS loaded!
+        this->rootStateNode_ = shared_ptr<GameStateTreeNode>(new GameStateTreeNode());
+        this->rootStateNode_->state_ = getState("emptyRootGameState");
+        this->activeStateNode_ = this->rootStateNode_;
+        this->activeStates_.push_back(this->rootStateNode_->state_);
 
         // reset statistics
@@ -86 +94 @@
         this->avgFPS_ = 0.0f;
         this->avgTickTime_ = 0.0f;
-
 
         // Set up a basic clock to keep time
@@ -147 +154 @@
     void Game::run()
     {
-        // Always start with the ROOT state
-        this->requestedStateNodes_.push_back(this->rootStateNode_);
-        this->activeStateNode_ = this->rootStateNode_;
-        this->loadState(this->rootStateNode_->state_);
+        if (this->requestedStateNodes_.empty())
+            COUT(0) << "Warning: Starting game without requesting GameState. This automatically terminates the program." << std::endl;
 
         // START GAME
         this->gameClock_->capture(); // first delta time should be about 0 seconds
-        while (!this->abort_ && !this->activeStates_.empty())
+        while (!this->bAbort_ && (!this->activeStates_.empty() || this->requestedStateNodes_.size() > 0))
         {
             this->gameClock_->capture();
@@ -165 +170 @@
 
             // UPDATE STATE STACK
-            while (this->requestedStateNodes_.size() > 1)
-            {
-                // Note: this->requestedStateNodes_.front() is the currently active state node
-                std::vector<shared_ptr<GameStateTreeNode> >::iterator it = this->requestedStateNodes_.begin() + 1;
-                if (*it == this->activeStateNode_->parent_.lock())
+            while (this->requestedStateNodes_.size() > 0)
+            {
+                shared_ptr<GameStateTreeNode> requestedStateNode = this->requestedStateNodes_.front();
+                assert(this->activeStateNode_);
+                if (!this->activeStateNode_->parent_.expired() && requestedStateNode == this->activeStateNode_->parent_.lock())
                     this->unloadState(this->activeStateNode_->state_);
                 else // has to be child
-                    this->loadState((*it)->state_);
-                this->activeStateNode_ = *it;
+                {
+                    try
+                    {
+                        this->loadState(requestedStateNode->state_);
+                    }
+                    catch (const std::exception& ex)
+                    {
+                        COUT(1) << "Error: Loading GameState '" << requestedStateNode->state_->getName() << "' failed: " << ex.what() << std::endl;
+                        // All scheduled operations have now been rendered inert --> flush them and issue a warning
+                        if (this->requestedStateNodes_.size() > 1)
+                            COUT(1) << "All " << this->requestedStateNodes_.size() - 1 << " scheduled transitions have been ignored." << std::endl;
+                        this->requestedStateNodes_.clear();
+                        break;
+                    }
+                }
+                this->activeStateNode_ = requestedStateNode;
                 this->requestedStateNodes_.erase(this->requestedStateNodes_.begin());
             }
 
-            // UPDATE, bottom to top in the stack
-            this->core_->update(*this->gameClock_);
-            for (std::vector<GameState*>::const_iterator it = this->activeStates_.begin();
+            // UPDATE, Core first
+            try
+            {
+                this->core_->update(*this->gameClock_);
+            }
+            catch (...)
+            {
+                COUT(0) << "An exception occured while ticking the Core. This should really never happen!" << std::endl;
+                COUT(0) << "Closing the program." << std::endl;
+                this->stop();
+                break;
+            }
+
+            // UPDATE, GameStates bottom to top in the stack
+            // Note: The first element is the empty root state, which doesn't need ticking
+            for (std::vector<GameState*>::const_iterator it = this->activeStates_.begin() + 1;
                 it != this->activeStates_.end(); ++it)
             {
-                // Add tick time for most of the states
-                uint64_t timeBeforeTick;
-                if ((*it)->getCountTickTime())
-                    timeBeforeTick = this->gameClock_->getRealMicroseconds();
-                
-                (*it)->update(*this->gameClock_);
-
-                if ((*it)->getCountTickTime())
-                    this->addTickTime(static_cast<uint32_t>(this->gameClock_->getRealMicroseconds() - timeBeforeTick));
+                try
+                {
+                    // Add tick time for most of the states
+                    uint64_t timeBeforeTick;
+                    if (!(*it)->ignoreTickTime())
+                        timeBeforeTick = this->gameClock_->getRealMicroseconds();
+                    (*it)->update(*this->gameClock_);
+                    if (!(*it)->ignoreTickTime())
+                        this->addTickTime(static_cast<uint32_t>(this->gameClock_->getRealMicroseconds() - timeBeforeTick));
+                }
+                catch (...)
+                {
+                    COUT(1) << "An exception occured while ticking GameState '" << (*it)->getName() << "'. This should really never happen!" << std::endl;
+                    COUT(1) << "Unloading all GameStates depending on the one that crashed." << std::endl;
+                    if ((*it)->getParent() != NULL)
+                        this->requestState((*it)->getParent()->getName());
+                    else
+                        this->stop();
+                    break;
+                }
+
             }
 
@@ -203 +247 @@
                     do
                     {
-                        assert(this->periodTickTime_ > it->tickLength);
+                        assert(this->periodTickTime_ >= it->tickLength);
                         this->periodTickTime_ -= it->tickLength;
                         ++it;
@@ -220 +264 @@
 
         // UNLOAD all remaining states
-        while (!this->activeStates_.empty())
+        while (this->activeStates_.size() > 1)
             this->unloadState(this->activeStates_.back());
-        this->activeStateNode_.reset();
+        this->activeStateNode_ = this->rootStateNode_;
         this->requestedStateNodes_.clear();
     }
@@ -228 +272 @@
     void Game::stop()
     {
-        this->abort_ = true;
+        this->bAbort_ = true;
     }
 
@@ -244 +288 @@
     {
         GameState* state = this->getState(name);
-        if (state == NULL || this->activeStateNode_ == NULL)
+        if (state == NULL)
             return;
 
-        shared_ptr<GameStateTreeNode> requestedNode;
-
-        // this->requestedStateNodes_.back() is the currently active state
-        shared_ptr<GameStateTreeNode> lastRequestedNode = this->requestedStateNodes_.back();
-
-        // Already the active node?
+        //if (this->bChangingState_)
+        //{
+        //    COUT(2) << "Warning: Requesting GameStates while loading/unloading a GameState is illegal! Ignoring." << std::endl;
+        //    return;
+        //}
+
+        shared_ptr<GameStateTreeNode> lastRequestedNode;
+        if (this->requestedStateNodes_.empty())
+            lastRequestedNode = this->activeStateNode_;
+        else
+            lastRequestedNode = this->requestedStateNodes_.back();
         if (state == lastRequestedNode->state_)
         {
@@ -260 +309 @@
 
         // Check children first
+        shared_ptr<GameStateTreeNode> requestedNode;
         for (unsigned int i = 0; i < lastRequestedNode->children_.size(); ++i)
         {
@@ -293 +343 @@
     void Game::popState()
     {
-        if (this->activeStateNode_ != NULL && this->requestedStateNodes_.back()->parent_.lock())
-            this->requestState(this->requestedStateNodes_.back()->parent_.lock()->state_->getName());
-        else
-            COUT(2) << "Warning: Could not pop GameState. Ignoring." << std::endl;
+        shared_ptr<GameStateTreeNode> lastRequestedNode;
+        if (this->requestedStateNodes_.empty())
+            lastRequestedNode = this->activeStateNode_;
+        else
+            lastRequestedNode = this->requestedStateNodes_.back();
+        if (lastRequestedNode != this->rootStateNode_)
+            this->requestState(lastRequestedNode->parent_.lock()->state_->getName());
+        else
+            COUT(2) << "Warning: Can't pop the internal dummy root GameState" << std::endl;
     }
 
@@ -325 +380 @@
             while(pos < str.size() && str[pos] != ' ')
                 ++pos;
-            stateStrings.push_back(std::pair<std::string, unsigned>(
-                str.substr(startPos, pos - startPos), indentation));
+            stateStrings.push_back(std::make_pair(str.substr(startPos, pos - startPos), indentation));
         }
         unsigned int currentLevel = 0;
-        shared_ptr<GameStateTreeNode> currentNode;
+        shared_ptr<GameStateTreeNode> currentNode = this->rootStateNode_;
         for (std::vector<std::pair<std::string, unsigned> >::const_iterator it = stateStrings.begin(); it != stateStrings.end(); ++it)
         {
             std::string newStateName = it->first;
-            unsigned newLevel = it->second;
+            unsigned newLevel = it->second + 1; // empty root is 0
             GameState* newState = this->getState(newStateName);
             if (!newState)
-                ThrowException(GameState, std::string("GameState with name '") + newStateName + "' not found!");
-            if (newLevel == 0)
-            {
-                // root
-                if (this->rootStateNode_ != NULL)
-                    ThrowException(GameState, "No two root GameStates are allowed!");
-                shared_ptr<GameStateTreeNode> newNode(new GameStateTreeNode);
-                newNode->state_ = newState;
-                this->rootStateNode_ = newNode;
-                currentNode = this->rootStateNode_;
-            }
-            else if (currentNode)
-            {
-                shared_ptr<GameStateTreeNode> newNode(new GameStateTreeNode);
-                newNode->state_ = newState;
-                if (newLevel < currentLevel)
-                {
-                    // Get down the hierarchy
-                    do
-                        currentNode = currentNode->parent_.lock();
-                    while (newLevel < --currentLevel);
-                }
-                if (newLevel == currentLevel)
-                {
-                    // same level
-                    newNode->parent_ = currentNode->parent_;
-                    newNode->parent_.lock()->children_.push_back(newNode);
-                }
-                else if (newLevel == currentLevel + 1)
-                {
-                    // child
-                    newNode->parent_ = currentNode;
-                    currentNode->children_.push_back(newNode);
-                }
-                else
-                    ThrowException(GameState, "Indentation error while parsing the hierarchy.");
-                currentNode = newNode;
-                currentLevel = newLevel;
+                ThrowException(GameState, "GameState with name '" << newStateName << "' not found!");
+            if (newState == this->rootStateNode_->state_)
+                ThrowException(GameState, "You shouldn't use 'emptyRootGameState' in the hierarchy...");
+            shared_ptr<GameStateTreeNode> newNode(new GameStateTreeNode);
+            newNode->state_ = newState;
+
+            if (newLevel <= currentLevel)
+            {
+                do
+                    currentNode = currentNode->parent_.lock();
+                while (newLevel <= --currentLevel);
+            }
+            if (newLevel == currentLevel + 1)
+            {
+                // Add the child
+                newNode->parent_ = currentNode;
+                currentNode->children_.push_back(newNode);
+                currentNode->state_->addChild(newNode->state_);
             }
             else
-            {
-                ThrowException(GameState, "No root GameState specified!");
-            }
+                ThrowException(GameState, "Indentation error while parsing the hierarchy.");
+            currentNode = newNode;
+            currentLevel = newLevel;
         }
     }
@@ -386 +420 @@
     void Game::loadState(GameState* state)
     {
+        this->bChangingState_ = true;
+        state->activate();
         if (!this->activeStates_.empty())
             this->activeStates_.back()->activity_.topState = false;
-        state->activate();
+        this->activeStates_.push_back(state);
         state->activity_.topState = true;
-        this->activeStates_.push_back(state);
+        this->bChangingState_ = false;
     }
 
     void Game::unloadState(orxonox::GameState* state)
     {
+        this->bChangingState_ = true;
         state->activity_.topState = false;
-        state->deactivate();
         this->activeStates_.pop_back();
         if (!this->activeStates_.empty())
             this->activeStates_.back()->activity_.topState = true;
+        try
+        {
+            state->deactivate();
+        }
+        catch (const std::exception& ex)
+        {
+            COUT(2) << "Warning: Unloading GameState '" << state->getName() << "' threw an exception: " << ex.what() << std::endl;
+            COUT(2) << "         There might be potential resource leaks involved! To avoid this, improve exception-safety." << std::endl;
+        }
+        this->bChangingState_ = false;
     }
 

code/branches/core4/src/core/Game.h

@@ -118 +118 @@
         Clock*                          gameClock_;
 
-        bool                            abort_;
+        bool                            bChangingState_;
+        bool                            bAbort_;
 
         // variables for time statistics

code/branches/core4/src/core/GameState.cc

@@ -45 +45 @@
         Constructor only initialises variables and sets the name permanently.
     */
-    GameState::GameState(const std::string& name, bool countTickTime)
+    GameState::GameState(const std::string& name, bool ignoreTickTime)
         : name_(name)
-        , bCountTickTime_(countTickTime)
+        , bIgnoreTickTime_(ignoreTickTime)
         , parent_(0)
     {

code/branches/core4/src/core/GameState.h

@@ -77 +77 @@
 
     public:
-        GameState(const std::string& name, bool countTicktime = true);
+        GameState(const std::string& name, bool ignoreTicktime = false);
         virtual ~GameState();
 
@@ -84 +84 @@
         GameState* getParent()       const { return this->parent_; }
 
-        bool getCountTickTime()      const { return this->bCountTickTime_; }
+        bool ignoreTickTime()        const { return this->bIgnoreTickTime_; }
 
         void addChild(GameState* state);
@@ -90 +90 @@
 
     protected:
-        virtual void activate() = 0;
-        virtual void deactivate() = 0;
-        virtual void update(const Clock& time) = 0;
+        virtual void activate() { }
+        virtual void deactivate() { }
+        virtual void update(const Clock& time) { }
 
     private:
@@ -103 +103 @@
         const std::string                        name_;
         State                                    activity_;
-        const bool                               bCountTickTime_;
+        const bool                               bIgnoreTickTime_;
         GameState*                               parent_;
         std::map<std::string, GameState*>        children_;

code/branches/core4/src/orxonox/Main.cc

@@ -68 +68 @@
         );
 
+        orxonox.requestState("root");
         orxonox.run();
 

code/branches/core4/src/orxonox/gamestates/GSGraphics.cc

@@ -57 +57 @@
 namespace orxonox
 {
-    AddGameState(GSGraphics, "graphics", false);
+    AddGameState(GSGraphics, "graphics", true);
 
     GSGraphics::GSGraphics(const std::string& name, bool countTickTime)
@@ -198 +198 @@
     void GSGraphics::toggleGUI()
     {
-            GUIManager::getInstance().executeCode("toggleGUI()");
+        GUIManager::getInstance().executeCode("toggleGUI()");
     }
 

code/branches/core4/src/orxonox/gamestates/GSRoot.cc

@@ -40 +40 @@
 namespace orxonox
 {
-    AddGameState(GSRoot, "root", false);
+    AddGameState(GSRoot, "root", true);
     SetCommandLineSwitch(console);
     // Shortcuts for easy direct loading

code/branches/core4/src/util/Exception.cc

@@ -64 +64 @@
             std::ostringstream fullDesc;
 
-            fullDesc << this->getTypeName() << "_EXCEPTION";
+            fullDesc << this->getTypeName() << "Exception";
 
             if (this->filename_ != "")
@@ -87 +87 @@
         return fullDescription_;
     }
+
+    //! Returns the error description
+    const char* Exception::what() const throw()
+    {
+        return getDescription().c_str();
+    }
 }

code/branches/core4/src/util/Exception.h

@@ -68 +68 @@
         //! Needed for compatibility with std::exception
         virtual ~Exception() throw() { }
+        const char* what() const throw();
 
         //! Returns a full description with type, line, file and function
@@ -81 +82 @@
         //! Returns the filename in which the exception occurred.
         virtual const std::string& getFilename()        const { return this->filename_; }
-
-        //! Returns a full description of the error.
-        const char* what() const throw() { return getFullDescription().c_str(); }
 
     protected:
@@ -152 +150 @@
     Exception description as string
 */
-#define ThrowException(type, description, ...) \
+#define ThrowException(type, description) \
     throw orxonox::exceptionThrowerHelper(type##Exception(static_cast<std::ostringstream&>(std::ostringstream().flush() << description).str(), __LINE__, __FILE__, __FUNCTIONNAME__))