-
Notifications
You must be signed in to change notification settings - Fork 2k
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 an item pick up callback (2) #7712
Conversation
I think it would be useful to factor out the "call callbacks and give item to player" code into a separate function that is part of the modding API, similar to |
They can punch the item entity object. |
That's true |
What would be an example use for this feature? I'm worried that in many cases you would want the same callbacks to run if a player gets an item by digging a node (for example). |
Some examples:
|
Looks good. I have tested it and it works as promised. Test Lua code:
The only thing I'm missing is that you should add the |
For those who want to overwrite the item entity, all you need to do is to call the callback in your custom item definition in the event of a pickup. |
More use cases:
|
The existing crappiness of |
Let's wait what the core devs have to say. |
Actually, an implementation similar to the one for item drop would be nice to have. There could be a |
This needs to be implemented in a such a way that a Minecraft-like itemdrop mod can reuse these callbacks In MTG, this callback should be called when the player digs a node and it goes into the inventory, and when the player punches a dropped item. The |
A Minecraft-like itemdrop mod can call
No. That should be a separate callback. It would need very different parameters (such as itemstack). Maybe something like |
Does someone want to approve or request changes? @minetest This PR adds a useful callback. Even minetest_game would profit from it. |
After some testing I found out that I made a mistake: the itemstring is not always exactly the itemname. |
I did tests. Everything works as in doc described. |
What about mods that add vacuuming nodes? Shouldn't they also be able to get the correct picked-up item? Such mods have no way to tell the punch where to put the items (unless you use the forbidden hack of temporarily using a player's inventory for storage, and then transferring the item to the node's inventory). |
They can either use a fake player or (Actually there's a missing check for |
fake player may work but should not be the intended solution because it's a hack. nil won't work because there is no inventory to place the item in. Since this is a new feature, it can be designed not to need hacks. |
Yes, but that would probably make the change over complex which prevents a merge. :/ And it's not that hacky if a vacuuming mod runs the callbacks itself with Of course I'm open to suggestions btw. |
#7712 (comment), is it possible to follow this approach? raymoo also suggested this, IIRC. |
What would be the benefit of that approach? For minetest/src/script/cpp_api/s_item.cpp Line 40 in 325bf68
But is it needed for on_pickup ?
And |
A function implementing default pickup behavior could be useful for use in I think what I was suggesting is different though. It would be some kind of function |
Resolved comments. |
builtin/game/item_entity.lua
Outdated
if not core.run_callbacks(core.registered_on_item_pickups, 3, | ||
ItemStack(itemstack), hitter, self.object, ...) then | ||
return | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would you think of making this similar to minetest.item_eat
? If added to the itemdef metatable, the function could serve as a default for all picked up items.
Just thinking out loud because these two look very similar, and having less different concepts would be great to have. Like this, you could use on_pickup = minetest.item_eat(2),
in an ItemDef to eat by picking up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed the obj parameter to a pointed_thing
, so on_pickup = minetest.item_eat(2),
should work now.
In that sense, it is now more similar to on_use
.
Idk. how I should make it more similar to item_eat
.
API proposal: https://0x0.st/o4v6.patch
|
* formatting issues (space indentation and whitespace at end `-´) * ipairs * if no inv, everything is leftover * item_pickup is default in itemdef, similar to on_place * itemstack:get_definition() always returns a def table * set itemstring to "", to avoid item duplication * devtest: keep the cases where we return nil * devtest: only print for callback items
* copy itemstack before calling callbacks * builtin item set_item always resets rotation (probably because automatic_rotate is set (might be an engine limitation)) * set_name returns a bool * dont spam chat for all items
The callbacks are now pretty similar to Now, the question arises whether we even need the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, the question arises whether we even need the registered_on_item_pickups
I don't mind. It's up to you whether or not to keep this callback.
Tested the PR again. Works as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few things.
Co-authored-by: SmallJoker <[email protected]>
Co-authored-by: Jude Melton-Houghton <[email protected]>
* optional args * doc params only once
I don't mind much either. minetest/builtin/game/item.lua Lines 559 to 562 in 525fc38
Comments addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Co-authored-by: Jude Melton-Houghton <[email protected]>
Co-authored-by: SmallJoker <[email protected]> Co-authored-by: Jude Melton-Houghton <[email protected]>
This PR adds a callback for when items are picked up via punch.
There's also an item in devtest for testing.
(Old PR was: #6359)
API:
on_pickup
callback in the item definition.register_on_item_pickup
function with that you can register callback functions for all items being picked up.lua_api.txt
.Use-cases:
on_pickup
callback to item definition that setsself.itemstring
to another itemstring and returnstrue
.