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

Avoiding forseeable memory problems...

Started by TheWanderer Apr 11, 2005 at 1:54 PM 7 replies 800+ views
Original Post
TheWanderer
TheWanderer
I recently coded up a variation on a game state management policy in which a virtual class is created for game states and all states are derived from it. Here is the code: 

class State{

 public:
  static State *current_state;

  State(){}
  virtual ~State(){}

  virtual void init()=0;     //Load resources
  virtual void cleanup()=0;  //Unload resources

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

  void changeState(State *next_state )
  {
   current_state->cleanup();
   delete current_state;
   current_state = next_state;
   current_state->init();
  }

  virtual void getInput()=0;
  virtual void draw()=0;
  virtual void update()=0;

};

extern State *current_state;
This is the main loop that uses it: 
int main()
{
 State::current_state = new Menu;

 while(game_running)
 {
  State::current_state->getInput();
  State::current_state->update();
  State::current_state->draw();
 }

 printf("Game Finished\n");

 return 0;
}
I've inlcuded the one function which I think could cause trouble (changeState). Note that when it says "delete current_state" it is, in essence, deleting itself. It also followed by two executable lines. I have had not trouble so far, but I might have been lucky. Does the fact that the object delets itself while there are two remaining executable lines render this code prones to errors? When does the current_state object(the one carrying the changeState) actually get deleted?
Drew_Benton
Drew_Benton
The only thing you have to worry about is three cases:
Case 1 - current_state is NULL and since you do not check for it, it will crash from accessin the NULL pointer
Case 2 - current_state is not NULL, but some random uninitialized memory, to which will crash as case 1 would (as what would happen right now in your release mode!)
Case 3 - next_state is NULL and you get your case one again.

So how about this:

State(){ current_state = 0; } // Remember, NULL your ptr's!!void changeState(State *next_state ){   if( !next_state )      return; // or handle that here   // If there is a current state   if( current_state )   {      current_state->cleanup();      delete current_state;   }   // Everything is fine now [smile]   current_state = next_state;   current_state->init();}


Oh and the main assumption is that current_state has been allocated with new somehow not seen in the code, probabally by the derived class or whatnot and next_state does point to new memory and NOT a reference to a State. That is very important.
TheWanderer
TheWanderer
So there is no problem in the fact that "delete current_state" is essentially equivalent to "delete this"?

(And thank you for the suggestions. They have been added to my program :))
Drew_Benton
Drew_Benton
Quote:
Original post by TheWanderer
So there is no problem in the fact that "delete current_state" is essentially equivalent to "delete this"?


Well I think I totally missed that static modifer when I read it, so if I am wrong, someone feel free to point that out [wink]. I think that those are two different statements all together, this will point to the class state while current_state is a variable pointing to some class state, but I would have to test it out to make sure. I'll do that real fast [smile].

[edit]
Ok you are right about that the this == current_state. However, since you are giving it new memory, it should work fine. Here is the test program that I did not see anything wrong with in execution.

#include <stdio.h>#include <windows.h>class State{ public:  static State *current_state;  State(){}  virtual ~State(){}  virtual void cleanup() = 0;  virtual void draw() = 0;  void changeState(State *next_state )  {   current_state->cleanup();   delete current_state;   current_state = next_state;   //current_state->init();  }};State *State::current_state;class Menu2 : public State{public:	void cleanup()	{	}	void draw()	{		printf("Menu2 Draw\n");	}	};class Menu : public State{public:	void cleanup()	{	}	void draw()	{		printf("Menu Draw\n");	}};int main(int argc, char* argv[]){	DWORD start = GetTickCount();	State::current_state = new Menu;	State* temp = new Menu2;	bool game_running = 1;	bool done = 0;	while( game_running )	{		State::current_state->draw();		if( !done && GetTickCount() - start >= 3000 )		{			State::current_state->changeState(temp);			done = 1;			start = GetTickCount();		}		if( GetTickCount() - start >= 3000 )			game_running = 0;	}	delete State::current_state;	printf("Game Finished\n");	return 0;}


