Revert "fix card moving on server (#4413)"

This reverts commit c25bf491e4.

common/server_cardzone.cpp

@@ -44,8 +44,6 @@ Server_CardZone::~Server_CardZone()
 
 void Server_CardZone::shuffle(int start, int end)
 {
-    cardsBeingLookedAt = 0;
-
     // Size 0 or 1 decks are sorted
     if (cards.size() < 2)
         return;
@@ -123,22 +121,11 @@ void Server_CardZone::insertCardIntoCoordMap(Server_Card *card, int x, int y)
 }
 
 int Server_CardZone::removeCard(Server_Card *card)
-{
-    bool wasLookedAt;
-    return removeCard(card, wasLookedAt);
-}
-
-int Server_CardZone::removeCard(Server_Card *card, bool &wasLookedAt)
 {
     int index = cards.indexOf(card);
-    wasLookedAt = isCardAtPosLookedAt(index);
-    if (wasLookedAt && cardsBeingLookedAt > 0) {
-        cardsBeingLookedAt -= 1;
-    }
     cards.removeAt(index);
-    if (has_coords) {
+    if (has_coords)
         removeCardFromCoordMap(card, card->getX(), card->getY());
-    }
     card->setZone(nullptr);
 
     return index;
@@ -174,11 +161,6 @@ Server_Card *Server_CardZone::getCard(int id, int *position, bool remove)
     }
 }
 
-bool Server_CardZone::isCardAtPosLookedAt(int pos) const
-{
-    return type == ServerInfo_Zone::HiddenZone && (cardsBeingLookedAt == -1 || cardsBeingLookedAt > pos);
-}
-
 int Server_CardZone::getFreeGridColumn(int x, int y, const QString &cardName, bool dontStackSameName) const
 {
     const QMap<int, Server_Card *> &coordMap = coordinateMap.value(y);
@@ -292,11 +274,10 @@ void Server_CardZone::insertCard(Server_Card *card, int x, int y)
         insertCardIntoCoordMap(card, x, y);
     } else {
         card->setCoords(0, 0);
-        if (x == -1) {
+        if (x == -1)
             cards.append(card);
-        } else {
+        else
             cards.insert(x, card);
-        }
     }
     card->setZone(this);
 }
@@ -310,7 +291,6 @@ void Server_CardZone::clear()
     freePilesMap.clear();
     freeSpaceMap.clear();
     playersWithWritePermission.clear();
-    cardsBeingLookedAt = 0;
 }
 
 void Server_CardZone::addWritePermission(int playerId)

common/server_cardzone.h

@@ -37,7 +37,7 @@ class Server_CardZone
 private:
     Server_Player *player;
     QString name;
-    bool has_coords; // having coords means this zone has x and y coordinates
+    bool has_coords;
     ServerInfo_Zone::ZoneType type;
     int cardsBeingLookedAt;
     QSet<int> playersWithWritePermission;
@@ -59,7 +59,6 @@ public:
         return cards;
     }
     int removeCard(Server_Card *card);
-    int removeCard(Server_Card *card, bool &wasLookedAt);
     Server_Card *getCard(int id, int *position = nullptr, bool remove = false);
 
     int getCardsBeingLookedAt() const
@@ -70,7 +69,6 @@ public:
     {
         cardsBeingLookedAt = _cardsBeingLookedAt;
     }
