Program crushes..

[Sean11]

Hey guys i had open thread here and got much help from you last time so i encountered new problems
i created class called Bullet and when the bullet collide and enemy(spaceship) it is deleting the bullet,the spaceship, and spaceship pointer that is inside the vector
but when it hits the enemy the program crushes.. now i have tried to delete 1 spaceship from the spaceship row when the spaceships reach certain Y() but still when i remove from the scene 1 spaceship
even if i remove the spaceship from the scene, delete the certain spaceship from the vector and delete the object it self the program crushes. plz anyhelp
here is my code:
https://github.com/SeanR11/Spaceship---first-game
another question i got:
i want to add level(difficulty) class that incharge of changing levels from 1-30 on each level the spaceship ill be in different order or somthing like this any suggestions how to start it? what to inherits from attributes or methods?

[PierreA]

Can you run the program in gdb? That should tell you what's happening. It's likely that you're calling a method of an object with a pointer to the object when the pointer is null. Make sure that you compile it with debugging enabled.

If you are used to debugging console programs, debugging Qt programs is a little different. When you say "where", you'll get some frames that are in your code, several that aren't, and some more frames that are. The frames that aren't are Qt's event loop and the signal-slot calls.

[d_stranz]

even if i remove the spaceship from the scene, delete the certain spaceship from the vector and delete the object it self the program crushes.

Sounds to me like you are deleting the spaceship instance twice or calling one of the spaceship methods after that instance has been deleted, which is why the program crashes.

The reason why I wrote "POINTER" instead of lowercase pointer in one of my earlier posts was to emphasize the fact the the QVector was simply storing a copy of the pointer to the spaceship object that is stored in (and owned) by the scene.

When you want to delete a single item in the scene, you need to do the following:

1 - retrieve the item pointer from the scene (using one of the itemAt() or items() methods)
2 - call QGraphicsScene::removeItem()
3 - call delete on the item pointer.

In your case, since you are also separately storing a copy of the pointer to the item in the QVector, you also need to call QVector::erase() or QVector::remove() before you delete the item (i.e. do that before step 3 above). If you delete the item before removing from the vector, then you run the risk of the QTimer firing and the timeout slot trying to update the position of an object that no longer exists. Note that since you are only storing a copy of the item's pointer in the QVector, you do not call delete on that copyt after you remove it.

[Sean11]

i have single row of 5 spaceships goes from 1 side to the other.
each time they reach the end of the line they go 1 line down
when they reach y() >= 100 i want to delete single spaceship from the row and let them keep going
from side to side.
how can i delete the copy pointer that in spaceships[0] and the program will still run the spaceship ?
if i delete spaceships[0], will spaceships[1] will become spaceships[0]?

if(spaceships[0]->y() >= 100){
        qDebug() << "you lost";
           Spaceship * spaceship = spaceships[ 0 ]; // spaceships[N] = QVector that stores copy of spaceship(object) pointer 
           scene->removeItem(spaceship);
           delete(spaceship);
           }

if(spaceships[0]->y() >= 100){
        qDebug() << "you lost";
           Spaceship * spaceship = spaceships[ 0 ]; // spaceships[N] = QVector that stores copy of spaceship(object) pointer 
           scene->removeItem(spaceship);
           delete(spaceship);
           }

To copy to clipboard, switch view to plain text mode 

[PierreA]

What you're doing is deleting the spaceship while there's still a pointer to it. What you want to do is delete the spaceship and the pointer to it. Try this:

if (spaceships[0]->y()>=100)
  {
    scene->removeItem(spaceship[0]);
    swap(spaceships[0],spaceships.back());
    spaceships.resize(spaceships.size()-1);
  }

if (spaceships[0]->y()>=100)
  {
    scene->removeItem(spaceship[0]);
    swap(spaceships[0],spaceships.back());
    spaceships.resize(spaceships.size()-1);
  }

To copy to clipboard, switch view to plain text mode 

