Inventory: Properly revert client predictions (#8945)

Caused by incremental inventory sending
Previously everything was overwritten by serializing the entire inventory

diff --git a/src/inventory.cpp b/src/inventory.cpp
index 26e6eaa73c46fa1be52abaf62023a7bb87a1b152..77ecf5876bf36500206ba8b7eb5697c0e7a2c033 100644 (file)
--- a/src/inventory.cpp
+++ b/src/inventory.cpp
@@ -785,17 +785,17 @@ Inventory::~Inventory()
 
 void Inventory::clear()
 {
-       m_dirty = true;
        for (auto &m_list : m_lists) {
                delete m_list;
        }
        m_lists.clear();
+       setModified();
 }
 
 Inventory::Inventory(IItemDefManager *itemdef)
 {
-       m_dirty = false;
        m_itemdef = itemdef;
+       setModified();
 }
 
 Inventory::Inventory(const Inventory &other)
@@ -808,12 +808,12 @@ Inventory & Inventory::operator = (const Inventory &other)
        // Gracefully handle self assignment
        if(this != &other)
        {
-               m_dirty = true;
                clear();
                m_itemdef = other.m_itemdef;
                for (InventoryList *list : other.m_lists) {
                        m_lists.push_back(new InventoryList(*list));
                }
+               setModified();
        }
        return *this;
 }
@@ -833,6 +833,7 @@ bool Inventory::operator == (const Inventory &other) const
 
 void Inventory::serialize(std::ostream &os, bool incremental) const
 {
+       //std::cout << "Serialize " << (int)incremental << ", n=" << m_lists.size() << std::endl;
        for (const InventoryList *list : m_lists) {
                if (!incremental || list->checkModified()) {
                        os << "List " << list->getName() << " " << list->getSize() << "\n";
@@ -867,7 +868,7 @@ void Inventory::deSerialize(std::istream &is)
 
                                delete list;
                                list = nullptr;
-                               m_dirty = true;
+                               setModified();
                        }
                        m_lists.erase(std::remove(m_lists.begin(), m_lists.end(),
                                        nullptr), m_lists.end());
@@ -920,7 +921,7 @@ void Inventory::deSerialize(std::istream &is)
 
 InventoryList * Inventory::addList(const std::string &name, u32 size)
 {
-       m_dirty = true;
+       setModified();
        s32 i = getListIndex(name);
        if(i != -1)
        {
@@ -966,7 +967,8 @@ bool Inventory::deleteList(const std::string &name)
        s32 i = getListIndex(name);
        if(i == -1)
                return false;
-       m_dirty = true;
+
+       setModified();
        delete m_lists[i];
        m_lists.erase(m_lists.begin() + i);
        return true;

diff --git a/src/inventory.h b/src/inventory.h
index b7a93553d879aacf3a159a3da51edbf2bc4c4eb6..2828d3e5ad935dd6dc1338afd32975854f920e7e 100644 (file)
--- a/src/inventory.h
+++ b/src/inventory.h
@@ -323,11 +323,14 @@ public:
                return false;
        }
 
-       inline void setModified(bool dirty)
+       inline void setModified(bool dirty = true)
        {
                m_dirty = dirty;
-               for (const auto &list : m_lists)
-                       list->setModified(dirty);
+               // Set all as handled
+               if (!dirty) {
+                       for (const auto &list : m_lists)
+                               list->setModified(dirty);
+               }
        }
 private:
        // -1 if not found

diff --git a/src/inventorymanager.cpp b/src/inventorymanager.cpp
index fccfdea16cbdd77b2247c88fdceaff5630aed0fb..57b561477b03c07020aa92a85676e9e1bea168f9 100644 (file)
--- a/src/inventorymanager.cpp
+++ b/src/inventorymanager.cpp
@@ -348,6 +348,13 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 
        /* If no items will be moved, don't go further */
        if (count == 0) {
+               // Undo client prediction. See 'clientApply'
+               if (from_inv.type == InventoryLocation::PLAYER)
+                       list_from->setModified();
+
+               if (to_inv.type == InventoryLocation::PLAYER)
+                       list_to->setModified();
+
                infostream<<"IMoveAction::apply(): move was completely disallowed:"
                                <<" count="<<old_count
                                <<" from inv=\""<<from_inv.dump()<<"\""
@@ -658,8 +665,10 @@ void IDropAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 
                if (actually_dropped_count == 0) {
                        infostream<<"Actually dropped no items"<<std::endl;
-                       // Revert client prediction
-                       mgr->setInventoryModified(from_inv);
+
+                       // Revert client prediction. See 'clientApply'
+                       if (from_inv.type == InventoryLocation::PLAYER)
+                               list_from->setModified();
                        return;
                }