views:

143

answers:

2

Let's say I want to store a vector of LPD3DXSPRITE objects. The line to declare this code would be std::vector<LPD3DXSPRITE> sprites; I should be able to create my sprite with:

LPD3DXSPRITE sprite = NULL;
D3DXCreateSprite(myRenderingDevice, &sprite);

Finally, I should be able to add this to the vector like so:

sprites.push_back(sprite);

At least with my understanding, that should be plausible. However, this compiles but gives runtime errors. Why is this? Am I going about it wrong? How might I fix it?

edit:

This may be helpful as well. The call stack yields for this function that vector<ID3DXSprite *, std::allocator<ID3DXSprite *>>::push_back(ID3DXSprite * const &_Val=0x0036fd38 is what is called. This is not the vector that it was passed.
However, LPD3DXSPRITE is just a typedef for ID3DXSprite *. Could this bring anything to light?

A: 

The code looks ok to me. But you are storing a pointer to ID3DXSprite in the vector. How is this de-allocated? Is it possible the sprite has been de-allocated so the vector ends up holding an invalid pointer and then attempts to reference the pointer?

Jeremy Simon
+4  A: 

After looking through your code, I found the problem. Something to look at when you get any breaks in your application is the "Autos" tab or the Locals tab. Here you'll notice something about the this pointer: it's null!

That means the instance that AddSprite is being called on doesn't exist. This is your SpriteManager, which I see is a singleton. In your main, you don't create an instance of it.

I had to do a couple things to get it working. I included "LudoRenderer/SpriteManager.h" in Main.cpp, and added the CreateInstance call:

SpriteManager::CreateInstance();

The only problem with this was that you had declared your constructor/destructor private, like other singletons, but never defined them, so I did that as well:

SpriteManager::SpriteManager(){}
SpriteManager::~SpriteManager(){}

After those changes, it "worked". That's in quotes because your problem is solved, but there is another error later in the code m_GameManager->SetWagon(m_Wagon);.

Here, m_GameManager is not initialized. I uncommented m_GameManager = GameManager::GetInstance(); on line 43 in LudoEngine.cpp, which put us in the same problem as before, no CreateInstance is ever called. I added the necessary header in main, called the create method. This fixed the problem, and your engine ran (cool demo, too!)

There was a crash on exit, in ErrorLogger::LogError, because ErrorLogger was null. It was being called in LudoMemory's destructor, but I'll leave this one for you. :)

Now, two tips I tihnk that would help. The first is about the issue we're solving. Normally, singletons will create themselves if they aren't already. I'd change your singleton GetInstance to something like this:

static T *GetInstance ( )
{
    if (!m_Singleton) // or == NULL or whatever you prefer
    {
        CreateInstance();
    }

    return m_Singleton; // not sure what the cast was for
}

This will force creation of the singleton if it hasn't been already. Now, if you'd like users to call CreateInstance before trying to GetInstance, you could add some sort of warning:

static T *GetInstance ( )
{
    if (!m_Singleton) // or == NULL or whatever you prefer
    {
        CreateInstance();
        std::cerr << "Warning, late creation of singleton!" << std::endl;

        // or perhaps:
        ErrorLogger::GetInstance()->
                        LogError(L"Warning, late creation of singleton!");
    }

    return m_Singleton;
}

Since that leaves out the important information "which singleton?", you could always try to add typeinfo to it:

#include <typeinfo>

// ...

std::cerr << "Warning, late creation of singleton: "
          << typeid(T).name() << std::endl;

To try to get some type names in there.

And lastly, it's okay to delete 0, your checked delete macro is not needed.

To clarify, you have LUDO_SAFE_DELETE, which checks if it's not null before it calls delete. In C++, deleting null has no effect, so your check isn't needed. All instances of your safe delete could be replaced with just your LUDO_DELETE.

GMan
Amazing amount of detail. Huge thanks for this. Really helpful tips, as well. With respect to your edit, I'm not sure that I understand what you mean by it being okay to delete 0.
4501
"ok to delete 0" means you can call `delete` on a NULL pointer and nohting bad should happen (http://discuss.joelonsoftware.com/default.asp?design.4.194233.15)
bobobobo