search

S-S-X / mineunit

Minetest core / engine libraries for regression tests
Other
10 stars 6 forks source link

How am I meant to test empty ItemStacks from crafting recipes? #80

Open Montandalar opened 2 years ago

Montandalar commented 2 years ago

Hi,

So I am implementing a privileged-protected recipe system using register_craft_predict and register_on_craft. In these callbacks you get a table with 9 ItemStacks, and in my test for the empty slots in the grid I thought I would insert ItemStack(""), but the source of Mineunit says it's meant to cause an error with an empty string. I don't think my function will work without "" as the argument though. Do you know how I should proceed?

Code is here:

S-S-X commented 2 years ago

I think I've just followed documentation here:

An ItemStack is a stack of items.

It can be created via ItemStack(x), where x is an ItemStack, an itemstring, a table or nil.

Empty string however is valid with actual engine, for some reason I did leave explicit note "do not fix" there but I do not really anymore remember reasoning for that. Not too long ago I was going to change that to allow empty string but in the end for some reason did not change it...

Reasoning could have been strict rules: "be explicit" which would mean that empty stack should be created with

stack = ItemStack(nil)

instead of

stack = ItemStack("")

Now I'm pretty sure this is a bug in Mineunit and now again I'm leaning towards fixing it, would be nice to remember exact reason why I originally decided to do that...

Also now that I tested engine behavior:

So even if it was for "strict" rules it would still be bad option as ItemStack() is allowed and crashes older engine while ItemStack("") succeeds with old engine.

S-S-X commented 2 years ago

For your test you can simply use ItemStack() instead of ItemStack(""), I'll leave this issue hanging here for a moment attempting to recall why I've stated "do not fix" for apparent bug.

That explicit "do not fix" note tells me that there might be some actual reason behind it, when I wrote note I already knew that I'm going to spot bug some day there so there has to be some reason why I wrote that note...

S-S-X commented 2 years ago

So far I've not found any real reason for what I've written there... this should be fixed, preferably with behavior depending on engine version and warnings for usage that is not universally compatible: call without args throws warning or error.