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

Liquids: Prevent liquid spreading on ignore #3581

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@paramat
Copy link
Member

commented Jan 17, 2016

screenshot_20160117_053200

^ Before
After v

screenshot_20160117_071156

See #2977

When the node below water is 'ignore' the 'flowing_down' flag is set to true to prevent water spreading outwards on the surface of ignore, however because water cannot penetrate ignore it doesn't actually flow down it just stops at ignore.
It does not fix the updating of stuck water but at least it's not as ugly as the spreading and could be considered water dissolving into spray at the column base.
Later some sort of liquid updating ABM could be added to MTgame, see below, obviously the spreading must be solved first otherwise all that spread water will start to flow down.
Tests were in floatlands by placing a source and watchiing the water column flow down below until it reaches an ungenerated mapchunk and either stops or spreads out.

//////////////////////////////////////////////////////

The ABM below is not part of this commit.

Flying down to the column base does not start it flowing down again, so i experimented with a hacky ABM for updating water nodes to start the column flowing down again (nodeupdate(pos) does not work):

minetest.register_abm({
    nodenames = {"group:water"},
    neighbors = {"air"},
    interval = 8,
    chance = 256,
    catch_up = false,
    action = function(pos, node)
        minetest.place_node(pos, {name = node.name})
    end
})

Even with a single node column this will start it flowing again within a minute or two, while hopefully being lightweight enough for the case of being near sealevel.
If updating the water by ABM is eventually done perhaps we need a new API that updates a water node by adding it to the liquid queue, i will look into this.
EDIT It already exists 'transforming_liquid_add(pos)'.
EDIT But it doesn't work.

Some time later after descending the column using the ABM:

screenshot_20160117_073010

screenshot_20160117_073210

Making liquid columns less ugly is essential for floatlands, and this (with an ABM) allows long water columns for vertical travel between floatlands.

@paramat

This comment has been minimized.

Copy link
Member Author

commented Jan 17, 2016

I think we need a specially designed new API that is as lightweight as possible while still updating a water column. Using 'place node' 16 times per interval (chance 256) when near sea level seems too heavy. Perhaps that can be a separate PR.

@paramat paramat changed the title Liquid flow: Prevent water spreading on ignore WIP Liquid flow: Prevent water spreading on ignore Jan 17, 2016

@paramat paramat removed the WIP label Jan 17, 2016

@paramat paramat changed the title Liquid flow: Prevent water spreading on ignore Liquids: Prevent liquid spreading on ignore Jan 17, 2016

@pinkysnowman

This comment has been minimized.

Copy link
Contributor

commented Jan 17, 2016

this could also be recoded to solve issue #3529 at the same time C:

@Rogier-5

This comment has been minimized.

Copy link
Contributor

commented Jan 17, 2016

so i experimented with a hacky ABM for updating water nodes to start the column flowing down again

Maybe this could be efficiently solved by adding a lua callback on_load_block, similar to on_generate, that is called at most once for every newly loaded block, but of course not sooner than that a player comes within active_block_range blocks of that block.

Alternatively, while liquid is spreading, the the liquid queue could replace water nodes with alternative 'spreading' water nodes (e.g. water_flowing_spreading and water_source_spreading or so, with the same colors and properties of water_flowing and water_source). only after the liquid has stopped spreading, does the liquid queue finally replace it with regular (non-spreading) liquid_flowing or liquid_source. For nodes which haven't stopped spreading for some reason, and which are therefore still of the spreading type, an ABM can take care of adding these nodes back to the liquid queue at a later time. As the ABM does not run for regular liquid, but only for the spreading type, which should be rare, the extra load of such an ABM should be minimal.

Having such regular and spreading liquid nodes, liquid spreading might even be implemented entirely in lua ? Emptying a bucket would result in a node of the spreading type, and when removing a liquid node, its destructor would change the static liquid neigbors to their spreading counterparts. Mapgen would also only generate the spreading type of liquid.

@paramat

This comment has been minimized.

Copy link
Member Author

commented Jan 17, 2016

pinkysnowman, possibly, thanks i'll consider that.
Just realised the ABM should be on flowing liquids only (lava too), it will be less processing.

@Rogier-5

This comment has been minimized.

