Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inventory list: inv:set_list() not working anymore according to docs #13490

Closed
BluebirdGreycoat opened this issue May 10, 2023 · 6 comments
Closed
Labels
Bug Issues that were confirmed to be a bug Regression Something that used to work no longer does @ Script API
Milestone

Comments

@BluebirdGreycoat
Copy link
Contributor

Obligatory Minetest version
Minetest 5.8.0-dev-65692ad1b (Linux)
OS / Hardware

Operating system: 6.2.6-76060206-generic
CPU: dunno, why?

Summary

Use of inv:set_list("listname", {}) inside on_metadata_inventory_take causes error from Lua:

Runtime error from mod 'bleh' in callback nodemeta_inventory_OnTake(): InventoryList 'whatev' is currently in use and cannot be deleted or resized.

This is a problem, because the docs for inv:set_list() state:

set full list (size will not change)

There's a mod running on my server which expected inv:set_list() to do what it used to do a couple Minetest versions ago: that is, to set all (or just most) of the itemstacks in a list at once, without changing its size. (IIRC, I think Minetest would clear out the bits of the inventory list not present in the method's table argument, in the case that the table contained too few elements to fill the whole list?)

Pedantic steps to reproduce
minetest.register_node("invlist:test", {
  description = "Test",
  tiles = {"default_stone.png"},

  on_construct = function(pos)
    local formspec =
      "size[8,2]" ..
      "list[context;test;0,0;8,1;]"..
      "list[current_player;main;0,1;8,1;]"

    local meta = minetest.get_meta(pos)
    meta:set_string("formspec", formspec)

    local inv = meta:get_inventory()
    inv:set_size("test", 5)
    inv:set_stack("test", 5, "default:mese")
  end,

  on_metadata_inventory_take = function(pos, listname, index, stack, player)
    local meta = minetest.get_meta(pos)
    local inv = meta:get_inventory()
    inv:set_list("test", {"default:wood", "default:stick"})
    
    -- It doesn't work even if the number of entries is exactly right >:(
    inv:set_list("test", {"default:wood", "default:stick", "default:apple", "default:chest", "default:tree"})
  end,
})
@BluebirdGreycoat BluebirdGreycoat added the Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible label May 10, 2023
@BluebirdGreycoat BluebirdGreycoat changed the title Inventory list Inventory list: inv:set_list() not working anymore according to docs May 10, 2023
@sfan5
Copy link
Member

sfan5 commented May 10, 2023

dup #13465

@sfan5 sfan5 closed this as completed May 10, 2023
@BluebirdGreycoat
Copy link
Contributor Author

BluebirdGreycoat commented May 10, 2023

How is this a dup? The mentioned issue (#13465) was already closed with a merged solution, but I encountered this bug with the latest git pull.

In the meantime I have to work around this everywhere by using a inv:set_stack() loop in place of inv:set_list().

@Desour Desour added Bug Issues that were confirmed to be a bug Regression Something that used to work no longer does and removed Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible Duplicate labels May 10, 2023
@Desour
Copy link
Member

Desour commented May 10, 2023

#13465 is similar, but it was fixed by allowing resizing in more situations (i.e. if the list is the destination in a listring operation).
The non-resizing set_list() can, and should still be made to work though.

@Desour Desour reopened this May 10, 2023
@sfan5
Copy link
Member

sfan5 commented May 10, 2023

but I encountered this bug with the latest git pull.

I admit I didn't check the commit hash and assumed that we'd already fixed this entirely.

@SmallJoker
Copy link
Member

SmallJoker commented May 10, 2023

The solution here might be to refresh all inventory list pointers within the inventory action code after each callback execution to avoid bad memory access in subsequent callbacks. That will again make the code slower and more complicated/bloated than it already is but eventually solves the issue.

@SmallJoker
Copy link
Member

Cannot reproduce in current master, either due to #13497 or follow-up PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues that were confirmed to be a bug Regression Something that used to work no longer does @ Script API
Projects
None yet
5 participants