Add node texture variants

[sfence]

This is the adoption of #12928. Closes #9193

I have rebased the original @TurkeyMcMac code and tested it.
It looks to be working without some artifact on my Linux.

To do

This PR is Ready for Review.

How to test

There are test nodes in the developer test game. They can be used to test.
There is also the command /node_visual which allows to change variant_offset for the wanted node.
Example of command: /node_visual testnodes:variant_color variant_offset 2

[sfence]

According to this: https://irc.minetest.net/minetest-dev/2023-01-08#i_6046142

it seems that there is nothing left to be done.

Am I right?

[Bituvo]

Would it be possible to add weights for each variant?

[sfence]

Would it be possible to add weights for each variant?

What do you mean by it?

[Bituvo]

What do you mean by it?

So that one texture is more likely to appear than another, for example. Like, dirt_1.png could have a 75% chance of appearing, but dirt_2.png could have a 25% chance.

[sfence]

What do you mean by it?

So that one texture is more likely to appear than another, for example. Like, dirt_1.png could have a 75% chance of appearing, but dirt_2.png could have a 25% chance.

So, I understand now.

This uses the node param2 value to choose the texture. So if the mod author wants to make some texture more common, he only needs to reflect in on_construct callback or/and in the mapgen generation code.

[doxygen-spammer]

Here Github frustrates me again, as it refuses to expand the C++ diffs. :(

I left comments in the code.

Maybe it should be made clear if you need to override the same set of properties in each variant, or if it is allowed to e. g. add an overlay tile only to variants 3 and 6.

[sfence]

Here Github frustrates me again, as it refuses to expand the C++ diffs. :(

I left comments in the code.

Maybe it should be made clear if you need to override the same set of properties in each variant, or if it is allowed to e. g. add an overlay tile only to variants 3 and 6.

So, It should be documented now as well.

There is probably a bug in overlay_tiles which uses color. I will look at it later.

[doxygen-spammer]

Documentation updates are satisfying. I wonder if item metadata needs to have both palette_index and variant. Probably yes, because in non-nodes they can be independent. I got the whole patch now, and have no further comments.

…

--- I don’t want to put a requirement on this pull request—but I got the idea that `description` should be variant-able. Then you can use one node definition for all colors of wool, with each color having the appropriate description, without manually creating item drops. (And with the configurable bitfield, it also works with colorfacedir. :) )

[sfence]

With the last commit, this PR should be able to fix issue #13417 as well and possibly make issue #13112 obsolete.

[doxygen-spammer]

Yeah, the per-player-per-nodedef variant offset is a cool idea. Finally allows to make nodes visible only to certain players, like ParticleSource.playername can do it for particles. The “barriers” of Minecraft, which are only visible while wielding a barrier, can be implemented this way. Honestly I do not think it _needs_ to be included in this pull request. But if it is included, it seems the documentation in lua_api.md is still missing. (I would also like this on the clientside modding API, for responsiveness.)

[doxygen-spammer]

*lightbulbgoeson* Ah, the variant offset explains why the API currently allows to define a higher variant count than can be accessed from the param2 bitfield. :)

[sfence]

@doxygen-spammer
I have rebased this. It includes lua_doc.md update, so the set/get_node_visual methods are documented now.

[doxygen-spammer]

Nice!

[kromka-chleba]

With the last commit, this PR should be able to fix issue #13417 as well and possibly make issue #13112 obsolete.

It would indeed fix the first issue but not the second one. ABMs and LBMs do more stuff than simply swapping textures. Sometimes you want to remove the node completely, move the nodes or maybe spawn a structure. Not being able to change ABMs/LBMs after they are created makes them inflexible while hitting performance.

[Zughy]

@sfence rebase needed

[sfence]

Wouldn't it be better to make it so metadata can modify the image texture. And the param2 can be used for facedir, or color instead ?

it is possible to do it that way. However, it will probably be a controversial solution because it can result in many metadata reading during rendering.
Using metadata sounds acceptable for some nodes, where we can expect sporadic occurrence.

[DragonWrangler1]

Once I get some time later I’ll test this out again.

[DragonWrangler1]

Ok. So after some playing around I made a node that uses player:set_node_visual in an on_construct function, and it seems that it force closes mt (unless I made a mistake in the function), but here's what I get in the debug log.

2024-08-18 08:37:41: ERROR[Server]: In thread 743b937fe6c0:
2024-08-18 08:37:41: ERROR[Server]: /home/user/minetest-variant/src/server/clientiface.cpp:306: void ClientInterface::send(session_t, NetworkPacket*): A fatal error occurred: packet type missing in table

[Zughy]

@sfence rebase needed

[cx384]

It's a little late, and maybe I'm the only one who thinks this way, but wouldn't it be way better to put this feature into the tile definition instead of polluting the node definition table even more. This way, you would also avoid problems with future tile features.
It would for example look like this.

tiles = {{
    name = "mod_grass1.png",
    color="green",
    variants = {"mod_grass2.png", {name = "mod_grass3.png"}, {name = "mod_grass4.png", color="red"}},
}}

I like this feature very much, but I don't like how the API looks like.

[Zughy]

@sfence rebase needed

[doxygen-spammer]

wouldn't it be way better to put this feature into the tile definition instead of polluting the node definition table even more.

I think it would be more pollution if node variants are expanded to cover sounds, description, etc. Otherwise it would seem better to me, at least slightly better.

[cx384]

I think it would be more pollution if node variants are expanded to cover
sounds, description, etc.

I get where you're coming from, but being able to override arbitrary node def fields with param2 is nonsense.
In most cases, it is better to just register a new node (also for textures), and when you want to make something dynamically changeable, param2 is the last thing you would use if nothing else works, since it has only 8 bits.
E.g. for sounds it is better to use node meta and the description is only for the item and can already be changed with item meta.
(being able to dynamically change a node attribute in multiple ways (param2 & meta) would create a mess, and make it unnecessary complicated to figure out which one is actually used.)

Also, for a feature to occupy such a limited resource like param2 bits, you need sufficient use cases. It is not worth maintaining useless features that nobody uses.
Texture variants with param2 is more of an exception, it is like you would define your own paramtype2. Maybe other node fields will be changeable with param2 in the future, but in this case, I still think it would be better to put it into the corresponding node def field.

Edit:
I don't want to hold back this PR too much, because of bikeshedding. So keep the API if you insist, it's not that important.

[sfan5]

Feel free to wait for my review before rebasing.