diff --git a/cockatrice/src/client/tabs/tab_game.cpp b/cockatrice/src/client/tabs/tab_game.cpp index b79968a5c..f18e5ce06 100644 --- a/cockatrice/src/client/tabs/tab_game.cpp +++ b/cockatrice/src/client/tabs/tab_game.cpp @@ -189,14 +189,7 @@ void TabGame::emitUserEvent() TabGame::~TabGame() { - scene->clearViews(); - delete replay; - - QMapIterator i(players); - while (i.hasNext()) { - delete i.next().value(); - } } void TabGame::updatePlayerListDockTitle() @@ -1097,6 +1090,7 @@ void TabGame::eventLeave(const Event_Leave &event, int eventPlayerId, const Game players.remove(eventPlayerId); emit playerRemoved(player); player->clear(); + scene->removePlayer(player); player->deleteLater(); // Rearrange all remaining zones so that attachment relationship updates take place diff --git a/cockatrice/src/game/cards/card_item.cpp b/cockatrice/src/game/cards/card_item.cpp index 31c3b7c0f..19cff1df1 100644 --- a/cockatrice/src/game/cards/card_item.cpp +++ b/cockatrice/src/game/cards/card_item.cpp @@ -38,16 +38,9 @@ CardItem::CardItem(Player *_owner, CardItem::~CardItem() { - prepareDelete(); - - if (scene()) - static_cast(scene())->unregisterAnimationItem(this); - delete cardMenu; delete ptMenu; delete moveMenu; - - deleteDragItem(); } void CardItem::prepareDelete() @@ -74,6 +67,8 @@ void CardItem::prepareDelete() void CardItem::deleteLater() { prepareDelete(); + if (scene()) + static_cast(scene())->unregisterAnimationItem(this); AbstractCardItem::deleteLater(); } diff --git a/cockatrice/src/game/game_scene.cpp b/cockatrice/src/game/game_scene.cpp index af1e40f96..e649bbb2a 100644 --- a/cockatrice/src/game/game_scene.cpp +++ b/cockatrice/src/game/game_scene.cpp @@ -29,6 +29,13 @@ GameScene::GameScene(PhasesToolbar *_phasesToolbar, QObject *parent) GameScene::~GameScene() { delete animationTimer; + + // DO NOT call clearViews() here + // clearViews calls close() on the zoneViews, which sends signals; sending signals in destructors leads to segfaults + // deleteLater() deletes the zoneView without allowing it to send signals + for (const auto &zoneView : zoneViews) { + zoneView->deleteLater(); + } } void GameScene::retranslateUi() @@ -76,7 +83,7 @@ void GameScene::rearrange() QListIterator playersIter(players); while (playersIter.hasNext()) { Player *p = playersIter.next(); - if (!p->getConceded()) { + if (p && !p->getConceded()) { playersPlaying.append(p); if (!firstPlayerFound && (p->getLocal())) { firstPlayerIndex = playersPlaying.size() - 1; diff --git a/cockatrice/src/game/player/player.cpp b/cockatrice/src/game/player/player.cpp index d25f3ef63..71a9a0a3f 100644 --- a/cockatrice/src/game/player/player.cpp +++ b/cockatrice/src/game/player/player.cpp @@ -564,9 +564,6 @@ Player::~Player() { qCDebug(PlayerLog) << "Player destructor:" << getName(); - static_cast(scene())->removePlayer(this); - - clear(); QMapIterator i(zones); while (i.hasNext()) delete i.next().value(); @@ -2086,7 +2083,7 @@ void Player::eventShuffle(const Event_Shuffle &event) int length = view->getCards().length(); // we want to close empty views as well if (length == 0 || length > absStart) { // note this assumes views always start at the top of the library - view->deleteLater(); + view->close(); break; } } else { diff --git a/cockatrice/src/game/zones/card_zone.cpp b/cockatrice/src/game/zones/card_zone.cpp index 893a9b181..f444cb464 100644 --- a/cockatrice/src/game/zones/card_zone.cpp +++ b/cockatrice/src/game/zones/card_zone.cpp @@ -40,17 +40,6 @@ CardZone::CardZone(Player *_p, &CardZone::refreshCardInfos); } -CardZone::~CardZone() -{ - qCDebug(CardZoneLog) << "CardZone destructor: " << name; - for (auto *view : views) { - if (view != nullptr) { - view->deleteLater(); - } - } - clearContents(); -} - void CardZone::retranslateUi() { for (int i = 0; i < cards.size(); ++i) diff --git a/cockatrice/src/game/zones/card_zone.h b/cockatrice/src/game/zones/card_zone.h index 20f58d4ba..55ea606ab 100644 --- a/cockatrice/src/game/zones/card_zone.h +++ b/cockatrice/src/game/zones/card_zone.h @@ -67,7 +67,6 @@ public: bool _contentsKnown, QGraphicsItem *parent = nullptr, bool _isView = false); - ~CardZone(); void retranslateUi(); void clearContents(); bool getHasCardAttr() const diff --git a/cockatrice/src/game/zones/view_zone.cpp b/cockatrice/src/game/zones/view_zone.cpp index 9e311f51b..67c3c4b94 100644 --- a/cockatrice/src/game/zones/view_zone.cpp +++ b/cockatrice/src/game/zones/view_zone.cpp @@ -40,13 +40,17 @@ ZoneViewZone::ZoneViewZone(Player *_p, } } -ZoneViewZone::~ZoneViewZone() +/** + * Deletes this ZoneView and removes it from the origZone's views. + * You should normally call this method instead of deleteLater() + */ +void ZoneViewZone::close() { - emit beingDeleted(); - qDebug("ZoneViewZone destructor"); + emit closed(); if (!(revealZone && !writeableRevealZone)) { origZone->getViews().removeOne(this); } + deleteLater(); } QRectF ZoneViewZone::boundingRect() const diff --git a/cockatrice/src/game/zones/view_zone.h b/cockatrice/src/game/zones/view_zone.h index 6eda0fd23..196b1bca4 100644 --- a/cockatrice/src/game/zones/view_zone.h +++ b/cockatrice/src/game/zones/view_zone.h @@ -65,7 +65,6 @@ public: bool _writeableRevealZone = false, QGraphicsItem *parent = nullptr, bool _isReversed = false); - ~ZoneViewZone(); QRectF boundingRect() const; void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget); void reorganizeCards(); @@ -94,13 +93,14 @@ public: return isReversed; } public slots: + void close(); void setGroupBy(CardList::SortOption _groupBy); void setSortBy(CardList::SortOption _sortBy); void setPileView(int _pileView); private slots: void zoneDumpReceived(const Response &r); signals: - void beingDeleted(); + void closed(); void optimumRectChanged(); void wheelEventReceived(QGraphicsSceneWheelEvent *event); diff --git a/cockatrice/src/game/zones/view_zone_widget.cpp b/cockatrice/src/game/zones/view_zone_widget.cpp index 4bc596531..531ca06df 100644 --- a/cockatrice/src/game/zones/view_zone_widget.cpp +++ b/cockatrice/src/game/zones/view_zone_widget.cpp @@ -133,7 +133,7 @@ ZoneViewWidget::ZoneViewWidget(Player *_player, setLayout(vbox); connect(zone, &ZoneViewZone::optimumRectChanged, this, &ZoneViewWidget::resizeToZoneContents); - connect(zone, &ZoneViewZone::beingDeleted, this, &ZoneViewWidget::zoneDeleted); + connect(zone, &ZoneViewZone::closed, this, &ZoneViewWidget::zoneDeleted); zone->initializeCards(cardList); // QLabel sizes aren't taken into account until the widget is rendered. @@ -314,11 +314,12 @@ void ZoneViewWidget::handleScrollBarChange(int value) void ZoneViewWidget::closeEvent(QCloseEvent *event) { - disconnect(zone, &ZoneViewZone::beingDeleted, this, 0); + disconnect(zone, &ZoneViewZone::closed, this, 0); + // manually call zone->close in order to remove it from the origZones views + zone->close(); if (shuffleCheckBox.isChecked()) player->sendGameCommand(Command_Shuffle()); - emit closePressed(this); - deleteLater(); + zoneDeleted(); event->accept(); }