Using New and Delete in inherited classes and stored pointers to the new object

Started by
10 comments, last by ASnogarD 11 years, 8 months ago
I am using much of the framework I got from http://www.sdltutorials.com/ , and while its entity class is great for creating new player entities I cant get the thing to make bullet entities... least not without generating exception faults.

The way to make a player entity is straight forward, declare it in the source file , load its assets when entering the in game state and clear the assets when exiting ... no problem.

The bullets however pose a number of issues, I cant declare each bullet beforehand.... they need to be created dynamically so I create one each time the player fires, and store it in the entity vector like the player... and when the bullet collides with a wall it dies, so I delete it and erase the element in the vector it was stored in.

Code to create a bullet :


Entity_Bullet* pBullet = new Entity_Bullet;
pBullet->OnLoad(16,16,1,ENTITY_TYPE_BULLET_PLAYER);


if(Facing == ENT_DIR_RIGHT)
{
pBullet->MoveRight = true;
pBullet->MoveLeft = false;

pBullet->X = X + Width;
pBullet->Y = Y + (Height/2);
}
if(Facing == ENT_DIR_LEFT)
{
pBullet->MoveLeft = true;
pBullet->MoveRight = false;
pBullet->X = X - 20;
pBullet->Y = Y + (Height/2);
}

Entity::EntityList.push_back(pBullet);



The entity list is declared like this : std::vector<Entity*> Entity::EntityList;
The code to remove the bullet is in the ingame state loop, where it goes through the entity vector and calls up each element.


// Move , Animate and calculate actions for ALL entities
for(unsigned int i = 0; i < Entity::EntityList.size(); i++)
{
if(!Entity::EntityList) continue;

Entity::EntityList->OnLoop();

// If the entity is not a player
if(Entity::EntityList->Type != ENTITY_TYPE_PLAYER)
{
// If the Entity is suppose to be destroyed
if(Entity::EntityList->Dead == true)
{
Text::AddText("Dead one here", 10, 400);
Entity::EntityList->OnCleanup();
if(Entity::EntityList->Type == ENTITY_TYPE_BULLET_PLAYER)
{
delete Entity::EntityList;
}

Entity::EntityList.erase(Entity::EntityList.begin() + (i --));
}
}

}


The issue seems to be in the clearing up portion, I think there is a conflict in the vector when a few bullets are made and destroyed... I am thinking it is trying to store a bullet in a vector that was deleted but not cleared ?
If I remove the delete and erase portions of the entity loop it runs without crashing, just leaves bullets all over the place.

I would prefer the actual bullet class to delete the bullet during the OnCleanup() call, but using delete this crashes the game as soon as a bullet is fired, and I cant think of a way to identify the particular bullet beyond a this call.

Hmm, would the bullet object go out of scope if the pointer ( or rather the element the pointer was erased ? ).

Tips , ideas , advice welcome ... thanks.



FYI - I havent just copied the tutorials code , I have modified a number of things in the code except for portions where I like how it works, ie I havent just copy pasted someone elses code and want you to sort out the bugs.
Advertisement
Update:

As a test I disabled :

if(Entity::EntityList->Type == ENTITY_TYPE_BULLET_PLAYER)
{
delete Entity::EntityList;
}
Entity::EntityList.erase(Entity::EntityList.begin() + (i --));

I re-enabled : Entity::EntityList.erase(Entity::EntityList.begin() + (i --)); to clear the element and it clears out the bullets from the game... but I cant tell if it actually removes the bullets from memory as well.... or am I just removing the pointers to the objects from the vector but not getting rid of the actual objects ?

It doesnt crash the game, and the bullets are not left of the screen... just wonder if the bullet objects are still in memory ?
If you created an object with new and didn't call delete on it, then the object will be there. Just getting rid of all pointers to the objects will not get rid of the objects themselves - if you are using normal pointers. If you use smart pointers then they will automatically get rid of objects when the last smart pointer referencing the object goes away. If you possess a relatively up to date C++ compiler you can use std::shared_ptr. If not, look into using boost::shared_ptr.
Thanks for the reply, I will look into smart pointers at some time.

