Inventory tracker bug


#1

Summary: Reproducible inventory_tracker error related to item qualities when the only item of a type that doesn’t inherently allow variable quality is having its quality set.

Steps to reproduce:

  1. Create a standard Microworld.
  2. Create a stockpile and wait until all default spawned items are stored in it.
  3. Select the Silkweed Bundle (or the Oak Log) and set its quality with the following function (or similar):
e:add_component('stonehearth:item_quality'):initialize_quality(2, 'test', nil, {override_allow_variable_quality = true})
  1. If you had the game paused, unpause it now.

Expected Results: No error, item qualities properly applied.

Actual Results: Item qualities all seem to be working properly, as does the town overview inventory tracking, but some item tracker somewhere may be failing, because this error occurs:

release-947 (x64)[M][C]
invalid key to 'next'
stack traceback:
	[C]: ?
	[C]: in function '(for generator)'
	...arth/services/server/inventory/inventory_tracker.lua:185: in function <...arth/services/server/inventory/inventory_tracker.lua:163>
	[C]: ?
	[C]: in function 'update'
	radiant/modules/events.lua:59: in function '_update'
	radiant/server.lua:67: in function <radiant/server.lua:64>

Notes: If you spawn a second Silkweed bundle first before setting the quality of one of them, the error doesn’t occur. You can then set the quality of both of them without the error occurring. If you then destroy both of those items and create a new third bundle and set its item quality, you get the error. So it appears to be related to having the only item of a type being tracked change item qualities (probably trying to destroy and create the appropriate item quality tables, which are members of the same container table that’s trying to get iterated through, at the same time).

Also, this doesn’t appear to happen with items that are designed to have variable item qualities, e.g., a Comfy Bed. Is there some sort of extra preparation/management that goes into handling such items that doesn’t happen with other items where item quality is forced with the override_allow_variable_quality flag?

For my purposes (with ACE), I can probably mixin to a bunch of entities to allow variable quality for anything that might cause a problem like this, but it still seems like something that should be fixed.

Version Number and Mods in use: Microworld


#2

After looking at the code, you are exactly correct. You got for loop for a table, but inside that loop you delete entries while you looping. In addition, the code uses an iterator next() so when you modified the table it went bonkers.

As for the comfy bed and why it doesn’t error. Well it also doesn’t properly update the Inventory List neither.
If you look at line 207 it says iconics don’t have their own item quality component.

If you place down a comfy bed in it’s true form (non iconic) then upgrade it, you will see the error popup and the inventory list will be updated properly.

Edit: solution? once you update the item_quality_entry just break the for loop. at that point the tracking_data should be updated with the proper counts in relation to that item being updated.


#3

It’s been a while since I looked at that code, but IIRC the assumption everywhere is that once an item is added to an inventory, its quality never changes.


#4

Given that assumption, I suppose the more appropriate solution is, whenever I want to change the item quality of an item that’s already in an inventory, to first remove it from the inventory, then change the quality, then add it back. I’ll have to make sure that removing an item from a player’s inventory while a hearthling is carrying that item doesn’t break things.


#5

I think the safest approach there is to destroy the old item and create a new one with the right quality. Things generally listen for item destruction and clean up properly.


#6

I’ll look into it more, but that seems like a bit of overkill when the tracker is already listening for an item quality changed event. Why even have the event if nothing could ever use it? It may be enough to just investigate the places where item quality is used and make sure it’s not simply being cached without respect for changes. But that would be a longer-term goal; for the time being it is probably simplest to destroy the items and create new ones.