Inventory: Properly revert client predictions (#8945)

Caused by incremental inventory sending
Previously everything was overwritten by serializing the entire inventory
This commit is contained in:
SmallJoker 2019-09-18 18:47:09 +02:00 committed by GitHub
parent 05a7da6279
commit 94a5df795c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 25 additions and 11 deletions

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

@ -323,11 +323,14 @@ public:
return false; return false;
} }
inline void setModified(bool dirty) inline void setModified(bool dirty = true)
{ {
m_dirty = dirty; m_dirty = dirty;
for (const auto &list : m_lists) // Set all as handled
list->setModified(dirty); if (!dirty) {
for (const auto &list : m_lists)
list->setModified(dirty);
}
} }
private: private:
// -1 if not found // -1 if not found

@ -348,6 +348,13 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
/* If no items will be moved, don't go further */ /* If no items will be moved, don't go further */
if (count == 0) { 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:" infostream<<"IMoveAction::apply(): move was completely disallowed:"
<<" count="<<old_count <<" count="<<old_count
<<" from inv=\""<<from_inv.dump()<<"\"" <<" from inv=\""<<from_inv.dump()<<"\""
@ -658,8 +665,10 @@ void IDropAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
if (actually_dropped_count == 0) { if (actually_dropped_count == 0) {
infostream<<"Actually dropped no items"<<std::endl; 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; return;
} }