Patchwork of changes..mostly related tracing a cursor queue bug (see changelog)

changelog.txt

@@ -1,5 +1,15 @@
 EQEMu Changelog (Started on Sept 24, 2003 15:50)
 -------------------------------------------------------
+== 01/15/2015 ==
+Uleat: Attempted fix for elusive inventory bug:
+  - Removed 'iter_queue' typedef and converted lcast to explicit or auto defines
+  - Reworked several functions that manipulate the cursor queue
+  - Found/corrected one occurrence of post-processing iterator incrementing in an ItemInstQueue handler
+  - Added many scope declarations in code that handles inventory manipulation (loose macros are bad...)
+Uleat: Added Item_Struct pointer checks to ItemInst methods that did not have them
+Uleat: Changed IsEquippable(race,class) to use bit-wise 'and' (&) over '(x%2)==1' in conditional check.
+Uleat: Changed DyeArmor() assignment of 'armor_color' to use bit-wise 'or' and bit-shifting (|,<<) over multiplication and addition (*,+).
+
 == 01/13/2015 ==
 Uleat: Placed an upper limit on the cursor queue save loop.
 Trevius: (RoF2) Guild invites now add new members as members instead of recruits, and /guild chat works properly.

common/item.h

@@ -34,7 +34,6 @@ class EvolveInfo;			// Stores information about an evolving item family
 #include <map>
 
 // Helper typedefs
-typedef std::list<ItemInst*>::const_iterator				iter_queue;
 typedef std::map<int16, ItemInst*>::const_iterator			iter_inst;
 typedef std::map<uint8, ItemInst*>::const_iterator			iter_contents;
 
@@ -87,15 +86,17 @@ public:
 	// Public Methods
 	/////////////////////////
 
-	inline iter_queue begin()	{ return m_list.begin(); }
+	inline std::list<ItemInst*>::const_iterator begin() { return m_list.begin(); }
-	inline iter_queue end()		{ return m_list.end(); }
+	inline std::list<ItemInst*>::const_iterator end() { return m_list.end(); }
+
+	inline int size() { return static_cast<int>(m_list.size()); } // TODO: change to size_t
+	inline bool empty() { return m_list.empty(); }
 
 	void push(ItemInst* inst);
 	void push_front(ItemInst* inst);
 	ItemInst* pop();
 	ItemInst* pop_back();
 	ItemInst* peek_front() const;
-	inline int size()		{ return static_cast<int>(m_list.size()); }
 
 protected:
 	/////////////////////////
@@ -103,7 +104,6 @@ protected:
 	/////////////////////////
 
 	std::list<ItemInst*> m_list;
-
 };
 
 // ########################################
@@ -140,9 +140,11 @@ public:
 	ItemInst* GetItem(int16 slot_id) const;
 	ItemInst* GetItem(int16 slot_id, uint8 bagidx) const;
 
-	inline iter_queue cursor_begin()	{ return m_cursor.begin(); }
+	inline std::list<ItemInst*>::const_iterator cursor_begin() { return m_cursor.begin(); }
-	inline iter_queue cursor_end()		{ return m_cursor.end(); }
+	inline std::list<ItemInst*>::const_iterator cursor_end() { return m_cursor.end(); }
-	inline bool CursorEmpty()			{ return (m_cursor.size() == 0); }
+
+	inline int CursorSize() { return m_cursor.size(); }
+	inline bool CursorEmpty() { return m_cursor.empty(); }
 
 	// Retrieve a read-only item from inventory
 	inline const ItemInst* operator[](int16 slot_id) const { return GetItem(slot_id); }
@@ -291,15 +293,15 @@ public:
 	bool IsEquipable(int16 slot_id) const;
 
 	//
-	// Augements
+	// Augments
 	//
