Item system update

- Set ITEM_FIELD_CREATE_PLAYED_TIME to 0 in Item::Create, should fix crash described in issue 1199
- Change some function calls so on Player::_saveInventory the core doesn't have to check for existance of item anymore when iterating over m_refundableItems. The loop will now only update played time, removal of items from this set is done in other functions.
- Cleanup in affected functions.

--HG--
branch : trunk

src/game/Item.cpp

@@ -248,7 +248,10 @@ Item::Item( )
     m_lootGenerated = false;
     mb_in_trade = false;
     m_lastPlayedTimeUpdate = time(NULL);
-    m_RefundData = NULL;
+    
+    m_refundRecipient = 0;
+    m_paidMoney = 0;
+    m_paidExtendedCost = 0;
 }
 
 bool Item::Create( uint32 guidlow, uint32 itemid, Player const* owner)
@@ -274,6 +277,8 @@ bool Item::Create( uint32 guidlow, uint32 itemid, Player const* owner)
 
     SetUInt32Value(ITEM_FIELD_FLAGS, itemProto->Flags);
     SetUInt32Value(ITEM_FIELD_DURATION, abs(itemProto->Duration));
+    
+    SetUInt32Value(ITEM_FIELD_CREATE_PLAYED_TIME, 0);
 
     return true;
 }
@@ -330,8 +335,6 @@ void Item::SaveToDB()
             CharacterDatabase.PExecute("DELETE FROM item_instance WHERE guid = '%u'", guid);
             if (HasFlag(ITEM_FIELD_FLAGS, ITEM_FLAGS_WRAPPED))
                 CharacterDatabase.PExecute("DELETE FROM character_gifts WHERE item_guid = '%u'", GetGUIDLow());
-            if (HasFlag(ITEM_FIELD_FLAGS, ITEM_FLAGS_REFUNDABLE))
-                DeleteRefundDataFromDB();
             delete this;
             return;
         }
@@ -1050,16 +1053,12 @@ void Item::BuildUpdate(UpdateDataMapType& data_map)
 
 void Item::SaveRefundDataToDB()
 {
-    ItemRefund* RefundData = GetRefundData();
-    if (!RefundData)
-        return;
-
     std::ostringstream ss;
     ss << "INSERT INTO item_refund_instance VALUES(";
     ss << GetGUIDLow() << ",";
-    ss << RefundData->eligibleFor << ",";
+    ss << GetRefundRecipient() << ",";
-    ss << RefundData->paidMoney << ",";
+    ss << GetPaidMoney() << ",";
-    ss << RefundData->paidExtendedCost;
+    ss << GetPaidExtendedCost();
     ss << ")";
     
     CharacterDatabase.Execute(ss.str().c_str());
@@ -1075,30 +1074,53 @@ void Item::SetNotRefundable(Player *owner)
     if (!HasFlag(ITEM_FIELD_FLAGS, ITEM_FLAGS_REFUNDABLE))
         return;
 
-    if (m_RefundData)
-        delete m_RefundData;
-        
     RemoveFlag(ITEM_FIELD_FLAGS, ITEM_FLAGS_REFUNDABLE);
     SetState(ITEM_CHANGED, owner);
-    DeleteRefundDataFromDB();
+    
+    SetRefundRecipient(0);
+    SetPaidMoney(0);
+    SetPaidExtendedCost(0);
+    DeleteRefundDataFromDB();   
+    
     owner->DeleteRefundReference(this);
 }
 
 void Item::UpdatePlayedTime(Player *owner)
 {
+    /*  Here we update our played time
+        We simply add a number to the current played time,
+        based on the time elapsed since the last update hereof.
+    */
+    // Get current played time
+    uint32 current_playtime = GetUInt32Value(ITEM_FIELD_CREATE_PLAYED_TIME);
+    // Calculate time elapsed since last played time update
     time_t curtime = time(NULL);
     uint32 elapsed = curtime - m_lastPlayedTimeUpdate;
-    SetUInt32Value(ITEM_FIELD_CREATE_PLAYED_TIME, GetPlayedTime(true) + elapsed);
+    uint32 new_playtime = current_playtime + elapsed;
-    SetState(ITEM_CHANGED, owner);
+    // Check if the refund timer has expired yet
-    m_lastPlayedTimeUpdate = curtime;
+    if (new_playtime <= 2*HOUR)
+    {
+        // No? Proceed.
+        // Update the data field
+        SetUInt32Value(ITEM_FIELD_CREATE_PLAYED_TIME, new_playtime);
+        // Flag as changed to get saved to DB
+        SetState(ITEM_CHANGED, owner);
+        // Speaks for itself
+        m_lastPlayedTimeUpdate = curtime;
+        return;
+    }
+    // Yes
+    SetNotRefundable(owner);
 }
 
-uint32 Item::GetPlayedTime(bool raw)
+uint32 Item::GetPlayedTime()
 {
-    if (raw)
-        return GetUInt32Value(ITEM_FIELD_CREATE_PLAYED_TIME);
-        
     time_t curtime = time(NULL);
     uint32 elapsed = curtime - m_lastPlayedTimeUpdate;
-    return uint32(GetUInt32Value(ITEM_FIELD_CREATE_PLAYED_TIME) + elapsed);
+    return GetUInt32Value(ITEM_FIELD_CREATE_PLAYED_TIME) + elapsed;
+}
+
+bool Item::IsRefundExpired()
+{
+    return (GetPlayedTime() > 2*HOUR);
 }

src/game/Item.h

@@ -218,14 +218,6 @@ struct ItemRequiredTarget
 
 bool ItemCanGoIntoBag(ItemPrototype const *proto, ItemPrototype const *pBagProto);
 
-struct ItemRefund
-{
-    ItemRefund() : paidMoney(0), paidExtendedCost(0) {}
-    uint32 eligibleFor;
-    uint32 paidMoney;
-    uint32 paidExtendedCost;
-};
-
 class Item : public Object
 {
     public:
@@ -331,11 +323,17 @@ class Item : public Object
         bool IsConjuredConsumable() const { return GetProto()->IsConjuredConsumable(); }
 
         // Item Refund system
-        ItemRefund *GetRefundData() { return m_RefundData; }
-        void SetRefundData(ItemRefund *RefundData) { m_RefundData = RefundData; }
         void SetNotRefundable(Player *owner);
+        void SetRefundRecipient(uint32 pGuidLow) { m_refundRecipient = pGuidLow; }
+        void SetPaidMoney(uint32 money) { m_paidMoney = money; }
+        void SetPaidExtendedCost(uint32 iece) { m_paidExtendedCost = iece; }
+        uint32 GetRefundRecipient() { return m_refundRecipient; }
+        uint32 GetPaidMoney() { return m_paidMoney; }
+        uint32 GetPaidExtendedCost() { return m_paidExtendedCost; }
+        
         void UpdatePlayedTime(Player *owner);
-        uint32 GetPlayedTime(bool raw = false);
+        uint32 GetPlayedTime();
+        bool IsRefundExpired();
         
         void BuildUpdate(UpdateDataMapType& );
 
@@ -346,6 +344,8 @@ class Item : public Object
         int16 uQueuePos;
         bool mb_in_trade;                                   // true if item is currently in trade-window
         time_t m_lastPlayedTimeUpdate;
-        ItemRefund *m_RefundData;
+        uint32 m_refundRecipient;
+        uint32 m_paidMoney;
+        uint32 m_paidExtendedCost;
 };
 #endif

src/game/ItemHandler.cpp

@@ -1359,41 +1359,32 @@ void WorldSession::HandleItemRefundInfoRequest(WorldPacket& recv_data)
         return;
     }
 
+    // This function call unsets ITEM_FLAGS_REFUNDABLE if played time is over 2 hours.
+    item->UpdatePlayedTime(GetPlayer());
+        
     if (!item->HasFlag(ITEM_FIELD_FLAGS, ITEM_FLAGS_REFUNDABLE))
     {
         sLog.outDebug("Item refund: item not refundable!");
         return;
     }
-
-    if (item->GetPlayedTime() > (2*HOUR))    // item refund has expired
-        item->SetNotRefundable(GetPlayer());
         
