0

I want to detect all items in collision with a QGraphicsItem (item), yet the code below crashes, id like to know the reason.

void PlayerDefences::checkCollisions(QGraphicsItem* item)
    {
        QList<QGraphicsItem*> itemCollidesWithShip = item->collidingItems(); ///Returns a list of all items that collide with this item.

 
    for (int i = 0; i < itemCollidesWithShip.size(); i++) {
        if(itemCollidesWithShip[i]==nullptr) continue;
 
        if(dynamic_cast<Bullet*>(itemCollidesWithShip[i])){ //if the colliding item is of type Bullet*
            life-=10;
            setActualLife();
            item->scene()->removeItem(itemCollidesWithShip[i]);
            std::vector<Bullet*>* bullets = Level1::getBulletContainer();
 
            bullets->erase(std::remove(bullets->begin(), bullets->end(), itemCollidesWithShip[i]), bullets->end());
            delete itemCollidesWithShip[i] ;
        }
    }
}

The crash goes away, when i add this line below delete:

itemCollidesWithShip.erase(std::remove(itemCollidesWithShip.begin(), itemCollidesWithShip.end(), itemCollidesWithShip[i]), itemCollidesWithShip.end())
black_gay
  • 143
  • 7
  • what container type is itemCollidesWithShip ? – pm100 Apr 14 '22 at 16:41
  • It's of type QList – black_gay Apr 14 '22 at 16:47
  • I tried with adding itemsCollideWithShip[i] = nullptr; but it also crashes. The only thing that prevents crashes is either adding itemsCollideWithShip.erase(std::remove(itemsCollideWithShip.begin(),itemsCollideWithShip.end(), itemsCollideWithShip[i]), itemsCollideWithShip.end()); – black_gay Apr 14 '22 at 17:03
  • Perhaps the pointer indicated by `itemCollidesWithShip[i]` is present in the QList more than once? If so, then after deleting it once, subsequent pointers to that same address in the QList would now be dangling-pointers, and trying to dereference them would invoke undefined behavior. (Dunno why the same pointer would be returned more than once by `collidingItems()`, but it would fit the observed behavior if that's the case) – Jeremy Friesner Apr 14 '22 at 19:15
  • but wouldnt "if(itemsCollideWithShip[i]==nullptr) continue;" at the beginning of the for loop stop it from dereferencing? – black_gay Apr 14 '22 at 19:47
  • Could it be that `collidingItems()` returns some item twice? – mugiseyebrows Apr 14 '22 at 21:57
  • No, because i put a for loop inside this function and displayed addresses of all the elements at loops entry and in the end, and no address is the same, after delete, the order of the elements is the same also, the only difference is that one is set to 0 as nullpointer. https://i.ibb.co/7SrJW5F/obraz.png – black_gay Apr 14 '22 at 22:12

1 Answers1

0

Looking at the rest of the shown code you probably need

delete itemCollidesWithShip[i] ;
itemCollidesWithShip[i] = nullptr;

This seems to be how you flag empty slots. The remove/erase combo you did removes the whole entry from the table, that works too. But at the moment you have an entry in your table that has an invalid pointer (it points at something you deleted),

pm100
  • 48,078
  • 23
  • 82
  • 145
  • I tried with adding itemsCollideWithShip[i] = nullptr; but it also crashes. The only thing that prevents crashes is either adding itemsCollideWithShip.erase(std::remove(itemsCollideWithShip.begin(),itemsCollideWithShip.end(), itemsCollideWithShip[i]), itemsCollideWithShip.end()); – black_gay Apr 14 '22 at 17:04