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

minetest.dig_node in protected areas will not work (even if you own the area) #13563

Closed
ancientmarinerdev opened this issue Jun 5, 2023 · 8 comments · Fixed by #14505
Closed
Labels
Concept approved Approved by a core dev: PRs welcomed! Feature request Issues that request the addition or enhancement of a feature @ Script API

Comments

@ancientmarinerdev
Copy link

ancientmarinerdev commented Jun 5, 2023

Minetest version

5.7

Summary
minetest.dig_node(pos)

    Dig node with the same effects that a player would cause
    Returns true if successful, false on failure (e.g. protected location)

If I have a protected area, and I want to dig it, I have no way to specify the player. In mineclone2, our pistons cannot dig a block in a protected area because it considers them protected, even if the triggerer is the owner of the protection. It returns false regardless of who calls it. I have no way to tell the engine of the player so it can actually understand if it owns the area.

"Dig node with the same effects that a player would cause" would mean that a player who owns an area should be able to affect it. I would like the facility to pass in a player_name or a digger reference. I fear a digger reference won't cover this with things like daylight sensors, or delayed time based stuff, for example. So passing a name gives the modder control in multiple contexts.

As a modder/game developer, I would like to control who I consider to be calling this, so I can pass this in so a protection check can be correctly done. With remove_node, I can check the protection myself and call, and it will do it, but dig_node prevents me from having any control of this, and I would likely have to implement this logic from scratch to get it to work (get item, get drops, remove node, and add drops in).

I guess two ways to proceed with this are to either remove the protection check and leave that to the modder to do (as it doesn't work), or enable the modder to give enough context for the code to be able to do this correctly.

This requirement has also been mentioned here: #11336 (comment) but for a different issue, so it's worth considering that issue alongside this one, but they are different issues. That is a feature request, this is a bug where protection checks aren't fit for purpose.

Steps to reproduce

Protect an area as a user, for example users areas mod. Let user trigger something that digs the area, it will return false when called from a piston for example and no easy way to get around this and utilise this behaviour.

An overriden function minetest.handle_node_drops won't be called at all

@ancientmarinerdev ancientmarinerdev added the Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible label Jun 5, 2023
@SmallJoker
Copy link
Member

If minetest.node_dig(pos, node, digger) does not suffice for your needs (e.g. offline player), the common approach is currently to fake a player like this:
https://github.com/mt-mods/pipeworks/blob/master/common.lua#L226

@wsor4035
Copy link
Contributor

dropping #11477 here on the subject of fake players.

@wsor4035
Copy link
Contributor

wsor4035 commented Oct 14, 2023

converting this to a feature request, because its asking for a specific player param/capability, not something that is explicitly broken. also unsubscribing, so please ping if you have something further relating to this issue

EDIT: if another triager or core dev thinks this should be a bug, please mark it as such instead of unconfirmed so its no longer on the list to confirm still

@wsor4035 wsor4035 added Feature request Issues that request the addition or enhancement of a feature and removed Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible labels Oct 14, 2023
@ancientmarinerdev
Copy link
Author

ancientmarinerdev commented Oct 15, 2023

converting this to a feature request, because its asking for a specific player param/capability, not something that is explicitly broken. also unsubscribing, so please ping if you have something further relating to this issue

I don't think this is a feature request. The engine supports protection, but minetest.dig_node doesn't support protection.

Creating a fake player is a hacky workaround to make up for the fact the engine doesn't support this. Most systems that need authentication to work, usually allow an authoritative user to support this. The engine in this area isn't fit for purpose.

I think removing the bug label sets a very dangerous precedence of saying the intention is for the engine to have fundamental flaws.

In all honesty, I couldn't actually create a fake user as it's such a bad hack, it feels absolutely rubbish to be adding that into a codebase just to do something that should already be supported.

Closing bug tickets or removing the labels may sweep problems under the carpet. It may make mt devs feel better about it, but it won't solve any problems or contribute to their solution. If there are too many bugs, just add priority levels in for bugs so devs can focus on the highest priority first.

@nauta-turbidus
Copy link

@wsor4035 It is a bug because:

  • minetest.dig_node() is supposed to let the modder (among other things) add unorthodox ways for players to dig nodes – same effects as a player, meaning it can be substituted for normal node-digging-through-punching
  • it doesn't work for some usecases

@ancientmarinerdev
Copy link
Author

A software bug is an error, flaw or fault in the design, development, or operation of computer software that causes it to produce an incorrect or unexpected result, or to behave in unintended ways.

https://en.wikipedia.org/wiki/Software_bug

I strongly believe that that the fact you cannot dig as a player because you cannot pass it in leads to this not behaving in the expected manner (you cannot wear a tool or dig in a protected area). How can you wear a tool if you do not know the digger. In present form, all this does is trigger call backs, and nothing logical or practical from a modding perspective.

@wsor4035
Copy link
Contributor

i was pinged elsewhere rather than here initially, but i digress
posting this here:

im going to speculate here and the current disagreement comes down effects that a player would cause i relate this as the engine does, callbacks that traditionally happen when a player digs the node. see

// Dig it out with a NULL digger (appears in Lua as a
// non-functional ObjectRef)
bool success = scriptIfaceNode->node_on_dig(pos, n, NULL);
. I believe that we are reading the documentation a different way and interpreting this to mean it (should do callbacks as if a player dug it) AND (run as a specified player name). because of the second part of the of the AND equation is why i marked it as feature request

@rubenwardy since you have upvoted my original comment, do you support that this should be kept as a feature request, or converted to a bug?

if ruben does not agree, will convert this to a bug since I am on the fence and that is what the current comments in the issue lean towards

@rubenwardy rubenwardy added the Concept approved Approved by a core dev: PRs welcomed! label Nov 10, 2023
@rubenwardy
Copy link
Member

rubenwardy commented Nov 10, 2023

Honestly not sure whether this should be considered a new feature or a defect as the API isn't super useful as is, but it is working as documented. I don't think it matters a huge amount either way as I support resolving it. (side-note: I find the whole node callback API a bit convoluted, the fact there's both dig_node and node_dig screwed me up quite a few times)

As for the API, it should take an objectref as that's what the rest of the callbacks requires. For offline players, you'll have to use a fake objectref for now like pipeworks. For an engine solution, there's #11477

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Concept approved Approved by a core dev: PRs welcomed! Feature request Issues that request the addition or enhancement of a feature @ Script API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants