minetest-mods / moreblocks

More Blocks
https://content.minetest.net/packages/Calinou/moreblocks/
zlib License
45 stars 67 forks source link

Crash when recycling default stairs #174

Closed Caellian closed 3 years ago

Caellian commented 3 years ago

Steps to reproduce:

What I believe is happening:

Due to alias definition, contains_item returns true for default stairs, but get_cost does not take aliases into consideration and returns nil.

This happened with acacia stairs, but is likely a problem with all aliased nodes.

Adding

if not cost then
  return 0
end 

before line 261 (if (incost + cost) > maxcost then in circular_saw.allow_metadata_inventory_put) prevents crash from occurring, but a better approach would be to keep track of aliased nodes in aliases.lua try mapping old node into a new one to consistently calculate cost.

Calinou commented 3 years ago

Adding

if not cost then
    return 0
end

before line 261 (if (incost + cost) > maxcost then in circular_saw.allow_metadata_inventory_put) prevents crash from occurring, but a better approach would be to keep track of aliased nodes in aliases.lua try mapping old node into a new one to consistently calculate cost.

Maybe we should return an extremely high cost in this case? We should avoid introducing a duplication bug while fixing this 🙂

Caellian commented 3 years ago

Maybe we should return an extremely high cost in this case? We should avoid introducing a duplication bug while fixing this

Returning 0 from circular_saw.allow_metadata_inventory_put prevents the item from being inserted. Returning a high cost from circular_saw:get_cost is logically worse than returning nil. Cost for an unhandled item does not exist, thus it makes sense for circular_saw:get_cost to return nil.

I suggested using aliases instead of simply returning 0 as it would result in expected behaviour instead of preventing players from recycling default stairs which might be confusing if they don't know what's going on.

So something along the lines of:

function circular_saw:get_cost(inv, stackname)
    local name = minetest.registered_aliases[stackname] or stackname
    for i, item in pairs(inv:get_list("output")) do
        if item:get_name() == name then
            return circular_saw.cost_in_microblocks[i]
        end
    end
end

Right, that fixes the problem. I'll submit a PR.