Sign in to follow this  

Rate my gamestate system

This topic is 4303 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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.

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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!

Share this post


Link to post
Share on other sites
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<cGameState*>, (or std::stack< boost::shared_ptr<cGameState> >), 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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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 view
class 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 class
class 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 loop
void 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,

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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,

Share this post


Link to post
Share on other sites

This topic is 4303 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this