-	inline bool IsAugmentable() const { return m_item->AugSlotType[0] != 0 || m_item->AugSlotType[1] != 0 || m_item->AugSlotType[2] != 0 || m_item->AugSlotType[3] != 0 || m_item->AugSlotType[4] != 0 || m_item->AugSlotType[5] != 0; }
+	bool IsAugmentable() const;
 	bool AvailableWearSlot(uint32 aug_wear_slots) const;
 	int8 AvailableAugmentSlot(int32 augtype) const;
 	bool IsAugmentSlotAvailable(int32 augtype, uint8 slot) const;
-	inline int32 GetAugmentType() const { return m_item->AugType; }
+	inline int32 GetAugmentType() const { return ((m_item) ? m_item->AugType : NO_ITEM); }
 
-	inline bool IsExpendable() const { return ((m_item->Click.Type == ET_Expendable ) || (m_item->ItemType == ItemTypePotion)); }
+	inline bool IsExpendable() const { return ((m_item) ? ((m_item->Click.Type == ET_Expendable ) || (m_item->ItemType == ItemTypePotion)) : false); }
 
 	//
 	// Contents
@@ -337,8 +339,8 @@ public:
 	bool IsAmmo() const;
 
 	// Accessors
-	const uint32 GetID() const { return m_item->ID; }
+	const uint32 GetID() const { return ((m_item) ? m_item->ID : NO_ITEM); }
-	const uint32 GetItemScriptID() const { return m_item->ScriptFileID; }
+	const uint32 GetItemScriptID() const { return ((m_item) ? m_item->ScriptFileID : NO_ITEM); }
 	const Item_Struct* GetItem() const;
 	const Item_Struct* GetUnscaledItem() const;
 

common/shareddb.cpp

@@ -110,9 +110,11 @@ bool SharedDatabase::SaveCursor(uint32 char_id, std::list<ItemInst*>::const_iter
     for(auto it = start; it != end; ++it, i++) {
 		if (i > 8999) { break; } // shouldn't be anything in the queue that indexes this high
         ItemInst *inst = *it;
-        if (!SaveInventory(char_id,inst,(i == 8000) ? MainCursor : i))
+		int16 use_slot = (i == 8000) ? MainCursor : i;
+		if (!SaveInventory(char_id, inst, use_slot)) {
 			return false;
 		}
+    }
 
 	return true;
 }
@@ -171,8 +173,9 @@ bool SharedDatabase::SaveInventory(uint32 char_id, const ItemInst* inst, int16 s
 		else
             return UpdateSharedBankSlot(char_id, inst, slot_id);
 	}
-	else if (!inst) // All other inventory
+	else if (!inst) { // All other inventory
 		return DeleteInventorySlot(char_id, slot_id);
+	}
 
     return UpdateInventorySlot(char_id, inst, slot_id);
 }
