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

Make it more convenient to implement item/player/entity abilities that modify node drops #15872

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

j-r
Copy link
Contributor

@j-r j-r commented Mar 7, 2025

Context

core.node_dig(pos, node, digger) uses core.get_node_drops(node, toolname) to determine the drops that should result from digging a node. The builtin implementation of get_node_drops uses a well documented not so simple algorithm to compute the drops from data recorded in the node and tool definition while builtin node_dig handles varied tasks like removing the node, applying wear to the wielded tool, and calling callbacks. For most games/mods it's not desirable to completely replace the builtin implementations, but rather hook into them to e.g. implement player abilities or tool enchantments that may modify yield. But since the builtin implementation of get_node_drops only needs the name of the tool and the node data, nothing more is available and while the other function related to drops, core.handle_node_drops(pos, drops, digger), has access to the digger and therefore the wielded tool, that one is only called after wear is applied and the tool potentially destroyed. Workarounds are possible (e.g. overriding node_dig to add some fields to the node parameter that is currently passed unmodified to get_node_drops and then calling the original node_dig function), but a satisfying solution within the existing API at least isn't obvious.

Proposed solution

The simple solution proposed in this PR is making node_dig also pass the tool itemstack and the digger object reference to get_node_drops. These aren't used by the builtin implementation, therefore there is no functional change, but provide overrides with the access to relevant meta data. An argument could be made to also pass pos to get access to node metadata, and I'm open to adding that, too, but my feeling is that preserve_metadata in the node def is already a viable option for use cases related to node metadata.

Related issues

This PR is perhaps a tiny little bit related to (closed) issue #13740 in so far as it opens more ways to modify node drops.

Status

This PR is Ready for Review.

How to test

Since this is mostly a documentation change, there should be no observable change at all as long as mods are not using the additional parameters provided to core.get_node_drops.

This gives games/mods the ability to modify node drops depending on item
and/or player metadata without overriding node_dig or other workarounds.
@cx384 cx384 added @ Script API Feature ✨ PRs that add or enhance a feature labels Mar 7, 2025
@Zughy Zughy added the Roadmap The change matches an item on the current roadmap label Mar 8, 2025
@CandyFiend
Copy link

CandyFiend commented Mar 28, 2025

If I understood correctly, core.node_dig(pos, node, digger) and core.get_node_drops(node, toolname) is what I was looking for to solve my problem. I can redefine core.node_dig(pos, node, digger) and pass pos and digger to core.get_node_drops(node, toolname). And I can change the logic of drop table processing in core.get_node_drops(node, toolname), for example, so that the engine processes my custom fields that I come up with.

I've prescribed this for developers who will be looking for information on custom drop table processing in the future so they know what to do.

Node position is not necessarily needed for meta, it is quite possible that I will want to check neighboring nodes or players.

For example, I am developing an RPG. The player is wearing an item that increases the number of coins (energy for using skills) dropped from monsters dying nearby. And according to the intended mechanics, it doesn't matter who exactly killed, it's enough that the player with the bonus is nearby.

Or I need to check that there are other nodes around the excavated node. I'm redefining core.get_node_drops(node, toolname) by adding a call to the condition checking functions from the drop table. And I need to pass the pos into one of these functions, which will check neighboring nodes. To do this, I will have to rewrite core.node_dig(pos, node, digger). Probably, if necessary, you can do it yourself.

@@ -514,7 +514,7 @@ function core.node_dig(pos, node, digger)
.. node.name .. " at " .. core.pos_to_string(pos))

local wielded = digger and digger:get_wielded_item()
local drops = core.get_node_drops(node, wielded and wielded:get_name())
local drops = core.get_node_drops(node, wielded and wielded:get_name(), wielded, digger)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
local drops = core.get_node_drops(node, wielded and wielded:get_name(), wielded, digger)
local drops = core.get_node_drops(node, wielded and wielded:get_name(), ItemStack(wielded), digger)

should probably do this so the callback does not accidentally modify the wielded item

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature ✨ PRs that add or enhance a feature Roadmap The change matches an item on the current roadmap @ Script API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants