Crash Inventory::getItemCount when saving lua methods to global

[eradicator]

Reproduction

This prints "0" to the console 10 times, then crashes.

Code: Select all

/c

local p = game.player
global.x = p.surface.create_entity{
  name = 'steel-chest',
  position = p.position,
  force = p.force,
  }

global.f = global.x.get_inventory(defines.inventory.chest).get_item_count

script.on_event(defines.events.on_tick, function()
  print(global.f())
  end)

Expected behavior

Function references stored in global should stay usable for as long as the instance runs. When saving happens they should not be stored in the save.zip file, but remain in the lua state untouched. The mere act of storing them shouldn't crash the game :D.

Explanation

While trying to optimize performance I tried to cache the inventory methods directly in addition to caching the LuaInventory, expecting to be able to restore them in on_load. I tried caching them directly in a global subtable, and in an __index metatable, but both implementations crash. If saving happens before the crash the console *does* print a message that N functions have been removed from global as expected.

I've used get_item_count for the example, but I'm assuming this happens for other methods too.

[Rseding91]

Thanks for the report however this isn't going to be fixed. You need to keep the LuaInventory reference around if you are going to keep the method reference to it.

[eradicator]

You need to keep the LuaInventory reference around if you are going to keep the method reference to it.

Thanks for the tip ! This seems to work as expected and is about 10% faster even if I have to hide them in an __index metatable. It does feel a bit odd that cached methods are included in the hash of global after on_load, and thus trigger a crc warning when I restore them in on_load (even though they're removed before saving). Is that intended?

[Rseding91]

You need to keep the LuaInventory reference around if you are going to keep the method reference to it.

Thanks for the tip ! This seems to work as expected and is about 10% faster even if I have to hide them in an __index metatable. It does feel a bit odd that cached methods are included in the hash of global after on_load, and thus trigger a crc warning when I restore them in on_load (even though they're removed before saving). Is that intended?

Just don't store them in global. Store them in some other table outside of global.

[eradicator]

You need to keep the LuaInventory reference around if you are going to keep the method reference to it.

Thanks for the tip :oops: :D ! This seems to work as expected and is about 10% faster even if I have to hide them in an __index metatable. It does feel a bit odd that cached methods are included in the hash of global after on_load, and thus trigger a crc warning when I restore them in on_load (even though they're removed before saving). Is that intended?

Just don't store them in global. Store them in some other table outside of global.

That's what i meant with "hiding them in an __index metatable". It just seemed odd to me that they're part of the game-state-crc even though they're auto-deleted from the save.zip. But I guess your answer means that's working as intended.

[Rseding91]

Function references aren't part of the CRC, but if you store a full LuaObject anywhere it is part of the CRC. Any full LuaObjects should always be stored in the global table.

[eradicator]

but if you store a full LuaObject anywhere it is part of the CRC. Any full LuaObjects should always be stored in the global table.

Yes, I know.

Function references aren't part of the CRC,

That's my point: It seemed to me that they *are*. Here's a simplified version of my on_load. The inventory is already stored.

Code: Select all

script.on_load(function()
  for _, data in pairs(global.datas) do
    data.get_item_count = data.inventory.get_item_count
    data.insert = data.inventory.insert
    data.remove = data.inventory.remove
    end
  end)  

So you're saying this should NOT cause a crc error right? Because this does raise a crc error for me.

[Rseding91]

That is storing the function references in the global table still. You need to not store them in the "global" table. Some other table.

[eradicator]

You need to not store them in the "global" table. Some other table.

Yes. My workaround is storing them in __index so that they can be used *as if* they were in global without actually being in global. Modification of simplified code from above:

Code: Select all

script.on_load(function()
  for _, data in pairs(global.datas) do
    setmetatable(data, {__index={
      get_item_count = data.inventory.get_item_count
      insert = data.inventory.insert
      remove = data.inventory.remove
      }}
    end
  end)

That is storing the function references in the global table still.

Yes, but by your own account:

Function references aren't part of the CRC

If they aren't part of the CRC why do I get a CRC error if I store them in global? Those two statements contradict each other.

[Rseding91]

Things in the 'global' table and all LuaObjects are in the CRC. The creation of a function reference isn't forced to be in the CRC like the creation of a LuaObject. But, by storing it in the global table it now is.

[eradicator]

Things in the 'global' table and all LuaObjects are in the CRC.

So LuaObjects are in the CRC even if they're stored in local variables? Interesting. Thanks for taking the time to clarify!

I guess I could try storing the function references in a local table indexed by unit_number of the owning entity (unless inventories have a unique index too that I'm not aware of?).