Jump to content
  • Advertisement
Sign in to follow this  
c4c0d3m0n

Singleton + Inheritance = ???

This topic is 3650 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

I'm getting my feet wet in the areas of Inheritance and Singletons, all at the same time. I thought Singletons wouldn't be so hard. My code won't compile, the linker is spitting this at me:
rob@fruchtsirup:~/dev/demo$ make all
g++ -c main.cpp
g++ -c game-engine.cpp
g++ -c first-state.cpp

g++ -g -o demo main.o game-engine.o first-state.o `sdl-config --cflags --libs`
main.o: In function `CFirstState::instance()':
main.cpp:(.text._ZN11CFirstState8instanceEv[CFirstState::instance()]+0x4): undefined reference to `CFirstState::m_FirstState'
collect2: ld returned 1 exit status
make: *** [all] Error 1
These are my sources:
/**
  * @author         Rob Spoel
  * @copyright      2008
  *
  * @file           main.cpp
  *
  *
**/


#include <cstdlib>
#include <iostream>

#include "game-engine.h"

#include "first-state.h"



int
main( int argc, char* argv [] )
{
    std::cout << "main() : Demo" << std::endl;

    CGameEngine demo;

    demo.changeState( CFirstState::instance() );


    while ( demo.running() ) {
        demo.handleEvents();
        demo.update();
        demo.draw();
    }


    return 0;
}



/// EOF

/**
  * @author         Rob Spoel
  * @copyright      2008
  *
  * @file           game-state.h
  *
  *
**/


#ifndef GAME_STATE_H
#define GAME_STATE_H

#include "game-engine.h"



class CGameState
{
  public:

    virtual void init()    = 0;
    virtual void destroy() = 0;


    virtual void pause()  = 0;
    virtual void resume() = 0;


    virtual void handleEvents( CGameEngine* game ) = 0;
    virtual void update( CGameEngine* game )       = 0;
    virtual void draw( CGameEngine* game )         = 0;


    void changeState( CGameEngine* game, CGameState* state ) {
        game->changeState( state );
    }


  protected:

    CGameState() { }

};



#endif // GAME_STATE_H

/**
  * @author         Rob Spoel
  * @copyright      2008
  *
  * @file           first-state.h
  *
  *
**/


#ifndef FIRST_STATE_H
#define FIRST_STATE_H

#include "game-state.h"



class CFirstState : public CGameState
{
  public:

    void init();
    void destroy();


    void pause();
    void resume();


    void handleEvents( CGameEngine* game );
    void update( CGameEngine* game );
    void draw( CGameEngine* game );


    static CFirstState* instance() { return &m_FirstState; }


  protected:

    CFirstState() { }


  private:

    static CFirstState m_FirstState;

    // Implementation

};



#endif // FIRST_STATE_H

/**
  * @author         Rob Spoel
  * @copyright      2008
  *
  * @file           first-state.cpp
  *
  *
**/


#include "first-state.h"

#include <iostream>



void
CFirstState::
init()
{
    std::cout << "FirstState::init()" << std::endl;
}


void
CFirstState::
destroy()
{
    std::cout << "FirstState::destroy()" << std::endl;
}




void
CFirstState::
pause()
{
    std::cout << "FirstState::pause()" << std::endl;
}


void
CFirstState::
resume()
{
    std::cout << "FirstState::resume()" << std::endl;
}




void
CFirstState::
handleEvents( CGameEngine* game )
{
    // TODO
}


void
CFirstState::
update( CGameEngine* game )
{
    // TODO
}


void
CFirstState::
draw( CGameEngine* game )
{
    // TODO
}



/// EOF

What's wrong here?

Share this post


Link to post
Share on other sites
Advertisement
Quote:
Original post by c4c0d3m0n
What's wrong here?


Above all else, the fact that you're using a singleton. More.

To fix your error, put this in first-state.cpp:

CFirstState CFirstState::m_FirstState;

Share this post


Link to post
Share on other sites
You don't have a Singleton, you have a global. You need to define this global. In exactly one source file (e.g. first-state.cpp) write this:


CFirstState CFirstState::m_FirstState;




The Singleton is often a "pattern without a cause". This would appear to be the case for you. Unless you can come up with a good reason as to why something should be a Singleton - don't make any. A Singleton is a type of global, and globals in general are usually indicative of poor (or lack of) design.

Share this post


Link to post
Share on other sites
I was following this article, and it says that we would only want one of each specific gamestate. I gve it a thought, and I find it to be a valid argument. I'm creating a little demo app that will show some different things I programmed. It would be poor design imho if I end up having more than one of each state.

Share this post


Link to post
Share on other sites
Quote:
Original post by c4c0d3m0n
I was following this article, and it says that we would only want one of each specific gamestate. I gve it a thought, and I find it to be a valid argument.


The article I linked addresses this argument. I really recommend that you read it.

Short answer - If you only need one state, create only one state.

Share this post


Link to post
Share on other sites
Quote:

It would be poor design imho if I end up having more than one of each state.

Not really, and it's actually poorer design to lock yourself into that assumption so early and so needlessly.

For example, it can actually come up quite frequently that you will want your gamestate objects to maintain some other kind of stateful information, so you essentially end up in a situation where you have a "MainMenuState" with property A set to Foo, and another instance of that MainMenuState with property A set to Bar. You can't do that if MainMenuState is a singleton, you have to hack around it by maintaining the state in external objects, which can engender pathological coupling between the external object and the state object.

Furthermore you can quickly end up with a class that feels like an object but is actually a collection of procedural methods (stateless, operate on data), in which case your design is skewing off further because it is both poor OO and poor procedural code, a half-baked bastard child of both paradigms.

Your objection falls under the second and third sections in the Performant Singleton article. Learn to discipline yourself (and create only one), don't learn to cripple yourself (and force yourself to have only one).

Share this post


Link to post
Share on other sites
Requirements change. *Want* is not sufficient. One part of the Singleton definition is "there can only be one" - with the unsaid rule that chaos and death will occur if there are more than one. Another part of the Singleton definition is "and you require global access to this instance". This is not required in your example.

It will probably be far simpler to write your program such that you only make one. If you are *really* paranoid, it is relatively easy to catch the case where you create more than one (for a single threaded application):

SomeClass::SomeClass()
{
static bool thereShouldOnlyBeOne = true;
assert(thereShouldOnlyBeOne);
thereShouldOnlyBeOne = false;
}





Now your program will assert if you accidentally code it such that more unwanted instances are created. But the benefits are:

* The "there should only be one" rule doesn't leak into the classes external interface.

* Its a single point of change should you need to relax the rule

More reading

Share this post


Link to post
Share on other sites
Quote:
Original post by c4c0d3m0n
I was following this article, and it says that we would only want one of each specific gamestate. I gve it a thought, and I find it to be a valid argument.

It might be a valid requirement, but that it's not a valid reason to use a singleton, in fact it is only 50% of the reason to use a singleton: When you need one instance that is globally accessible.
If both requirements were needed then a singleton would be the ideal choice, however in practice you never need both of those together, hence the singleton is really an anti-pattern.

Quote:
It would be poor design imho if I end up having more than one of each state.

Two things spring to mind: Firstly, if you don't need more than one, then why would you make more than one? Secondly, you might, in the future, need more than one, for example in a split screen game, if each screen is allowed to switch game states independantly then it's entirely possible that both screens happen to be in the same game state at once.

Share this post


Link to post
Share on other sites
Hmm, that is a very good article. I now see why I shouldn't use Singletons. I have an additional question though.

My CGameEngine class has a std::vector with pointers to CGameStates. Seeing I'm now going to rewrite my game state to have proper constructors/destructors instead of init() and destroy(), how would I make sure I clean up the mess? Ie, how do I destroy objects that I remove from the vector?


/**
* @author Rob Spoel
* @copyright 2008
*
* @file game-engine.h
*
*
**/



#ifndef GAME_ENGINE_H
#define GAME_ENGINE_H

#include <vector>


class CGameState;



class CGameEngine
{
public:

CGameEngine();
~CGameEngine();


void changeState( CGameState* state );
void pushState( CGameState* state );
void popState();


void handleEvents();
void update();
void draw();


bool running() const { return m_running; }
void quit() { m_running = false; }


private:

std::vector<CGameState*> state_stack;

bool m_running;

};



#endif // GAME_ENGINE_H


/**
* @author Rob Spoel
* @copyright 2008
*
* @file game-engine.cpp
*
*
**/



#include "game-engine.h"
#include "game-state.h"

#include <iostream>
#include <stdexcept>
#include <string>



/// Constructor

CGameEngine::
CGameEngine()
: m_running ( true )
{
std::cout << "GameEngine init" << std::endl;
}


CGameEngine::
~CGameEngine()
{
// Clean up states
while ( !state_stack.empty() ) {
state_stack.back()->destroy();
state_stack.pop_back();
}

std::cout << "GameEngine destroyed..." << std::endl;
}




/// States

void
CGameEngine::
changeState( CGameState* state )
{
// Clean up states
while ( !state_stack.empty() ) {
state_stack.back()->destroy();
state_stack.pop_back();
}

// Start new state
state_stack.push_back( state );
state_stack.back()->init();
}


void
CGameEngine::
pushState( CGameState* state )
{
// Pause last state
if ( !state_stack.empty() )
state_stack.back()->pause();

// Store & start new state
state_stack.push_back( state );
state_stack.back()->init();
}


void
CGameEngine::
popState()
{
// Clean up current state
if ( !state_stack.empty() ) {
state_stack.back()->destroy();
state_stack.pop_back();
}

// Resume previous state
if ( !state_stack.empty() )
state_stack.back()->resume();
}




/// Logistics

void
CGameEngine::
handleEvents()
{
if ( !state_stack.empty() )
state_stack.back()->handleEvents( this );

else
throw std::runtime_error
( "CGameEngine::handleEvents() : state_stack is empty" );

}


void
CGameEngine::
update()
{
if ( !state_stack.empty() )
state_stack.back()->update( this );

else
throw std::runtime_error
( "CGameEngine::update() : state_stack is empty" );

}


void
CGameEngine::
draw()
{
if ( !state_stack.empty() )
state_stack.back()->draw( this );

else
throw std::runtime_error
( "CGameEngine::draw() : state_stack is empty" );

}



/// EOF

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!