[Orxonox-commit 706] r3238 - in branches/core4/src: core orxonox orxonox/gamestates util

rgrieder at orxonox.net rgrieder at orxonox.net
Sun Jun 28 13:47:58 CEST 2009


Author: rgrieder
Date: 2009-06-28 13:47:57 +0200 (Sun, 28 Jun 2009)
New Revision: 3238

Modified:
   branches/core4/src/core/Game.cc
   branches/core4/src/core/Game.h
   branches/core4/src/core/GameState.cc
   branches/core4/src/core/GameState.h
   branches/core4/src/orxonox/Main.cc
   branches/core4/src/orxonox/gamestates/GSGraphics.cc
   branches/core4/src/orxonox/gamestates/GSRoot.cc
   branches/core4/src/util/Exception.cc
   branches/core4/src/util/Exception.h
Log:
Improved exception-safety in the Game class and fixed some issues and bugs resulting from the changes.

Modified: branches/core4/src/core/Game.cc
===================================================================
--- branches/core4/src/core/Game.cc	2009-06-28 11:44:23 UTC (rev 3237)
+++ branches/core4/src/core/Game.cc	2009-06-28 11:47:57 UTC (rev 3238)
@@ -53,9 +53,14 @@
     using boost::shared_ptr;
     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,9 +69,6 @@
         std::vector<shared_ptr<GameStateTreeNode> > children_;
     };
 
-    std::map<std::string, GameState*> Game::allStates_s;
-    Game* Game::singletonRef_s = 0;
-
     /**
     @brief
         Non-initialising constructor.
@@ -76,7 +78,13 @@
         assert(singletonRef_s == 0);
         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
         this->statisticsStartTime_ = 0;
@@ -86,7 +94,6 @@
         this->avgFPS_ = 0.0f;
         this->avgTickTime_ = 0.0f;
 
-
         // Set up a basic clock to keep time
         this->gameClock_ = new Clock();
 
@@ -146,14 +153,12 @@
     */
     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();
             uint64_t currentTime = this->gameClock_->getMicroseconds();
@@ -164,32 +169,71 @@
             this->periodTime_ += this->gameClock_->getDeltaTimeMicroseconds();
 
             // UPDATE STATE STACK
-            while (this->requestedStateNodes_.size() > 1)
+            while (this->requestedStateNodes_.size() > 0)
             {
-                // 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())
+                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_);
+                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;
+                }
 
-                if ((*it)->getCountTickTime())
-                    this->addTickTime(static_cast<uint32_t>(this->gameClock_->getRealMicroseconds() - timeBeforeTick));
             }
 
             // STATISTICS
@@ -202,7 +246,7 @@
                 {
                     do
                     {
-                        assert(this->periodTickTime_ > it->tickLength);
+                        assert(this->periodTickTime_ >= it->tickLength);
                         this->periodTickTime_ -= it->tickLength;
                         ++it;
                         assert(it != this->statisticsTickTimes_.end());
@@ -219,15 +263,15 @@
         }
 
         // 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();
     }
 
     void Game::stop()
     {
-        this->abort_ = true;
+        this->bAbort_ = true;
     }
 
     void Game::addTickTime(uint32_t length)
@@ -243,15 +287,20 @@
     void Game::requestState(const std::string& name)
     {
         GameState* state = this->getState(name);
-        if (state == NULL || this->activeStateNode_ == NULL)
+        if (state == NULL)
             return;
 
-        shared_ptr<GameStateTreeNode> requestedNode;
+        //if (this->bChangingState_)
+        //{
+        //    COUT(2) << "Warning: Requesting GameStates while loading/unloading a GameState is illegal! Ignoring." << std::endl;
+        //    return;
+        //}
 
-        // this->requestedStateNodes_.back() is the currently active state
-        shared_ptr<GameStateTreeNode> lastRequestedNode = this->requestedStateNodes_.back();
-
-        // Already the active node?
+        shared_ptr<GameStateTreeNode> lastRequestedNode;
+        if (this->requestedStateNodes_.empty())
+            lastRequestedNode = this->activeStateNode_;
+        else
+            lastRequestedNode = this->requestedStateNodes_.back();
         if (state == lastRequestedNode->state_)
         {
             COUT(2) << "Warning: Requesting the currently active state! Ignoring." << std::endl;
@@ -259,6 +308,7 @@
         }
 
         // Check children first
+        shared_ptr<GameStateTreeNode> requestedNode;
         for (unsigned int i = 0; i < lastRequestedNode->children_.size(); ++i)
         {
             if (lastRequestedNode->children_[i]->state_ == state)
@@ -292,10 +342,15 @@
 
     void Game::popState()
     {
-        if (this->activeStateNode_ != NULL && this->requestedStateNodes_.back()->parent_.lock())
-            this->requestState(this->requestedStateNodes_.back()->parent_.lock()->state_->getName());
+        shared_ptr<GameStateTreeNode> lastRequestedNode;
+        if (this->requestedStateNodes_.empty())
+            lastRequestedNode = this->activeStateNode_;
         else
-            COUT(2) << "Warning: Could not pop GameState. Ignoring." << std::endl;
+            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;
     }
 
     GameState* Game::getState(const std::string& name)
@@ -324,60 +379,39 @@
             startPos = pos;
             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)
+                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)
             {
-                // 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_;
+                do
+                    currentNode = currentNode->parent_.lock();
+                while (newLevel <= --currentLevel);
             }
-            else if (currentNode)
+            if (newLevel == currentLevel + 1)
             {
-                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;
+                // 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;
         }
     }
 
@@ -385,20 +419,32 @@
 
     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;
     }
 
     /*static*/ bool Game::addGameState(GameState* state)

Modified: branches/core4/src/core/Game.h
===================================================================
--- branches/core4/src/core/Game.h	2009-06-28 11:44:23 UTC (rev 3237)
+++ branches/core4/src/core/Game.h	2009-06-28 11:47:57 UTC (rev 3238)
@@ -117,7 +117,8 @@
         Core*                           core_;
         Clock*                          gameClock_;
 
-        bool                            abort_;
+        bool                            bChangingState_;
+        bool                            bAbort_;
 
         // variables for time statistics
         uint64_t                        statisticsStartTime_;

Modified: branches/core4/src/core/GameState.cc
===================================================================
--- branches/core4/src/core/GameState.cc	2009-06-28 11:44:23 UTC (rev 3237)
+++ branches/core4/src/core/GameState.cc	2009-06-28 11:47:57 UTC (rev 3238)
@@ -44,9 +44,9 @@
     @brief
         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)
     {
         this->activity_.activating   = false;

Modified: branches/core4/src/core/GameState.h
===================================================================
--- branches/core4/src/core/GameState.h	2009-06-28 11:44:23 UTC (rev 3237)
+++ branches/core4/src/core/GameState.h	2009-06-28 11:47:57 UTC (rev 3238)
@@ -76,22 +76,22 @@
         };
 
     public:
-        GameState(const std::string& name, bool countTicktime = true);
+        GameState(const std::string& name, bool ignoreTicktime = false);
         virtual ~GameState();
 
         const std::string& getName() const { return name_; }
         State getActivity()          const { return this->activity_; }
         GameState* getParent()       const { return this->parent_; }
 
-        bool getCountTickTime()      const { return this->bCountTickTime_; }
+        bool ignoreTickTime()        const { return this->bIgnoreTickTime_; }
 
         void addChild(GameState* state);
         void removeChild(GameState* state);
 
     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:
         void setParent(GameState* state) { this->parent_ = state; }
@@ -102,7 +102,7 @@
 
         const std::string                        name_;
         State                                    activity_;
