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 minetest.override_entity() #6909

Open
bell07 opened this issue Jan 12, 2018 · 11 comments · May be fixed by #14178
Open

Add minetest.override_entity() #6909

bell07 opened this issue Jan 12, 2018 · 11 comments · May be fixed by #14178
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

@bell07
Copy link

bell07 commented Jan 12, 2018

The "dropped item" entity '__builtin:item' should have an callback in on_step to allow game-specific behaviors like other physics, environment manipulations, light, knock-back, and many others using mods. The best way is a function that is executed before the builtin logic and if the function returns false the builtin logic is skipped. Something like minetest.register_item_entity_on_step(func(obj)) ?

EDIT 2018-05-08: after some discussion the preferred way is to add minetest.override_entity() function that can redefine entity methods without the full re-registration. Like minetest.override_items() does for nodes and items.

@raymoo
Copy link
Contributor

raymoo commented Jan 12, 2018

I don't know if all of these could be solved with on_step callbacks, but I agree this could be useful. An example would be having liquid flow or conveyor belts move items around.

@paramat paramat added the Request / Suggestion The issue makes a suggestion for something that should be done but isn't a new feature. label Jan 13, 2018
@sofar
Copy link
Contributor

sofar commented May 8, 2018

Having an on_step may be costly. How about an event for when the entity is created, and when it is destroyed?

@bell07
Copy link
Author

bell07 commented May 8, 2018

The '__builtin:item' does have already an on_step function to handle falling and item merging. I search for a way to enhance them for other physics. on create and on destroy are maybe usefull to, but not enough.
An other way could be something like
minetest.override_entity('__builtin:item', on_step = new_on_step)

@Ferk
Copy link
Contributor

Ferk commented May 8, 2018

Adding another callback is not really a solution.
What if more than one mod wants to add its own things to handle inside the on_step?

You'll have the same problem, you'll have to override this new callback, and to make sure you don't conflict you should call the old callback inside your own callback.

At the moment there's no register_step for entities (analog to register_globalstep).
Until there's such thing overriding makes more sense than adding a new callback for mods to fight for, imho. Just make sure you do call the original on_step within the new on_step.
That way mods can stack up features without having to add special logic in builtin that has to check for callbacks in every step.

@bell07
Copy link
Author

bell07 commented May 8, 2018

What if more than one mod wants to add its own things to handle inside the on_step?

Primary I search for any way to change the builtin behaviour. Usually it is enoug to redefine the functionality once in subgame. If multiple mods needs to redefine the behaviour, it is possible to create an "on_step = local old_onstep + new_onstep" wrapper chain, like sometimes seen for node's on_use. If really a lot of mods requires them, an builtin_item_manager mod can be written that redefines once the builtin's on_step and the provides the register_item_entity_on_step() function or something like.

New idea is to do it like the minetest_game abm for lava-cooling

core.builtin_item_on_step = function( ...
--
end

register entity(..
on step(...)
    return core.builtin_item_on_step(...)
end

This way it is possible just to replace the core.builtin_item_on_step in other mods

But I prefer the minetest.override_entity() for now (like exitsitng minetest.override_item()) because of the most flexibility

@rubenwardy
Copy link
Member

👎

You should use override entity and wrap around the old on step

@bell07
Copy link
Author

bell07 commented May 8, 2018

But I prefer the minetest.override_entity() for now (like exitsitng minetest.override_item()) because of the most flexibility

Should I adjust the initial post to request the minetest.override_entity()?

@rubenwardy
Copy link
Member

You can already override the entity using minetest.registered_entities, but an override_entity method would be nice

@bell07
Copy link
Author

bell07 commented May 8, 2018

I do not know how the table content minetest.registered_entities is synchronized to the engine, therefore I access such tables read only. To change builtin things an method should exists.

I know the way to get the full definition from minetest.registered_entities, modify the method and then re-register using core.register_entity(":__builtin:item" modified_def), but I do not like this way

@Ferk
Copy link
Contributor

Ferk commented May 8, 2018

This should work:

local builtin_item = minetest.registered_entities["__builtin:item"]
local item = {
    on_step = function(self, dtime)
        builtin_item.on_step(self, dtime)
        -- do your stuff
    end,
    --- additional stuff if needed
}
setmetatable(item, builtin_item)
minetest.register_entity(":__builtin:item", item)

You don't have to directly write into minetest.registered_entities (although you can), the right way would be to use minetest.register_entity to override it, prefixing the name with a ":".

@rubenwardy
Copy link
Member

Entities aren't synced to the engine at all, the engine reads the table. The only reason that items are synced with the engine is so they can be sent to clients - entities have object properties binstead

@paramat paramat added Feature request Issues that request the addition or enhancement of a feature and removed Request / Suggestion The issue makes a suggestion for something that should be done but isn't a new feature. labels Sep 22, 2018
@paramat paramat changed the title Feature request: __builtin:item on_step callback Add minetest.override_entity() Dec 7, 2018
@paramat paramat added the Concept approved Approved by a core dev: PRs welcomed! label Jan 22, 2019
@sfence sfence linked a pull request Dec 28, 2023 that will close this issue
4 tasks
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.

7 participants