[Game] [Arrows] Use arrowData/registry and generate unique server-side ids (#6973)

* [Game] [Arrows] Track creatorId, use arrowData in arrowItem, use registry, generate unique arrow id's on server side and delete-on-exist inserts.

Took 2 minutes

Took 1 minute

* Fix emitting slot instead of signal.

Took 15 minutes

* Clear arrows locally in special circumstances i.e. teardown.

Took 28 minutes

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cockatrice/CMakeLists.txt

@@ -56,6 +56,7 @@ set(cockatrice_SOURCES
     src/filters/filter_tree_model.cpp
     src/filters/syntax_help.cpp
     src/game/abstract_game.cpp
+    src/game/arrow_registry.cpp
     src/game/board/abstract_card_drag_item.cpp
     src/game/board/abstract_card_item.cpp
     src/game/board/abstract_counter.cpp

cockatrice/src/game/arrow_registry.cpp

@@ -0,0 +1,48 @@
+#include "arrow_registry.h"
+
+#include "board/arrow_item.h"
+
+void ArrowRegistry::insert(QSharedPointer<ArrowData> data, ArrowItem *arrow)
+{
+    const ArrowKey key{data->creatorId, data->id};
+
+    if (auto *existing = take(data->creatorId, data->id)) {
+        existing->delArrow();
+    }
+
+    dataStore.insert(key, data);
+    items.insert(key, arrow);
+    byPlayer[data->creatorId].insert(data->id);
+}
+
+ArrowItem *ArrowRegistry::take(int creatorId, int arrowId)
+{
+    const ArrowKey key{creatorId, arrowId};
+    dataStore.remove(key);
+    auto &playerSet = byPlayer[creatorId];
+    playerSet.remove(arrowId);
+    if (playerSet.isEmpty()) {
+        byPlayer.remove(creatorId);
+    }
+    return items.take(key);
+}
+
+ArrowItem *ArrowRegistry::get(int creatorId, int arrowId) const
+{
+    return items.value(ArrowKey{creatorId, arrowId}, nullptr);
+}
+
+bool ArrowRegistry::contains(int creatorId, int arrowId) const
+{
+    return items.contains(ArrowKey{creatorId, arrowId});
+}
+
+QSet<int> ArrowRegistry::idsForPlayer(int playerId) const
+{
+    return byPlayer.value(playerId);
+}
+
+QList<ArrowItem *> ArrowRegistry::all() const
+{
+    return items.values();
+}

cockatrice/src/game/arrow_registry.h

@@ -0,0 +1,43 @@
+#ifndef COCKATRICE_ARROW_REGISTRY_H
+#define COCKATRICE_ARROW_REGISTRY_H
+
+#include "board/arrow_data.h"
+
+#include <QMap>
+#include <QSet>
+#include <QSharedPointer>
+
+class ArrowItem;
+
+struct ArrowKey
+{
+    int creatorId;
+    int arrowId;
+
+    bool operator<(const ArrowKey &other) const
+    {
+        if (creatorId != other.creatorId) {
+            return creatorId < other.creatorId;
+        }
+        return arrowId < other.arrowId;
+    }
+};
+
+class ArrowRegistry
+{
+public:
+    void insert(QSharedPointer<ArrowData> data, ArrowItem *arrow);
+    ArrowItem *take(int creatorId, int arrowId);
+
+    [[nodiscard]] ArrowItem *get(int creatorId, int arrowId) const;
+    [[nodiscard]] bool contains(int creatorId, int arrowId) const;
+    [[nodiscard]] QSet<int> idsForPlayer(int playerId) const;
+    [[nodiscard]] QList<ArrowItem *> all() const;
+
+private:
+    QMap<ArrowKey, QSharedPointer<ArrowData>> dataStore;
+    QMap<ArrowKey, ArrowItem *> items;
+    QMap<int, QSet<int>> byPlayer;
+};
+
+#endif

cockatrice/src/game/board/arrow_data.cpp

@@ -1,8 +1,10 @@
 #include "arrow_data.h"
 
-ArrowData ArrowData::fromProto(const ServerInfo_Arrow &arrow)
+ArrowData ArrowData::fromProto(const ServerInfo_Arrow &arrow, int creatorId, bool isLocalCreator)
 {
     ArrowData data;
+    data.creatorId = creatorId;
+    data.isLocalCreator = isLocalCreator;
     data.id = arrow.id();
     data.startPlayerId = arrow.start_player_id();
     data.startZone = QString::fromStdString(arrow.start_zone());

cockatrice/src/game/board/arrow_data.h

@@ -8,16 +8,18 @@
 
 struct ArrowData
 {
-    int id;
-    int startPlayerId;
-    QString startZone;
-    int startCardId;
-    int targetPlayerId;
-    QString targetZone;    // empty = targeting a player
-    int targetCardId = -1; // -1 = targeting a player
-    QColor color;
+    int creatorId = -1;
+    bool isLocalCreator = false;
+    int id = -1;
+    int startPlayerId = -1;
+    QString startZone = "";
+    int startCardId = -1;
+    int targetPlayerId = -1;
+    QString targetZone = "";
+    int targetCardId = -1;
+    QColor color = "";
 
-    static ArrowData fromProto(const ServerInfo_Arrow &arrow);
+    static ArrowData fromProto(const ServerInfo_Arrow &arrow, int creatorId, bool isLocalCreator);
 
     bool isPlayerTargeted() const
     {

cockatrice/src/game/board/arrow_item.cpp

@@ -21,12 +21,8 @@
 #include <libcockatrice/utility/color.h>
 #include <libcockatrice/utility/zone_names.h>
 
-ArrowItem::ArrowItem(PlayerLogic *_player,
-                     int _id,
-                     ArrowTarget *_startItem,
-                     ArrowTarget *_targetItem,
-                     const QColor &_color)
-    : player(_player), id(_id), startItem(_startItem), targetItem(_targetItem), color(_color)
+ArrowItem::ArrowItem(QSharedPointer<const ArrowData> _data, ArrowTarget *_startItem, ArrowTarget *_targetItem)
+    : data(std::move(_data)), startItem(_startItem), targetItem(_targetItem)
 {
     setZValue(ZValues::ARROWS);
 
@@ -52,7 +48,7 @@ ArrowItem::ArrowItem(PlayerLogic *_player,
 
 void ArrowItem::onTargetDestroyed()
 {
-    emit requestDeletion(id);
+    emit requestDeletion(data->creatorId, data->id);
 }
 
 void ArrowItem::delArrow()
@@ -130,7 +126,7 @@ void ArrowItem::updatePath(const QPointF &endPoint)
 
 void ArrowItem::paint(QPainter *painter, const QStyleOptionGraphicsItem * /*option*/, QWidget * /*widget*/)
 {
-    QColor paintColor(color);
+    QColor paintColor(data->color);
     if (fullColor) {
         paintColor.setAlpha(200);
     } else {
@@ -142,7 +138,7 @@ void ArrowItem::paint(QPainter *painter, const QStyleOptionGraphicsItem * /*opti
 
 void ArrowItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
 {
-    if (!player->getPlayerInfo()->getLocal()) {
+    if (!data->isLocalCreator) {
         event->ignore();
         return;
     }
@@ -156,14 +152,20 @@ void ArrowItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
 
     event->accept();
     if (event->button() == Qt::RightButton) {
-        emit requestDeletion(id);
+        emit requestDeletion(data->creatorId, data->id);
     }
 }
 
 // ArrowDragItem
 
 ArrowDragItem::ArrowDragItem(PlayerLogic *_owner, ArrowTarget *_startItem, const QColor &_color, int _deleteInPhase)
-    : ArrowItem(_owner, -1, _startItem, nullptr, _color), deleteInPhase(_deleteInPhase)
+    : ArrowItem(QSharedPointer<ArrowData>::create(ArrowData{.creatorId = _owner->getPlayerInfo()->getId(),
+                                                            .isLocalCreator = true,
+                                                            .id = -1,
+                                                            .color = _color}),
+                _startItem,
+                nullptr),
+      player(_owner), deleteInPhase(_deleteInPhase)
 {
 }
 
@@ -238,7 +240,7 @@ void ArrowDragItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
         CardZoneLogic *startZone = startCard->getZone();
 
         Command_CreateArrow cmd;
-        cmd.mutable_arrow_color()->CopyFrom(convertQColorToColor(color));
+        cmd.mutable_arrow_color()->CopyFrom(convertQColorToColor(data->color));
         cmd.set_start_player_id(startZone->getPlayer()->getPlayerInfo()->getId());
         cmd.set_start_zone(startZone->getName().toStdString());
         cmd.set_start_card_id(startCard->getId());
@@ -284,7 +286,14 @@ void ArrowDragItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
 
 // ArrowAttachItem
 ArrowAttachItem::ArrowAttachItem(ArrowTarget *_startItem)
-    : ArrowItem(_startItem->getOwner(), -1, _startItem, nullptr, Qt::green)
+    : ArrowItem(
+          QSharedPointer<ArrowData>::create(ArrowData{.creatorId = _startItem->getOwner()->getPlayerInfo()->getId(),
+                                                      .isLocalCreator = true,
+                                                      .id = -1,
+                                                      .color = Qt::green}),
+          _startItem,
+          nullptr),
+      player(_startItem->getOwner())
 {
 }
 

cockatrice/src/game/board/arrow_item.h

@@ -1,20 +1,15 @@
-/**
- * @file arrow_item.h
- * @ingroup GameGraphics
- */
-//! \todo Document this file.
-
 #ifndef ARROWITEM_H
 #define ARROWITEM_H
 
+#include "arrow_data.h"
 #include "arrow_target.h"
 
 #include <QGraphicsItem>
 #include <QPointer>
+#include <QSharedPointer>
 
 class CardItem;
 class QGraphicsSceneMouseEvent;
-class QMenu;
 class PlayerLogic;
 
 class ArrowItem : public QObject, public QGraphicsItem
@@ -22,25 +17,27 @@ class ArrowItem : public QObject, public QGraphicsItem
     Q_OBJECT
     Q_INTERFACES(QGraphicsItem)
 signals:
-    void requestDeletion(int id);
+    void requestDeletion(int creatorId, int id);
 
 private:
     QPainterPath path;
 
 protected:
-    PlayerLogic *player;
-    int id;
+    QSharedPointer<const ArrowData> data;
     QPointer<ArrowTarget> startItem;
     QPointer<ArrowTarget> targetItem;
     bool targetLocked = false;
-    QColor color;
     bool fullColor = true;
 
     void mousePressEvent(QGraphicsSceneMouseEvent *event) override;
 
 public:
-    ArrowItem(PlayerLogic *_player, int _id, ArrowTarget *_startItem, ArrowTarget *_targetItem, const QColor &_color);
+    ArrowItem(QSharedPointer<const ArrowData> _data, ArrowTarget *_startItem, ArrowTarget *_targetItem);
+
     void onTargetDestroyed();
+    void delArrow();
+    void updatePath();
+    void updatePath(const QPointF &endPoint);
 
     void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) override;
     [[nodiscard]] QRectF boundingRect() const override
@@ -51,17 +48,13 @@ public:
     {
         return path;
     }
-
-    void updatePath();
-    void updatePath(const QPointF &endPoint);
-
     [[nodiscard]] int getId() const
     {
-        return id;
+        return data->id;
     }
-    [[nodiscard]] PlayerLogic *getPlayer() const
+    [[nodiscard]] int getCreatorId() const
     {
-        return player;
+        return data->creatorId;
     }
     [[nodiscard]] ArrowTarget *getStartItem() const
     {
@@ -75,14 +68,13 @@ public:
     {
         targetLocked = _targetLocked;
     }
-
-    void delArrow();
 };
 
 class ArrowDragItem : public ArrowItem
 {
     Q_OBJECT
 private:
+    PlayerLogic *player;
     int deleteInPhase;
     QList<ArrowDragItem *> childArrows;
     QMetaObject::Connection positionConnection;
@@ -100,6 +92,7 @@ class ArrowAttachItem : public ArrowItem
 {
     Q_OBJECT
 private:
+    PlayerLogic *player;
     QList<ArrowAttachItem *> childArrows;
     QMetaObject::Connection positionConnection;
     void attachCards(CardItem *startCard, const CardItem *targetCard);
@@ -113,4 +106,4 @@ protected:
     void mouseReleaseEvent(QGraphicsSceneMouseEvent *event) override;
 };
 
-#endif // ARROWITEM_H
+#endif

cockatrice/src/game/game_event_handler.cpp

@@ -213,23 +213,24 @@ void GameEventHandler::handleChatMessageSent(const QString &chatMessage)
     sendGameCommand(cmd);
 }
 
-void GameEventHandler::handleArrowDeletion(int arrowId)
+void GameEventHandler::handleArrowDeletion(int creatorId, int arrowId)
 {
     Command_DeleteArrow cmd;
     cmd.set_arrow_id(arrowId);
 
     auto preparedCommand = prepareGameCommand(cmd);
 
-    connect(preparedCommand, &PendingCommand::finished, this,
-            [arrowId, this](const Response &response) { handleArrowDeletionFinished(response, arrowId); });
+    connect(preparedCommand, &PendingCommand::finished, this, [creatorId, arrowId, this](const Response &response) {
+        handleArrowDeletionFinished(response, creatorId, arrowId);
+    });
 
     sendGameCommand(preparedCommand);
 }
 
-void GameEventHandler::handleArrowDeletionFinished(const Response &response, int arrowId)
+void GameEventHandler::handleArrowDeletionFinished(const Response &response, int creatorId, int arrowId)
 {
     if (response.response_code() == Response::RespNameNotFound) {
-        emit arrowDeleted(arrowId);
+        emit arrowDeleted(creatorId, arrowId);
     }
 }
 

cockatrice/src/game/game_event_handler.h

@@ -60,8 +60,8 @@ public:
     void handleActivePhaseChanged(int phase);
     void handleGameLeft();
     void handleChatMessageSent(const QString &chatMessage);
-    void handleArrowDeletion(int arrowId);
-    void handleArrowDeletionFinished(const Response &response, int arrowId);
+    void handleArrowDeletion(int creatorId, int arrowId);
+    void handleArrowDeletionFinished(const Response &response, int creatorId, int arrowId);
 
     void eventSpectatorSay(const Event_GameSay &event, int eventPlayerId, const GameEventContext &context);
     void eventSpectatorLeave(const Event_Leave &event, int eventPlayerId, const GameEventContext &context);
@@ -113,7 +113,7 @@ signals:
     void containerProcessingStarted(GameEventContext context);
     void setContextJudgeName(QString judgeName);
     void containerProcessingDone();
-    void arrowDeleted(int arrowId);
+    void arrowDeleted(int creatorId, int arrowId);
     void logSpectatorSay(ServerInfo_User userInfo, QString message);
     void logSpectatorLeave(QString name, QString reason);
     void logGameStart();

cockatrice/src/game/game_scene.cpp

@@ -96,8 +96,8 @@ void GameScene::addPlayer(PlayerLogic *player)
     connect(player, &PlayerLogic::arrowDeleted, this, &GameScene::deleteArrow);
     connect(player, &PlayerLogic::arrowCreateRequested, this, &GameScene::addArrow);
     connect(player, &PlayerLogic::arrowDeleteRequested, this, &GameScene::requestArrowDeletion);
-    connect(player, &PlayerLogic::arrowsCleared, this,
-            [this, id = player->getPlayerInfo()->getId()]() { clearArrowsForPlayer(id); });
+    connect(player, &PlayerLogic::arrowsClearedLocally, this,
+            [this, id = player->getPlayerInfo()->getId()]() { clearArrowsForPlayerLocally(id); });
 
     connect(player->getPlayerEventHandler(), &PlayerEventHandler::cardZoneChanged, this, &GameScene::onCardZoneChanged);
 
@@ -367,86 +367,60 @@ void GameScene::resizeColumnsAndPlayers(const QList<qreal> &minWidthByColumn, qr
     }
 }
 
-void GameScene::addArrow(const ArrowData &data)
+void GameScene::addArrow(QSharedPointer<ArrowData> data)
 {
-    auto *startView = playerViews.value(data.startPlayerId);
-    auto *targetView = playerViews.value(data.targetPlayerId);
+    auto *startView = playerViews.value(data->startPlayerId);
+    auto *targetView = playerViews.value(data->targetPlayerId);
     if (!startView || !targetView) {
         return;
     }
 
-    PlayerLogic *startLogic = startView->getPlayer();
-    auto *startZone = startLogic->getZones().value(data.startZone);
+    auto *startZone = startView->getPlayer()->getZones().value(data->startZone);
     if (!startZone) {
         return;
     }
 
-    CardItem *startCard = startZone->getCard(data.startCardId);
+    CardItem *startCard = startZone->getCard(data->startCardId);
     if (!startCard) {
         return;
     }
 
     ArrowTarget *targetItem = nullptr;
-    if (data.isPlayerTargeted()) {
+    if (data->isPlayerTargeted()) {
         targetItem = targetView->getPlayerTarget();
     } else {
-        auto *zone = targetView->getPlayer()->getZones().value(data.targetZone);
-        if (zone) {
-            targetItem = zone->getCard(data.targetCardId);
+        if (auto *zone = targetView->getPlayer()->getZones().value(data->targetZone)) {
+            targetItem = zone->getCard(data->targetCardId);
         }
     }
     if (!targetItem) {
         return;
     }
 
-    auto *arrow = new ArrowItem(startView->getPlayer(), data.id, startCard, targetItem, data.color);
+    auto *arrow = new ArrowItem(data, startCard, targetItem);
     addItem(arrow);
-    arrowRegistry.insert(data.id, arrow);
+    arrowRegistry.insert(data, arrow);
     connect(arrow, &ArrowItem::requestDeletion, this, &GameScene::requestArrowDeletion);
 }
 
-void GameScene::deleteArrow(int arrowId)
+void GameScene::deleteArrow(int playerId, int arrowId)
 {
-    if (arrowRegistry.contains(arrowId)) {
-        arrowRegistry.take(arrowId)->delArrow();
+    if (auto *arrow = arrowRegistry.take(playerId, arrowId)) {
+        arrow->delArrow();
     }
 }
 
-void GameScene::clearArrowsForPlayer(int playerId)
+void GameScene::requestArrowDeletion(int playerId, int arrowId)
 {
-    QList<int> toDelete;
-    for (auto i = arrowRegistry.cbegin(); i != arrowRegistry.cend(); ++i) {
-        auto *arrow = i.value();
-        if (arrow->getPlayer()->getPlayerInfo()->getId() == playerId) {
-            toDelete.append(i.key());
-        }
-    }
-
-    for (int arrowId : toDelete) {
-        arrowRegistry.take(arrowId)->delArrow();
-    }
-}
-
-void GameScene::requestArrowDeletion(int arrowId)
-{
-    if (arrowRegistry.contains(arrowId)) {
-        emit arrowDeletionRequested(arrowId);
-    }
-}
-
-void GameScene::requestClearArrowsForPlayer(int playerId)
-{
-    for (auto *arrow : arrowRegistry.values()) {
-        if (arrow->getPlayer()->getPlayerInfo()->getId() == playerId) {
-            emit requestArrowDeletion(arrow->getId());
-        }
+    if (arrowRegistry.contains(playerId, arrowId)) {
+        emit arrowDeletionRequested(playerId, arrowId);
     }
 }
 
 void GameScene::onCardZoneChanged(CardItem *card, bool sameZone)
 {
     QList<ArrowItem *> toDelete;
-    for (auto *arrow : arrowRegistry.values()) {
+    for (auto *arrow : arrowRegistry.all()) {
         if (arrow->getStartItem() == card || arrow->getTargetItem() == card) {
             if (sameZone) {
                 arrow->updatePath();
@@ -456,7 +430,21 @@ void GameScene::onCardZoneChanged(CardItem *card, bool sameZone)
         }
     }
     for (auto *arrow : toDelete) {
-        deleteArrow(arrow->getId());
+        deleteArrow(arrow->getCreatorId(), arrow->getId());
+    }
+}
+
+void GameScene::clearArrowsForPlayer(int playerId)
+{
+    for (int arrowId : arrowRegistry.idsForPlayer(playerId)) {
+        emit requestArrowDeletion(playerId, arrowId);
+    }
+}
+
+void GameScene::clearArrowsForPlayerLocally(int playerId)
+{
+    for (int arrowId : arrowRegistry.idsForPlayer(playerId)) {
+        arrowRegistry.take(playerId, arrowId)->delArrow();
     }
 }
 

cockatrice/src/game/game_scene.h

@@ -1,6 +1,7 @@
 #ifndef GAMESCENE_H
 #define GAMESCENE_H
 
+#include "arrow_registry.h"
 #include "board/arrow_data.h"
 #include "board/arrow_item.h"
 #include "zones/card_zone_logic.h"
@@ -45,7 +46,7 @@ private:
     PhasesToolbar *phasesToolbar;                       ///< Toolbar showing game phases
     QMap<int, PlayerGraphicsItem *> playerViews;        ///< ID lookup for player graphics items
     QList<QList<PlayerGraphicsItem *>> playersByColumn; ///< Players organized by column
-    QMap<int, ArrowItem *> arrowRegistry;               ///< ID registry for arrow graphics items
+    ArrowRegistry arrowRegistry;                        ///< ID registry for arrow graphics items
     QList<ZoneViewWidget *> zoneViews;                  ///< Active zone view widgets
     QSize viewSize;                                     ///< Current view size
     QPointer<CardItem> hoveredCard;                     ///< Currently hovered card
@@ -202,13 +203,13 @@ public slots:
     QTransform getViewportTransform() const;
 
     /// Directly modifies the scene
-    void addArrow(const ArrowData &data);
-    void deleteArrow(int arrowId);
+    void addArrow(QSharedPointer<ArrowData> data);
+    void deleteArrow(int playerId, int arrowId);
     void clearArrowsForPlayer(int playerId);
+    void clearArrowsForPlayerLocally(int playerId);
 
     /// Queues up arrow deletion but doesn't directly modify the scene
-    void requestArrowDeletion(int arrowId);
-    void requestClearArrowsForPlayer(int playerId);
+    void requestArrowDeletion(int playerId, int arrowId);
 
     void onCardZoneChanged(CardItem *card, bool sameZone);
 
@@ -223,7 +224,7 @@ signals:
     void sigStartRubberBand(const QPointF &selectionOrigin);
     void sigResizeRubberBand(const QPointF &cursorPoint, int selectedCount);
     void sigStopRubberBand();
-    void arrowDeletionRequested(int arrowId);
+    void arrowDeletionRequested(int creatorId, int arrowId);
 };
 
 #endif

cockatrice/src/game/player/player_event_handler.cpp

@@ -92,26 +92,24 @@ void PlayerEventHandler::eventRollDie(const Event_RollDie &event)
 
 void PlayerEventHandler::eventCreateArrow(const Event_CreateArrow &event)
 {
-    const ArrowData data = ArrowData::fromProto(event.arrow_info());
+    auto data = QSharedPointer<ArrowData>::create(ArrowData::fromProto(
+        event.arrow_info(), player->getPlayerInfo()->getId(), player->getPlayerInfo()->getLocal()));
 
-    // Resolve names for logging
     const auto &playerList = player->getGame()->getPlayerManager()->getPlayers();
-    PlayerLogic *startPlayer = playerList.value(data.startPlayerId);
-    PlayerLogic *targetPlayer = playerList.value(data.targetPlayerId);
+    PlayerLogic *startPlayer = playerList.value(data->startPlayerId);
+    PlayerLogic *targetPlayer = playerList.value(data->targetPlayerId);
 
     QString startCardName, targetCardName;
     if (startPlayer) {
-        auto *zone = startPlayer->getZones().value(data.startZone);
-        if (zone) {
-            if (auto *card = zone->getCard(data.startCardId)) {
+        if (auto *zone = startPlayer->getZones().value(data->startZone)) {
+            if (auto *card = zone->getCard(data->startCardId)) {
                 startCardName = card->getName();
             }
         }
     }
-    if (!data.isPlayerTargeted() && targetPlayer) {
-        auto *zone = targetPlayer->getZones().value(data.targetZone);
-        if (zone) {
-            if (auto *card = zone->getCard(data.targetCardId)) {
+    if (!data->isPlayerTargeted() && targetPlayer) {
+        if (auto *zone = targetPlayer->getZones().value(data->targetZone)) {
+            if (auto *card = zone->getCard(data->targetCardId)) {
                 targetCardName = card->getName();
             }
         }
@@ -119,16 +117,15 @@ void PlayerEventHandler::eventCreateArrow(const Event_CreateArrow &event)
 
     emit player->arrowCreateRequested(data);
 
-    const bool validForLogging = !startCardName.isEmpty() && (data.isPlayerTargeted() || !targetCardName.isEmpty());
-
-    if (startPlayer && targetPlayer && validForLogging) {
-        emit logCreateArrow(player, startPlayer, startCardName, targetPlayer, targetCardName, data.isPlayerTargeted());
+    if (startPlayer && targetPlayer && !startCardName.isEmpty() &&
+        (data->isPlayerTargeted() || !targetCardName.isEmpty())) {
+        emit logCreateArrow(player, startPlayer, startCardName, targetPlayer, targetCardName, data->isPlayerTargeted());
     }
 }
 
 void PlayerEventHandler::eventDeleteArrow(const Event_DeleteArrow &event)
 {
-    emit player->arrowDeleted(event.arrow_id());
+    emit player->arrowDeleted(player->getPlayerInfo()->getId(), event.arrow_id());
 }
 
 void PlayerEventHandler::eventCreateToken(const Event_CreateToken &event)

cockatrice/src/game/player/player_logic.cpp

@@ -74,7 +74,7 @@ PlayerLogic::~PlayerLogic()
 
 void PlayerLogic::clear()
 {
-    emit arrowsCleared();
+    emit arrowsClearedLocally();
 
     QMapIterator<QString, CardZoneLogic *> i(zones);
     while (i.hasNext()) {
@@ -115,7 +115,7 @@ void PlayerLogic::processPlayerInfo(const ServerInfo_Player &info)
                                       /* HandZone */
                                       ZoneNames::HAND};
     clearCounters();
-    emit arrowsCleared();
+    emit arrowsClearedLocally();
 
     QMutableMapIterator<QString, CardZoneLogic *> zoneIt(zones);
     while (zoneIt.hasNext()) {
@@ -231,7 +231,8 @@ void PlayerLogic::processCardAttachment(const ServerInfo_Player &info)
 
     const int arrowListSize = info.arrow_list_size();
     for (int i = 0; i < arrowListSize; ++i) {
-        emit arrowCreateRequested(ArrowData::fromProto(info.arrow_list(i)));
+        emit arrowCreateRequested(QSharedPointer<ArrowData>::create(
+            ArrowData::fromProto(info.arrow_list(i), getPlayerInfo()->getId(), getPlayerInfo()->getLocal())));
     }
 }
 

cockatrice/src/game/player/player_logic.h

@@ -78,10 +78,10 @@ signals:
     void clearCustomZonesMenu();
     void addViewCustomZoneActionToCustomZoneMenu(QString zoneName);
     void resetTopCardMenuActions();
-    void arrowCreateRequested(ArrowData data);
-    void arrowDeleteRequested(int arrowId);
-    void arrowDeleted(int arrowId);
-    void arrowsCleared(); // fires on clear() and processPlayerInfo
+    void arrowCreateRequested(QSharedPointer<ArrowData> data);
+    void arrowDeleteRequested(int creatorId, int arrowId);
+    void arrowDeleted(int creatorId, int arrowId);
+    void arrowsClearedLocally(); // fires on clear() and processPlayerInfo
 
 public slots:
     void setActive(bool _active);

cockatrice/src/interface/widgets/tabs/tab_game.cpp

@@ -608,7 +608,7 @@ void TabGame::actRemoveLocalArrows()
 {
     auto *local = game->getPlayerManager()->getActiveLocalPlayer(game->getGameState()->getActivePlayer());
     if (local) {
-        scene->requestClearArrowsForPlayer(local->getPlayerInfo()->getId());
+        scene->clearArrowsForPlayer(local->getPlayerInfo()->getId());
     }
 }
 
@@ -895,11 +895,6 @@ void TabGame::newCardAdded(AbstractCardItem *card)
     connect(card, &AbstractCardItem::showCardInfoPopup, this, &TabGame::showCardInfoPopup);
     connect(card, SIGNAL(deleteCardInfoPopup(QString)), this, SLOT(deleteCardInfoPopup(QString)));
     connect(card, &AbstractCardItem::cardShiftClicked, this, &TabGame::linkCardToChat);
-    CardItem *cardItem = qobject_cast<CardItem *>(card);
-    if (cardItem) {
-        connect(cardItem->getState(), &CardState::zoneChanged, scene,
-                [this, cardItem]() { scene->onCardZoneChanged(cardItem, false); });
-    }
 }
 
 QString TabGame::getTabText() const

libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.cpp

@@ -81,17 +81,6 @@ int Server_AbstractPlayer::newCardId()
     return nextCardId++;
 }
 
-int Server_AbstractPlayer::newArrowId() const
-{
-    int id = 0;
-    for (Server_Arrow *a : arrows) {
-        if (a->getId() > id) {
-            id = a->getId();
-        }
-    }
-    return id + 1;
-}
-
 void Server_AbstractPlayer::setupZones()
 {
     nextCardId = 0;
@@ -1144,7 +1133,7 @@ Server_AbstractPlayer::cmdCreateToken(const Command_CreateToken &cmd, ResponseCo
 
                         Event_CreateArrow createEvent;
                         ServerInfo_Arrow *arrowInfo = createEvent.mutable_arrow_info();
-                        const int newId = player->newArrowId();
+                        const int newId = game->generateArrowId();
                         arrow->setId(newId);
                         arrowInfo->set_id(newId);
                         arrowInfo->set_start_player_id(player->getPlayerId());
@@ -1267,7 +1256,8 @@ Server_AbstractPlayer::cmdCreateArrow(const Command_CreateArrow &cmd, ResponseCo
 
     int currentPhase = game->getActivePhase();
     int deletionPhase = cmd.has_delete_in_phase() ? cmd.delete_in_phase() : currentPhase;
-    auto arrow = new Server_Arrow(newArrowId(), startCard, targetItem, cmd.arrow_color(), currentPhase, deletionPhase);
+    auto arrow = new Server_Arrow(game->generateArrowId(), startCard, targetItem, cmd.arrow_color(), currentPhase,
+                                  deletionPhase);
     addArrow(arrow);
 
     Event_CreateArrow event;

libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.h

@@ -74,7 +74,6 @@ public:
     }
 
     int newCardId();
-    int newArrowId() const;
 
     void addZone(Server_CardZone *zone);
     void addArrow(Server_Arrow *arrow);

libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp

@@ -697,6 +697,11 @@ void Server_Game::setActivePhase(int newPhase)
     sendGameEventContainer(prepareGameEvent(event, -1));
 }
 
+qint64 Server_Game::generateArrowId()
+{
+    return nextArrowId++;
+}
+
 void Server_Game::removeArrows(int newPhase, bool force)
 {
     QMutexLocker locker(&gameMutex);

libcockatrice_network/libcockatrice/network/server/remote/game/server_game.h

@@ -49,6 +49,7 @@ class Server_Game : public QObject
 private:
     Server_Room *room;
     int nextPlayerId;
+    std::atomic<qint64> nextArrowId = 1;
     int hostId;
     ServerInfo_User *creatorInfo;
     QMap<int, Server_AbstractParticipant *> participants;
@@ -196,6 +197,7 @@ public:
     }
     void setActivePlayer(int newPlayer);
     void setActivePhase(int newPhase);
+    qint64 generateArrowId();
     void removeArrows(int newPhase, bool force = false);
     void nextTurn();
     int getSecondsElapsed() const