search

minetest / minetest_game

Minetest Game - A lightweight and well-maintained base for modding [https://github.com/minetest/minetest/]
http://minetest.net/
Other
1.42k stars 572 forks source link

Discuss: Should leaf decay check for a bit flag instead of == 1? #3035

Open Montandalar opened 1 year ago

Montandalar commented 1 year ago

Currently, leaves will decay as long as their param2 is not equal to 1. This means that leaves with param2 > 1 will also decay.

This other values may be used by mods to indicate direction or other values that are important to them.

The question is: should we try to help modders who want to use param2 to hold more information on leaves than just whether they were player placed, and should we try to do so by using a bit flag like 1, 2, 4 (and so on, powers of 2) to indicate manual placement? We have the bitop library to make this easy too.

If default will not accommodate mods, can we instead at least give some kind of best practice for how to handle modded leaf decay - should mods use some kind of library for leaf decay that overrides the default, maybe a library that handles modded leaves differently and leaves the default leaves alone.. or are they best to roll their own, or something else?

Discussion follows from a forum topic discussing leaf decay in the context of Realistic Trees mod. https://forum.minetest.net/viewtopic.php?p=425650#p425650

appgurueu commented 1 year ago

Ultimately, using param2 to store this bit of information is an ugly hack; it would be cleaner to store this in the metadata, but I'm afraid that would have way too much overhead.

Another clean solution would be to have different node registrations for "natural" and "artificial" (player-placed) leaves, but that might end up breaking backwards compatibility. An advantage of this approach would be that tools like WE could be used to set leaves without decay (not sure how you'd currently set all param1s to 1 with WE).

If param2 is only partially used, this hack could be kept by using a bitset, but if mods want to use the full param2 (say, for colored leaves), they are screwed.

I think the best solution as far as MTG is concerned is indeed to expose a "leafdecay" API for modders to override. This pushes the responsibility of making this choice onto the modders; they can keep using a param2 hack if they wish (and if their paramtype2 allows it), or they may use a separate node registration.

sfan5 commented 1 year ago

not sure how you'd currently set all param1s to 1 with WE

//param2 1

To add something to the discussion: I support giving modders an overrideable API but changing it to a bitfield would also be fine.

sirrobzeroone commented 1 year ago

I asked if it was better stored in node meta data back in this issue - https://github.com/minetest/minetest_game/issues/2877

The two issues are close enough that if this one was addressed I believe it fix the problem that caused me to raise the the above.

I know the meta isn't as easy to reference as param2 so that would add code complexity but as the decayability or not does seem specific to nodes I think it does fit with the idea of node meta data....anyways more just a thought/musing I don't have the programming knowledge or skill of yourselves appgurueu/sfan5 so what ever you both decide I know will be the best way fwd.

appgurueu commented 1 year ago

Yes, metadata would be the cleanest solution in principle, but as said, it isn't really low-overhead and our leaves currently don't use param2, so it makes sense to store one bit of information there. For this to be a problem, modders need to override / register leaves which use param2 for something else. Thus I think providing an API as sfan5 proposed is the proper solution, pushing the responsibility onto the modders; they can then decide how they want to store this bit of information (often, just using a different bit of param2 will probably do the trick; many drawtypes don't use the full param2). We could also add support for having a decaying and a non-decaying node variant.

Montandalar commented 9 months ago

Just a thought for a proposal: The API defaults to using the least significant bit for leaf decay, and this is the default, but mods are allowed to provide a callback function which is a predicate (function that returns only true or false) as to whether the node should decay based on the node table. Then mods like [https://forum.minetest.net/viewtopic.php?p=425650](Realistic trees) that use param2 for the plantlike info can use metadata, but others that are simpler can use a bit flag.

Or perhaps for strongest backwards-compatibility, we use == 1 as the default predicate and a predicate that uses the bitop library is provided in the API.