minetest icon indicating copy to clipboard operation
minetest copied to clipboard

Add drawtype sunken and covered

Open sfence opened this issue 1 year ago • 32 comments

Add new drawtypes sunken and covered.

  • Extend drawtypes.
  • Ability to render node in the node.
  • For sunken doors in shipwrecks...
  • For some archeology mod -> possibility to uncover node covered by sand etc.
  • it simply adds a link to another node and draws two nodes in the same position.
  • Is related to #3074 at least.

To do

This PR is a Work in Progress.

  • [x] Documentation in lua_doc.md
  • [x] More examples
  • [x] Update transformLiquids logic to prevent overwriting of the sunken node by alternative_source
  • [x] Rendering logic for liquids and sunken node updated.
  • [ ] Change param2 usage in covered drawtype for more flexibility.
  • [ ] Add lua script check to prevent setting another sunken or covered drawtype node as inner_node of sunken/covered node to prevent the possibility of infinite rendering loop.

How to test

Run game devtest and place added test nodes testnodes: testnodes:sunken_torchlike, testnodes:sunken_nodebox, testnodes:sunken_mesh, testnodes:covered_torchlike_node, testnodes:covered_nodebox_node and/or testnodes:covered_meshnode. You can change param2 of testnodes:covered_torchlike to simulate the uncovering process.

screenshot_20231214_153449 screenshot_20231214_164317

sfence avatar Dec 14 '23 12:12 sfence

I don't think those new drawtypes are particularly bad, but it would be good to have a discussion first, how useful they are and some use cases. plantlike_rooted for example already works in water and you can use special_tiles to make it look similar to your covered drawtype.

cx384 avatar Dec 14 '23 13:12 cx384

I don't think those new drawtypes are particularly bad, but it would be good to have a discussion first, how useful they are and some use cases. plantlike_rooted for example already works in water and you can use special_tiles to make it look similar to your covered drawtype.

I know about planlike_rooted, but how it can be used to do sunken two-node height doors on shipwrecks for example? Doors typically included the main node and the invisible node above. And what about the sunken trap door? Can I use plantlike_rooted to make nodebox or mesh-based node look like covered or sunken?

Or can be special_tiles used to show, particularly covered nodes?

I have to add more examples. :)

sfence avatar Dec 14 '23 14:12 sfence

@cx384 So I have added a new screenshot to show how it works with testnodes:nodebox_fixed node.

sfence avatar Dec 14 '23 14:12 sfence

I'm still not very convinced. For the nodebox inside a node covered it may be better to just have a drawtype that allows multiple nodeboxes inside one node (with different textures).

Or add the possibility for node boxes to use special_tiles instead of the covered type.

For liquids your PR may be able to fix #3074.

cx384 avatar Dec 14 '23 15:12 cx384

@cx384 It can be an interesting update for nodebox nodes. Some solution like it comes to my mind too. But I think this solution is more flexible.

I only include "link" to another node a draw two nodes in one position.

I think you can't do this with nodebox at all: screenshot_20231214_164317 (it is all "testnodes:covered_mesth_node")

So, you can use it with mesh, plant, nodebox, torchlike, etc nodes. And probably whatever type of node that will be added in the future.

sfence avatar Dec 14 '23 16:12 sfence

just popping in to say plantlike_rooted is a hack that sorta gets you suken/covered nodes, as it requires you to have the node underneath accessible/or extend your node very long (see kelp), and then add additional logic to to handle breaking "per node". also it requires you to have multiple variations if you want different types of nodes underneath(sand/gravel/dirt/etc for example)

wsor4035 avatar Dec 14 '23 16:12 wsor4035

covered may not be necessary for mesh, since you can incorporate the cover into the mesh, but it is still useful.

A use case I can think of is a sapling covered in snow.

cx384 avatar Dec 14 '23 17:12 cx384

covered may not be necessary for mesh, since you can incorporate the cover into the mesh, but it is still useful.

A use case I can think of is a sapling covered in snow.

unless im misunderstanding something here, to do this currently you would have to make #levels meshes/nodes (cutting into the 32k limit) currently since the covered type means the height updates with param2

wsor4035 avatar Dec 14 '23 17:12 wsor4035

This is extremely useful, if it solves the problem it claims to - doors and other items creating air pockets in Mesecraft is very annoying and also buggy- its possible to breathe underwater for as long as you like by placing some node that creates an airpocket (like a slab). A conceptual issue is, will this allow sunken nodes to be mesh or nodebox drawtypes?

MisterE123 avatar Dec 14 '23 22:12 MisterE123

If I understand correctly, you could superimpose any node?

