Segmentation fault using event receiver

[lenx]

Hi everyone.
First i'm truly sorry for the overhead here, but it looks like my only way out.
I'm trying to code tetris as a first project to learn better C++ and of course irrlicht Now i've come to the point where i want to write code to move my blocks left and right. When i use the eventreceiver, i get a segmentation fault, even when only moving the cursor inside the game window, and i really have no clue why! I already commented all my functional game code so only the structure of the program is left, but still it crashes.

One this is for sure, it's happening when MyEventreciever is called (for example when moving the mouse inside the game window). I'm running it on Ubuntu linux btw, but i don't think it has anything to do with that (i ran other irrlicht event receiver programs just fine)

What is the problem here?? Here's my code in the shortest way possible. (It does compile).

main
-------

Code: Select all

#include "playfieldMgr.h"

int main()
{
	PlayfieldMgr playfieldMgr;

	while(playfieldMgr.getDevice()->run())
	{
		if (playfieldMgr.getDevice()->isWindowActive())
		{
			playfieldMgr.update();
		}
	}
	playfieldMgr.~PlayfieldMgr();
}

playfieldmgr.h:
---------------------

Code: Select all

#ifndef PLAYFIELDMGR_H_INCLUDED
#define PLAYFIELDMGR_H_INCLUDED

#include <irrlicht.h>
#include "eventreceiver.h"

class PlayfieldMgr{
private:
	irr::IrrlichtDevice *device;
	irr::video::IVideoDriver* driver;
	irr::scene::ISceneManager* smgr;

public:
    PlayfieldMgr();
    ~PlayfieldMgr();
    void update();
    IrrlichtDevice* getDevice();
};
#endif

playfieldmgr.cpp
------------------------

Code: Select all

#include "playfieldMgr.h"

using namespace irr;
using namespace core;
using namespace scene;
using namespace video;
using namespace io;

PlayfieldMgr::PlayfieldMgr()
{
	device = createDevice( video::EDT_SOFTWARE, dimension2d<s32>(640, 480), 16, false, false, false);

	MyEventReceiver receiver;
	device->setEventReceiver(&receiver);

	driver = device->getVideoDriver();
	smgr = device->getSceneManager();

	smgr->addCameraSceneNode(0, core::vector3df(50,100,-200), core::vector3df(50,100,0));
}

PlayfieldMgr::~PlayfieldMgr()
{
    device->drop();
}

void PlayfieldMgr::update()
{
	driver->beginScene(true, true, SColor(100,100,100,100));
	smgr->drawAll();
	driver->endScene();
}

IrrlichtDevice* PlayfieldMgr::getDevice()
{
	return device;
}

eventreceiver.h
----------------------

Code: Select all

#ifndef EVENTRECEIVER_H_INCLUDED
#define EVENTRECEIVER_H_INCLUDED

#include <irrlicht.h>

using namespace irr;

class MyEventReceiver : public IEventReceiver
{
public:
    virtual bool OnEvent(const SEvent&);
};

#endif

eventreceiver.cpp
-------------------------

Code: Select all

#include "eventreceiver.h"
#include <iostream>

bool MyEventReceiver::OnEvent(const SEvent& event)
{
    if (event.EventType == irr::EET_KEY_INPUT_EVENT && !event.KeyInput.PressedDown)
    {
        switch(event.KeyInput.Key)
        {
		case KEY_LEFT:
            std::cout << "left\n";
		case KEY_RIGHT:
            std::cout << "right\n";
        default:
            std::cout << "smth else\n";
		}
		return true;
    }
	return false;
}

[CuteAlien]

Ok, a few problems. The cause for the crash is probably that your eventreceiver is only a local variable. This means as soon as the PlayfieldMgr constructor is finished the memory for receiver will no longer be valid.

There are several ways to avoid this. I recommend making receiver a membervariable of your PlayfieldMgr class. Then it will be valid as long as your PlayfieldMgr object lives. Which is probably what you want. You could also make it global, but that's not the OO-way.

Furthermore two smaller things I noticed:
- You should not call destructors for variables on the stack explicitely. So the playfieldMgr.~PlayfieldMgr(); should be removed. Your playfieldMgr object will be destroyed automatically when main ends. Actually you should never call destructors explicitely - for objects on the heap (those which you created with new ) you call delete which will call the destructor.
- Each case should be ended with a "break;". Otherwise it will fall through - which means that for KEY_LEFT you will also execute the code for KEY_RIGHT and for default.

[vitek]

Since CuteAlien already answered, I'm going to nit-pick his comments... :)

Actually you should never call destructors explicitely - for objects on the heap (those which you created with new ) you call delete which will call the destructor.

Not necessarily. You must call destructors explicitly when you use placement new to create an object instance in a block of memory. Like this...

Code: Select all

Thing* thing = (Thing*)malloc (sizeof (Thing)); // just a block of memory
new (thing) Thing; // initialize

// destroy
thing->~Thing();
free (thing);

Each case should be ended with a "break;"

In this example, yes. More generally a case can fall-through to the next, or a return can be used to exit the function.

Travis

[CuteAlien]

Since CuteAlien already answered, I'm going to nit-pick his comments... :)

Don't confuse the man - this is his first project :-)

Lets just say - what I said is correct for your situation. But everything in programming has exceptions... and there are even exceptions to that ;-)

[rogerborg]

Irrlicht doesn't use exceptions.

[CuteAlien]

aaaargh

[lenx]

There are several ways to avoid this. I recommend making receiver a membervariable of your PlayfieldMgr class. Then it will be valid as long as your PlayfieldMgr object lives. Which is probably what you want. You could also make it global, but that's not the OO-way.

Aha! i think i see it now. "receiver" should be declared in eventreceiver.h! Now that's just stupid of me, that's what you get when copy-pasting code from examples Unbelievable how you guys can see those issues in an instant. I'll give it a shot this evening. Thanks for the other comments too btw.

Maybe one more question that is on my mind. As you can see i'm declaring my objects not as pointers:

Code: Select all

PlayfieldMgr playfieldMgr;
playfieldMgr.update();

But i noticed in irrlicht every object is declared as a poiner. Should i also do that with my objects? Then it would just look like this, right?

Code: Select all

PlayfieldMgr *playfieldMgr;
playfieldMgr->update();

And what is the advantage of doing it this way? Is it speed?

[JP]

Go and learn a bit about pointers and you'll understand better

The pointer code for your playfieldmgr you posted there will just crash because your pointer hasn't been initialised.

So you'd have to do: PlayfieldMgr* playfieldMgr = new PlayFieldMgr();

[lenx]

Go and learn a bit about pointers and you'll understand better

The pointer code for your playfieldmgr you posted there will just crash because your pointer hasn't been initialised.

So you'd have to do: PlayfieldMgr* playfieldMgr = new PlayFieldMgr();

Yes sir
I perfectly know what pointers are though. Only it's been a while when i was a student and made some crappy programs, and i wasn't really into all that stuff so the C++ is a bit rusty to say the least.

However now i really set myself the goal to learn game programming WITHOUT giving up (something i did too many times in the last few years ). And you know what? It's working. I already learned so much with one week of programming tetris!