-    ItemRefund* RefundData = item->GetRefundData();
+    if (GetPlayer()->GetGUIDLow() != item->GetRefundRecipient()) // Formerly refundable item got traded
-    if (!RefundData)
-    {
-        sLog.outDebug("Item refund: cannot find refundable data.");
-        item->SetNotRefundable(GetPlayer());
-        return;
-    }
-    
-    if (GetPlayer()->GetGUIDLow() != RefundData->eligibleFor) // Formerly refundable item got traded
     {
         sLog.outDebug("Item refund: item was traded!");
         item->SetNotRefundable(GetPlayer());
         return;
     }
 
-    ItemExtendedCostEntry const* iece = sItemExtendedCostStore.LookupEntry(RefundData->paidExtendedCost);
+    ItemExtendedCostEntry const* iece = sItemExtendedCostStore.LookupEntry(item->GetPaidExtendedCost());
     if (!iece)
     {
         sLog.outDebug("Item refund: cannot find extendedcost data.");
         return;
     }
     
-    item->UpdatePlayedTime(GetPlayer());
     WorldPacket data(SMSG_ITEM_REFUND_INFO_RESPONSE, 8+4+4+4+4*4+4*4+4+4);
     data << uint64(guid);                          // item guid
-    data << uint32(RefundData->paidMoney);          // money cost
+    data << uint32(item->GetPaidMoney());          // money cost
     data << uint32(iece->reqhonorpoints);    // honor point cost
     data << uint32(iece->reqarenapoints);   // arena point cost
     for (uint8 i = 0; i < 5; ++i)                  // item cost data
@@ -1425,7 +1416,7 @@ void WorldSession::HandleItemRefund(WorldPacket &recv_data)
         return;
     }
 
-    if (item->GetPlayedTime() > (2*HOUR))    // item refund has expired
+    if (item->IsRefundExpired())    // item refund has expired
     {
         item->SetNotRefundable(GetPlayer());
         WorldPacket data(SMSG_ITEM_REFUND_RESULT, 8+4);
@@ -1435,28 +1426,22 @@ void WorldSession::HandleItemRefund(WorldPacket &recv_data)
         return;
     }
 
-    ItemRefund* RefundData = item->GetRefundData();
+    if (GetPlayer()->GetGUIDLow() != item->GetRefundRecipient()) // Formerly refundable item got traded
-    if (!RefundData)
-    {
-        sLog.outDebug("Item refund: cannot find refundable data.");
-        item->SetNotRefundable(GetPlayer());
-        return;
-    }
-    
-    if (GetPlayer()->GetGUIDLow() != RefundData->eligibleFor) // Formerly refundable item got traded
     {
         sLog.outDebug("Item refund: item was traded!");
         item->SetNotRefundable(GetPlayer());
         return;
     }
     
-    ItemExtendedCostEntry const* iece = sItemExtendedCostStore.LookupEntry(RefundData->paidExtendedCost);
+    ItemExtendedCostEntry const* iece = sItemExtendedCostStore.LookupEntry(item->GetPaidExtendedCost());
     if (!iece)
     {
         sLog.outDebug("Item refund: cannot find extendedcost data.");
         return;
     }
     
+    uint32 moneyRefund = item->GetPaidMoney();
+
     bool store_error = false;
     for (uint8 i = 0; i < 5; ++i)
     {
@@ -1487,7 +1472,7 @@ void WorldSession::HandleItemRefund(WorldPacket &recv_data)
     WorldPacket data(SMSG_ITEM_REFUND_RESULT, 8+4+4+4+4+4*4+4*4);
     data << uint64(guid);                          // item guid
     data << uint32(0);                             // 0, or error code
-    data << uint32(RefundData->paidMoney);          // money cost
+    data << uint32(moneyRefund);          // money cost
     data << uint32(iece->reqhonorpoints);    // honor point cost
     data << uint32(iece->reqarenapoints);   // arena point cost
     for (uint8 i = 0; i < 5; ++i)                  // item cost data
@@ -1498,8 +1483,8 @@ void WorldSession::HandleItemRefund(WorldPacket &recv_data)
     SendPacket(&data);
 
     // Delete any references to the refund data
-    item->DeleteRefundDataFromDB();
+    item->SetNotRefundable(GetPlayer());
-    GetPlayer()->DeleteRefundReference(item);
+
     // Destroy item
     _player->DestroyItem(item->GetBagSlot(), item->GetSlot(), true);
     
@@ -1519,7 +1504,6 @@ void WorldSession::HandleItemRefund(WorldPacket &recv_data)
     }
     
     // Grant back money
