Make drill and laser use the custom on_dig of node(s) it digs

[KaylebJay]

There is a conflict with the drawers mod, which means that when the drill takes a drawer, all the items will disappear that were in the drawer, and you will get the drawer in your inventory. Simplest way to fix this for Tunnelers' Abyss was to do this - so porting here. I have not tested if this is true for the mining laser too, yet.

[auouymous]

The problem is they don't call the on_dig function provided by the node. The following fixes the drill and laser for all mods that use custom on_dig functions.

--- a/technic/tools/mining_drill.lua
+++ b/technic/tools/mining_drill.lua
@@ -57,7 +57,7 @@ local function drill_dig_it0 (pos,player)
         if node.name == "default:lava_flowing" then return end
         if node.name == "default:water_source" then minetest.remove_node(pos) return end
         if node.name == "default:water_flowing" then minetest.remove_node(pos) return end
-        minetest.node_dig(pos,node,player)
+        minetest.registered_nodes[node.name].on_dig(pos, node, player)
 end

 local function drill_dig_it1 (player)
--- a/technic/tools/mining_lasers.lua
+++ b/technic/tools/mining_lasers.lua
@@ -46,7 +46,7 @@ local function laser_node(pos, node, player)
                 })
                 return
         end
-        minetest.node_dig(pos, node, player)
+        minetest.registered_nodes[node.name].on_dig(pos, node, player)
 end

 local keep_node = {air = true}

[KaylebJay]

Looks good. :thumbsup: Would you like to make a separate PR, or shall I instead integrate this?

[auouymous]

Go ahead.

[KaylebJay]

I personally don't think drawers should be diggable with a drill but lets have a dev look at this and decide. I'll remove that if they want :)

[auouymous]

It is inconsistent behavior to allow breaking by hand and with other tools (including the laser) but not with the drill. Ideally, the drawers mod should have preventing breaking when they contain items, like every other inventory mod in the game.

[KaylebJay]

Honestly, that's quite true, drawers shouldn't drop thousands of entities when dug. Since this would be a PR for the drawers mod, I think we can safely remove the drawers overrides for the drill and just use your fix. I'm probably going to end up making a patch anyway to prevent drawers from being dug if they have items, for TA server - several users have complained about it. @SmallJoker when you get some time this (and pull 557) are ready for review :-)

[auouymous]

Do you know that commits can be removed from local branch and the branch force pushed with --force? It keeps the bloody sausage making process out of the master branch. ;)

[KaylebJay]

Never tried that. Sounds tempting. I hope I don't make this PR too messy while trying to figure it out ;)

[KaylebJay]

Thanks for letting me know about that! A great feature, I should have known about it earlier :)

[KaylebJay]

Fixed both, squashed the commits and force pushed again.

[auouymous]

That fix queries node def twice for all normal nodes and doesn't need to (it can be stored in a local). The laser currently does this before calling laser_node and again inside laser_node.

[auouymous]

Your indentation doesn't match the surrounding source, use tabs not spaces.

Your patch adds a second check for node.name=="ignore", look at the line after you query node def.

It would be better to query node def at the bottom because it isn't needed for the block of air/lava/water checks.

And if not def then return end would not only match the block if checks above it, but use two fewer lines.

[KaylebJay]

My editor, for some reason, has the indentation one 'space' behind the code here. And it all looks lined up on the editor. o_O

[auouymous]

The solution isn't to add more spaces, it is to turn off tab-to-space conversion in your editor and make it use actual tab characters.

[auouymous]

Could you uncommit all three commits and push all as a single commit? And it doesn't look like you fixed the indentation, all three patches contain spaces.

[SmallJoker]

@auouymous

GitHub can squash all commits automatically, and the commit text can be edited too. I prefer to see follow-up changes (i.e. doing requested changes) as new commits so that only the diff must be checked. This is extremely helpful for larger PRs (> 300 changes lines) to not re-review the entire thing again to check for newly introduced bugs. EDIT: In this case it does not matter at all. Feel free to squash, or I'll do it on merge.

[KaylebJay]

This is ready for merge, as far as I know.