SFML community forums
Help => Graphics => Topic started by: stevend on January 19, 2010, 07:51:13 pm
-
ok so i am basically using the example code from the tutorials.
i have made a little resource manager and all i do is load
and display a sprite.
when i press esc or the X my program crashes instead of gracefully exiting.....
when i try and debug i get this
Windows has triggered a breakpoint in 2dtile.exe.
This may be due to a corruption of the heap, which indicates a bug in 2dtile.exe or any of the DLLs it has loaded.
This may also be due to the user pressing F12 while 2dtile.exe has focus.
The output window may have more diagnostic information.
any one have any ideas what could be happening? ps: there is no more diagnostics information :roll:
-
ok so if i dont load and display the sprite it doesnt crash on exit.
should i be disposing of the loaded sf::Image before i exit possibly?
-
Can you show the code that crashes?
-
i was right, i fixed it by putting this in my resource manager deconstructor:
for (int i = 0; i < (int) _images.size(); i++)
{
delete _images.at(i);
}
and calling this before i call app->close();
this->ResourceManager.~resman();
sorry for worrying so much. this is where everything got sticky a long time ago when i tried to use sfml and now it seems im getting the hang of it faster. thanks anyways!
-
Deleting your images is not supposed to fix this crash, you just fixed a memory leak.
You can't explicitely call a destructor, your second piece of code is wrong.
You should really show us your code so that we can find the real problem, and a clean fix :)
-
deconstructor:
You mean destructor.
this->ResourceManager.~resman();
As Laurent said, don't do that. Although it's possible (and even meaningful in some cases, as counterpart to placement new), it will probably lead to undefined behaviour (UB) at you because your object is destroyed twice.
Defeating UB by creating even more UB is the worst you can do.
-
this is my resource manager.
#pragma once
#include "include.h"
namespace engine
{
class resman
{
public:
resman ()
{
};
~resman (void)
{
for (int i = 0; i < (int) _images.size(); i++)
{
delete _images.at(i);
}
};
//! Load or retreive an image from memory
sf::Image * GetImage(string filepath);
private:
//! Array of images loaded into memory
std::vector <sf::Image *> _images;
//! Array of path names of images loaded into memory
std::vector <std::string> images;
};
};
#include "include.h"
#include "factory.h"
class factory;
namespace engine
{
sf::Image * resman::GetImage(string filepath)
{
for (int i = 0; i < (int)images.size(); i++)
{
if (images.at(i).compare(filepath)==0)
return _images.at(i);
}
// Not Found?
sf::Image *ImgTemp = new sf::Image();
if(ImgTemp->LoadFromFile(filepath))
{
_images.push_back(ImgTemp);
images.push_back(filepath);
return ImgTemp;
}
else
{
delete ImgTemp;
}
return NULL;
}
}
i put the code in the destructor so i might as well move it to a function and call it then from what you say.
i didnt see the reason why it would matter if i destroyed the resource manager class right before i call app.close();
// Close window : exit
if (Event.Type == sf::Event::Closed)
{
this->ResourceManager.~resman();
window->Close();
}
thats where i would call it[/code]
-
i didnt see the reason why it would matter if i destroyed the resource manager class right before i call app.close();
For the third time: Calling destructors explicitly results in undefined program behaviour. As mentioned, there are exceptions, but you needn't care about them at the moment. Just don't do it.
Destructors are called automatically if you have got an object with automatic storage class ("stack") – we're not in GC languages with probabilistic destruction behaviour. Otherwise, if you allocate your class dynamically (new operator), you have to free the memory by delete. In this case, the delete operator calls the destructor.
-
yeah, thats why i said i didnt see
i didnt see the reason why it would matter
So i fixed a memory leak, and i also fixed my program crashing on exit. dont ask me why or how but me putting the contents or resman's destructor into a separate and calling it where i would call the destructor has fixed my problem. all my code is above. if you see anything im doing wrong now, please, let me know.
-
Is your resman instance global?
-
resman is declared like
resman ResourceManager;
and is located in a singleton factory class
-
Can you show this singleton factory class?
-
if you see anything im doing wrong now, please, let me know.
Just don't call the destructor by hand. Why don't you believe me? Do you know what "undefined" means? It means everything can happen. Don't draw any conclusion just because your program seems to run correctly. You fixed NOTHING, you did the opposite.
Ok. Your resman instance is a member of a class, right?
resman ResourceManager;
Therefore, the destructor is automatically called in the surrounding class's destructor. Manually calling ~resman() leads to a double destruction, and the mentioned UB. Clear?
-
class factory
{
protected:
//! Pointer to the factory
static factory *dinstance;
factory ()
{
};
~factory (void) { };
public:
//! Creates or returns the factory instance
static factory *instance();
//! Initialize the game engine
void init (int width, int height, bool fullscr);
private:
//! SFML Rendering Device
sf::RenderWindow * window;
//! SFML events Device
sf::Event Event;
//! Main Game Loop
void GameLoop ();
//! Main Event Loop
void EventLoop ();
//! Resource Manager Instance
resman ResourceManager;
//! State Manager Instance
stateman StateManager;
};
-
and im not calling it any more. as my previous posts have stated i have changed my code and no longer call the destructor. i do beleive you. lol
-
How are the functions instance() and init() implemented?
Concerning the destructor: Sorry, I got you wrong (I read too vaguely). But do you understand why the code in another function doesn't lead to an error anymore?
-
yes i do now :P it was just really early and i havent touched c++ for a few months :lol:
//! Creates or returns the factory instance
factory * factory::instance()
{
if (!dinstance)
dinstance = new factory;
return dinstance;
}
//! Creates the SFML window and passes off execution
void factory::init (int width, int height, bool fullscr)
{
window = new sf::RenderWindow(sf::VideoMode(width, height, 32), "2dtile");
window->UseVerticalSync(true);
window->SetFramerateLimit(60);
this->StateManager.Init();
this->GameLoop();
}
do you like my engine framework :P ? im designing a 2d mmorpg. oh noes! ;)
-
You never delete your factory instance?
-
oh right i suppose i should do that too.
i added a unload function to my factory class and i call it in main after the game has finished running and the window has closed.
-
Why don't you use the meyers singleton? Very easy, the accessor function creates the instance at first call and destroys it at program shutdown.
factory& factory::instance()
{
static factory inst;
return inst;
}
Besides, I wouldn't allocate window dynamically. And what if the user forgets to call init()?
-
With this kind of singleton you can't control the destruction of the instance, that happens at global exit. This can lead to problems, especially if the singleton contains members that need SFML to be "alive" in order to destroy themselves properly (sf::Image, sf::Font, ...).
-
You are right, I didn't think of the dependencies. In this case, a manual call to a cleanup function would be the better alternative. However, one should ensure that the singleton isn't used after destruction (at least by assert).
Or, if this is anyhow possible (and it often is), no singleton at all. As we all know, global variables are evil. :)
-
In this case, a manual call to a cleanup function would be the better alternative
This means using dynamic allocation for these members, in order to be able to destroy them in the function. I think I'd prefer doing so for the whole singleton rather than for its members ;)
-
Actually, I meant something similar, a counterpart to the functionality of creating the instance. ;)
#ifdef SFML_DEBUG
#define DEBUG_ONLY(EXPR) EXPR
#else
#define DEBUG_ONLY(EXPR)
#endif
class Factory
{
private:
static Factory* InstancePtr;
DEBUG_ONLY(static bool Destroyed;)
public:
static Factory& Instance()
{
assert(!Destroyed);
if (InstancePtr == NULL)
InstancePtr = new Instance();
return *InstancePtr;
}
static void Cleanup()
{
delete InstancePtr;
DEBUG_ONLY( Destroyed = true; )
}
};
Factory* Factory::InstancePtr = NULL;
DEBUG_ONLY(bool Factory::Destroyed = false;)
A check of invalid calls is only performed in debug-mode. There are a lot of tactics, for example the phoenix singleton which recreates the instance if accessed after destruction. Some of them are explained in Modern C++ Design (Andrei Alexandrescu).
Here, I don't think an exception would make much sense because the user can't really react to it. Accessing the singleton after its destruction is a logic error in my opinion, that's why I chose the assertion.