• Announcements

    • khawk

      Download the Game Design and Indie Game Marketing Freebook   07/19/17

      GameDev.net and CRC Press have teamed up to bring a free ebook of content curated from top titles published by CRC Press. The freebook, Practices of Game Design & Indie Game Marketing, includes chapters from The Art of Game Design: A Book of Lenses, A Practical Guide to Indie Game Marketing, and An Architectural Approach to Level Design. The GameDev.net FreeBook is relevant to game designers, developers, and those interested in learning more about the challenges in game development. We know game development can be a tough discipline and business, so we picked several chapters from CRC Press titles that we thought would be of interest to you, the GameDev.net audience, in your journey to design, develop, and market your next game. The free ebook is available through CRC Press by clicking here. The Curated Books The Art of Game Design: A Book of Lenses, Second Edition, by Jesse Schell Presents 100+ sets of questions, or different lenses, for viewing a game’s design, encompassing diverse fields such as psychology, architecture, music, film, software engineering, theme park design, mathematics, anthropology, and more. Written by one of the world's top game designers, this book describes the deepest and most fundamental principles of game design, demonstrating how tactics used in board, card, and athletic games also work in video games. It provides practical instruction on creating world-class games that will be played again and again. View it here. A Practical Guide to Indie Game Marketing, by Joel Dreskin Marketing is an essential but too frequently overlooked or minimized component of the release plan for indie games. A Practical Guide to Indie Game Marketing provides you with the tools needed to build visibility and sell your indie games. With special focus on those developers with small budgets and limited staff and resources, this book is packed with tangible recommendations and techniques that you can put to use immediately. As a seasoned professional of the indie game arena, author Joel Dreskin gives you insight into practical, real-world experiences of marketing numerous successful games and also provides stories of the failures. View it here. An Architectural Approach to Level Design This is one of the first books to integrate architectural and spatial design theory with the field of level design. The book presents architectural techniques and theories for level designers to use in their own work. It connects architecture and level design in different ways that address the practical elements of how designers construct space and the experiential elements of how and why humans interact with this space. Throughout the text, readers learn skills for spatial layout, evoking emotion through gamespaces, and creating better levels through architectural theory. View it here. Learn more and download the ebook by clicking here. Did you know? GameDev.net and CRC Press also recently teamed up to bring GDNet+ Members up to a 20% discount on all CRC Press books. Learn more about this and other benefits here.
Sign in to follow this  
Followers 0
parinho7

Game crashes for no obvious reason, help please

11 posts in this topic

I do one thing it runs fine. I try it again, it crashes. what's happening?

I am trying to use for the first time a finite state machine in my game. The whole reason of making the game is to use a FSM. So, I have two states, STATE_MAIN_MENU and STATE_EXIT.

I have my abstract class, two derived classes for the two states and all. I have already done a similar console application and everything works fine, but here when I am getting from main menu to exit the game crashes, but not all the times. It runs fine for 2-3 times (deletes currentState instance of State_Main_Menu and then currentState = new State_Exit()). The thing is that I put a cout at the constructor of State_Exit and it does construct the class. But then when a function of class is about to run it crushes.

I can't explain it any better, I am attaching the code if anyone could help me I'd appreciate it, because I am getting crazy here!

Thanks in advance! Edited by parinho7
0

Share this post


Link to post
Share on other sites
I hope this will help:

I am not familiar with the debugger, I think that the State instance doesn't have a value.
For some reason this doesn't work well

void CStateManager::change_state(GameStates *cur_state, int next_state){

if (next_state != STATE_NULL)
delete cur_state;

switch (next_state) {
case STATE_MAIN_MENU: cur_state = new State_Main_Menu(); break;
case STATE_EXIT: cur_state = new State_Exit(); break;
}
}

and it doesn't execute the cur_state = new State_Exit();

BUT, I have a cout at the constructor and I do get the output so the object is constructed. Edited by parinho7
0

Share this post


Link to post
Share on other sites

You have a scope problem. The value of your pointer, cur_state, being an argument, only exists within this function, and will be forgotten once you return from it (while any reference it was pointing to, will not). So, the delete instruction will destroy the reference the pointer is pointing to, and the new instruction will correctly create a new instance and make cur_state point to it.

 

However, as soon as you return from the function, the value of the cur_state pointer (which is, the address of the instance you just allocated) will be lost, and the caller will be left with the old value, which was the address of the instance you just deleted a few lines earlier. This is, in essence, because you passed the pointer by value and not by reference. The solution is to pass the pointer by reference, or pass a pointer to the pointer, or return the new pointer instead.

 

Basically, the thing to understand is that you did not pass cur_state to your change_state function, but only its value. Whatever pointer variable you passed as an argument to this function, will be left unchanged, a copy of it will be made, and discarded after the function returns. The unfortunate result being, that the pointer to "new State_Exit()" never made it beyond the "return" statement. As soon as the function returned, the copy holding the new, valid pointer was discarded, and you kept on going with the old value, which now points to a deleted instance.

 

The new State_Exit() object is there, well and alive. You just don't know where it is, and have no reference to it anywhere in your code and no way to access it beyond the function where it was created, because you lost your pointer to it after said function returned.

 

Try making cur_state a reference to a pointer instead, that way you will be affecting the real cur_state variable and not just a copy, and your code should work. You can also use a pointer to a pointer (so that you can now affect the inner pointer outside the function too, as well as the object, which is pointed to by the pointed pointer), but this is more C-ish and more error-prone.

Edited by Bacterius
0

Share this post


Link to post
Share on other sites
You have a scope problem. The value of your pointer, cur_state, being an argument, only exists within this function, and will be forgotten once you return from it (while any reference it was pointing to, will not). So, the delete instruction will destroy the reference the pointer is pointing to, and the new instruction will correctly create a new instance and make cur_state point to it.

However, as soon as you return from the function, the value of the cur_state pointer (which is, the address of the instance you just allocated) will be lost, and the caller will be left with the old value, which was the address of the instance you just deleted a few lines earlier. This is, in essence, because you passed the pointer by value and not by reference. The solution is to pass the pointer by reference, or pass a pointer to the pointer, or return the new pointer instead.

Basically, the thing to understand is that you did not pass cur_state to your change_state function, but only its value. Whatever pointer variable you passed as an argument to this function, will be left unchanged, a copy of it will be made, and discarded after the function returns. The unfortunate result being, that the pointer to "new State_Exit()" never made it beyond the "return" statement. As soon as the function returned, the copy holding the new, valid pointer was discarded, and you kept on going with the old value, which now points to a deleted instance.

The new State_Exit() object is there, well and alive. You just don't know where it is, and have no reference to it anywhere in your code and no way to access it beyond the function where it was created, because you lost your pointer to it after said function returned.

Try making cur_state a reference to a pointer instead, that way you will be affecting the real cur_state variable and not just a copy, and your code should work. You can also use a pointer to a pointer (so that you can now affect the inner pointer outside the function too, as well as the object, which is pointed to by the pointed pointer), but this is more C-ish and more error-prone.

I understand the problem now, thank you.
The call to the function was this:

CStateManager::change_state(this, nextState);

How can I modify it so that "this" is passed by reference? (If it's possible at all) Edited by parinho7
0

Share this post


Link to post
Share on other sites
I understand the problem now, thank you.
The call to the function was this:

CStateManager::change_state(this, nextState);

How can I modify it so that "this" is passed by reference? (If it's possible at all)

 

 

Ah, I see. No, it is illegal to modify "this", at least in C++. This means your design has failed, and you need to rethink it. Why do you need different classes for State_Main_Menu, State_Exit, etc...? Is an enum flag not enough? What are you trying to do, exactly? It looks like you are trying to implement a state machine for your GUI, but this is clearly not the right approach here, as the language is very clearly telling you "don't do this".

Edited by Bacterius
0

Share this post


Link to post
Share on other sites


I understand the problem now, thank you.
The call to the function was this:

CStateManager::change_state(this, nextState);

How can I modify it so that "this" is passed by reference? (If it's possible at all)



Ah, I see. No, it is illegal to modify "this", at least in C++. This means your design has failed, and you need to rethink it. Why do you need different classes for State_Main_Menu, State_Exit, etc...? Is an enum flag not enough? What are you trying to do, exactly? It looks like you are trying to implement a state machine for your GUI, but this is clearly not the right approach here, as the language is very clearly telling you "don't do this".




It's my first try to use a finite state machine in my game (the point of the whole game was to learn how to write a FSM), to navigate through the menus. Apparently I need to find some other way to do this. But as I say to my original post I did almost an exact console application that worked this way.

Here is the code

void CStateManager::change_state(GameStates *cur_state, int next_state){

	if (next_state != STATE_NULL)
		delete cur_state;

	switch (next_state){
		case STATE_MAIN_MENU: cur_state = new State_Main_Menu(); break;
		case STATE_PLAYING: cur_state = new State_Playing(); break;
		case STATE_EXIT: cur_state = new State_Exit(); break;
	}
}

State_Main_Menu::State_Main_Menu(){
	stateID = STATE_MAIN_MENU;
	nextState = STATE_NULL;
}

void State_Main_Menu::Event(){
	cin >> nextState;
}

void State_Main_Menu::Update(){
	CStateManager::change_state(this, nextState);
}

void State_Main_Menu::Render(){
	cout << "MAIN MENU" << endl;
	cout << "Press '3' to play"  << endl;
	cout << "Press '0' to continue in Main Menu"  << endl;
//	system("cls");
}

int main(int argc, char* argv[]){
	
	GameStates *myState = new State_Main_Menu();

	while (myState->Get_nextState() != STATE_EXIT){
		
		myState->Event();
		myState->Update();
		myState->Render();
	}

	return 0;
}
and it worked as intended, never get an exception by that code. All I was doing was somewhat copying that and adding the SDL stuff! Edited by parinho7
0

Share this post


Link to post
Share on other sites

I'm sure I posted this a lot, but: just because something "seems" to be working just fine, doesn't mean it's actually right. C++ is a language where half the standard seems to say "this is undefined behavior, and this, and this, and this". It is absolutely the last language you should be using for trial and error coding, because stuff will work just fine when you "try something" and will blow up in your face two days later. Why? Because somewhere something completed unrelated in your code will affect the memory layout or eventually overwrite the objects memory you have so happily been accessing and executing after you already deleted it.

 

The language itself does little to no error checking for you, the compiler will mostly point out syntax errors and many things won't trigger a warning. There is no hand holding and you are expected to know what you're doing. That's also why most people on this forum will tell beginners to stay away from it and start with another language. Not because it is so hard to make something compile. Not because the syntax is difficult or the standard library is so complex. Because you can write horribly bad and bug ridden code and it will still let you believe that everything is working perfectly fine... until it doesn't.

 

So the usual reply to 99% of all "but it was working before" is "yes, because you have been incredibly lucky".

 

Reasons why it worked before? Maybe you never called it with a different state. Maybe it kept using the deleted state and you never noticed. Maybe you were incredibly lucky that your new state was placed at exactly the location of your previous state. There's lots of reasons why something happened to work in C++ when it never should have.

2

Share this post


Link to post
Share on other sites

[quote name='Trienco' timestamp='1357106333' post='5016582']

Reasons why it worked before? Maybe you never called it with a different state. Maybe it kept using the deleted state and you never noticed. Maybe you were incredibly lucky that your new state was placed at exactly the location of your previous state. There's lots of reasons why something happened to work in C++ when it never should have.

[/quote]

My guess is because this is was simple console application, when he deleted and recreated "this", it ended up being created at the same address it was before, so it really just happened to work by pure chance, since the old and new pointers were the same (so the old value was still valid). Now in his game with SDL thrown in the mix, the heap is more disorganized and this doesn't happen... and boom.

 

I could be completely wrong, of course, but something in this code just doesn't seem right. It's not supposed to work.

0

Share this post


Link to post
Share on other sites
A pattern that I frequently use is to have my state->update() method return a shared_ptr to the state the game should switch to. If update returns an empty pointer, no state change occurs. Something like this (very quick, incomplete and un-tested, but it shows the gist of it)
 
enum EStates
{
	STATE_NULL,
	STATE_MENU,
	STATE_PLAYING,
	STATE_EXIT
};

class GameState
{
	public:
	GameState(){}
	virtual ~GameState(){}

	virtual std::shared_ptr<GameState> update()=0;
	virtual void event()=0;
	virtual void render()=0;

	virtual unsigned int getState()=0;
};


class MainMenuState : public GameState
{
	public:
	MainMenuState() : GameState(), nextState(STATE_NULL){}
	~MainMenuState(){}

	unsigned int getState()
	{
		return STATE_MENU;
	}

	std::shared_ptr<GameState> update()
	{
		switch(nextState)
		{
			case STATE_NULL: return std::shared_ptr<GameState>(); break;  // Return empty ptr
			case STATE_PLAYING: return std::shared_ptr<GameState>(std::make_shared<PlayingState>()); break;
			case STATE_EXIT: return std::shared_ptr<GameState>(std::make_shared<ExitState>()); break;
			default: return std::shared_ptr<GameState>(); break;
		}
	}

	void event()
	{
		cin >> nextState;
	}

	void render()
	{
		cout << "MAIN MENU" << endl;
		cout << "Press '3' to play"  << endl;
		cout << "Press '0' to continue in Main Menu"  << endl;
	}

	private:
	unsigned int nextState;
};

int main()
{
	std::shared_ptr<GameState> state=std::make_shared<MainMenuState>();

	while(state->getState()!=STATE_EXIT)
	{
	    state->render();
	    state->event();
		std::shared_ptr<GameState> newstate=state->update();
		if(newstate)
		{
			state=newstate;
			newstate.reset();
		}
	}
}

Update performs a switch on nextState as in your code, and returns either an empty pointer if no state change should occur, or it creates a new state object (std::make_shared news an object and assigns it to a shared_ptr in one call, safely) and returns a shared_ptr to the new state object. In the main loop, the return result of update is checked, and if it is a non-empty pointer, then state is assigned to the new pointer. This essentially overwrites the old state that was assigned previously, which will then drop out and be automatically and safely deleted.

Any time you find yourself doing what amounts to delete this you are almost certainly making an error, regardless of whether or not it seems to work. That sort of thing is just not wise to do unless you are experienced enough to know what is really going on. Edited by JTippetts
0

Share this post


Link to post
Share on other sites
A pattern that I frequently use is to have my state->update() method return a shared_ptr to the state the game should switch to. If update returns an empty pointer, no state change occurs. Something like this (very quick, incomplete and un-tested, but it shows the gist of it)

enum EStates{	STATE_NULL,	STATE_MENU,	STATE_PLAYING,	STATE_EXIT};class GameState{	public:	GameState(){}	virtual ~GameState(){}	virtual std::shared_ptr update()=0;	virtual void event()=0;	virtual void render()=0;	virtual unsigned int getState()=0;};class MainMenuState : public GameState{	public:	MainMenuState() : GameState(), nextState(STATE_NULL){}	~MainMenuState(){}	unsigned int getState()	{		return STATE_MENU;	}	std::shared_ptr update()	{		switch(nextState)		{			case STATE_NULL: return std::shared_ptr(); break;  // Return empty ptr			case STATE_PLAYING: return std::shared_ptr(std::make_shared()); break;			case STATE_EXIT: return std::shared_ptr(std::make_shared()); break;			default: return std::shared_ptr(); break;		}	}	void event()	{		cin >> nextState;	}	void render()	{		cout << "MAIN MENU" << endl;		cout << "Press '3' to play"  << endl;		cout << "Press '0' to continue in Main Menu"  << endl;	}	private:	unsigned int nextState;};int main(){	std::shared_ptr state=std::make_shared();	while(state->getState()!=STATE_EXIT)	{	    state->render();	    state->event();		std::shared_ptr newstate=state->update();		if(newstate)		{			state=newstate;			newstate.reset();		}	}}
Update performs a switch on nextState as in your code, and returns either an empty pointer if no state change should occur, or it creates a new state object (std::make_shared news an object and assigns it to a shared_ptr in one call, safely) and returns a shared_ptr to the new state object. In the main loop, the return result of update is checked, and if it is a non-empty pointer, then state is assigned to the new pointer. This essentially overwrites the old state that was assigned previously, which will then drop out and be automatically and safely deleted.

Any time you find yourself doing what amounts to delete this you are almost certainly making an error, regardless of whether or not it seems to work. That sort of thing is just not wise to do unless you are experienced enough to know what is really going on.




It works perfectly, thank you.

Thanks everyone for the replies, you were all very helpful. May I ask one last thing? Recommendations on book and links to study about FSM for games and also some advanced matters of C++. For example, I now know what this line does MainMenuState() : GameState() but I don't know what the syntax actually mean. Where can I find that? I was also unaware of the existence of "shared_ptr" where can I study about stuff I don't know they exist?!.

Thanks again!! Edited by parinho7
0

Share this post


Link to post
Share on other sites
It calls the default constructor of the base class GameState. It's not actually necessary to do that since if you don't call the base class constructor it calls the default constructor (if there is one) anyway.
0

Share this post


Link to post
Share on other sites

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  
Followers 0