-    bool isCardAtPosLookedAt(int pos) const;
     bool hasCoords() const
     {
         return has_coords;

common/server_player.cpp

@@ -83,25 +83,6 @@
 #include <QDebug>
 #include <algorithm>
 
-struct MoveCardStruct
-{
-    Server_Card *card;
-    int position;
-    const CardToMove *cardToMove;
-    int xCoord, yCoord;
-    MoveCardStruct(Server_Card *_card, int _position, const CardToMove *_cardToMove)
-        : card(_card), position(_position), cardToMove(_cardToMove), xCoord(_card->getX()), yCoord(_card->getY())
-
-    {
-    }
-    bool operator<(const MoveCardStruct &other) const
-    {
-        return (yCoord == other.yCoord &&
-                ((xCoord == other.xCoord && position < other.position) || xCoord < other.xCoord)) ||
-               yCoord < other.yCoord;
-    }
-};
-
 Server_Player::Server_Player(Server_Game *_game,
                              int _playerId,
                              const ServerInfo_User &_userInfo,
@@ -370,12 +351,39 @@ void Server_Player::revealTopCardIfNeeded(Server_CardZone *zone, GameEventStorag
     }
 }
 
+class Server_Player::MoveCardCompareFunctor
+{
+private:
+    int x;
+
+public:
+    explicit MoveCardCompareFunctor(int _x) : x(_x)
+    {
+    }
+    inline bool operator()(QPair<Server_Card *, int> a, QPair<Server_Card *, int> b)
+    {
+        if (a.second < x) {
+            if (b.second >= x) {
+                return false;
+            } else {
+                return (a.second > b.second);
+            }
+        } else {
+            if (b.second < x) {
+                return true;
+            } else {
+                return (a.second < b.second);
+            }
+        }
+    }
+};
+
 Response::ResponseCode Server_Player::moveCard(GameEventStorage &ges,
                                                Server_CardZone *startzone,
                                                const QList<const CardToMove *> &_cards,
                                                Server_CardZone *targetzone,
-                                               int xCoord,
+                                               int x,
-                                               int yCoord,
+                                               int y,
                                                bool fixFreeSpaces,
                                                bool undoingDraw)
 {
@@ -385,11 +393,12 @@ Response::ResponseCode Server_Player::moveCard(GameEventStorage &ges,
         return Response::RespContextError;
     }
 
-    if (!targetzone->hasCoords() && (xCoord <= -1)) {
+    if (!targetzone->hasCoords() && (x <= -1)) {
-        xCoord = targetzone->getCards().size();
+        x = targetzone->getCards().size();
     }
 
-    std::set<MoveCardStruct> cardsToMove;
+    QList<QPair<Server_Card *, int>> cardsToMove;
+    QMap<Server_Card *, const CardToMove *> cardProperties;
     QSet<int> cardIdsToMove;
     for (auto _card : _cards) {
         // The same card being moved twice would lead to undefined behaviour.
@@ -407,30 +416,35 @@ Response::ResponseCode Server_Player::moveCard(GameEventStorage &ges,
         if (card->getParentCard()) {
             continue;
         }
-        if (!card->getAttachedCards().isEmpty() && !targetzone->isColumnEmpty(xCoord, yCoord)) {
+        if (!card->getAttachedCards().isEmpty() && !targetzone->isColumnEmpty(x, y)) {
             continue;
         }
-
+        cardsToMove.append(QPair<Server_Card *, int>(card, position));
-        cardsToMove.insert(MoveCardStruct{card, position, _card});
+        cardProperties.insert(card, _card);
     }
     // In case all moves were filtered out, abort.
-    if (cardsToMove.empty()) {
+    if (cardsToMove.isEmpty()) {
         return Response::RespContextError;
     }
 
+    // 0 performs no sorting
+    // 1 reverses the sorting
+    MoveCardCompareFunctor cmp(0);
+    std::sort(cardsToMove.begin(), cardsToMove.end(), cmp);
+
+    bool secondHalf = false;
     int xIndex = -1;
-    bool revealTopStart = false;
-    bool revealTopTarget = false;
 
-    for (auto cardStruct : cardsToMove) {
+    for (int cardIndex = cardsToMove.size() - 1; cardIndex > -1; --cardIndex) {
-        Server_Card *card = cardStruct.card;
+        Server_Card *card = cardsToMove[cardIndex].first;
-        const CardToMove *thisCardProperties = cardStruct.cardToMove;
+        const CardToMove *thisCardProperties = cardProperties.value(card);
-        int originalPosition = cardStruct.position;
+        bool faceDown = thisCardProperties->has_face_down() ? thisCardProperties->face_down() : card->getFaceDown();
-        bool faceDown = targetzone->hasCoords() &&
+        if (!targetzone->hasCoords()) {
-                        (thisCardProperties->has_face_down() ? thisCardProperties->face_down() : card->getFaceDown());
+            faceDown = false;
+        }
 
-        bool sourceBeingLookedAt;
+        int originalPosition = cardsToMove[cardIndex].second;
-        int position = startzone->removeCard(card, sourceBeingLookedAt);
+        int position = startzone->removeCard(card);
 
         // "Undo draw" should only remain valid if the just-drawn card stays within the user's hand (e.g., they only
         // reorder their hand). If a just-drawn card leaves the hand then remove cards before it from the list
@@ -481,37 +495,71 @@ Response::ResponseCode Server_Player::moveCard(GameEventStorage &ges,
 
             card->deleteLater();
         } else {
-            ++xIndex;
+            if ((startzone == targetzone) && !startzone->hasCoords()) {
-            int newX = xCoord + xIndex;
+                if (!secondHalf && (originalPosition < x)) {
-
+                    xIndex = -1;
-            if (targetzone->hasCoords()) {
+                    secondHalf = true;
-                newX = targetzone->getFreeGridColumn(newX, yCoord, card->getName(), faceDown);
+                } else if (secondHalf) {
-            } else {
+                    --xIndex;
-                yCoord = 0;
-                card->resetState();
-            }
-
-            targetzone->insertCard(card, newX, yCoord);
-            int targetLookedCards = targetzone->getCardsBeingLookedAt();
-            bool sourceKnownToPlayer = sourceBeingLookedAt && !card->getFaceDown();
-            if (targetzone->getType() == ServerInfo_Zone::HiddenZone && targetLookedCards >= newX) {
-                if (sourceKnownToPlayer) {
-                    targetLookedCards += 1;
                 } else {
-                    targetLookedCards = newX;
+                    ++xIndex;
                 }
-                targetzone->setCardsBeingLookedAt(targetLookedCards);
+            } else {
+                ++xIndex;
+            }
+            int newX = x + xIndex;
+
+            if (!targetzone->hasCoords()) {
+                y = 0;
+                card->resetState();
+            } else {
+                newX = targetzone->getFreeGridColumn(newX, y, card->getName(), faceDown);
             }
 
+            targetzone->insertCard(card, newX, y);
+
+            bool targetBeingLookedAt = (targetzone->getType() != ServerInfo_Zone::HiddenZone) ||
+                                       (targetzone->getCardsBeingLookedAt() > newX) ||
+                                       (targetzone->getCardsBeingLookedAt() == -1);
+            bool sourceBeingLookedAt = (startzone->getType() != ServerInfo_Zone::HiddenZone) ||
+                                       (startzone->getCardsBeingLookedAt() > position) ||
+                                       (startzone->getCardsBeingLookedAt() == -1);
+
+            bool targetHiddenToPlayer = faceDown || !targetBeingLookedAt;
             bool targetHiddenToOthers = faceDown || (targetzone->getType() != ServerInfo_Zone::PublicZone);
+            bool sourceHiddenToPlayer = card->getFaceDown() || !sourceBeingLookedAt;
             bool sourceHiddenToOthers = card->getFaceDown() || (startzone->getType() != ServerInfo_Zone::PublicZone);
 
+            QString privateCardName, publicCardName;
+            if (!(sourceHiddenToPlayer && targetHiddenToPlayer)) {
+                privateCardName = card->getName();
+            }
+            if (!(sourceHiddenToOthers && targetHiddenToOthers)) {
+                publicCardName = card->getName();
+            }
+
             int oldCardId = card->getId();
             if ((faceDown && (startzone != targetzone)) || (targetzone->getPlayer() != startzone->getPlayer())) {
                 card->setId(targetzone->getPlayer()->newCardId());
             }
             card->setFaceDown(faceDown);
 
+            // The player does not get to see which card he moved if it moves between two parts of hidden zones which
+            // are not being looked at.
+            int privateNewCardId = card->getId();
+            int privateOldCardId = oldCardId;
+            if (!targetBeingLookedAt && !sourceBeingLookedAt) {
+                privateOldCardId = -1;
+                privateNewCardId = -1;
+                privateCardName = QString();
+            }
+            int privatePosition = -1;
+            if (startzone->getType() == ServerInfo_Zone::HiddenZone) {
+                privatePosition = position;
+            }
+
+            int publicNewX = newX;
+
             Event_MoveCard eventOthers;
             eventOthers.set_start_player_id(startzone->getPlayer()->getPlayerId());
             eventOthers.set_start_zone(startzone->getName().toStdString());
@@ -519,28 +567,16 @@ Response::ResponseCode Server_Player::moveCard(GameEventStorage &ges,
             if (startzone != targetzone) {
                 eventOthers.set_target_zone(targetzone->getName().toStdString());
             }
-            eventOthers.set_y(yCoord);
+            eventOthers.set_y(y);
             eventOthers.set_face_down(faceDown);
 
             Event_MoveCard eventPrivate(eventOthers);
-            if (targetzone->getType() != ServerInfo_Zone::HiddenZone ||
+            eventPrivate.set_card_id(privateOldCardId);
-                (startzone->getType() == ServerInfo_Zone::HiddenZone && sourceBeingLookedAt)) {
+            if (!privateCardName.isEmpty()) {
-                eventPrivate.set_card_id(oldCardId);
-                eventPrivate.set_new_card_id(card->getId());
-            } else {
-                eventPrivate.set_card_id(-1);
-                eventPrivate.set_new_card_id(-1);
-            }
-            if (sourceKnownToPlayer || !(faceDown || targetzone->getType() == ServerInfo_Zone::HiddenZone)) {
-                QString privateCardName = card->getName();
                 eventPrivate.set_card_name(privateCardName.toStdString());
             }
-            if (startzone->getType() == ServerInfo_Zone::HiddenZone) {
+            eventPrivate.set_position(privatePosition);
-                eventPrivate.set_position(position);
+            eventPrivate.set_new_card_id(privateNewCardId);
-            } else {
-                eventPrivate.set_position(-1);
-            }
-
             eventPrivate.set_x(newX);
 
             // Other players do not get to see the start and/or target position of the card if the respective
@@ -549,22 +585,19 @@ Response::ResponseCode Server_Player::moveCard(GameEventStorage &ges,
             if (((startzone->getType() == ServerInfo_Zone::HiddenZone) &&
                  ((startzone->getCardsBeingLookedAt() > position) || (startzone->getCardsBeingLookedAt() == -1))) ||
                 (startzone->getType() == ServerInfo_Zone::PublicZone)) {
-                eventOthers.set_position(-1);
+                position = -1;
-            } else {
-                eventOthers.set_position(position);
             }
             if ((targetzone->getType() == ServerInfo_Zone::HiddenZone) &&
                 ((targetzone->getCardsBeingLookedAt() > newX) || (targetzone->getCardsBeingLookedAt() == -1))) {
-                eventOthers.set_x(-1);
+                publicNewX = -1;
-            } else {
-                eventOthers.set_x(newX);
             }
 
+            eventOthers.set_x(publicNewX);
+            eventOthers.set_position(position);
             if ((startzone->getType() == ServerInfo_Zone::PublicZone) ||
                 (targetzone->getType() == ServerInfo_Zone::PublicZone)) {
                 eventOthers.set_card_id(oldCardId);
-                if (!(sourceHiddenToOthers && targetHiddenToOthers)) {
+                if (!publicCardName.isEmpty()) {
-                    QString publicCardName = card->getName();
                     eventOthers.set_card_name(publicCardName.toStdString());
                 }
                 eventOthers.set_new_card_id(card->getId());
@@ -582,21 +615,14 @@ Response::ResponseCode Server_Player::moveCard(GameEventStorage &ges,
                 setCardAttrHelper(ges, targetzone->getPlayer()->getPlayerId(), targetzone->getName(), card->getId(),
                                   AttrPT, ptString);
             }
-
             if (originalPosition == 0) {
-                revealTopStart = true;
+                revealTopCardIfNeeded(startzone, ges);
             }
             if (newX == 0) {
-                revealTopTarget = true;
+                revealTopCardIfNeeded(targetzone, ges);
             }
         }
     }
-    if (revealTopStart) {
-        revealTopCardIfNeeded(startzone, ges);
-    }
-    if (targetzone != startzone && revealTopTarget) {
-        revealTopCardIfNeeded(targetzone, ges);
-    }
     if (undoingDraw) {
         ges.setGameEventContext(Context_UndoDraw());
     } else {
@@ -1311,26 +1337,26 @@ Server_Player::cmdCreateToken(const Command_CreateToken &cmd, ResponseContainer
     }
 
     QString cardName = QString::fromStdString(cmd.card_name());
-    int xCoord = cmd.x();
+    int x = cmd.x();
-    int yCoord = cmd.y();
+    int y = cmd.y();
     if (zone->hasCoords()) {
-        xCoord = zone->getFreeGridColumn(xCoord, yCoord, cardName, false);
+        x = zone->getFreeGridColumn(x, y, cardName, false);
     }
-    if (xCoord < 0) {
+    if (x < 0) {
-        xCoord = 0;
+        x = 0;
     }
-    if (yCoord < 0) {
+    if (y < 0) {
-        yCoord = 0;
+        y = 0;
     }
 
-    auto *card = new Server_Card(cardName, newCardId(), xCoord, yCoord);
+    auto *card = new Server_Card(cardName, newCardId(), x, y);
     card->moveToThread(thread());
     card->setPT(QString::fromStdString(cmd.pt()));
     card->setColor(QString::fromStdString(cmd.color()));
     card->setAnnotation(QString::fromStdString(cmd.annotation()));
     card->setDestroyOnZoneChange(cmd.destroy_on_zone_change());
 
-    zone->insertCard(card, xCoord, yCoord);
+    zone->insertCard(card, x, y);
 
     Event_CreateToken event;
     event.set_zone_name(zone->getName().toStdString());
@@ -1340,8 +1366,8 @@ Server_Player::cmdCreateToken(const Command_CreateToken &cmd, ResponseContainer
     event.set_pt(card->getPT().toStdString());
     event.set_annotation(card->getAnnotation().toStdString());
     event.set_destroy_on_zone_change(card->getDestroyOnZoneChange());
-    event.set_x(xCoord);
+    event.set_x(x);
-    event.set_y(yCoord);
+    event.set_y(y);
     ges.enqueueGameEvent(event, playerId);
 
     // check if the token is a replacement for an existing card

common/server_player.h

@@ -175,8 +175,8 @@ public:
                                     Server_CardZone *startzone,
                                     const QList<const CardToMove *> &_cards,
                                     Server_CardZone *targetzone,
-                                    int xCoord,
+                                    int x,
-                                    int yCoord,
+                                    int y,
                                     bool fixFreeSpaces = true,
                                     bool undoingDraw = false);
     void unattachCard(GameEventStorage &ges, Server_Card *card);