So *I think* you should be fine as long as the necessary memory is passed. The last thing though is that you have to make sure to delete the final state so no leaks occur. This is a pretty interesting system, I have to look at it some more to really see if it is adventageous.
TheWanderer
TheWanderer
Well, they way I intended for this to work was for each program to be in charge of managing their own state changes. I saw similar approach somewhere on the web (I'll look it up later), but it needed static variables for each and every state. I tried to centralize it into one variable (current_state).

Now, change_state() would be called because of some input on that state. So when I call change_state() with some parameter, I am having it destroy itself and initialize the next state . I just want to make sure that I do not incur some uncertainty from the fact thatthe object just deleted itself, i.e. the memory in that area gets wiped before I am able to initialize the next state.

I could probably centralize all of this into a State Manager, but I wanted to give this method a go since it is fairly simple and elengant.
Drew_Benton
Drew_Benton
The last example of where this could go wrong was when a reference was used instead, as mentioned above:

#include <stdio.h>#include <windows.h>class State{ public:  static State *current_state;  State(){}  virtual ~State(){}  virtual void cleanup() = 0;  virtual void draw() = 0;  void changeState(State *next_state )  {   current_state->cleanup();   delete current_state;   current_state = next_state;   //current_state->init();  }};State *State::current_state;class Menu2 : public State{public:	void cleanup()	{	}	void draw()	{		printf("Menu2 Draw\n");	}	};class Menu : public State{public:	void cleanup()	{	}	void draw()	{		printf("Menu Draw\n");	}};class Menu3 : public State{public:	void cleanup()	{	}	void draw()	{		printf("Menu3 Draw\n");	}};int main(int argc, char* argv[]){	DWORD start = GetTickCount();	State::current_state = new Menu;	State* temp = new Menu2;        // Uh oh!	Menu3 m3;	bool game_running = 1;	bool done = 0;	while( game_running )	{		State::current_state->draw();		if( !done && GetTickCount() - start >= 3000 )		{			State::current_state->changeState(&m3);			done = 1;			start = GetTickCount();		}		if( GetTickCount() - start >= 3000 )			game_running = 0;	}        // Even if this was not called, if you did another changeState, it would still happen inevitably [sad]	delete State::current_state; // *BOOM*	printf("Game Finished\n");	return 0;}


So that you have to watch out for as well.
TheWanderer
TheWanderer
Well, if that is all you can see going wrong, then I guess I will refine it a little more. But it seems that it is a good solution for the problem of changing game states.
Deyja
Deyja
Make change_state(...) a static member. All doubt erased.

You will also probably find a state stack to be much more usefull. With a state stack, you would just push the new state (say, the in-game pause menu) onto the stack. When it's done (the player clicks resume) you just pop it off; and, bam. Right back to the game, just how the player left it.
TheWanderer
TheWanderer
Quote:
Original post by Deyja
You will also probably find a state stack to be much more usefull. With a state stack, you would just push the new state (say, the in-game pause menu) onto the stack. When it's done (the player clicks resume) you just pop it off; and, bam. Right back to the game, just how the player left it.


That is exactly where I am headed. It definetly is a good idea for managing state transitions as well as coming back to something you left without having to re-initialize.

As for the changeState() function, I've gone ahead and changed it to this (at least temporarily until I include the Stack). It should avoid the memory problem I was afraid of.

class State{ public:  static State *current_state; //set to NULL (0) in the cpp file  State(){}  virtual ~State(){}   virtual void init()=0;  virtual void cleanup()=0;  virtual void pause()=0;  virtual void resume()=0;  void changeState(State *next_state )  {   current_state->cleanup();   if(next_state   {    current_state = next_state;    current_state->init();   }   delete this;  }  virtual void getInput()=0;  virtual void draw()=0;  virtual void update()=0;};

Topic Locked

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

Sign in to reply to this topic.