using expressions to find inventory items

[Celesteria]

hi,

I was writing a bot which goes to my guild hall and collects crafting mats at night. so I run into the problem that the mats I harvest have different names and ids according to the level of the building. In the german client this are f.e. "Unbekanntes Erz I", "Unbekanntes Erz II", "Unbekanntes Erz VIII" etc.

I dont know all the ids and dont want to edit my file again after every levelup of the buildings. I also dont want to go through a list of ids using findItem (). so I added the following function to the inventory class, which based on the original CInventory:findItem ()

Code: Select all

function CInventory:findItems (pattern, range)
  local first, last, location = getInventoryRange (range) -- get bag slot range
  if location ~= "inventory" and location ~= nil then
    printf ("You can only use inventory ranges with 'inventory:findItems'. You cannot use '%s' which is in %s\n", range, location)
  end

  if first == nil then
    first, last = 1, 240 -- default, search all
  end

  local itemList = {}
  local item
  for slot = first, last do
    item = self.BagSlot[slot]
    item:update ()
    if item.Available and (not item.InUse) and string.find (item.Name, pattern) then
      if (os.clock () - item.LastMovedTime) > ITEM_REUSE_DELAY then
        table.insert (itemList, item)
      end
    end
  end
  return (#itemList>0 and itemList or nil)
end

this function makes it possible to do something like:

Code: Select all

local listOfItems = inventory:findItems ("^Unbekanntes Erz [IVX]$")

It would be nice, if the functionality will be added to the rombot. the only question is whats the best way to return the results. at the time it only returns a list of items (if any). maybe a better way is to return two values - first the amount of found items and second the list itself? and maybe the list should be sorted in any way?

any ideas would be fine

greetings
Celesteria

[rock5]

In regards to returning a list of items, we did something similar with player:findNearestNameOrId. Before, it only returned the closest match but now it returns the closest match and the list of matches. This way it's still backward compatible.

We could do something similar here; have it return the smallestStack and a table of matches.

In regards to doing a pattern match, I believe it does an exact match because there is too much chance of making a false match with pattern matching. Example, if you wanted to discard all Dexterity I runes and you did a pattern match for "Dexterity I", you would end up discarding Dexterity II, III and IV runes as well.

That said, if you want that functionality maybe we can think of some way to do it safely. Maybe an extra argument indicating you want to do a pattern match eg. inventory:findItem("^Unbekanntes Erz [IVX]$", range, usePattern). Or maybe use a special character in the string to indicate a pattern search eg. inventory:findItem("#^Unbekanntes Erz [IVX]$", range)

[Celesteria]

I didnt really find the "best item" or a "closest item" because there can be different items that match the pattern. So I think its the best way to simply return the first item and the list. I also merged both functionality together into one function. So the original function can be replaced without compatibility problems with old scripts. I prefered to use the usePattern argument as new third argument...

Code: Select all

function CInventory:findItem (itemNameOrIdOrPattern, range, usePattern)
	local itemList = nil
	local first, last, location = getInventoryRange(range) -- get bag slot range
	local smallestStack = nil
	local item

	if location ~= "inventory" and location ~= nil then
		printf("You can only use inventory ranges with 'inventory:findItem'. You cannot use '%s' which is in %s\n", range, location)
	end

	if first == nil then
		first , last = 1, 240 -- default, search all
	end

	for slot = first, last do
		item = self.BagSlot[slot]
		item:update()
		if item.Available and (not item.InUse) and (usePattern and string.find (item.Name, itemNameOrIdOrPattern) or (item.Name == itemNameOrIdOrPattern or item.Id == itemNameOrIdOrPattern)) then
			if (os.clock() - item.LastMovedTime) > ITEM_REUSE_DELAY then
				itemList = itemList or {}
				table.insert (itemList, item)
				if not usePattern then
					-- find smallest stack
					if smallestStack == nil or smallestStack.ItemCount > item.ItemCount then
						smallestStack = item
					end
				end
			end
		end
	end
	if usePattern then
		return (itemList and #itemList) and itemList[1] or nil, itemList
	else
		return smallestStack, itemList
	end
end

[rock5]

Nice. A few comments.

1. They shouldn't do it, but if a user uses an id and sets pattern matching to true then it wont find a match. I think usePattern should only be used with the name match, eg.

Code: Select all

		if item.Available and (not item.InUse) and (item.Id == itemNameOrIdOrPattern or (usePattern and string.find (item.Name, itemNameOrIdOrPattern) or (item.Name == itemNameOrIdOrPattern ))) then

2. I'm not sure we should discard finding the smallest stack just because we are doing a pattern match. There are 2 situations, I can think of, where you would use pattern matching. The first is to create a list of objects you want to interact with. In this case you would use the returned itemList and the order most likely wont matter. The second is to find 1 of a number of similar named items of which you don't care which one is returned. In this case you will use the returned smallest stack and all the reasons for returning the smallest stack still apply. So in that case you would still want the smallest stack. So I think we should still have the smallest stack functionality when using pattern matching.

3. A small thing but you do

Code: Select all

itemList = itemList or {} 

in every single loop. It might be better to declare the variable as an empty table before the loop instead. If you want to return nil if the table is empty you can change change it back to nil after the loop.

4. If we restore the smallest stack functionality then the following line wont be necessary.

Code: Select all

return (itemList and #itemList) and itemList[1] or nil, itemList

But I don't think this works as you intended it. I suspect you think #itemList will evaluate to false if 0. This is not the case. In lua the only values that evaluate to false are false and nil. All other values are evaluated to true. You would have had to use #itemList>0

Keep up the good work.

[Celesteria]

hello rock5,

I agree with you. Next version implements most of your thinkings.

(1) I added an info message and switch usePattern to false if itemNameOrIdOrPattern is numeric. think this is the smartest way to handle this.

(2) the smallest stack using pattern maybe not allway what the user expected. its just the lowest stack of all matches or the FIRST matching item with stacksize of 1. I think about ordering the itemList by stacksize, but this maybe to much overhead. what do you think? the function now returns in all cases the smallest stack and the full itemlist.

(3) (4) solved...

Code: Select all

function CInventory:findItem (itemNameOrIdOrPattern, range, usePattern)
	local itemList = {}
	local first, last, location = getInventoryRange(range) -- get bag slot range
	local smallestStack = nil
	local item

	if location ~= "inventory" and location ~= nil then
		printf("You can only use inventory ranges with 'inventory:findItem'. You cannot use '%s' which is in %s\n", range, location)
	end

	if type(itemNameOrIdOrPattern)=='number' then
		printf("[inventory:findItem] Searching for a numeric Id will switch usePattern to false")
		usePattern = false
	end

	if first == nil then
		first , last = 1, 240 -- default, search all
	end

	for slot = first, last do
		item = self.BagSlot[slot]
		item:update()
		if item.Available and (not item.InUse) and (usePattern and string.find (item.Name, itemNameOrIdOrPattern) or (item.Name == itemNameOrIdOrPattern or item.Id == itemNameOrIdOrPattern)) then
			if (os.clock() - item.LastMovedTime) > ITEM_REUSE_DELAY then
				table.insert (itemList, item)
				-- find smallest stack
				if smallestStack == nil or smallestStack.ItemCount > item.ItemCount then
					smallestStack = item
				end
			end
		end
	end
	itemList = #itemList>0 and itemList or nil
	return smallestStack, itemList
end

[rock5]

(1) I added an info message and switch usePattern to false if itemNameOrIdOrPattern is numeric. think this is the smartest way to handle this.

I was expecting the code to say something like "if itemNameOrIdOrPattern == number and usePattern == true then show message". I'm surprised you have it print the message every single time an id is used. Even so, I don't think the message is necessary. We need to keep unnecessary spammed messages to a minimum. Just because an id and usePattern are used doesn't mean the user made a mistake. For instance just say there is a userfunction that accepts an id or name from the user and then searches for it using inventory:findItem. The userfunction wont know if the user used an id or name. You should be able to do the following even though you don't know if the argument is an id or name.

Code: Select all

item = inventory:findItem(userSpecifiedNameOrId, "bags", true)

And the findItem function should be just able to handle it. Don't need a message every time an id is used.

(2) the smallest stack using pattern maybe not allway what the user expected.

I figure the user either wants the smallest stack or doesn't care. I don't see any harm in returning the smallest stack if the user doesn't care which one they get.

its just the lowest stack of all matches or the FIRST matching item with stacksize of 1

That's the way it is now. The reason it returns the FIRST matching stacksize of 1 is because there is no point in searching any more as you can't have a smaller stack than 1. Of course if you are going to return the list of items anyway then there is no need to check for the stacksize of 1, because we are going to go through the whole list anyway, so you were right to remove it

I think about ordering the itemList by stacksize, but this maybe to much overhead. what do you think?

Can't see any need for it.

So, besides the print message, I'm happy with it. I'll add it as it is minus the print message. Thanks for the work.

[Celesteria]

yes, the message was a little buggy

Code: Select all

function CInventory:findItem (itemNameOrIdOrPattern, range, usePattern)
	local itemList = {}
	local first, last, location = getInventoryRange(range) -- get bag slot range
	local smallestStack = nil
	local item

	if location ~= "inventory" and location ~= nil then
		printf("You can only use inventory ranges with 'inventory:findItem'. You cannot use '%s' which is in %s\n", range, location)
	end

	if type(itemNameOrIdOrPattern)=='number' and usePattern==true then
		printf("[inventory:findItem] Searching for a numberic Id will switch usePattern to false")
		usePattern = false
	end

	if first == nil then
		first , last = 1, 240 -- default, search all
	end

	for slot = first, last do
		item = self.BagSlot[slot]
		item:update()
		if item.Available and (not item.InUse) and (usePattern and string.find (item.Name, itemNameOrIdOrPattern) or (item.Name == itemNameOrIdOrPattern or item.Id == itemNameOrIdOrPattern)) then
			if (os.clock() - item.LastMovedTime) > ITEM_REUSE_DELAY then
				table.insert (itemList, item)
				-- find smallest stack
				if smallestStack == nil or smallestStack.ItemCount > item.ItemCount then
					smallestStack = item
				end
			end
		end
	end
	itemList = #itemList>0 and itemList or nil
	return smallestStack, itemList
end

[rock5]

Seeing as I'm not including the message I just left it as

Code: Select all

	if type(itemNameOrIdOrPattern)=='number' then
		usePattern = false
	end