Rewrote my GameState Manager class...

Okay well I talked to another developer about this directly, and he recommended that I not allow the user to specify the ID of the item, but have the class automatically select one for the user. Here is the class. ( I know it is a mess right now, but I am still in the process of cleaning it ).

My main question is if I am doing something the wrong way, or if I missed something in the implementation. Seems kind of messy in some places to me.

Well tell me what you think. I know I am asking a lot, but I just can't seem to see the little problems when I write code. Thank you so much ahead of time

Declaration

Code:

---

//Include for Singleton HGE manager
#include "gHge.h"
#include <map>
#include <list>

#include "GameState.h"

//Zero in the GameStateMgr is reserved for an error
#define ERROR 0

class gsMgr
{
public:
    gsMgr();
    ~gsMgr();

    size_t CreateState(GameState* newState);
    size_t DeleteState(size_t ID);
    bool  SetState(size_t ID);

    bool  InitiateState(size_t State = 0);
    bool  ShutdownState(size_t State = 0);

    //Update and Render functions are restricted to only working on the current valid GameState
    bool  UpdateState(float delta);
    bool  RenderState();

    //Clears all gamestates
    void  FreeAll();

    inline static gsMgr& Ref()
    {
        if (!singleton)
        {
            singleton = new gsMgr();
            if ( hge::Ref().Get() )
                hge::Ref()->System_Log("Initiated singleton GameStateManager.\n");
        }
        return *singleton;
    };

    inline static gsMgr* Ptr()
    {
        if (!singleton)
        {
            singleton = new gsMgr();
            if ( hge::Ref().Get() )
                hge::Ref()->System_Log("Initiated singleton GameStateManager.\n");
        }
        return singleton;
    };
private:
    //The current game state
    size_t curState;

    //The current amount of state
    size_t curKeyCount;

    //Associative container of GameStates
    std::map<size_t, GameState*> GameStateMap;

    //Linked list for holding used Keys
    std::list<size_t> UnusedKeys;

    //Singleton object
    static gsMgr* singleton;

};

---

Now the ridiculously big implementation.

Code:

---

#include "stdafx.h"
#include "GameStateMgr.h"

gsMgr* gsMgr::singleton = 0;

gsMgr::gsMgr():curState(0),curKeyCount(0)
{}

gsMgr::~gsMgr()
{
    FreeAll();
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("Passed: Freed GameStateMgr.\n");
}

size_t gsMgr::CreateState(GameState* State)
{
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("Method: Attempting to create new GameState.");
    if ( State != 0 )
    {
        size_t NewKey = 1;
        if ( UnusedKeys.empty() )
        {
            ++curKeyCount;
            NewKey = (curKeyCount);
            //This should never happen, but just to be safe
            std::map<size_t, GameState*>::iterator find = GameStateMap.find(NewKey);
            if ( find != GameStateMap.end() )
            {
                //Free GameState pointer. Could not use it.
                delete State;
                if ( hge::Ref().Get() )
                    hge::Ref()->System_Log("-->Failed: Could not use requested StateID(&#37;d).\n", NewKey);
                return ERROR;
            }

            //Assign GameState pointer
            GameStateMap[NewKey] = State;
            if ( hge::Ref().Get() )
                hge::Ref()->System_Log("-->Passed: GameState created using StateID(%d).\n", NewKey);
            return NewKey;
        }
        else
        {
            //Get NewKey off unused list
            NewKey = UnusedKeys.back();
            if ( NewKey != 0 )
            {
                UnusedKeys.pop_back();
                GameStateMap[NewKey] = State;
                if ( hge::Ref().Get() )
                    hge::Ref()->System_Log("-->Passed: GameState created using StateID(%d).\n", NewKey);
                return NewKey;
            }
            else
            {
                //Free uneeded GameState pointer
                delete State;
                if ( hge::Ref().Get() )
                    hge::Ref()->System_Log("-->Failed: Invalid StateID(%d) in list.\n", NewKey);
                return ERROR;
            }
        }
    }
    //Pointer passed was not valid
    return ERROR;
}

size_t gsMgr::DeleteState(size_t ID)
{
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("Method: Attempting to delete GameState(%d).", ID);
    std::map<size_t, GameState*>::iterator find = GameStateMap.find(ID);
    if ( find != GameStateMap.end() )
    {
        if ( GameStateMap[ID] != 0 )
        {
            GameStateMap[ID]->Shutdown();
            delete GameStateMap[ID];
            GameStateMap[ID] = 0;
            UnusedKeys.push_front(ID);
            if ( hge::Ref().Get() )
                hge::Ref()->System_Log("-->Passed: GameState(%d) deleted.\n",ID);
            return ID;
        }
        else
        {
            if ( hge::Ref().Get() )
                hge::Ref()->System_Log("-->Failed: Tried to delete <null> GameState(%d).\n",ID);
            return ERROR;
        }
    }
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("-->Failed: GameState(%d) could not be found.\n",ID);
    return ERROR;
}