I got the code to delete the objects successfully with delete this , the crash seems to be caused by too many events when I press the fire button too quickly... I guess there is a problem when you pass an event too much.
Do you think passing SDL_Event objects through a number of functions could cause issues ?
Currently an event gets passed from the main loop, to the statemanager, to the ingame state, to the player object... if I press the fire button.

*sigh* coding is such a pain when it goes wrong tongue.png

I give for now... to test if it was me mashing the fire button to get lots of bullets out.... I disabled the actual bullet create portion of the code...so each time I fired it merely reported to my ingame console bullet fired... and didnt crash at all.
So the issue is still with the entities.

I got the code to delete the objects successfully with [color=#ff0000]delete this , <snipped>


do you actually have something like that in your code:
[source lang="cpp"]void SomeClass::someMethod()
{
delete this;
}[/source]

When using construct like that, there are certain precaution you may need to be aware.
In brief the delete this was in a cleanup() called via a pointer to the base class of the bullets , the pointer is stored in a vector of pointers of entity object types and there is a loop that basically calls the cleanup like :

Entity::EntityList->OnCleanup();

It was working, I had a static counter counting each time the constructor made a object and it took one off each time the destructor was called... the destructor was called each time the OnCleanup() was called to delete this on the bullets ( ie the destructor was called each time the bullet got deleted - confirmed because I had the destructor report each time it was called).

Its moot now as the code crashed out in the loop when firing, it would sometimes work for a bit then crash or crash as soon as I shoot the first bullet... it was unstable as hell.
I basically deleted all the entity, player and collision classes and going to rewrite it in my own way instead of trying to adapt the tutorials entity system (its wierd the tutorial seemed to offer so much flexibility and elegant solutions to dealing with lots of entities... but on closer inspection, it seemed limited to only making 1 or 2 player entities ).


edit: ym seplilign scuks.
If I understand correctly, the Entity was deleted by using [color=#ff0000]delete this inside OnCleanup() method, but have you removed that pointer from the list Entity::EntityList? If you did not, then you have what is called dangling pointer. It underscores what SiCrane was saying about using smart pointer to manage memory and life time of objects.

http://en.wikipedia....angling_pointer
The element in the vector that held the pointer is erased after the OnCleanup() call, using :

Entity::EntityList.erase(Entity::EntityList.begin() + (i --));

I was under the impression that erasing the element holding the pointer would remove the pointer from the system ?

What really makes it confusing is the crash is pointing to the part of the code that control the game states ( ie menu or in game, or in the editor )... and that is pretty basic, I cant see how creating and deleting entities in the ingame state would crash the state manager... and when I remove the statemanager and all the states so the game is called directly by the main loop ...it crashes there, but not in the for loop that runs through the EntityList vector.

I am re-writing the player entity code, same stuff but checking each line more carefully ( had re-wrote the whole lot a few times and got lazy in some sections ).
[source lang="cpp"]Entity::EntityList->OnCleanup();
if(Entity::EntityList->Type == ENTITY_TYPE_BULLET_PLAYER)
{
delete Entity::EntityList;
}
[/source]

Just to clarify here.
when you call OnCleanup() method, you said that you are doing this in it: [color=#b22222]delete this. This will delete that instance of an object which in your case is object at Entity::EntityList, but after that you are trying to access member variable with this line:
[source lang="cpp"]Entity::EntityList->Type[/source]
and then you end-up deleting that object
[source lang="cpp"]delete Entity::EntityList;[/source]
which was already deleted by using delete this, hence you will get undefined behavior here.

If you goggle delete this you will find allot of resources and explanations. Here is a first one to get you started: http://www.parashift...elete-this.html
That may of been a cause for sure, however I deleted those classes to re-write them so I cant go check now... however I am pretty certain I resorted to a delete this in the cleanup after the delete Entity::EntityList seemed to cause crashes, but as I tried a number of things in a rush I cant remember for certain.

I will ensure that it doesnt do both in the new re-write I am busy with, thanks for the help... I do appreciate the effort, and the links .

This topic is closed to new replies.

Advertisement