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

Add optional InvRef to minetest.dig_node(pos) #11336

Open
Sokomine opened this issue Jun 11, 2021 · 9 comments
Open

Add optional InvRef to minetest.dig_node(pos) #11336

Sokomine opened this issue Jun 11, 2021 · 9 comments
Labels
Feature request Issues that request the addition or enhancement of a feature @ Script API

Comments

@Sokomine
Copy link
Contributor

Observed behaviour: Nodes digged with minetest.dig_node(pos) are properly digged - and then their drops are dropped as items.

NPCs and/or nodebreakers do need quite a lot of code to handle proper node digging. It is not very helpful if the items are dropped to the ground - they can't pick them up easily and thus need to implement their own functionality.

It would be very helpful if an optional target InvRef could be specified that receives the node drops. Perhaps even with the target list as a further additional parameter (or else "main").

It might also help to be able to act in the name of a given player in order to operate in protected areas.

And last, specifying a tool for the digging might sometimes be useful.

@Sokomine Sokomine added the Feature request Issues that request the addition or enhancement of a feature label Jun 11, 2021
@hlqkj
Copy link

hlqkj commented Jun 11, 2021

It might also help to be able to act in the name of a given player in order to operate in protected areas.

Maybe a little out of topic here, but it would be really nice to have something like minetest.create_fake_player(def) included in builtin, to be used to get a valid puncher in such situations. Mods created their own implementations over time (e.g. see the pipeworks mod) but would be nicer to have a standard API for that imho.

@Sokomine
Copy link
Contributor Author

Such a fake player function inside MT would definitely be great and help NPC mods as well - even in more situations than just dig_node.

@hlqkj
Copy link

hlqkj commented Jun 11, 2021

Also, adding a digger param to minetest.dig_node(...) would help your case, as digger would (optionally) expose both the wielded item and an InvRef of the "fake" player.
And of course a player name too, so you can impersonate one and still be able to check the protection.

Same applies to punch_node(...) of course.

I like the idea of being able to specify in which inventory list to put the drops, though!

@Emojigit
Copy link
Contributor

Emojigit commented Jun 13, 2021

I suggest: minetest.dig_node(DiggerRef) EDIT: minetest.dig_node(pos,DiggerRef)
DiggerRef can be:

  • ItemStack, apply the special effect of the tool (for example, Fortune of MineClone2) to the digging event, the player name is blank while checking protections.
  • Entity, check protection with its player name if it is a player, otherwise check with a blank name. Then, use it wield item or his "hand" to break that node.
  • string that contains a player name.

@wsor4035
Copy link
Contributor

I suggest: minetest.dig_node(DiggerRef)
DiggerRef can be:

  • ItemStack, apply the special effect of the tool (for example, Fortune of MineClone2) to the digging event, the player name is blank while checking protections.
  • Entity, check protection with its player name if it is a player, otherwise check with a blank name. Then, use it wield item or his "hand" to break that node.
  • string that contains a player name.

this will break backwards compat, it is better to add additional parameters as sokomine originally suggested

@hlqkj
Copy link

hlqkj commented Jun 14, 2021

this will break backwards compat, it is better to add additional parameters as sokomine originally suggested

How so? Just adding digger as an additional parameter that behaves like @Emojigit suggested would mean older code just keeps functioning like it is now by just ignoring that parameter.

Adding discrete params like initially suggested is limiting in conparison to all the stuff that can be included in a "DiggerRef" imho...

EDIT: to be even clearer on what I mean, I'm talking about re-using a PlayerRef as digger -- not inventing anything new. That "object" already has all the needed stuff to implement things like digging with a specific wielded item, an inventory into which place drops, a "player" name to check protection, etc.

@wsor4035
Copy link
Contributor

wsor4035 commented Jun 14, 2021

How so? Just adding digger as an additional parameter that behaves like @Emojigit suggested would mean older code just keeps functioning like it is now by just ignoring that parameter.****

they are suggesting DiggerRef as the only parameter with 3 types in it. this WILL break mods that expect minetest.dig_node to accept only a pos. as Sokomine suggested originally, adding a additional optional field will keep backwords compatibility

EDIT: not to mention there idea fails to include a pos input at all for the node to be dug

@hlqkj
Copy link

hlqkj commented Jun 14, 2021

Granted, my bad. I assumed they were meaning to add a parameter...

@Emojigit
Copy link
Contributor

this will break backwards compat, it is better to add additional parameters as sokomine originally suggested

Updated my suggestion to make backwards compat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature request Issues that request the addition or enhancement of a feature @ Script API
Projects
None yet
Development

No branches or pull requests

5 participants