bool gsMgr::SetState(size_t ID)
{
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("Method: Attempting to change from GameState(%d) to GameState(%d).",curState,ID);
    std::map<size_t, GameState*>::iterator find = GameStateMap.find(ID);
    if ( find != GameStateMap.end() )
    {
        if ( ID == curState )
        {
            if ( hge::Ref().Get() )
                hge::Ref()->System_Log("-->Failed: Current GameState is already GameState(%d).\n",ID);
            return false;
        }
        if ( GameStateMap[ID] == 0 )
        {
            if ( hge::Ref().Get() )
                hge::Ref()->System_Log("-->Failed: Requested GameState(%d) is <null>.\n", ID);
            return false;
        }
        if ( !InitiateState(ID) )
            return false;
        if ( !ShutdownState(0) )
        {
            //Reinitialize the old state
            InitiateState(0);//if this fails the program will probably crash
            return false;
        }
        if ( hge::Ref().Get() )
            hge::Ref()->System_Log("Passed: Changed from GameState(%d) to GameState(%d).\n",curState, ID);
        curState = ID;
        return true;
    }
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("-->Failed: GameState(%d) could not be found.\n",ID);
    return false;
}

bool gsMgr::InitiateState(size_t ID)
{
    if ( ID == 0 )
        ID = curState;
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("Method: Attempting to Initiate GameState(%d).",ID);
    std::map<size_t, GameState*>::iterator find = GameStateMap.find(ID);
    if ( find != GameStateMap.end() )
    {
        if ( GameStateMap[ID] != 0 )
        {
            if ( GameStateMap[ID]->Initiate() )
            {
                if ( hge::Ref().Get() )
                    hge::Ref()->System_Log("-->Passed: Initiated GameState(%d).\n",ID);
                return true;
            }
            else
            {
                if ( hge::Ref().Get() )
                    hge::Ref()->System_Log("-->Failed: Could not Initiate GameState(%d).\n",ID);
                return false;
            }
        }
        if ( hge::Ref().Get() )
            hge::Ref()->System_Log("-->Failed: Requested GameState(%d) is <null>.\n",ID);
        return false;
    }
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("-->Failed: Requested GameState(%d) could not be found.\n",ID);
    return false;
};

bool gsMgr::ShutdownState(size_t ID)
{
    if ( ID == 0 )
        ID = curState;
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("Method: Attempting to Shutdown GameState(%d).",ID);
    std::map<size_t, GameState*>::iterator find = GameStateMap.find(ID);
    if ( find != GameStateMap.end() )
    {
        if ( GameStateMap[ID] != 0 )
        {
            if ( GameStateMap[ID]->Shutdown() )
            {
                if ( hge::Ref().Get() )
                    hge::Ref()->System_Log("-->Passed: Shutdown GameState(%d).\n",ID);
                return true;
            }
            else
            {
                if ( hge::Ref().Get() )
                    hge::Ref()->System_Log("-->Failed: Could not Shutdown GameState(%d).\n",ID);
                return false;
            }
        }
        if ( hge::Ref().Get() )
            hge::Ref()->System_Log("-->Failed: Requested GameState(%d) is <null>.\n",ID);
        return false;
    }
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("-->Failed: Requested GameState(%d) could not be found.\n",ID);
    return false;
};

bool gsMgr::UpdateState(float delta)
{
    //Check for invalid state
    if ( GameStateMap[curState] == 0 )
    {
        return false;
    }
    if ( GameStateMap[curState]->Update(delta) )
    {
        return true;
    }
    return false;
}

bool gsMgr::RenderState()
{
    //Check for invalid state
    if ( GameStateMap[curState] == 0 )
    {
        return false;
    }
    if ( GameStateMap[curState]->Render() )
    {
        return true;
    }
    return false;
}

void gsMgr::FreeAll()
{
    //Safe delete every element
    if ( hge::Ref().Get() )
        hge::Ref()->System_Log("Method: Freeing all GameStates.");
    std::map<size_t, GameState*>::iterator iter;
    for ( iter = GameStateMap.begin(); iter != GameStateMap.end(); ++iter )
    {
        DeleteState((*iter).first);
    }
}

---

Well thank you for any insight.

Quote:

---

Okay well I talked to another developer about this directly, and he recommended that I not allow the user to specify the ID of the item, but have the class automatically select one for the user. Here is the class. ( I know it is a mess right now, but I am still in the process of cleaning it ).

---

I have heard that is better to have the class give out the ID as well. In my code I pass an integer to the function as a reference, and assign the ID to that integer