You could just have two superimposed water nodes for example? and the entites would be affected by both of their properties? Would they be cumulated? So twice as slow for two water nodes?

Do you have to register a different node for it to be sunken in different other nodes? door in water, door in river water, door in lava Could flowing water seamlessly flow through nodes? Could it stop or flow depending on the door/trapdoor position?

And I assume lava would have to check for water and the sunken drawtype to work again, right?

If the node is sunken in lava, I'm assuming the lava ABM would not be used, right? Which means every node that could be sunken in lava would need an ABM that checks if it is currently sunken in lava (or any other liquid with an ABM), and trigger that ABM if so.

Lemente avatar Dec 14 '23 23:12 Lemente

@wsor4035 Yes, I think you get it. In this proof on-concept version I am using the top 4 bits in param2 to calculate covered node height.

sfence avatar Dec 15 '23 05:12 sfence

@MisterE123 Yes, it is a target for sunken drawtype. I have to fix rendering on contact with the original liquid and sunken node. Node sides should not be rendered here to make it look well. This problem is connected with liquids updating logic in the transformLiquids method via liquid_alternatives files in node definition. So I have to find some nice way to fix it with existing node definition fields or add some field.

sfence avatar Dec 15 '23 05:12 sfence

@Lemente

If I understand correctly, you could superimpose any node?

Yes, this solution can be applied to any other registered node.

You could just have two superimposed water nodes for example? and the entites would be affected by both of their properties? Would they be cumulated? So twice as slow for two water nodes?

You can apply it to another liquid node as well, but you have to keep in mind, that it is only a rendering feature. So any special logic for mixed liquid nodes will have to be scripted. For the server only properties of the parent node are relevant.

Do you have to register a different node for it to be sunken in different other nodes? door in water, door in river water, door in lava Could flowing water seamlessly flow through nodes? Could it stop or flow depending on the door/trapdoor position?

Yes, you have to register a node for every possible combination of sunken node and liquid. I do not see a way to go around this without using metadata for rendering or something similar. However, the use of metadata for rendering will probably have a bad effect on rendering performance.

I am working with two solutions in my mind.

  1. Simply keep in the hands of ABMs and Lua callbacks.
  2. Try to do some integration with the actual liquid updating engine - maybe this can be done in another pull request later if this pull request is approved.

And I assume lava would have to check for water and the sunken drawtype to work again, right?

I think this is solved by lava ABM looking for nodes in group:water. So simply adding node sunken in water to that group should be enough.

If the node is sunken in lava, I'm assuming the lava ABM would not be used, right? Which means every node that could be sunken in lava would need an ABM that checks if it is currently sunken in lava (or any other liquid with an ABM), and trigger that ABM if so.

It depends on ABM's definition. If ABM works only for node default:lava_soruce for example, it will not work. If ABM is defined to work for all nodes in group group:lava, it can be enough to add a sunken node into this group. But depending on the situation, some changes in ABM logic can be required too.

sfence avatar Dec 15 '23 06:12 sfence

Commit e73b190 fix liquid source replacing iwth alternative liquid source. This is also related to problem mentioned here https://github.com/minetest-mods/wielded_light/issues/4 by @bell07. So if this will be merged, wielded_light mod liquid sources can be updated to be better integrated.

This also allows sunken node to generate flowing liquid of the wanted type. This can be also used in wielded_light.

sfence avatar Dec 16 '23 10:12 sfence

screenshot_20231216_110559 screenshot_20231217_131933

So, the sunken nodes now look natural in the associated liquid.

sfence avatar Dec 16 '23 10:12 sfence

could this be used for waterlogged nodes ? , if so is it possible to have a filter for where to cover a node (so we could waterlog only 1 side of glass pane for instance)

tigercoding56 avatar Jan 03 '24 00:01 tigercoding56

could this be used for waterlogged nodes ? , if so is it possible to have a filter for where to cover a node (so we could waterlog only 1 side of glass pane for instance)

Liquid nodes are rendered as solid nodes by the engine. So, it means that you can use different texture for each side and the node should be rotable with param2. So, it should be possible to realize it in a way, which will look like a glass pane with water on one side.

It will be a good idea to set liquid_range to 0 and liquid_renewable to false for nodes like this.

sfence avatar Jan 03 '24 03:01 sfence

So, this PR is related to a "Supported by core dev" issue, but

  1. it looks like it adds more than that
  2. the label was put by a former core dev, who left the organisation years ago

For the aforementioned reasons, I'll leave the choice about what to do with this PR to core devs in the next meeting => https://dev.minetest.net/Meetings#2024-01-21

Zughy avatar Jan 14 '24 22:01 Zughy

It's quite hard to judge if I support this if there's no detailed description of what this actually does. Doc draft needed.

