jeirmeister/Cockatrice
1
0
Fork 0
mirror of https://github.com/Cockatrice/Cockatrice.git synced 2026-09-21 09:05:10 -07:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[GameScene] Sever animated item destroy connections at teardown

Browse source 
Fix crash when a replay's board is closed (GameScene teardown abort).

The old QObject::disconnect(nullptr, nullptr, this, nullptr) is invalid per
Qt docs (the sender must never be nullptr), so it never severed the PMF
destroyed -> GameScene::removeAnimatedItem connections that fire when
QGraphicsScene::~QGraphicsScene -> clear() destroys the remaining items.

Store the QMetaObject::Connection handle for each animated item and
disconnect them all in ~GameScene via the connection-handle overload.
Dedup connections on the connection map rather than animatedItems, since
the animation timer clears animatedItems on completion, which let a
re-registered item (e.g. a life counter flashed repeatedly) accumulate
orphaned duplicate destroyed connections that survived teardown.
This commit is contained in:
Lukas Brübach 2026-09-01 09:41:16 +02:00
parent 7c842ef350
commit c3c4ccb427
2 changed files with 24 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

30
cockatrice/src/game_graphics/game_scene.cpp
View file

@ -44,16 +44,16 @@ GameScene::GameScene(PhasesToolbar *_phasesToolbar, QObject *parent)
GameScene::~GameScene()
{
// Sever the destroy-tracking connections before the members and base-class
// teardown destroy the items. The receiver (this) cannot be matched with a
// nullptr sender (QObject::disconnect forbids one) so disconnect each
// tracked sender explicitly. Otherwise, when the base QGraphicsScene destructor
// destroys the remaining items, their destroyed() signals would re-enter
// removeAnimatedItem() and touch members that no longer exist.
const auto animatedSenders = animatedItems.keys();
for (QObject *sender : animatedSenders) {
disconnect(sender, &QObject::destroyed, this, &GameScene::removeAnimatedItem);
// Sever all destroyed->removeAnimatedItem connections before the members below
// are destroyed: the base QGraphicsScene destructor destroys the remaining items,
// and their destroyed() signals must not reach slots that reference members that
// no longer exist. The connection handle overload is used because the string-based
// disconnect(nullptr, nullptr, this, nullptr) is invalid (the sender must never be
// nullptr) and would otherwise fail to sever these pointer-to-member connections.
for (auto it = animationItemConnections.constBegin(); it != animationItemConnections.constEnd(); ++it) {
QObject::disconnect(*it);
}
animationItemConnections.clear();
delete animationTimer;
animationTimer = nullptr;
@ -782,8 +782,15 @@ void GameScene::registerAnimationItem(IAnimatedItem *item)
if (!object) {
return;
}
if (!animatedItems.contains(object)) {
connect(object, &QObject::destroyed, this, &GameScene::removeAnimatedItem);
// Guard against duplicate connections using the connection map, not
// animatedItems: the animation timer removes entries from animatedItems when an
// animation completes, but the destroyed->removeAnimatedItem connection must
// persist until the object is destroyed. Relying on animatedItems here would let
// a re-registered item (e.g. a life counter that flashes repeatedly) accumulate
// duplicate destroyed connections, the older ones of which would survive teardown.
if (!animationItemConnections.contains(object)) {
animationItemConnections.insert(object,
connect(object, &QObject::destroyed, this, &GameScene::removeAnimatedItem));
}
animatedItems.insert(object, item);
if (animationTimer && !animationTimer->isActive()) {
@ -802,6 +809,7 @@ void GameScene::unregisterAnimationItem(IAnimatedItem *item)
void GameScene::removeAnimatedItem(QObject *item)
{
animatedItems.remove(item);
animationItemConnections.remove(item);
if (animationTimer && animatedItems.isEmpty()) {
animationTimer->stop();
}

8
cockatrice/src/game_graphics/game_scene.h
View file

@ -54,9 +54,11 @@ private:
QPointer<CardItem> hoveredCard; ///< Currently hovered card
QBasicTimer *animationTimer; ///< Timer for scene animations
QHash<QObject *, IAnimatedItem *> animatedItems; ///< Items currently animating
int playerRotation; ///< Rotation offset for player layout
bool rearranging = false; ///< Guard against re-entrant rearrange
bool needsReArrange = false; ///< Pending rearrange requested during a pass
QHash<QObject *, QMetaObject::Connection>
animationItemConnections; ///< destroyed->removeAnimatedItem handles per animated item
int playerRotation; ///< Rotation offset for player layout
bool rearranging = false; ///< Guard against re-entrant rearrange
bool needsReArrange = false; ///< Pending rearrange requested during a pass
/**
* @brief Updates which card is currently hovered based on scene coordinates.