-    uint32 moneyRefund = RefundData->paidMoney;
     if (moneyRefund)
         _player->ModifyMoney(moneyRefund);
 
@@ -1533,5 +1517,4 @@ void WorldSession::HandleItemRefund(WorldPacket &recv_data)
     if (arenaRefund)
         _player->ModifyArenaPoints(arenaRefund);
 
-    delete RefundData;
 }

src/game/Player.cpp

@@ -11580,7 +11580,7 @@ void Player::DestroyItem(uint8 bag, uint8 slot, bool update)
         RemoveItemDurations(pItem);
         
         if (pItem->HasFlag(ITEM_FIELD_FLAGS, ITEM_FLAGS_REFUNDABLE))
-            DeleteRefundReference(pItem);
+            pItem->SetNotRefundable(this);
 
         ItemRemovedQuestCheck( pItem->GetEntry(), pItem->GetCount() );
 
@@ -16409,11 +16409,9 @@ void Player::_LoadInventory(QueryResult_AutoPtr result, uint32 timediff)
                     else
                     {    
                         fields = result2->Fetch();
-                        ItemRefund* RefundData = new ItemRefund();
+                        item->SetRefundRecipient(fields[0].GetUInt32());
-                        RefundData->eligibleFor = fields[0].GetUInt32();
+                        item->SetPaidMoney(fields[1].GetUInt32());
-                        RefundData->paidMoney = fields[1].GetUInt32();
+                        item->SetPaidExtendedCost(fields[2].GetUInt32());
-                        RefundData->paidExtendedCost = fields[2].GetUInt32();
-                        item->SetRefundData(RefundData);
                         AddRefundReference(item);
                     }
                 }
@@ -17497,21 +17495,15 @@ void Player::_SaveInventory()
         m_items[i]->FSetState(ITEM_NEW);
     }
 
-    // Updated played time for refundable items
+    // Updated played time for refundable items. We don't do this in Player::Update because there's simply no need for it,
-    for (std::set<Item*>::iterator itr = m_refundableItems.begin(); itr!= m_refundableItems.end();)
+    // the client auto counts down in real time after having received the initial played time on the first
+    // SMSG_ITEM_REFUND_INFO_RESPONSE packet.
+    // Item::UpdatePlayedTime is only called when needed, which is in DB saves, and item refund info requests.
+    for (std::set<Item*>::iterator itr = m_refundableItems.begin(); itr!= m_refundableItems.end(); ++itr)
     {
-        // Item could be deleted, or traded.
+        Item* iPtr = *itr;
-        // In the first case, DeleteRefundDataFromDB() was already called in Item::SaveToDB()
+        ASSERT(iPtr); // Sanity check, if this assertion is hit then the item wasn't removed from the set correctly./
-        Item* iPtr = (*itr);
+        iPtr->UpdatePlayedTime(this);
-        if (!iPtr)
-            m_refundableItems.erase(itr++);
-        else
-        {
-            iPtr->UpdatePlayedTime(this);
-            if (iPtr->GetPlayedTime() > (2*HOUR))
-                iPtr->SetNotRefundable(this);
-            ++itr;
-        }    
     }
 
     // update enchantment durations
