Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mining Drill Breaks Unbreakable Items #261

Open
Bituvo opened this issue Feb 13, 2022 · 5 comments
Open

Mining Drill Breaks Unbreakable Items #261

Bituvo opened this issue Feb 13, 2022 · 5 comments
Labels
Bug Something isn't working Good first issue Good for newcomers

Comments

@Bituvo
Copy link

Bituvo commented Feb 13, 2022

The mining drills provided by this mod are able to break normally unbreakable blocks, such as default:cloud.

Should be simple enough to fix

@BuckarooBanzay BuckarooBanzay added Bug Something isn't working Good first issue Good for newcomers labels Feb 14, 2022
@BuckarooBanzay
Copy link
Member

BuckarooBanzay commented Feb 14, 2022

Relevant code:

local function drill_dig_it0 (pos,player)
if minetest.is_protected(pos, player:get_player_name()) then
minetest.record_protection_violation(pos, player:get_player_name())
return
end
local node = minetest.get_node(pos)
if node.name == "air" or node.name == "ignore" then return end
if node.name == "default:lava_source" then return end
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
local def = minetest.registered_nodes[node.name]
if not def then return end
def.on_dig(pos, node, player)
end
local function drill_dig_it1 (player)

Looks like there is basically just a hardcoded blacklist 😖

Parts of the quarry-code could be used to replace that, or even shared globally:

local function can_dig_node(pos, node_name, owner, digger)
if node_name == "air" or node_name == "vacuum:vacuum" then
return false
end
local def = minetest.registered_nodes[node_name]
if not def or not def.diggable or (def.can_dig and not def.can_dig(pos, digger)) then
return false
end
if minetest.is_protected(pos, owner) then
return false
end
return true
end

EDIT: the vacuum:vacuum node check is redundant btw, the node-def already has diggable = false on it

@OgelGames
Copy link
Contributor

Not sure if this could be a feature-bug or not, probably better to fix it though. (I wonder if it might be good to add fixes for other feature-bugs with a setting to allow them)

Looks like there is basically just a hardcoded blacklist 😖

Yep, the drills haven't gotten a rewrite yet :)

EDIT: the vacuum:vacuum node check is redundant btw, the node-def already has diggable = false on it

That was just a minor optimization for the quarry. (no clue if it actually makes any significant difference)

@Bituvo
Copy link
Author

Bituvo commented Feb 15, 2022

Can we just use the code from the quarry for the drill? It looks like it would work better, since it's less hardcoded.

@BuckarooBanzay
Copy link
Member

Can we just use the code from the quarry for the drill? It looks like it would work better, since it's less hardcoded.

Sure, that's the plan, no one is working on that currently so if you want to try that, go ahead 👍

@S-S-X
Copy link
Member

S-S-X commented Feb 16, 2022

no one is working on that currently

Not on that exact issue but I would still request minimal changes to avoid big conflicts with #233
Or I could finish that one quickly and fork all mods that have not accepted PR to mt-mods org.

There's just one PR I've added few days ago and promised to add another optional PR soon after. not done that yet but I do have code available that should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants