Skip to main content
GameDev.net gamedev.net
🔒 Locked

Rate my gamestate system

Started by walle Mar 1, 2006 at 7:22 PM 9 replies 1.8k views
Original Post
walle
walle
Hello. Finnished with my first non "big switch statement in main.cpp" gamestate manager. Would like some comments. The main idea is that the main part should be pGamestateManager->run(); and pGamestateManager->process_events( event );, the active state runs. So this is a basic game state: 
[source lang = cpp]
#ifndef GAMESTATE_H
#define GAMESTATE_H

#include "globals.h"

class cGamestate
{
    public:
            cGamestate() { }
            ~cGamestate() { }

            virtual void run() { /* Overloaded =) */  }
            virtual void process_events( SDL_Event event) {  /* Overloaded =) */  }
};

#endif
This is a pure base class, doesn't even have a own .cpp file. And no data, The gamestates that inherit this class adds their own data. An example: 
[source lang = cpp]
#ifndef PLAYSTATE_H
#define PLAYSTATE_H

#include "globals.h"

class cPlaystate : public cGamestate
{
    public:
            cPlaystate();
            ~cPlaystate();

            void run();
            void process_events( SDL_Event event);
            void manage_enemies();
            void manage_player();
            void draw_hud();
    private:
            cPlayer player;
            vector<cEnemy> enemies;

            bool paused;
            bool gameover;
            Uint8* keys;
};

#endif
The process_events function looks like this: 
[source lang = cpp]
void cPlaystate::process_events( SDL_Event event)
{
    while( SDL_PollEvent(&event) )
    {
        switch(event.type)
        {
            case SDL_QUIT:
                done = true;
                break;
            case SDL_KEYDOWN:
                switch(event.key.keysym.sym)
                {
                    case SDLK_ESCAPE:
                        done = true;
                        break;
                    case SDLK_F1:
                        Toggle_Fullscreen();
                        break;
                    case SDLK_SPACE:
                        player.shoot();
                        break;
                    case SDLK_RETURN:
                        paused = true;
                        break;
                    case SDLK_p:
                        if(paused == true) { paused = false; }
                        else { paused = true; }
                        break;
                }
            default:
                break;
        }
    }

    if( paused == false )
    {

        keys = SDL_GetKeyState(NULL);

        if(keys[SDLK_UP])
        {
            player.move(0, -350 * framerate.dtime());
        }

        if(keys[SDLK_DOWN])
        {
            if(player.sprite.gety() + player.sprite.geth() < screen->h ) { player.move(0, 350 * framerate.dtime()); }
        }

        if(keys[SDLK_LEFT])
        {
            player.move(-350 * framerate.dtime(), 0);
        }

        if(keys[SDLK_RIGHT])
        {
            if(player.sprite.getx() + player.sprite.getw() < screen->w ) { player.move(350 * framerate.dtime(), 0); }
        }
    }

}
And the gamestate manager that runs everything looks like this: 
[source lang = cpp]
#ifndef GAMESTATEMANAGER_H
#define GAMESTATEMANAGER_H

#include "globals.h"

class cGamestatemanager
{
    public:
            cGamestatemanager();
            cGamestatemanager( cGamestate *nstate);
            ~cGamestatemanager();

            void run();
            void process_events( SDL_Event event);
            void set_state( cGamestate *nstate );

    private:
            cGamestate *pGamestate;
};

#endif


[source lang = cpp]
#include "globals.h"

cGamestatemanager::cGamestatemanager()
{
    pGamestate = NULL;
}

cGamestatemanager::cGamestatemanager( cGamestate *nstate )
{
    pGamestate = nstate;
}

cGamestatemanager::~cGamestatemanager()
{
    if( pGamestate )
    {
        delete pGamestate;
        pGamestate = NULL;
    }
}

void cGamestatemanager::run()
{
    pGamestate->run();
}

void cGamestatemanager::process_events( SDL_Event event )
{
    pGamestate->process_events( event );
}

void cGamestatemanager::set_state( cGamestate *nstate )
{
    pGamestate = nstate;
}
Any comments, I know it's not that advanced, and probobly not very smart =) but I want to learn, so point out all the mistakes you have the strenght to find =) Thanks.
dave
dave
The problem with asking peoples opinion on this is not that they can say it is wrong, but more to do with if it gives you or enables you to do the things that you need it to do. Then comes the question of how versatile it is.

So really i can't comment on how good it is, but i can give you an idea of how i do things. A pretty different way. If you go to here you can see the decription of my framework in my first post.

I find that the method i use is very versatile, simple to implement and reasonably nice to expand.

Dave
DeadXorAlive
DeadXorAlive
Hey, good work there (i'm not really qualified to judge but just wanted to compliment anyway). I'm just finishing something similar myself and I think this is a really helpful thing. I've also tried to make several small test programs with it, that helps to spot where the code would need improvement.

I would like to point out two things. First, you could make cGamestate pure virtual (if thats the right term?). Declare it's member function in this way:
virtual void run() = 0;
virtual void process_events( SDL_Event event) = 0;
This will enforce classes that derive from it to override these functions, if they do not the compiler will freak out when you try to instantiate an object.

Another thing is that you may have a memory leak in the manager class if I saw that right.
This is the relevant code:
cGamestatemanager::~cGamestatemanager(){    if( pGamestate )    {        delete pGamestate; // From this line i assume that pGameState will be from the heap        pGamestate = NULL;    }}void cGamestatemanager::set_state( cGamestate *nstate ){    pGamestate = nstate;  // But then here the allocated memory where pGameState previously pointed to will leak}


I would recommend looking into std::auto_ptr and boost::shared_ptr. These smart pointers are easy to use and reduce the likelyhood of memory-leaks by a great deal. The first can't and the second can be used with the standard containers.

Keep in mind this is not coming from an expert or even average programmer, although I hope this may be helpful anyways.
walle
walle
Thanks. Well, it does what I want it to do Dave,so I think I've succeed from that point of view.
Thanks for pointing out that memory leak for me DeadXorAlive.
PiCkLeD
PiCkLeD
If you are interested, theres a fantastic introduction to a state system in the book called "Game AI by Example" - by Matt Buckland.

It really opened my eyes that chapter, he votes against a switch case style method for using a state system (he also explains why this is) and shows you how to implement it in a very clear cut and advanced method including a very cool message system tied in with it.

Good Luck!
DeadXorAlive
DeadXorAlive
I intented to mentioned this too but i forgot, ah well:
You could set your state system up as a stack. That way you could easily spawn an ingame menu for example and go back to the previous gamestate by pushing and popping states from the stack.
In your case it would simply be a matter of changing the cGameState* pointer in the manager class to std::stack, (or std::stack< boost::shared_ptr >), and add appropiate push and pop methods.
Here is a small tutorial which uses a std::vector, it seems to fit well with the way you do it.
walle
walle
Yes, I actually thought about doing that. I read a discussion here on gamedev about using a stack to keep the gamestates in. But I decided that this was a "easier" more straight forward design.
Kylotan
Kylotan
You may as well remove the SDL_Event parameter from your event handling function and make it a local, as it serves no purpose as a parameter.
Emmanuel Deloget
Emmanuel Deloget
Hello,

Before speaking of the design itself, let's see some stuff about the code.

  • if you pass a SDL_Event to your process_event method it often suppose that the event structure is already initialized. It means that your cPlaystate::process_event() don't have to poll the events - this task should be done by the game state manager.
  • there is no real difference between process_event() and run(). You are probably going to call process_event() and run() subsequently. Moreover, process_event() has to modify the game state, just as run(). As a consequence, removing run from your code should be possible.


Now, from a design point of view, your game state system lacks a lot of functionalities.

First, the game state manager is responsible for nothing. It only stores the current game state, but since you need to tell him what is the current game state I can safely assume that the object that will be responsible for holding the manager will also know what is the current game state.

Nevertheless, a game state manager is still needed, so let's see what it should do.
  1. it should be responsible for updating the current game state - it also mean that we must have a way to setup the startup game state.
  2. we should not take care of what is the current game state from outside
  3. since we don't know what is the current game state from the outside, we need to know when we need to exit. 

// I removed all the stuff we don't really care from a design point of viewclass GameStateManager{  GameState *mCurrentGameState;public:  // this method will poll the events and update the game state  void updateOnce();  // this method is called by the owner to check wether we need to quit or not  bool canExit();  // of course, we need to setup the startup game state  void setGameState(GameState *gameState);};

Now, let's do the game state class. What will be the needed actions ?

  1. we must be able to update the game state in response to a particular event
  2. because a particular action might modify the game state (from example, from the normal game to the equipement game state), the game state is also responsible fro telling to the game state manager which is the next game state (either itself or anotehr one)
  3. we can (optionnally) implement some kind of enter() and leave() methods that will be called when we enter or leave a particular game state

// this is an abstract classclass GameState{public:  // update the game state  virtual void updateOnce(const SDL_Event& event) = 0;  // the function is called by the manager to get the next game state  virtual GameState *getNextGameState() = 0;  // optional enter/leave methods  virtual void enter() = 0;  virtual void leave() = 0;};


Now we can implement our game state manager

void GameStateManager::updateOnce(){  // we don't have anything to do if we don't have any game state  if (mCurrentGameState) {    SDL_Event event;    GameState *gameState;    // poll for an event - I'm not sure how SDL_PollEvent works,     // I assume that it returns true if an event has been catched.    if (SDL_PollEvent(event)) {      // update the current game state      mCurrentGameState->updateOnce(event);      // get the next one      gameState = mCurrentGameState->getNextGameState();      // set the new game state      setGameState(gameState);    }  } }bool GameStateManager::canExit(){  // if we don't have a current game state then we can   // assume that we want to quit  return (mCurrentGameState == NULL);}void GameStateManager::setGameState(GameState *gameState){  // if the new game state is not the current one, we'll have  // some additional work to do  if (gameState != mCurrentGameState) {    // leaving the current game state    mCurrentGameState->leave();    mCurrentGameState = gameState;    if (mCurrentGameState) {      // nextState can be NULL; if it is not NULL, we must       // enter the new current game state      mCurrentGameState->enter();    }  }}

Using this game state manager is rather simple:
GameState *Game::createGameStates(){  // create the different game state  GameStateMain *mainGameState;  GameStatePlay *playGameState;  GameStateHiScore *hiscoreGameState;  GameStateNewHiScore *newHiscoreGameState;  GameStateCredits *creditsGameState;  creditsGameState = new GameStateCredits();  hiscoreGameState = new GameStateHiScore();  newHiscoreGameState = new GameStateNewHiScore();  playGameState = new GameStatePlay();  mainGameState = new GameStateMain();  // setup the game state graph  mainGameState->setPlayGameState(playGameState);  mainGameState->setCreditsGameState(creditsGameState);  mainGameState->setHiscoreGameState(hiscoreGameState);    playGameState->setNewHiscoreGameState(newHiscoreGameState);  playGameState->setBackGameState(mainGameState);  creditsGameState->setBackGameState(mainGameState);  hiscoreGameState->setBackGameState(mainGameState);  newHiscoreGameState->setBackGameState(playGameState);  // finally, return our startup game state  return mainGameState;}void Game::init(){  GameState *startupGameState;  // get the startup game state  startupGameState = createGameStates();  // now, set up the game state manager  mGameStateManager.setGameState(startupGameState);}// the main game loopvoid Game::mainLoop(){  do {    mGameStateManager.update();  } while (!mGameStateManager.canEcit())}

Of course, one can even make the Game class generic by creating a GameState abstract factory that will be responsible for creating the main game state (it will do everything in createGameStates())

This post was rather long, but I hope you found it informative [smile]

Regards,
walle
walle
Ok, didn't understand why I could make the SDL_Event local at first, but now I see =) Thanks for pointing that out. Don't think I would thought about that myself.

Emmanuel Deloget: Thanks for that massive post =)
At work now, and I skimmed through your post and I don't really understand it all. But I'll scrutinize it when I get home. It sounds alot more advanced than my simple design. Maybe something for the future evolvation of my code =)

Thanks.
Emmanuel Deloget
Emmanuel Deloget
Quote:
Original post by walle
Ok, didn't understand why I could make the SDL_Event local at first, but now I see =) Thanks for pointing that out. Don't think I would thought about that myself.

Emmanuel Deloget: Thanks for that massive post =)
At work now, and I skimmed through your post and I don't really understand it all. But I'll scrutinize it when I get home. It sounds alot more advanced than my simple design. Maybe something for the future evolvation of my code =)

Thanks.


It is not very advanced :) If you give a look to the class definition (GameState and GameStateManager) you'll see that they have only 3/4 methods.