@@ -17527,14 +17519,16 @@ void Player::_SaveInventory()
     for (size_t i = 0; i < m_itemUpdateQueue.size(); ++i)
     {
         Item *item = m_itemUpdateQueue[i];
-        if(!item || item->GetState() == ITEM_REMOVED) continue;
+        if (!item || item->GetState() == ITEM_REMOVED)
-        Item *test = GetItemByPos( item->GetBagSlot(), item->GetSlot());
+            continue;
+
+        Item *test = GetItemByPos(item->GetBagSlot(), item->GetSlot());
 
         if (test == NULL)
         {
             sLog.outCrash("Player(GUID: %u Name: %s)::_SaveInventory - the bag(%d) and slot(%d) values for the item with guid %d (state %d) are incorrect, the player doesn't have an item at that position!", GetGUIDLow(), GetName(), item->GetBagSlot(), item->GetSlot(), item->GetGUIDLow(), (int32)item->GetState());
             //error = true;
-            //now some items in bags cannot be saved but after cleansup they appear again
+            //Should the above line really be commented out?
         }
         else if (test != item)
         {
@@ -19364,11 +19358,9 @@ bool Player::BuyItemFromVendor(uint64 vendorguid, uint32 item, uint8 count, uint
             
             if (it->HasFlag(ITEM_FIELD_FLAGS, ITEM_FLAGS_REFUNDABLE) && crItem->ExtendedCost)
             {
-                ItemRefund* RefundData = new ItemRefund();
+                it->SetRefundRecipient(GetGUIDLow());
-                RefundData->eligibleFor = GetGUIDLow();
+                it->SetPaidMoney(price);
-                RefundData->paidMoney = price;
+                it->SetPaidExtendedCost(crItem->ExtendedCost);
-                RefundData->paidExtendedCost = crItem->ExtendedCost;
-                it->SetRefundData(RefundData);
                 it->SaveRefundDataToDB();
                 AddRefundReference(it);
             }
@@ -19424,11 +19416,9 @@ bool Player::BuyItemFromVendor(uint64 vendorguid, uint32 item, uint8 count, uint
 
             if (it->HasFlag(ITEM_FIELD_FLAGS, ITEM_FLAGS_REFUNDABLE) && crItem->ExtendedCost)
             {
-                ItemRefund* RefundData = new ItemRefund();
+                it->SetRefundRecipient(GetGUIDLow());
-                RefundData->eligibleFor = GetGUIDLow();
+                it->SetPaidMoney(price);
-                RefundData->paidMoney = price;
+                it->SetPaidExtendedCost(crItem->ExtendedCost);
-                RefundData->paidExtendedCost = crItem->ExtendedCost;
-                it->SetRefundData(RefundData);
                 it->SaveRefundDataToDB();
                 AddRefundReference(it);
             }

src/game/TradeHandler.cpp

@@ -202,9 +202,6 @@ void WorldSession::moveItems(Item* myItems[], Item* hisItems[])
                         _player->pTrader->GetName(),_player->pTrader->GetSession()->GetAccountId());
                 }
 
-                
-                // make not refundable
-                myItems[i]->SetNotRefundable(_player);
                 // store
                 _player->pTrader->MoveItemToInventory( traderDst, myItems[i], true, true);
             }
@@ -220,8 +217,6 @@ void WorldSession::moveItems(Item* myItems[], Item* hisItems[])
                         _player->GetName(),_player->GetSession()->GetAccountId());
                 }
 
-                // make not refundable
-                hisItems[i]->SetNotRefundable(_player->pTrader);
                 // store
                 _player->MoveItemToInventory( playerDst, hisItems[i], true, true);
             }
@@ -378,12 +373,14 @@ void WorldSession::HandleAcceptTradeOpcode(WorldPacket& /*recvPacket*/)
                 myItems[i]->SetUInt64Value( ITEM_FIELD_GIFTCREATOR, _player->GetGUID());
                 iPtr = _player->GetItemByGuid(_player->tradeItems[i]);
                 _player->MoveItemFromInventory(iPtr->GetBagSlot(), iPtr->GetSlot(), true);
+                myItems[i]->SetNotRefundable(_player);
             }
             if (hisItems[i])
             {
                 hisItems[i]->SetUInt64Value( ITEM_FIELD_GIFTCREATOR,_player->pTrader->GetGUID());
                 iPtr = _player->pTrader->GetItemByGuid(_player->pTrader->tradeItems[i]);
                 _player->pTrader->MoveItemFromInventory(iPtr->GetBagSlot(), iPtr->GetSlot(), true);
+                hisItems[i]->SetNotRefundable(_player->pTrader);
             }
         }