-        const bool                               bCountTickTime_;
+        const bool                               bIgnoreTickTime_;
         GameState*                               parent_;
         std::map<std::string, GameState*>        children_;
     };

Modified: branches/core4/src/orxonox/Main.cc
===================================================================
--- branches/core4/src/orxonox/Main.cc	2009-06-28 11:44:23 UTC (rev 3237)
+++ branches/core4/src/orxonox/Main.cc	2009-06-28 11:47:57 UTC (rev 3238)
@@ -67,6 +67,7 @@
         " ioConsole"
         );
 
+        orxonox.requestState("root");
         orxonox.run();
 
         // destroy the GameStates created pre-mainly

Modified: branches/core4/src/orxonox/gamestates/GSGraphics.cc
===================================================================
--- branches/core4/src/orxonox/gamestates/GSGraphics.cc	2009-06-28 11:44:23 UTC (rev 3237)
+++ branches/core4/src/orxonox/gamestates/GSGraphics.cc	2009-06-28 11:47:57 UTC (rev 3238)
@@ -56,7 +56,7 @@
 
 namespace orxonox
 {
-    AddGameState(GSGraphics, "graphics", false);
+    AddGameState(GSGraphics, "graphics", true);
 
     GSGraphics::GSGraphics(const std::string& name, bool countTickTime)
         : GameState(name, countTickTime)
@@ -197,7 +197,7 @@
     */
     void GSGraphics::toggleGUI()
     {
-            GUIManager::getInstance().executeCode("toggleGUI()");
+        GUIManager::getInstance().executeCode("toggleGUI()");
     }
 
     /**

Modified: branches/core4/src/orxonox/gamestates/GSRoot.cc
===================================================================
--- branches/core4/src/orxonox/gamestates/GSRoot.cc	2009-06-28 11:44:23 UTC (rev 3237)
+++ branches/core4/src/orxonox/gamestates/GSRoot.cc	2009-06-28 11:47:57 UTC (rev 3238)
@@ -39,7 +39,7 @@
 
 namespace orxonox
 {
-    AddGameState(GSRoot, "root", false);
+    AddGameState(GSRoot, "root", true);
     SetCommandLineSwitch(console);
     // Shortcuts for easy direct loading
     SetCommandLineSwitch(server);

Modified: branches/core4/src/util/Exception.cc
===================================================================
--- branches/core4/src/util/Exception.cc	2009-06-28 11:44:23 UTC (rev 3237)
+++ branches/core4/src/util/Exception.cc	2009-06-28 11:47:57 UTC (rev 3238)
@@ -63,7 +63,7 @@
         {
             std::ostringstream fullDesc;
 
-            fullDesc << this->getTypeName() << "_EXCEPTION";
+            fullDesc << this->getTypeName() << "Exception";
 
             if (this->filename_ != "")
             {
@@ -86,4 +86,10 @@
 
         return fullDescription_;
     }
+
+    //! Returns the error description
+    const char* Exception::what() const throw()
+    {
+        return getDescription().c_str();
+    }
 }

Modified: branches/core4/src/util/Exception.h
===================================================================
--- branches/core4/src/util/Exception.h	2009-06-28 11:44:23 UTC (rev 3237)
+++ branches/core4/src/util/Exception.h	2009-06-28 11:47:57 UTC (rev 3238)
@@ -67,6 +67,7 @@
 
         //! Needed for compatibility with std::exception
         virtual ~Exception() throw() { }
+        const char* what() const throw();
 
         //! Returns a full description with type, line, file and function
         virtual const std::string& getFullDescription() const;
@@ -81,9 +82,6 @@
         //! 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:
         std::string description_;             //!< User typed text about why the exception occurred
         unsigned int lineNumber_;             //!< Line on which the exception occurred
@@ -151,7 +149,7 @@
 @param description
     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__))
 
 } /* namespace orxonox */




More information about the Orxonox-commit mailing list