Desour avatar Jan 16 '24 23:01 Desour

@Desour I added some info to lua_doc.md, but screenshots are probably still better to show what it does than a thousand words.

If you have any additional questions, feel free to ask.

sfence avatar Jan 17 '24 18:01 sfence

Ah, I see now. So for sunken nodes, you need to register a new node for each liquid x inner node pair. And this doesn't work if the inner node needs param2, I suppose? It's obviously an imperfect solution. Idk if it is better than nothing.

The covered drawtype with level looks interesting to me for dust covered plants (like the snow sapling mentioned before).

=> I'm unsure and will wait for more opinions first.

Desour avatar Jan 17 '24 19:01 Desour

Ah, I see now. So for sunken nodes, you need to register a new node for each liquid x inner node pair. And this doesn't work if the inner node needs param2, I suppose? It's obviously an imperfect solution. Idk if it is better than nothing.

The covered drawtype with level looks interesting to me for dust covered plants (like the snow sapling mentioned before).

=> I'm unsure and will wait for more opinions first.

In the actual implementation of the covered draw type I use param2 & 0xF0 for level and param2 & 0x0F value is used by the inner node. So it is not ideal, but still useful for some nodes. Sunken drawtype does not use param2, so full param2 is available for the inner node now.

Because param1 is needed for light, the only different way how to implement it is probably to use metadata. However, using metadata for rendering maps does not sound like a great idea because of its performance. On the opposite side, with metadata, inner_node can be specified from metadata as well.

If it is requested, I will make the same changes to do it in the preferred way.

sfence avatar Jan 17 '24 19:01 sfence

New ideas in my mind about using param2 and field inner_node:

It can be determined by leveled_max value, how many bites can be used for the inner node.

And it will not be probably complicated to implement some loaded metadata cache based on unordered_map for example, to prevent accessing node metadata on every scene rendering. So, it should fix potential performance problems and sounds like a preferred solution to me.

sfence avatar Jan 17 '24 20:01 sfence

Discussed in the meeting: https://irc.minetest.net/minetest-dev/2024-01-21#i_6147292

  1. The main goal appears to be to submerge nodes under liquids. This requires to register each node combination again - which is very inefficient.
    • Proposed solution: introduce a node def field to indicate whether a node may be submerged (e.g. submerge = "none|liquids|custom"). The on_flood` callback also exists to implement special rules in Lua (e.g. replacing the node in certain liquids)
    • Merging two nodes cannot be seamless. There's only param2, thus limited possibilities. I did not check the code yet but I'd guess that you could not have a liquid level + node rotation at the same time. An additional submerge field would allow more possibilities.
  2. Observation: Some of the functionality appears to be similar in concept to glasslikeliquidlevel.
  3. The code is currently not very readable and possibly duplicated. Needs proper reviewing to provide suggestions.

If there are any questions, please reach out.

SmallJoker avatar Jan 21 '24 16:01 SmallJoker

The new version removed the need for node pair registration for visual purposes. Inner nodes are now defined by metadata fields inner_node and inner_param2. These fields are cached, so rendering does not need to access the metadata class. I believe that this solution can be expanded in the future for different purposes.

Would be nice if someone checked if this solution looks acceptable.

@SmallJoker I am not sure about submerge suggestion. Looks like this check if the node can be flooded can be done by on_flood callback. Similarly, if the node can be covered, it will require some custom Lua code to do so, so a check can be done in it. The server probably does not need to know if the node can be submerged. Or should I check in render logic the submerged node submerge field and render it only if the submerge field is in the expected value?

sfence avatar Jan 25 '24 16:01 sfence

@SmallJoker reminder

Zughy avatar Feb 21 '24 20:02 Zughy

@sfence floodable/on_flood describes the "physical" behaviour when nodes come in touch with liquid. What I meant with submerge is to only adapt the visuals to the surrounding liquid (for now, player physics are a different issue). This way, plants or any other node could be placed underwater without worrying about air gaps like seen here:

air gap of dry shrub

Getting a decent underwater appearance would then be pretty trivial for mods to enable. One nodedef field.

SmallJoker avatar Feb 24 '24 08:02 SmallJoker

@SmallJoker I do not think, that the submerge field will be enough to work in situations like this:

screenshot_20240224_100556 screenshot_20240224_100759

sfence avatar Feb 24 '24 09:02 sfence

What version of devtest is this?

nininik0 avatar Mar 23 '24 01:03 nininik0

What version of devtest is this?

This is an unmerged branch without the core developer's support. To try it, you must clone this branch and compile Minetest.

But at the moment, it looks like it will not be supported and merged.

sfence avatar Mar 23 '24 04:03 sfence