View Single Post
11/25/14, 01:27 PM   #23
circonian
AddOn Author - Click to view addons
Join Date: May 2014
Posts: 613
While I'm thinking of it:

Lua Code:
  1. -- You register for this:
  2. EVENT_MANAGER:RegisterForEvent(gAddonName, EVENT_INVENTORY_SINGLE_SLOT_UPDATE, FCOItemSaver_Inv_Update)
  3.  
  4. -- then in it you zo_calllater this:
  5. --scanInventory(bagId, slotId)
Then you do stuff with bagId, slotId. Three things I see here.

1) When the player moves an item two slots are updated. First the slot the item was moved from, then the slot it was moved to. This means this event fires twice. One of the times the item is no longer even in the slot. You seem to have the same problem with what if a user destroys or sells an item? the item vanishes, the slot gets updated, but the item is not in the slot anymore. It looks like your running a bunch of code on items that no longer exist. Even if your code is handling it correctly, not causing an error when the item no longer exists, I don't see any reason to run the code if the item no longer exists in that slot. You should probably have something in there like:
Lua Code:
  1. function FCOItemSaver_Inv_Update( eventCode, bagId, slotId, isNewItem, itemSoundCategory, updateReason )
  2.     if GetItemType(bagId, slotId) == ITEMTYPE_NONE then return end
This will cut back on how often this code runs a lot.


2) In FCOItemSaver_Inv_Update, why is this called with a zo_calllater? This serves no purpose? Delaying the call here doesn't do anything in this function or in scanInventory() ?
Lua Code:
  1. zo_callLater(function()
  2.                 debugMessage("Event 'Single inventory slot updated' executed now! bagId=" .. bagId .. ", slotIndex=" .. slotId, true)
  3.                 scanInventory(bagId, slotId)
  4.             end, 250)
My Only guess I can come up with is that I see in ScanInventory() your doing something related to researchAssistant
Lua Code:
  1. --Automatic marking of ornate and/or researchable items (researchAssistant addon needed!) is activated?
If you are delaying to wait for researchAssistant to do something theres better ways. If you need to be informed of when researchAssistant does something (I have no clue what that addon even does, but), lets say you need to wait for it to finish marking items that are needed for research or something...ask the author to put in a custom callback that he can fire once the addon finishes that task & then your addon will know exactly when to run this code. Instead of calling it from the slotUpdate & delaying & hoping that researchAssistant has finished its task.


3) In FCOItemSaver_Inv_Update, do you really need to run this code on items that are NOT new items??
It looks like it does nothing but call scanInventory() which in turn does nothing but save an itemId if the item is ornate or researchable
Lua Code:
  1. markedItems[3][itemId] = true
Well if you check it & get that itemId when the item is new (when they first get it) you don't need to ever check that item again.

Seems like this whole function could just be:
Lua Code:
  1. --Inventory slot gets updated function
  2. function FCOItemSaver_Inv_Update( eventCode, bagId, slotId, isNewItem, itemSoundCategory, updateReason )
  3.     if not isNewItem and bagId ~= BAG_BACKPACK then return end
  4.     if  GetItemType(bagId, slotId) == ITEMTYPE_NONE then return end
  5.    
  6.     scanInventory(bagId, slotId)
  7. end

4) I would change this function:
Lua Code:
  1. --Scan the inventory for ornate and/or researchable items
  2. local function scanInventory(p_bagId, p_slotIndex)

First you have it performing two very similar (practically identical) tasks...meaning you have a lot of code in there twice.
I only see two main tasks being done, I would create two separate functions out of them:
Lua Code:
  1. --Update ornate items
  2. CheckForOrnate(bagId, slotId, any other info needed....)  
  3. -- and
  4. --Update researchable items
  5. CheckForResearchable(bagId, slotId, any other info needed....)

Then rewrite scanInventory(bagId, slotId)
so that it ONLY works with a bagId, slotId. Theres no reason to write a bunch of code to make it scan the entire inventory when the bagId/slotId is nil, & do something else when their not. Make it only work when its given a bagId, slotId and if you want to update the entire inventory like on initialize...just loop through the inventory & pass scanInventory(bagId, slotId) the bag/slot's it needs updating each bag/slot.
  Reply With Quote