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 connections properly. (#7191)

Browse source 
* [GameScene] Sever connections properly.

Took 2 minutes


Took 54 minutes

* [GameScene] Sever animated item destroy connections at teardown

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.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
BruebachL 2026-09-01 11:31:11 +02:00 • committed by GitHub
parent 9bf2202739
commit d974501277
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
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() GameScene::~GameScene()
{ {
// Sever all incoming connections (animated item destroy-tracking) before the // Sever all destroyed->removeAnimatedItem connections before the members below
// members below are destroyed: the base QGraphicsScene destructor destroys the // are destroyed: the base QGraphicsScene destructor destroys the remaining items,
// remaining items, and their destroyed() signals must not reach slots that // and their destroyed() signals must not reach slots that reference members that
// reference members that no longer exist. // no longer exist. The connection handle overload is used because the string-based
QObject::disconnect(nullptr, nullptr, this, nullptr); // 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; delete animationTimer;
animationTimer = nullptr; animationTimer = nullptr;
@ -777,8 +782,15 @@ void GameScene::registerAnimationItem(IAnimatedItem *item)
if (!object) { if (!object) {
return; return;
} }
if (!animatedItems.contains(object)) { // Guard against duplicate connections using the connection map, not
connect(object, &QObject::destroyed, this, &GameScene::removeAnimatedItem); // 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); animatedItems.insert(object, item);
if (animationTimer && !animationTimer->isActive()) { if (animationTimer && !animationTimer->isActive()) {
@ -797,6 +809,7 @@ void GameScene::unregisterAnimationItem(IAnimatedItem *item)
void GameScene::removeAnimatedItem(QObject *item) void GameScene::removeAnimatedItem(QObject *item)
{ {
animatedItems.remove(item); animatedItems.remove(item);
animationItemConnections.remove(item);
if (animationTimer && animatedItems.isEmpty()) { if (animationTimer && animatedItems.isEmpty()) {
animationTimer->stop(); animationTimer->stop();
} }

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

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