Copy link
Contributor

commented Jan 17, 2016

Just realised the ABM should be on flowing liquids only, it will be less processing.

The way I understand it (but CMIIW), both liquid_source and liquid_flowing are static, unchanging nodes. Only if anything nearby changes will they start spreading. Both of them. The only thing actually flowing in a flowing liquid is the impression created by the texture and by its sloped nature...

Liquid_source spreads because it converts adjacent air nodes to liquid_flowing, and also because it may connect with another source nearby, and convert the node-in-between to source as well. Both these things shouldn't stop at block boundaries either (or anywhere else where they might have stopped just because the liquid queue was growing too much, and truncated)

If I'm correct, the ABM should process both types...

@paramat

This comment has been minimized.

Copy link
Member Author

commented Jan 17, 2016

I think that the liquid that stops above ignore will always be a flowing node either flowing down vertically or flowing horizontally, so that's all we need to update. The update might update nearby sources too. For this hacky ABM this may be the only option as otherwise it acts too often on all the source nodes in lakes.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Jan 17, 2016

neighbors = {"air"}

Not sure about this part, for example in caves it can hang near walls. Actually it can hang around everything on the sides :)

@paramat

This comment has been minimized.

Copy link
Member Author

commented Jan 17, 2016

Good point, with the ABM acting on flowing nodes only we can remove that.

@red-001

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2016

Good enough to be used for now, but an ABM on group:water doesn't sound much better then the solution I used. But then I don't know a lot about how ABM's work. It could be an option to add all water nodes in a block or at least around a block to transforming_liquid_queue this is the only other solution I can think of that has no chance of water getting stuck in the air. Also water shouldn't really spread over water so you might want to add that as well.

@paramat

This comment has been minimized.

Copy link
Member Author

commented Jan 18, 2016

The ABM isn't working reliably, updating water will be a challenge, see the other thread for something hmmmm tried.

@paramat

This comment has been minimized.

Copy link
Member Author

commented Jan 18, 2016

@paramat

This comment has been minimized.

Copy link
Member Author

commented Jan 20, 2016

@paramat paramat closed this Jan 20, 2016

@paramat

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2016

Re-opening because the discussion on ABMs and updating liquids is useful.

@paramat paramat reopened this Jan 22, 2016

@paramat

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2016

minetest.register_abm({
    nodenames = {"group:water"},
    neighbors = {"air"},
    interval = 8,
    chance = 256,
    catch_up = false,
    action = function(pos, node)
        minetest.place_node(pos, {name = node.name})
    end
})

I realised this may not be setting param2 properly, perhaps it needs to be:

        minetest.place_node(pos, {name = node.name, param2 = node.param2})

Unless not specifying param2 results in the previous value being preserved.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2016

Liquids should be fixed in engine (for sure, no short measures). Here follows some experimental stuff with lua.

I wanted to play with liquid activation, so I saved a multiplayer server with lots of buggy liquids and made some crude code just for fun to see what happens.
This is ABM that slowly scans flowing liquids and resets them, user needs some patience to see it working. This code adds some slight stutter (or very short 10%+ fps drop) when meshes regenerate and increases workload if it triggers stuck flowing nodes that start to spread. However, effect on worlds with few stuck liquids is little.

Game save is attached below, unpack to /worlds, start the game, walk south for liquids.
Paste this code to your /games/minetest_game/mods/default/functions.lua

--
-- Water and lava activation
--

minetest.register_abm({
    nodenames = {"default:water_flowing"},
    neighbors = {"air"},
    interval = 15,
    chance = 128,
    catch_up = false,
    action = function(pos, node)
    local above = {x = pos.x, y = pos.y + 1, z = pos.z}
    local below = {x = pos.x, y = pos.y - 1, z = pos.z}
    local name_above = minetest.get_node(above).name
    local name_below = minetest.get_node(below).name
    local nodedef_above = minetest.registered_nodes[name_above]
    local nodedef_below = minetest.registered_nodes[name_below]
    if nodedef_above and nodedef_below and (nodedef_below.liquidtype == "none" or nodedef_above.liquidtype == "none") then
            minetest.set_node(pos, {name = "default:water_flowing", param2 = node.param2})
        print("activ: "..pos.x..", "..pos.y..", "..pos.z)
        print("above: "..name_above..", "..above.x..", "..above.y..", "..above.z)
        print("below: "..name_below..", "..below.x..", "..below.y..", "..below.z)
        print("---")
    end
    end
})


