Fix crash in case of multiple item collection and different

item ids on client and server.

src/items/item_manager.cpp

@@ -226,13 +226,15 @@ unsigned int ItemManager::insertItem(Item *item)
     // previously deleted entry, otherwise at the end.
     int index = -1;
     for(index=(int)m_all_items.size()-1; index>=0 && m_all_items[index]; index--) {}
-
+    if (index == -1)
-    if(index==-1) index = (int)m_all_items.size();
+    {
-
+        index = (int)m_all_items.size();
-    if(index<(int)m_all_items.size())
-        m_all_items[index] = item;
-    else
         m_all_items.push_back(item);
+    }
+    else
+    {
+        m_all_items[index] = item;
+    }
     item->setItemId(index);
 
     // Now insert into the appropriate quad list, if there is a quad list
@@ -352,7 +354,7 @@ Item* ItemManager::placeTrigger(const Vec3& xyz, float distance,
  *  \param item The item that was collected.
  *  \param kart The kart that collected the item.
  */
-void ItemManager::collectedItem(Item *item, AbstractKart *kart)
+void ItemManager::collectedItem(ItemState *item, AbstractKart *kart)
 {
     assert(item);
     // Spare tire karts don't collect items

src/items/item_manager.hpp

@@ -133,7 +133,7 @@ public:
     void           updateGraphics  (float dt);
     void           checkItemHit    (AbstractKart* kart);
     void           reset           ();
-    virtual void   collectedItem   (Item *item, AbstractKart *kart);
+    virtual void   collectedItem   (ItemState *item, AbstractKart *kart);
     void           switchItems     ();
     bool           randomItemsForArena(const AlignedArray<btTransform>& pos);
     // ------------------------------------------------------------------------

src/items/network_item_manager.cpp

@@ -95,7 +95,7 @@ void NetworkItemManager::initClientConfirmState()
  *  \param item The item that was collected.
  *  \param kart The kart that collected the item.
  */
-void NetworkItemManager::collectedItem(Item *item, AbstractKart *kart)
+void NetworkItemManager::collectedItem(ItemState *item, AbstractKart *kart)
 {
     if(NetworkConfig::get()->isServer())
     {
@@ -142,7 +142,7 @@ Item* NetworkItemManager::dropNewItem(ItemState::ItemType type,
                                          kart->getXYZ() );
     m_item_events.unlock();
     return item;
-}   // newItem
+}   // dropNewItem
 
 // ----------------------------------------------------------------------------
 /** Called by the GameProtocol when a confirmation for an item event is
@@ -237,9 +237,9 @@ void NetworkItemManager::forwardTime(int ticks)
 
 //-----------------------------------------------------------------------------
 /** Restores the state of the items to the current world time. It takes the
- *  last saved
+ *  last saved confirmed state, applies any updates from the server, and
-
+ *  then syncs up the confirmed state to the in-race items.
- *  using exactly 'count' bytes of the message.
+ *  It uses exactly 'count' bytes of the message.
  *  \param buffer the state content.
  *  \param count Number of bytes used for this state.
  */
@@ -299,13 +299,14 @@ void NetworkItemManager::restoreState(BareNetworkString *buffer, int count)
         // Forward the saved state:
         if (dt>0) forwardTime(dt);
 
-        // TODO: apply the various events types, atm only collection is supported:
+        // TODO: apply the various events types, atm only collection
+        // and new items are supported.
         if(iei.isItemCollection())
         {
             int index = iei.getIndex();
             // An item on the track was collected:
             AbstractKart *kart = World::getWorld()->getKart(iei.getKartId());
-            m_confirmed_state[index]->collected(kart);
+            collectedItem(m_confirmed_state[index], kart);
             if (m_confirmed_state[index]->isUsedUp())
             {
                 delete m_confirmed_state[index];
@@ -327,7 +328,10 @@ void NetworkItemManager::restoreState(BareNetworkString *buffer, int count)
                 if (m_confirmed_state[is->getItemId()] == NULL)
                     m_confirmed_state[is->getItemId()] = is;
                 else
+                {
                     *m_confirmed_state[is->getItemId()] = *is;
+                    delete is;
+                }
             }
         }
         current_time = iei.getTicks();
@@ -351,7 +355,7 @@ void NetworkItemManager::restoreState(BareNetworkString *buffer, int count)
 
     for(unsigned int i=0; i<m_confirmed_state.size(); i++)
     {
-        Item *item = m_all_items[i];
+        Item *item = i < m_all_items.size() ?  m_all_items[i] : NULL;
         const ItemState *is = m_confirmed_state[i];
         if (is && item)
             *(ItemState*)item = *is;
@@ -360,10 +364,21 @@ void NetworkItemManager::restoreState(BareNetworkString *buffer, int count)
             Vec3 xyz = is->getXYZ();
             Item *item_new = dropNewItem(is->getType(), is->getPreviousOwner(),
                                          &xyz);
-            item_new->setPredicted(false);
+            if (i != item_new->getItemId())
-            item_new->setItemId(i);
+            {
-            item_new->setDeactivatedTicks(is->getDeactivatedTicks());
+                // The server has this item at a different index in the list
+                // of all items, which means the client has an incorrect
+                // item at the index given by the server - delete that item
+                Log::info("nim", "about to delete item with not matching index i %d item %d",
+                    i, item_new->getItemId());
+                if(m_all_items[i])
+                    deleteItem(m_all_items[i]);
+                m_all_items[item_new->getItemId()] = NULL;
                 m_all_items[i] = item_new;
+                item_new->setItemId(i);
+            }
+            item_new->setPredicted(false);
+            item_new->setDeactivatedTicks(is->getDeactivatedTicks());
             *((ItemState*)m_all_items[i]) = *is;
         }
         else if (!is && item)

src/items/network_item_manager.hpp

@@ -73,7 +73,7 @@ public:
     virtual void reset() OVERRIDE;
     virtual void setItemConfirmationTime(std::weak_ptr<STKPeer> peer,
                                          int ticks) OVERRIDE;
-    virtual void collectedItem(Item *item, AbstractKart *kart) OVERRIDE;
+    virtual void collectedItem(ItemState *item, AbstractKart *kart) OVERRIDE;
     virtual Item* dropNewItem(ItemState::ItemType type, const AbstractKart *kart,
                               const Vec3 *xyz=NULL) OVERRIDE;
     virtual BareNetworkString* saveState(std::vector<std::string>* ru)

src/modes/easter_egg_hunt.cpp

@@ -159,7 +159,8 @@ const std::string& EasterEggHunt::getIdent() const
 /** Called when a kart has collected an egg.
  *  \param kart The kart that collected an egg.
  */
-void EasterEggHunt::collectedItem(const AbstractKart *kart, const Item *item)
+void EasterEggHunt::collectedItem(const AbstractKart *kart,
+                                  const ItemState *item    )
 {
     if(item->getType() != ItemState::ITEM_EASTER_EGG) return;
 

src/modes/easter_egg_hunt.hpp

@@ -66,7 +66,7 @@ public:
     virtual void getKartsDisplayInfo(
                  std::vector<RaceGUIBase::KartIconDisplayInfo> *info) OVERRIDE;
     virtual void collectedItem(const AbstractKart *kart,
-                               const Item *item           ) OVERRIDE;
+                               const ItemState *item     ) OVERRIDE;
     void collectedEasterEggGhost(int world_id);
 
     const int  numberOfEggsFound() { return m_eggs_found; }

src/modes/world.hpp

@@ -42,7 +42,7 @@
 class AbstractKart;
 class btRigidBody;
 class Controller;
-class Item;
+class ItemState;
 class PhysicalObject;
 
 namespace Scripting
@@ -263,7 +263,8 @@ public:
                                            int *amount );
     // ------------------------------------------------------------------------
     /** Receives notification if an item is collected. Used for easter eggs. */
-    virtual void collectedItem(const AbstractKart *kart, const Item *item) {}
+    virtual void collectedItem(const AbstractKart *kart, 
+                               const ItemState *item    ) {}
     // ------------------------------------------------------------------------
     virtual void endRaceEarly() { return; }
     // ------------------------------------------------------------------------