BTW I also wanted to make the final code snippet clearer, so here is a more concrete (pseudocode) GameState example:

class GameStateHiscore : public GameState{  GameState *mBackState;  GameState *mNextGameState;  // this private method will render the high score table (hence its name)  void renderHighScoreTable();public:  // I decided to implement enter() and leave() this way. In a normal world,  // the high score table will be stored in memory and GameStateHiscore will  // only use it (we don't need to own it - moreover, GameStateNewHiscore will  // also need to be able to read/write it). Anyway, this is just an example :)  void enter()  {    read the high scores from a file and store them somewhere in memory    // + : we need to setup the next game state    mNextGameState = this;  }  void leave()  {    remove the high scores from memory, we don't need them anymore  }  // the updateOnce() method will only wait for a keystroke  void updateOnce(const SDL_Event& event)  {    if (event.type == SDL_KEYDOWN) {      if (event.key.keysym.sym == SDLK_ESCAPE) {        mNextGameState = mBackState;        return;      }    }     // if we don't return, we should update the screen in order    // to show the high score table    renderHighScoreTable();  }  GameState *getNextGameState()  {    return mNextGameState;  } };


I beleive it shows every tiny feature of my initial GameState class (which means: it implements the 4 virtual methods - not to hard [smile])

HTH,

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.