@@ -181,11 +184,12 @@ bool SharedDatabase::UpdateInventorySlot(uint32 char_id, const ItemInst* inst, i
 	// need to check 'inst' argument for valid pointer
 
 	uint32 augslot[EmuConstants::ITEM_COMMON_SIZE] = { NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM };
-    if (inst->IsType(ItemClassCommon))
+	if (inst->IsType(ItemClassCommon)) {
 		for (int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) {
 			ItemInst *auginst = inst->GetItem(i);
 			augslot[i] = (auginst && auginst->GetItem()) ? auginst->GetItem()->ID : NO_ITEM;
 		}
+	}
 
     uint16 charges = 0;
 	if(inst->GetCharges() >= 0)
@@ -226,11 +230,12 @@ bool SharedDatabase::UpdateSharedBankSlot(uint32 char_id, const ItemInst* inst,
 	// need to check 'inst' argument for valid pointer
 
 	uint32 augslot[EmuConstants::ITEM_COMMON_SIZE] = { NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM };
-    if (inst->IsType(ItemClassCommon))
+	if (inst->IsType(ItemClassCommon)) {
 		for (int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) {
 			ItemInst *auginst = inst->GetItem(i);
 			augslot[i] = (auginst && auginst->GetItem()) ? auginst->GetItem()->ID : NO_ITEM;
 		}
+	}
 
 // Update/Insert item
     uint32 account_id = GetAccountIDByChar(char_id);
@@ -252,11 +257,12 @@ bool SharedDatabase::UpdateSharedBankSlot(uint32 char_id, const ItemInst* inst,
     auto results = QueryDatabase(query);
 
     // Save bag contents, if slot supports bag contents
-	if (inst->IsType(ItemClassContainer) && Inventory::SupportsContainers(slot_id))
+	if (inst->IsType(ItemClassContainer) && Inventory::SupportsContainers(slot_id)) {
 		for (uint8 idx = SUB_BEGIN; idx < EmuConstants::ITEM_CONTAINER_SIZE; idx++) {
 			const ItemInst* baginst = inst->GetItem(idx);
 			SaveInventory(char_id, baginst, Inventory::CalcSlotId(slot_id, idx));
 		}
+	}
 
     if (!results.Success()) {
         LogFile->write(EQEmuLog::Error, "UpdateSharedBankSlot query '%s': %s", query.c_str(), results.ErrorMessage().c_str());
@@ -435,11 +441,10 @@ bool SharedDatabase::GetSharedBank(uint32 id, Inventory* inv, bool is_charid) {
         ItemInst* inst = CreateBaseItem(item, charges);
         if (inst && item->ItemClass == ItemClassCommon) {
             for(int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) {
-                if (aug[i]) {
+                if (aug[i])
 					inst->PutAugment(this, i, aug[i]);
             }
         }
-        }
 
         if(!row[9])
             continue;
@@ -576,10 +581,12 @@ bool SharedDatabase::GetInventory(uint32 char_id, Inventory* inv) {
 		else
             inst->SetCharges(charges);
 
-        if (item->ItemClass == ItemClassCommon)
+		if (item->ItemClass == ItemClassCommon) {
-            for(int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++)
+			for (int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) {
 				if (aug[i])
 					inst->PutAugment(this, i, aug[i]);
+			}
+		}
 
         if (slot_id >= 8000 && slot_id <= 8999)
 		{
@@ -691,10 +698,12 @@ bool SharedDatabase::GetInventory(uint32 account_id, char* name, Inventory* inv)
 
         inst->SetCharges(charges);
 
-        if (item->ItemClass == ItemClassCommon)
+		if (item->ItemClass == ItemClassCommon) {
-            for(int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++)
+			for (int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) {
 				if (aug[i])
 					inst->PutAugment(this, i, aug[i]);
+			}
+		}
 
         if (slot_id>=8000 && slot_id <= 8999)
             put_slot_id = inv->PushCursor(*inst);

zone/client_packet.cpp

@@ -1837,12 +1837,11 @@ void Client::Handle_Connect_OP_ZoneEntry(const EQApplicationPacket *app)
 	if (loaditems) { /* Dont load if a length error occurs */
 		BulkSendInventoryItems();
 		/* Send stuff on the cursor which isnt sent in bulk */
-		iter_queue it;
+		for (auto iter = m_inv.cursor_begin(); iter != m_inv.cursor_end(); ++iter) {
-		for (it = m_inv.cursor_begin(); it != m_inv.cursor_end(); ++it) {
 			/* First item cursor is sent in bulk inventory packet */
-			if (it == m_inv.cursor_begin())
+			if (iter == m_inv.cursor_begin())
 				continue;
-			const ItemInst *inst = *it;
+			const ItemInst *inst = *iter;
 			SendItemPacket(MainCursor, inst, ItemPacketSummonItem);
 		}
 	}

zone/command.cpp

@@ -2683,7 +2683,7 @@ void command_peekinv(Client *c, const Seperator *sep)
 		}
 		else {
 			int cursorDepth = 0;
-			for (iter_queue it = targetClient->GetInv().cursor_begin(); (it != targetClient->GetInv().cursor_end()); ++it, ++cursorDepth) {
+			for (auto it = targetClient->GetInv().cursor_begin(); (it != targetClient->GetInv().cursor_end()); ++it, ++cursorDepth) {
 				inst_main = *it;
 				item_data = (inst_main == nullptr) ? nullptr : inst_main->GetItem();
 				linker.SetItemInst(inst_main);

zone/corpse.cpp

@@ -363,9 +363,10 @@ Corpse::Corpse(Client* client, int32 in_rezexp) : Mob (
 		// solar: TODO soulbound items need not be added to corpse, but they need
 		// to go into the regular slots on the player, out of bags
 
-		// worn + inventory + cursor
+		// possessions
+		// TODO: accomodate soul-bound items
 		std::list<uint32> removed_list;
-		bool cursor = false;
+		//bool cursor = false;
 		for(i = MAIN_BEGIN; i < EmuConstants::MAP_POSSESSIONS_SIZE; i++) {
 			if(i == MainAmmo && client->GetClientVersion() >= EQClientSoF) {
 				item = client->GetInv().GetItem(MainPowerSource);
@@ -383,14 +384,18 @@ Corpse::Corpse(Client* client, int32 in_rezexp) : Mob (
 			}
 		}
 
+#if 0
+		// This will either be re-enabled or deleted at some point. The client doesn't appear
+		// to like to have items deleted from it's buffer..or, I just haven't figure out how -U
+		// (Besides, the 'corpse' slots equal the size of MapPossessions..not MapPossessions + MapCorpse)
+
 		// cursor queue // (change to first client that supports 'death hover' mode, if not SoF.)
 		if (!RuleB(Character, RespawnFromHover) || client->GetClientVersion() < EQClientSoF) {
 
 			// bumped starting assignment to 8001 because any in-memory 'slot 8000' item was moved above as 'slot 30'
 			// this was mainly for client profile state reflection..should match db player inventory entries now.
-
+			i = 8001;
-			iter_queue it;
+			for (auto it = client->GetInv().cursor_begin(); it != client->GetInv().cursor_end(); ++it, i++) {
-			for (it = client->GetInv().cursor_begin(), i = 8001; it != client->GetInv().cursor_end(); ++it, i++) {
 				item = *it;
 				if ((item && (!client->IsBecomeNPC())) || (item && client->IsBecomeNPC() && !item->GetItem()->NoRent)) {
 					std::list<uint32> slot_list = MoveItemToCorpse(client, item, i);
@@ -399,6 +404,7 @@ Corpse::Corpse(Client* client, int32 in_rezexp) : Mob (
 				}
 			}
 		}
+#endif
 
 		database.TransactionBegin();
 		if (removed_list.size() != 0) {
@@ -421,6 +427,7 @@ Corpse::Corpse(Client* client, int32 in_rezexp) : Mob (
 			database.QueryDatabase(ss.str().c_str());
 		}
 
+#if 0
 		if (cursor) { // all cursor items should be on corpse (client < SoF or RespawnFromHover = false)
 			while (!client->GetInv().CursorEmpty())
 				client->DeleteItemInInventory(MainCursor, 0, false, false);
@@ -430,8 +437,13 @@ Corpse::Corpse(Client* client, int32 in_rezexp) : Mob (
 			std::list<ItemInst*>::const_iterator finish = client->GetInv().cursor_end();
 			database.SaveCursor(client->CharacterID(), start, finish);
 		}
+#endif
 
-		client->CalcBonuses(); // will only affect offline profile viewing of dead characters..unneeded overhead
+		auto start = client->GetInv().cursor_begin();
+		auto finish = client->GetInv().cursor_end();
+		database.SaveCursor(client->CharacterID(), start, finish);
+
+		client->CalcBonuses();
 		client->Save();
 
 		IsRezzed(false);