jeirmeister/Cockatrice
1
0
Fork 0
mirror of https://github.com/Cockatrice/Cockatrice.git synced 2026-09-21 00:55:09 -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 b38afe9769
commit a0bd3a5c0c
2 changed files with 25 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -44,11 +44,16 @@ GameScene::GameScene(PhasesToolbar *_phasesToolbar, QObject *parent)
GameScene::~GameScene()
{
// Sever all incoming connections (animated item destroy-tracking) 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.
QObject::disconnect(nullptr, nullptr, this, nullptr);
// 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;
@ -777,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()) {
@ -797,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();
}

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

@ -54,6 +54,8 @@ private:
QPointer<CardItem> hoveredCard; ///< Currently hovered card
QBasicTimer *animationTimer; ///< Timer for scene animations
QHash<QObject *, IAnimatedItem *> animatedItems; ///< Items currently animating
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