Wrench support extend

[nixnoxus]

Adds wrench support for bones, connected_chests and some nodes from pipeworks and xdecor.

For more information see: https://github.com/mt-mods/wrench/tree/wrench_support_extend#supported-mods-and-nodes

Related to mt-mods/technic#251

[OgelGames]

I made some changes and added support for more_chests, I hope you don't mind, I thought it would be easier to work on the same branch so we can both work on support for different mods :)

[nixnoxus]

I think that unit tests can only ensure a stable and consistent API.

The interaction with other mods we can only ensure with integration tests (with loading other mods).

[S-S-X]

Could possibly try separate integration tests with actual player interaction by actually loading mods and writing simple test loop where player picks up and places whitelisted nodes.
Similar to node placement test for Technic mod https://github.com/mt-mods/technic/blob/7e466e9b41215306b220d83b932b754294f32e1d/technic/spec/nodes_spec.lua#L44-L46
And placement_test function: https://github.com/mt-mods/technic/blob/7e466e9b41215306b220d83b932b754294f32e1d/technic/spec/nodes_spec.lua#L15-L20

Probably need to implement some missing functionality for Mineunit and allow disabling test failures that happen because of deprecated Minetest Lua API use, many mods are using deprecated functions and currently Mineunit throws errors for some if deprecated version is used.

[nixnoxus]

@OgelGames i found some issues with your changes:

digistuff:detector
29a4857#diff-0de17c9b8ddfd83bc350c98c87ecc8490af2647c2c963e15d08ec519fac173c9L84-R77
=> config loss after replace

digistuff:eeprom
29a4857#diff-0de17c9b8ddfd83bc350c98c87ecc8490af2647c2c963e15d08ec519fac173c9L107-R102
=> right click dosn't work after replace

digistuff:piezo
29a4857#diff-0de17c9b8ddfd83bc350c98c87ecc8490af2647c2c963e15d08ec519fac173c9L216
=> can't pickup after digiline_send("beep", "longbeep")

[OgelGames]

digistuff:detector => config loss after replace

It works fine for me...

digistuff:eeprom => right click dosn't work after replace

Ah... that doesn't set it's formspec the same was as digistuff:ram does...

Fixed here: mt-mods/digistuff@b60d9b3

[nixnoxus]

digistuff:detector => config loss after replace

It works fine for me...

My test setup:
1. place digistuff:detector
2. set channel = c_ob, radius = 1
3. pickup digistuff:detector
4. (re)place digistuff:detector
5. right click digistuff:detector ~~
~~ => channel is now 0
same effect also with mt-mods/digistuff@b60d9b3

Sorry, my mistake

[nixnoxus]

Looks good to me so far.

I think it would be good if we include support for digistuff:advtouchscreen again since it is still included in the upstream digistuff mod. 🤔
(support remove here 29a4857#diff-0de17c9b8ddfd83bc350c98c87ecc8490af2647c2c963e15d08ec519fac173c9L276-L282)

[OgelGames]

I think it would be good if we include support for digistuff:advtouchscreen again since it is still included in the upstream digistuff mod.

IMO we should only support the versions stated in the readme, the API can be used for adding support for other versions/nodes. While it would be good to support more, I think in the long run it will be a lot easier to manage if we only support specific versions.

[OgelGames]

There is still stuff I want to add, but I don't have time for that at the moment, and it's better to make another PR anyway.

I think everything is good enough to merge 👍

[nixnoxus]

@S-S-X please review.

[S-S-X]

Picking up bones: functions.lua:150: bad argument #1 to 'pairs' (table expected, got nil)
Because placed bones do not have inventory, same situation can happen with any other node placed by other mods or direct commands, should be checked.

So no for blacklisting or whitelisting certain names but checking if that specific node has inventory and prevent pickup if it does not have, I would also recommend against digging said node because that can increase complexity and not exactly what wrench should be doing.

Simply do not do anything, maybe chat message telling that node cannot be picked up with wrench because it does not have inventory.

[nixnoxus]

Picking up bones: functions.lua:150: bad argument #1 to 'pairs' (table expected, got nil)

Fixed.

Simply do not do anything, maybe chat message telling that node cannot be picked up with wrench because it does not have inventory.

A chat message 'Cannot pickup node. Node has no inventory.' is written.

[nixnoxus]

if no objections come, I will merge this weekend and create a new branch / merge request for further customization.

[S-S-X]

Seems "mostly" fine... mostly because I've actually only read at most half of it, you know it is huge PR with over 2200 LoC added...

[OgelGames]

It's been two months since I approved this, I think we should just merge this now.

[OgelGames]

I don't know why github deleted the branch automatically, but I'm gonna leave it to exist for a little bit... :)