Client-side prediction of inventory changes, and some inventory menu fixes
authorKahrl <kahrl@gmx.net>
Sat, 21 Jan 2012 23:49:02 +0000 (00:49 +0100)
committerPerttu Ahola <celeron55@gmail.com>
Sun, 22 Jan 2012 15:31:20 +0000 (17:31 +0200)
src/client.cpp
src/client.h
src/guiInventoryMenu.cpp
src/inventory.cpp
src/inventory.h
src/inventorymanager.cpp
src/inventorymanager.h
src/server.cpp

index 602f0cf84abf44b4c444e681e2615c5724f21be5..0463aa81c9cf5f47f56bdeefb469267465829d01 100644 (file)
@@ -225,6 +225,8 @@ Client::Client(
        m_server_ser_ver(SER_FMT_VER_INVALID),
        m_playeritem(0),
        m_inventory_updated(false),
+       m_inventory_from_server(NULL),
+       m_inventory_from_server_age(0.0),
        m_time_of_day(0),
        m_map_seed(0),
        m_password(password),
@@ -269,6 +271,8 @@ Client::~Client()
        m_mesh_update_thread.setRun(false);
        while(m_mesh_update_thread.IsRunning())
                sleep_ms(100);
+
+       delete m_inventory_from_server;
 }
 
 void Client::connect(Address address)
@@ -657,6 +661,30 @@ void Client::step(float dtime)
                        }
                }
        }
+
+       /*
+               If the server didn't update the inventory in a while, revert
+               the local inventory (so the player notices the lag problem
+               and knows something is wrong).
+       */
+       if(m_inventory_from_server)
+       {
+               float interval = 10.0;
+               float count_before = floor(m_inventory_from_server_age / interval);
+
+               m_inventory_from_server_age += dtime;
+
+               float count_after = floor(m_inventory_from_server_age / interval);
+
+               if(count_after != count_before)
+               {
+                       // Do this every <interval> seconds after TOCLIENT_INVENTORY
+                       // Reset the locally changed inventory to the authoritative inventory
+                       Player *player = m_env.getLocalPlayer();
+                       player->inventory = *m_inventory_from_server;
+                       m_inventory_updated = true;
+               }
+       }
 }
 
 // Virtual methods from con::PeerHandler
@@ -975,6 +1003,10 @@ void Client::ProcessData(u8 *data, u32 datasize, u16 sender_peer_id)
 
                        m_inventory_updated = true;
 
+                       delete m_inventory_from_server;
+                       m_inventory_from_server = new Inventory(player->inventory);
+                       m_inventory_from_server_age = 0.0;
+
                        //infostream<<"Client got player inventory:"<<std::endl;
                        //player->inventory.print(infostream);
                }
@@ -1931,7 +1963,15 @@ Inventory* Client::getInventory(const InventoryLocation &loc)
 }
 void Client::inventoryAction(InventoryAction *a)
 {
+       /*
+               Send it to the server
+       */
        sendInventoryAction(a);
+
+       /*
+               Predict some local inventory changes
+       */
+       a->clientApply(this, this);
 }
 
 ClientActiveObject * Client::getSelectedActiveObject(
index 6d5e5c52503eaca3a0b418c76be79eb3e1f7905c..efdf315f7d78c73bb2f2765c5b0fa9cc9a534493 100644 (file)
@@ -367,6 +367,8 @@ private:
        u8 m_server_ser_ver;
        u16 m_playeritem;
        bool m_inventory_updated;
+       Inventory *m_inventory_from_server;
+       float m_inventory_from_server_age;
        core::map<v3s16, bool> m_active_blocks;
        PacketCounter m_packetcounter;
        // Received from the server. 0-23999
index a4b16b25168e020bf7938abee6d9459b8df8edcd..cd371d0628ecb6e8d8123a2b3ad2d3d8db9aba3a 100644 (file)
@@ -537,6 +537,11 @@ bool GUIInventoryMenu::OnEvent(const SEvent& event)
                                s.i = -1;  // make it invalid again
                }
 
+               bool identical = (m_selected_item != NULL) && s.isValid() &&
+                       (inv_selected == inv_s) &&
+                       (m_selected_item->listname == s.listname) &&
+                       (m_selected_item->i == s.i);
+
                // buttons: 0 = left, 1 = right, 2 = middle
                // up/down: 0 = down (press), 1 = up (release), 2 = unknown event
                int button = 0;
@@ -602,13 +607,26 @@ bool GUIInventoryMenu::OnEvent(const SEvent& event)
 
                                if(s.isValid())
                                {
-                                       // Clicked another slot: move
+                                       // Clicked a slot: move
                                        if(button == 1)  // right
                                                move_amount = 1;
                                        else if(button == 2)  // middle
                                                move_amount = MYMIN(m_selected_amount, 10);
                                        else  // left
                                                move_amount = m_selected_amount;
+                                       dstream << "move_amount=" << move_amount<<"\n";
+                                       dstream << "m_selected_amount=" << m_selected_amount<<"\n";
+
+                                       if(identical)
+                                       {
+                                               if(move_amount >= m_selected_amount)
+                                                       m_selected_amount = 0;
+                                               else
+                                                       m_selected_amount -= move_amount;
+                                               move_amount = 0;
+                                       }
+                                       dstream << "move_amount=" << move_amount<<"\n";
+                                       dstream << "m_selected_amount=" << m_selected_amount<<"\n";
                                }
                                else if(getAbsoluteClippingRect().isPointInside(m_pointer))
                                {
@@ -636,9 +654,7 @@ bool GUIInventoryMenu::OnEvent(const SEvent& event)
 
                        if(m_selected_item != NULL && m_selected_dragging && s.isValid())
                        {
-                               if((inv_selected != inv_s) ||
-                                       (m_selected_item->listname != s.listname) ||
-                                       (m_selected_item->i != s.i))
+                               if(!identical)
                                {
                                        // Dragged to different slot: move all selected
                                        move_amount = m_selected_amount;
@@ -675,18 +691,19 @@ bool GUIInventoryMenu::OnEvent(const SEvent& event)
                        if(leftover.count == stack_from.count)
                        {
                                // Swap the stacks
+                               m_selected_amount -= stack_to.count;
                        }
                        else if(leftover.empty())
                        {
                                // Item fits
+                               m_selected_amount -= move_amount;
                        }
                        else
                        {
                                // Item only fits partially
                                move_amount -= leftover.count;
+                               m_selected_amount -= move_amount;
                        }
-                       assert(move_amount > 0 && move_amount <= m_selected_amount);
-                       m_selected_amount -= move_amount;
 
                        infostream<<"Handing IACTION_MOVE to manager"<<std::endl;
                        IMoveAction *a = new IMoveAction();
@@ -741,6 +758,7 @@ bool GUIInventoryMenu::OnEvent(const SEvent& event)
                {
                        delete m_selected_item;
                        m_selected_item = NULL;
+                       m_selected_amount = 0;
                        m_selected_dragging = false;
                }
        }
index ebd0b9c23b471bb579e8aaf27c5083c6e99fb4a6..66101b831b3de0c4b19edf288beb8e2ef2a2c265 100644 (file)
@@ -733,6 +733,49 @@ ItemStack InventoryList::peekItem(u32 i, u32 peekcount) const
        return m_items[i].peekItem(peekcount);
 }
 
+void InventoryList::moveItem(u32 i, InventoryList *dest, u32 dest_i, u32 count)
+{
+       if(this == dest && i == dest_i)
+               return;
+
+       // Take item from source list
+       ItemStack item1;
+       if(count == 0)
+               item1 = changeItem(i, ItemStack());
+       else
+               item1 = takeItem(i, count);
+
+       if(item1.empty())
+               return;
+
+       // Try to add the item to destination list
+       u32 oldcount = item1.count;
+       item1 = dest->addItem(dest_i, item1);
+
+       // If something is returned, the item was not fully added
+       if(!item1.empty())
+       {
+               // If olditem is returned, nothing was added.
+               bool nothing_added = (item1.count == oldcount);
+
+               // If something else is returned, part of the item was left unadded.
+               // Add the other part back to the source item
+               addItem(i, item1);
+
+               // If olditem is returned, nothing was added.
+               // Swap the items
+               if(nothing_added)
+               {
+                       // Take item from source list
+                       item1 = changeItem(i, ItemStack());
+                       // Adding was not possible, swap the items.
+                       ItemStack item2 = dest->changeItem(dest_i, item1);
+                       // Put item from destination list to the source list
+                       changeItem(i, item2);
+               }
+       }
+}
+
 /*
        Inventory
 */
index 3f5d835895f65130fa31646daedd932652b8db14..fcac5f647153cb9ecbf110ebe9888ac066610f67 100644 (file)
@@ -233,6 +233,10 @@ public:
        // Similar to takeItem, but keeps the slot intact.
        ItemStack peekItem(u32 i, u32 peekcount) const;
 
+       // Move an item to a different list (or a different stack in the same list)
+       // count is the maximum number of items to move (0 for everything)
+       void moveItem(u32 i, InventoryList *dest, u32 dest_i, u32 count = 0);
+
 private:
        std::vector<ItemStack> m_items;
        u32 m_size;
index b42a80673574b15398d14b4670fe606f3c3a90d6..b04a1c1777522aec1af58e6399d78c796d4dcb23 100644 (file)
@@ -133,6 +133,10 @@ InventoryAction * InventoryAction::deSerialize(std::istream &is)
        return a;
 }
 
+/*
+       IMoveAction
+*/
+
 IMoveAction::IMoveAction(std::istream &is)
 {
        std::string ts;
@@ -193,59 +197,13 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
                                <<", to_list=\""<<to_list<<"\""<<std::endl;
                return;
        }
-       if(list_from->getItem(from_i).empty())
-       {
-               infostream<<"IMoveAction::apply(): FAIL: source item not found: "
-                               <<"from_inv=\""<<from_inv.dump()<<"\""
-                               <<", from_list=\""<<from_list<<"\""
-                               <<" from_i="<<from_i<<std::endl;
-               return;
-       }
        /*
-               If the source and the destination slots are the same
-       */
-       if(inv_from == inv_to && list_from == list_to && from_i == to_i)
-       {
-               infostream<<"IMoveAction::apply(): FAIL: source and destination slots "
-                               <<"are the same: inv=\""<<from_inv.dump()
-                               <<"\" list=\""<<from_list
-                               <<"\" i="<<from_i<<std::endl;
-               return;
-       }
-       
-       // Take item from source list
-       ItemStack item1;
-       if(count == 0)
-               item1 = list_from->changeItem(from_i, ItemStack());
-       else
-               item1 = list_from->takeItem(from_i, count);
+               This performs the actual movement
 
-       // Try to add the item to destination list
-       int oldcount = item1.count;
-       item1 = list_to->addItem(to_i, item1);
-
-       // If something is returned, the item was not fully added
-       if(!item1.empty())
-       {
-               // If olditem is returned, nothing was added.
-               bool nothing_added = (item1.count == oldcount);
-               
-               // If something else is returned, part of the item was left unadded.
-               // Add the other part back to the source item
-               list_from->addItem(from_i, item1);
-
-               // If olditem is returned, nothing was added.
-               // Swap the items
-               if(nothing_added)
-               {
-                       // Take item from source list
-                       item1 = list_from->changeItem(from_i, ItemStack());
-                       // Adding was not possible, swap the items.
-                       ItemStack item2 = list_to->changeItem(to_i, item1);
-                       // Put item from destination list to the source list
-                       list_from->changeItem(from_i, item2);
-               }
-       }
+               If something is wrong (source item is empty, destination is the
+               same as source), nothing happens
+       */
+       list_from->moveItem(from_i, list_to, to_i, count);
 
        mgr->setInventoryModified(from_inv);
        if(inv_from != inv_to)
@@ -262,6 +220,38 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
                        <<std::endl;
 }
 
+void IMoveAction::clientApply(InventoryManager *mgr, IGameDef *gamedef)
+{
+       // Optional InventoryAction operation that is run on the client
+       // to make lag less apparent.
+
+       Inventory *inv_from = mgr->getInventory(from_inv);
+       Inventory *inv_to = mgr->getInventory(to_inv);
+       if(!inv_from || !inv_to)
+               return;
+
+       InventoryLocation current_player;
+       current_player.setCurrentPlayer();
+       Inventory *inv_player = mgr->getInventory(current_player);
+       if(inv_from != inv_player || inv_to != inv_player)
+               return;
+
+       InventoryList *list_from = inv_from->getList(from_list);
+       InventoryList *list_to = inv_to->getList(to_list);
+       if(!list_from || !list_to)
+               return;
+
+       list_from->moveItem(from_i, list_to, to_i, count);
+
+       mgr->setInventoryModified(from_inv);
+       if(inv_from != inv_to)
+               mgr->setInventoryModified(to_inv);
+}
+
+/*
+       IDropAction
+*/
+
 IDropAction::IDropAction(std::istream &is)
 {
        std::string ts;
@@ -338,6 +328,37 @@ void IDropAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
                        <<std::endl;
 }
 
+void IDropAction::clientApply(InventoryManager *mgr, IGameDef *gamedef)
+{
+       // Optional InventoryAction operation that is run on the client
+       // to make lag less apparent.
+
+       Inventory *inv_from = mgr->getInventory(from_inv);
+       if(!inv_from)
+               return;
+
+       InventoryLocation current_player;
+       current_player.setCurrentPlayer();
+       Inventory *inv_player = mgr->getInventory(current_player);
+       if(inv_from != inv_player)
+               return;
+
+       InventoryList *list_from = inv_from->getList(from_list);
+       if(!list_from)
+               return;
+
+       if(count == 0)
+               list_from->changeItem(from_i, ItemStack());
+       else
+               list_from->takeItem(from_i, count);
+
+       mgr->setInventoryModified(from_inv);
+}
+
+/*
+       ICraftAction
+*/
+
 ICraftAction::ICraftAction(std::istream &is)
 {
        std::string ts;
@@ -412,6 +433,12 @@ void ICraftAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGam
                        <<std::endl;
 }
 
+void ICraftAction::clientApply(InventoryManager *mgr, IGameDef *gamedef)
+{
+       // Optional InventoryAction operation that is run on the client
+       // to make lag less apparent.
+}
+
 
 // Crafting helper
 bool getCraftingResult(Inventory *inv, ItemStack& result,
index 890d05168dd259d53c69859c79b82426a35c7931..55e8f840215b9345e3b5e87f5aaecef28b139d32 100644 (file)
@@ -102,6 +102,7 @@ struct InventoryAction
        virtual void serialize(std::ostream &os) const = 0;
        virtual void apply(InventoryManager *mgr, ServerActiveObject *player,
                        IGameDef *gamedef) = 0;
+       virtual void clientApply(InventoryManager *mgr, IGameDef *gamedef) = 0;
 };
 
 struct IMoveAction : public InventoryAction
@@ -142,6 +143,8 @@ struct IMoveAction : public InventoryAction
        }
 
        void apply(InventoryManager *mgr, ServerActiveObject *player, IGameDef *gamedef);
+
+       void clientApply(InventoryManager *mgr, IGameDef *gamedef);
 };
 
 struct IDropAction : public InventoryAction
@@ -175,6 +178,8 @@ struct IDropAction : public InventoryAction
        }
 
        void apply(InventoryManager *mgr, ServerActiveObject *player, IGameDef *gamedef);
+
+       void clientApply(InventoryManager *mgr, IGameDef *gamedef);
 };
 
 struct ICraftAction : public InventoryAction
@@ -203,6 +208,8 @@ struct ICraftAction : public InventoryAction
        }
 
        void apply(InventoryManager *mgr, ServerActiveObject *player, IGameDef *gamedef);
+
+       void clientApply(InventoryManager *mgr, IGameDef *gamedef);
 };
 
 // Crafting helper
index 177551881173e8d29ad5d4b3d5cac13f11bd37b7..a0c8a00928011740af8f9e862f7c03b6d3556871 100644 (file)
@@ -2372,6 +2372,11 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
                        return;
                }
 
+               /*
+                       Note: Always set inventory not sent, to repair cases
+                       where the client made a bad prediction.
+               */
+
                /*
                        Handle restrictions and special cases of the move action
                */
@@ -2382,6 +2387,9 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
                        ma->from_inv.applyCurrentPlayer(player->getName());
                        ma->to_inv.applyCurrentPlayer(player->getName());
 
+                       setInventoryModified(ma->from_inv);
+                       setInventoryModified(ma->to_inv);
+
                        bool from_inv_is_current_player =
                                (ma->from_inv.type == InventoryLocation::PLAYER) &&
                                (ma->from_inv.name == player->getName());
@@ -2461,6 +2469,8 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 
                        da->from_inv.applyCurrentPlayer(player->getName());
 
+                       setInventoryModified(da->from_inv);
+
                        // Disallow dropping items if not allowed to interact
                        if((getPlayerPrivs(player) & PRIV_INTERACT) == 0)
                        {
@@ -2490,6 +2500,8 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 
                        ca->craft_inv.applyCurrentPlayer(player->getName());
 
+                       setInventoryModified(ca->craft_inv);
+
                        //bool craft_inv_is_current_player =
                        //      (ca->craft_inv.type == InventoryLocation::PLAYER) &&
                        //      (ca->craft_inv.name == player->getName());