minetest.register_abm({
    nodenames = {"default:river_water_flowing"},
    neighbors = {"air"},
    interval = 30,
    chance = 128,
    catch_up = false,
    action = function(pos, node)
    local above = {x = pos.x, y = pos.y + 1, z = pos.z}
    local below = {x = pos.x, y = pos.y - 1, z = pos.z}
    local name_above = minetest.get_node(above).name
    local name_below = minetest.get_node(below).name
    local nodedef_above = minetest.registered_nodes[name_above]
    local nodedef_below = minetest.registered_nodes[name_below]
    if nodedef_above and nodedef_below and (nodedef_below.liquidtype == "none" or nodedef_above.liquidtype == "none") then
            minetest.set_node(pos, {name = "default:river_water_flowing", param2 = node.param2})
        print("activ: "..pos.x..", "..pos.y..", "..pos.z)
        print("above: "..name_above..", "..above.x..", "..above.y..", "..above.z)
        print("below: "..name_below..", "..below.x..", "..below.y..", "..below.z)
        print("---")
    end
    end
})


minetest.register_abm({
    nodenames = {"default:lava_flowing"},
    neighbors = {"air"},
    interval = 20,
    chance = 128,
    catch_up = false,
    action = function(pos, node)
    local above = {x = pos.x, y = pos.y + 1, z = pos.z}
    local below = {x = pos.x, y = pos.y - 1, z = pos.z}
    local name_above = minetest.get_node(above).name
    local name_below = minetest.get_node(below).name
    local nodedef_above = minetest.registered_nodes[name_above]
    local nodedef_below = minetest.registered_nodes[name_below]
    if nodedef_above and nodedef_below and (nodedef_below.liquidtype == "none" or nodedef_above.liquidtype == "none") then
            minetest.set_node(pos, {name = "default:lava_flowing", param2 = node.param2})
        print("activ: "..pos.x..", "..pos.y..", "..pos.z)
        print("above: "..name_above..", "..above.x..", "..above.y..", "..above.z)
        print("below: "..name_below..", "..below.x..", "..below.y..", "..below.z)
        print("---")
    end
    end
})

Download: Just Test.zip
2016-01-22 23_28_22-greenshot

@paramat

This comment has been minimized.

Copy link
Member Author

commented Jan 23, 2016

You can combine those 3 ABMs into 1 by using multiple nodenames inside 'nodenames = {}' and using 'set node {name = node.name ...}'.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2016

Good idea, so far, from testing I see that even with interval = 15, chance = 128 you need to wait for a very loooooong time... days to fix flowing liquids on bad cases. On the other hand with chance = 64 has some promise.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2016

Testing this kind. Now that is much more effective!

--
-- Water and lava activation
--

minetest.register_abm({
    nodenames = {"default:water_flowing", "default:river_water_flowing", "default:lava_flowing"},
    neighbors = {"air"},
    interval = 10,
    chance = 80,
    catch_up = false,
    action = function(pos, node)
    local above = {x = pos.x, y = pos.y + 1, z = pos.z}
    local below = {x = pos.x, y = pos.y - 1, z = pos.z}
    local name_above = minetest.get_node(above).name
    local name_below = minetest.get_node(below).name
    local nodedef_above = minetest.registered_nodes[name_above]
    local nodedef_below = minetest.registered_nodes[name_below]
    if nodedef_above and nodedef_below and (nodedef_below.liquidtype == "none" or nodedef_above.liquidtype == "none") then
            minetest.set_node(pos, {name = node.name, param2 = node.param2})
        print("activ: "..pos.x..", "..pos.y..", "..pos.z)
        print("above: "..name_above..", "..above.x..", "..above.y..", "..above.z)
        print("below: "..name_below..", "..below.x..", "..below.y..", "..below.z)
        print("---")
    end
    end
})

@paramat paramat closed this Jan 29, 2016

@paramat paramat deleted the paramat:waterignore branch Jan 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.