This is with an STL vector (I haven't worked with QVectors). Resizing a vector calls the destructor if you make it smaller or the constructor if you make it bigger. spaceships.back() is equivalent to spaceships[spaceships.size()-1] and can be used as an lvalue. I know it looks funny to put a function call on the left side, but it is doable. If you want to add more spaceships to the vector, use push_back.

[Sean11]

Unfortunately it did not work.. thanks for your answer tho.
how can i remove specific pointer from a vector array?

[d_stranz]

Resizing a vector calls the destructor if you make it smaller or the constructor if you make it bigger.

In this case, where *pointers* are being stored in the vector, the Spaceship destructor is not called, the "destructor" that is being called simply erases the location occupied by the pointer and the Spaceship instance itself still exists. That's a memory leak.

@Sean11: you remove something from a QVector simply by calling the QVector::remove() method.

Please do not get hung up thinking there is some magic about "spaceships[0]" or "spaceships[9]" or any other number in the square brackets. A vector is just a row of boxes. The boxes are numbered starting at 0 and ending at "number of boxes - 1" (in your case "number of boxes" 10 or 5 or whatever). The notation "spaceships[0]" is just shorthand for "the pointer to the Spaceship instance that is stored in the first box of the QVector".

You are storing your spaceship instances in two places: in the scene as well as in the QVector. In order to correctly remove a particular spaceship from the universe it needs to be removed from both places. You also have to be sure that the memory used by the Spaceship instance is freed up for reuse, otherwise you will have a memory leak. The trick is to do these three things in the correct order so that some part of your program won't try to use a pointer to a instance that has been deleted already. If your program tries to do that, it will crash.

If you want to delete the first spaceship in a row, the easiest way is to get the pointer from your QVector, since it is storing the pointers in order from left to right. This is "spaceships[0]". You then want to do the following, in this order:

int spaceshipToRemove = 0;  // Or any other number between 0 and spaceships.size() - 1
 
  // Do not try to get a spaceship if the vector is empty or the number is too high!
  if ( spaceships.size() > 0 && spaceshipToRemove < spaceships.size() )  
  {
      Spaceship * spaceship = spaceships[ spaceshipToRemove ];  // get the first spaceship
      scene->remove( spaceship );  // remove it from the scene
      spaceships.remove( spaceshipToRemove );  // remove it from the QVector
      delete spaceship;  // last thing to do to avoid a leak
   }

int spaceshipToRemove = 0;  // Or any other number between 0 and spaceships.size() - 1

  // Do not try to get a spaceship if the vector is empty or the number is too high!
  if ( spaceships.size() > 0 && spaceshipToRemove < spaceships.size() )  
  {
      Spaceship * spaceship = spaceships[ spaceshipToRemove ];  // get the first spaceship
      scene->remove( spaceship );  // remove it from the scene
      spaceships.remove( spaceshipToRemove );  // remove it from the QVector
      delete spaceship;  // last thing to do to avoid a leak
   }

To copy to clipboard, switch view to plain text mode 

Now at this point, "spaceships.size()" will return a number that is 1 less than before. Assuming you removed the spaceship # 0, if the size before deleting spaceship # 0 was 10, it is now 9. All of the other spaceship pointers have been shifted down in the QVector, in otherwords, there is a new pointer in spaceships[0], which used to be the pointer in spaceships[1], and so forth. There is no "spaceships[9]" any more because one of the spaceships has been removed and the vector has become shorter.

The same thing happens if you remove any other spaceship # n: The pointer at QVector position n will be erased, all the other pointers above it will be moved down, and he size of the vector will decrease by 1.

[Sean11]

@d_stranz I wanted to say thanks for the help i have learned a lot thanks to you.
i tested what u said and i did:

if(spaceships[0]->y() >= 100 && spaceships.size() == 5){
        qDebug() << "you lost";
          Spaceship * spaceship = spaceships[ 1 ];
          scene->removeItem(spaceship);
          spaceships.remove(1); // i have tried Spaceships.remove(spaceship) but it says: cannot convert QVector::remove() Spaceship * to int // i dont know why tho 
          delete(spaceship);
    }

if(spaceships[0]->y() >= 100 && spaceships.size() == 5){
        qDebug() << "you lost";
          Spaceship * spaceship = spaceships[ 1 ];
          scene->removeItem(spaceship);
          spaceships.remove(1); // i have tried Spaceships.remove(spaceship) but it says: cannot convert QVector::remove() Spaceship * to int // i dont know why tho 
          delete(spaceship);
    }

To copy to clipboard, switch view to plain text mode 

anyway when i tested it , it did remove the second spaceship from the row and kept moving from side to side. but when i added a function:

void Game::delSpaceshipVector(int x)
{
    Spaceship * spaceship = spaceships[x-1];
    scene->removeItem(spaceship);
    spaceships.remove(x-1);
    delete(spaceship);
    return;
 
}

void Game::delSpaceshipVector(int x)
{
    Spaceship * spaceship = spaceships[x-1];
    scene->removeItem(spaceship);
    spaceships.remove(x-1);
    delete(spaceship);
    return;

}

To copy to clipboard, switch view to plain text mode 

this function is called by this:

void Bullet::move()
{
    QList<QGraphicsItem *> SpaceshipHit= collidingItems();
    for(int i = 0, n = SpaceshipHit.size(); i < n; i++){
        if(typeid(*(SpaceshipHit[i])) == typeid(Spaceship)){
            scene()->removeItem(this);
            game->delSpaceshipVector(i);
            delete this;
            return;
        }
    }
 
 
    setPos(x(),y()-10);
}

void Bullet::move()
{
    QList<QGraphicsItem *> SpaceshipHit= collidingItems();
    for(int i = 0, n = SpaceshipHit.size(); i < n; i++){
        if(typeid(*(SpaceshipHit[i])) == typeid(Spaceship)){
            scene()->removeItem(this);
            game->delSpaceshipVector(i);
            delete this;
            return;
        }
    }


    setPos(x(),y()-10);
}

To copy to clipboard, switch view to plain text mode 

the last fucntion cause the bullet move forward when it collide type"spaceship" it is delete the bullet object then goes to delSpaceshipVector func inside game.cpp-> in delSpaceshipVector function does delete spaceship POINTER, spaceship from the scene and the object it self. when i tried the first code in this message it did worked. but when i used another function that calls from another cpp file (bullet.cpp) it crushed and says:
"QVector index out of range"

[d_stranz]

void Bullet::move()
{
    QList<QGraphicsItem *> SpaceshipHit= collidingItems();
    for(int i = 0, n = SpaceshipHit.size(); i < n; i++){
        if(typeid(*(SpaceshipHit[i])) == typeid(Spaceship)){
            scene()->removeItem(this);
            game->delSpaceshipVector(i);
            delete this;
            return;
        }
    }
 
 
    setPos(x(),y()-10);
}

void Bullet::move()
{
    QList<QGraphicsItem *> SpaceshipHit= collidingItems();
    for(int i = 0, n = SpaceshipHit.size(); i < n; i++){
        if(typeid(*(SpaceshipHit[i])) == typeid(Spaceship)){
            scene()->removeItem(this);
            game->delSpaceshipVector(i);
            delete this;
            return;
        }
    }


    setPos(x(),y()-10);
}

To copy to clipboard, switch view to plain text mode 

This code doesn't make any sense. You ask for the list of items your bullet has collided with, to see if you have hit a spaceship. In your loop, you check each item to see if it is in fact a spaceship. If it is, then for some strange reason, you try to delete the bullet.

Never try to delete an object from inside of one of its own member functions. That's almost always guaranteed to cause a crash, particularly in Qt where there are copies of object pointers in places you may not know about. Deleting an object when something else is still using the pointer will cause a crash.

You need to turn your logic around. Instead of the Bullet class determining whether it has hit something, the class which is controlling the bullet needs to do the checking. If this class determines that the bullet has hit something, then the bullet and the object it hit need to be destroyed and removed from all of the data structures that hold pointers to them. If the bullet didn't hit anything, then the controller class moves the bullet. The bullet doesn't move itself, some controller moves it. Only the controller class is aware of everything in the game; each player in the game only knows about itself.

void Game::delSpaceshipVector(int x)
    {
        Spaceship * spaceship = spaceships[x-1];
        scene->removeItem(spaceship);
        spaceships.remove(x-1);
        delete(spaceship);
        return;
 
    }

void Game::delSpaceshipVector(int x)
    {
        Spaceship * spaceship = spaceships[x-1];
        scene->removeItem(spaceship);
        spaceships.remove(x-1);
        delete(spaceship);
        return;
     
    }

To copy to clipboard, switch view to plain text mode 

This doesn't make sense either. If you want to remove spaceship # x, and if "x" is 0, then when you subtract 1 you get spaceships[-1] and that isn't valid. Get rid of the "x - 1" and replace it with just "x".

if(spaceships[0]->y() >= 100 && spaceships.size() == 5){

if(spaceships[0]->y() >= 100 && spaceships.size() == 5){

To copy to clipboard, switch view to plain text mode 

And this doesn't make sense either. As soon as you remove the first spaceship, spaceships.size() is no longer equal to 5, so the statement is false and the code in the if() clause won't execute. Think about what you really want this to say.

// i have tried Spaceships.remove(spaceship) but it says: cannot convert QVector::remove() Spaceship * to int // i dont know why tho

Have you read the documentation for QVector::remove()? That will tell you why it didn't compile.

[Sean11]

@d_stranz i dont know if it interesting you but i solved my problems thanks to you
i added few methods: bulletMove() and i added keyEventPress method to check if space is clicked to create bullet inside the game.cpp you said the game should manage any movement of objects and creating them and class object only response for self-object attributes and actions. inside the bulletMove() i crossed spaceships.x() and spaceships.y() with bullet and checked if they are collides. then it was much easier to delete both if they are collide now my program works fine for now
just last question that i did ask if u can help me with it:
if i want to make level class that response of difficulties change when the game runs.
what i want to do is make level class a list of voids (methods) that each method create the amount of spaceships for this level (difficulty) and when all spaceships is destroyed it does go on to the next void (next level) that creates different shape or amount of spaceships.
so my question is how do i start such a class what to inherits from and how to connect it to game.cpp
another question that if i create the spaceships outside game.cpp i need to transfer QVector points from level.cpp to game.cpp that holds spaceships(Vector) how i can do that?
i know my question lil bit annoying but thanks to you i have learned a lot on c++ and on QT environment and a lot about Vectors that i didnt know was so useful

[d_stranz]

I do not think you really need a "Level" class. What you need is to define (as part of your design) what level 0 should look like (how many spaceships, their shape, etc.), level 1, and so forth. Then your Game class needs a variable to know which is the current level ("int nLevel") and it needs a method "createLevel( int levelNumber )" that will create everything needed for that level.

If you add any class at all, it should be something like a "LevelParameters" class or struct that contains the definition of that level (how many spaceships, how many bullets, how many lives, etc.) These can be arguments to the LevelParameters constructor. You can make another QVector< LevelParameters > that stores the parameters for each one. You will build this vector in your Game constructor.

Game::Game( /* ... */ )
{
   // do whatever to initialize the game
 
   // Build parameters for each difficulty level
   LevelParameters  parameters;
 
   // Set parameters for level 0
   parameters.spaceshipCount = 5;
   parameters.bulletCount = 100;
   levelParameters.push_back( parameters );  // This makes a copy of "parameters"
 
    // Set parameters for level 1
    parameters.spaceshipCount = 10;
    parameters.bulletCount = 50;
    levelParameters.push_back( parameters );
 
    // etc.
}
 
void Game::createLevel( int levelNumber )
{
    // If the player is already at the highest level, just play that level again
    int level = levelNumber;
    if ( level > highestLevel )
        level = highestLevel;
 
    const LevelParameters & parameters = levelParameters[ level ];
 
    createSpaceships( parameters.spaceshipCount, level );
    createBullets( parameters.bulletCount, level );
 
    // etc.
 
   nLevel = level;  // Store current level in the class
}

Game::Game( /* ... */ )
{
   // do whatever to initialize the game

   // Build parameters for each difficulty level
   LevelParameters  parameters;

   // Set parameters for level 0
   parameters.spaceshipCount = 5;
   parameters.bulletCount = 100;
   levelParameters.push_back( parameters );  // This makes a copy of "parameters"

    // Set parameters for level 1
    parameters.spaceshipCount = 10;
    parameters.bulletCount = 50;
    levelParameters.push_back( parameters );

    // etc.
}

void Game::createLevel( int levelNumber )
{
    // If the player is already at the highest level, just play that level again
    int level = levelNumber;
    if ( level > highestLevel )
        level = highestLevel;

    const LevelParameters & parameters = levelParameters[ level ];

    createSpaceships( parameters.spaceshipCount, level );
    createBullets( parameters.bulletCount, level );

    // etc.
   
   nLevel = level;  // Store current level in the class
}

To copy to clipboard, switch view to plain text mode 

If Spaceship has a different shape for each level or different capabilities, then the Spaceship class should have a "nLevel" variable, and the Spaceship constructor should take this as an argument. Inside the constructor, you can change the shape or size of the Spaceship QGraphicsItem according to the level.

[Sean11]

Okay ill go for that idea but when i create a class i need to set her up

class Levelparameter: {  // what to add here? from what lib this class should inherits? QObject?
 
public:
         Levelparameter();
 
 
 
}

class Levelparameter: {  // what to add here? from what lib this class should inherits? QObject?

public:
         Levelparameter();



}

To copy to clipboard, switch view to plain text mode 

[d_stranz]

// what to add here? from what lib this class should inherits? QObject?

Nothing. The class can stand alone. It does not need to inherit from anything. It does not need signals or slots, so does not need to inherit from QObject (and you won't use the Q_OBJECT macro in the class definition).