Fix make unrepairable

[hlqkj]

I've just been reported an issue by which, after a first call to anvil.make_unrepairable, all the tools registered thereafter erroneously gets the group not_repaired_by_anvil = 1. The malfunctioning appeared after the server admin pulled the mod after my last pull request was merged.

I still don't understend well why this happens (a brief explanation would be greatly apreciated by the way :-) yet I've been able to solve it by creating a new groups table and using that when calling minetest.override_item instead of adding the group to the original one.

Tested and seems to work on official Windows x64 MT 5.3.0 stable, anvil and technic mods installed.

[hlqkj]

My apologies, I think I've made some mistake on here. Still not expert on how pull requests works...

[FaceDeer]

I wouldn't be surprised if there's some finnicky bug in minetest.override_item, I've run into problems with it before. Specifically, I discovered that using it to add a group to a node doesn't update whether that node is handled by ABMs: https://github.com/minetest/minetest/issues/5518

The workaround I used when I ran into that particular problem was to instead read the node's definition from minetest.registered_nodes, add the new group I needed, and then used minetest.register_node(":"..node_name, new_def) to override the node's definition. That seemed to cause the engine to update group-related references to the node where minetest.override_item failed.

It's possible that you've found a new manifestation and a new workaround for the override_item bug I linked above, if it works out I'll update the bug with a mention of this.

[OgelGames]

I still don't understend well why this happens (a brief explanation would be greatly apreciated by the way :-)

It's technically not a bug, it's just the way Lua works; in Lua, tables are essentially passed by reference:

There are eight basic types in Lua: nil, boolean, number, string, function, userdata, thread, and table...

Tables, functions, threads, and (full) userdata values are objects: variables do not actually contain these values, only references to them. Assignment, parameter passing, and function returns always manipulate references to such values; these operations do not imply any kind of copy.

http://www.lua.org/manual/5.1/manual.html#2.2

What is happening is that because the technic tools don't set a groups table, the item definition returns the default groups table (see here), so when it gets modified, it is actually the default table that is being modified, which means that every tool registered after it (that doesn't set a groups table) has that modified table.

This PR fixes that because it creates a new (separate) groups table for each tool, which means the default table is never modified, and doesn't effect other tools.

[hlqkj]

Thanks a lot @OgelGames! I did knew about Lua references and guessed the problem was something like that, hence the table copy. But was missing that the default item definition part, now it all makes sense...

[hlqkj]

Sorry if I stress again on this topic, but just to be sure (I am trying to improve my knowledge... :-). You were right of course, the first technic tool that was getting registered was technic:water_can and it did not defined any group, so the anvil mod was changing the core.tool_default.groups table.

But I realized after answering here that some of the tools that were affected by that issue (having the group when they should've not) were for example from the orbs_of_fimes mod. I checked and yhey actually do define their groups table (see here) so even if the anvil mod was messing the the core.tool_default.groups table these items should've not been affected, correct?

Having had a look at the core.register_item in the builtin mod I saw that either the groups is inherited from the core.tool_default table via the setmetatable() or is set in the itemdef argument given to the register function. I mean, the item groups are not a "merge" of the default ones and the ones given by the mod definition, that would have explained why the orbs had the not_repaired_by_anvilgroup... Am I missing something else?

[OgelGames]

Yes, you are correct, the __index metamethod only returns the default value from core.tool_default table if it doesn't exist in the itemdef table (more in-depth explanation here).

Am I missing something else?

I did notice that not all of the tools from that mod define their groups table, maybe you tested it with one of the ones that didn't?

[hlqkj]

Actually I was referring to that specific tool I linked (orbs_of_time:orb_day), which does define a group but also gets the not_repaired_by_anvil one, see the attached screenshot. I could try to add other mods to see if that affects their tools, but I already verified this behaviour on the server the admin of which reported me this issue.

[OgelGames]

Seems like some other mod must be changing something...

I just tested with only anvil, orbs_of_time, technic, and unified_inventory (along with minetest_game mods), and only orbs_of_time:orb_dawn and orbs_of_time:orb_dusk had the not_repaired_by_anvil group (the ones without groups defined).

[hlqkj]

That's what I'd have expected indeed... Don't know, I am surely missing something in my setup then, I'll keep investigating. Many thanks for you help, I really appreciate this! :)

[OgelGames]

@SmallJoker

[SmallJoker]

This issue is likely caused by a mod (or builtin?) that uses the same groups table to register multiple nodes. EDIT: already pointed out by https://github.com/minetest-mods/anvil/pull/25#issuecomment-732810666