LBMs are not performed reliably for generated but never activated blocks

[ryvnf]

Luanti version

Luanti 5.12.0-dev-4b85062ca (Linux)
Using LuaJIT 2.1.0-beta3
Built by GCC 12.2
Running on Linux/6.1.0 x86_64
BUILD_TYPE=Release
RUN_IN_PLACE=0
USE_CURL=1
USE_GETTEXT=1
USE_SOUND=1
STATIC_SHAREDIR="/usr/local/share/luanti"
STATIC_LOCALEDIR="/usr/local/share/locale"

Operating system and version

Debian

CPU model

No response

GPU model

No response

Active renderer

No response

Summary

LBMs are not being triggered reliably.

In Mineclonia I have made a branch that changes paramtype2 of grass to color4dir with random rotation to make the terrain look more natural (no repeated "grid" patterns). In order to convert old mapblocks I made an LBM to migrate the palette index from color (used for grass previously) to color4dir along with adding a random rotations.

I did however notice that the LBM is not reliably triggered for all mapblocks. Some mapblocks are clearly not updated, which becomes noticeable due to them having a different color and no random rotation.

I am pretty sure this is a Luanti bug considering the LBM is pretty simple:

-- Rotate old biomecolor4dir nodes
minetest.register_lbm({
	label = "Rotate old biomecolor4dir nodes",
	name = "mcl_core:rotate_old_biomecolor4dir",
	nodenames = {"group:biomecolor4dir"},
	action = function(pos, node)
		node.param2 = mcl_util.get_pos_biomecolor4dir(pos)
		minetest.swap_node(pos, node)
	end,
})

Steps to reproduce

1. Clone the Mineclonia repo to the games directory in Luanti
   • git clone https://codeberg.org/mineclonia/mineclonia/ ~/.minetest/games/mineclonia-test
2. Create a world with mapgen "flat" and world seed test
3. Join the world on the main branch, let it generate some terrain
4. Exit world, do git checkout biomecolor4dir
5. Join world again
6. Notice that the LBM converts mapblocks but leaves some mapblocks completely unaffected (see screenshot)
   

I can reproduce this both when using action and bulk_action for the LBM.

Note that the biomecolor4dir branch is destructive to Mineclonia worlds, so you probably want to delete the cloned repo or switch back to main to avoid accidentally triggering the LBM on a non-testing world.

[SmallJoker]

Can confirm with the devtest game, no mapgen mods enabled. Some blocks are processed, some aren't.
Minimal test code: (for any map, preferably with the dummy database backend)

assert(core.get_modpath("basenodes"), "Devtest needed")

minetest.register_lbm({
	name = ":lab:reliability_test",
	nodenames = {"basenodes:dirt_with_grass"},
	--run_at_every_load = true,
	action = function(pos, node)
		node.name = "basenodes:stone"
		minetest.swap_node(pos, node)
	end,
})

---

EDIT: This might not actually be a bug. When approaching the mapblock (i.e. it becomes active after being generated), the LBM kicks in and does convert all (in this case) grass nodes with stone.

[sfan5]

https://github.com/luanti-org/luanti/blob/master/doc/lua_api.md#lbm-loadingblockmodifier-definition

A loading block modifier (LBM) [...] is called [...] when a mapblock which contains such nodes gets activated (not loaded!).

[ryvnf]

EDIT: This might not actually be a bug. When approaching the mapblock (i.e. it becomes active after being generated), the LBM kicks in and does convert all (in this case) grass nodes with stone.

Just asking to be clear, are you saying that the issue I reported may not be a bug? Or that your way to reproduce may not reproduce the bug?

[kromka-chleba]

Honestly looks similar to the artifacts caused by the y-slice bug.
#9357 (comment)
I also got similar stripes when I do mapchunk-wide VMs that apparently run into a race condition with mapgen #13712

So I believe it is some sort of a race condition with mapgen caused by overgeneration.

[sfan5]

Just asking to be clear, are you saying that the issue I reported may not be a bug? Or that your way to reproduce may not reproduce the bug?

The fact that you can see blocks for which the LBM has not run yet is not a bug, due to the behavior mentioned above.
What would be a bug if a block gets activated and the LBM does not run.
You can check if the grass stays wrong if you get close (which should definitely activate the block).

By the way, there's a technical reason why LBMs run on activation and not load: The tracking of whether LBMs have been run depends on the timestamp of the block, which is only refreshed if it's activated.

[ryvnf]

The fact that you can see blocks for which the LBM has not run yet is not a bug, due to the behavior mentioned above.
What would be a bug if a block gets activated and the LBM does not run.
You can check if the grass stays wrong if you get close (which should definitely activate the block).

By the way, there's a technical reason why LBMs run on activation and not load: The tracking of whether LBMs have been run depends on the timestamp of the block, which is only refreshed if it's activated.

The LBMs are not run even when you get close and interact with the mapblocks. So it looks like a bug.

From what you are saying, it sounds like the bug could be that the timestamp is only initialized when a mapblock is activated. So an old generated mapblock that has never been activated will not have a timestamp. This results in the LBM not being run when it is activated for the first time. This is just a theory. Do you think that could be the reason?

[ryvnf]

From what you are saying, it sounds like the bug could be that the timestamp is only initialized when a mapblock is activated. So an old generated mapblock that has never been activated will not have a timestamp. This results in the LBM not being run when it is activated for the first time. This is just a theory. Do you think that could be the reason?

This appears to be the reason. I did a quick patch which treats BLOCK_TIMESTAMP_UNDEFINED as 0 instead of its numerical value 0xffffffff.

diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp
index bab243713..c6247d48f 100644
--- a/src/serverenvironment.cpp
+++ b/src/serverenvironment.cpp
@@ -1082,6 +1082,10 @@ void ServerEnvironment::activateBlock(MapBlock *block, u32 additional_dtime)
        if (block->isOrphan())
                return;
 
+       if (stamp == BLOCK_TIMESTAMP_UNDEFINED) {
+               stamp = 0;
+       }
+
        /* Handle LoadingBlockModifiers */
        m_lbm_mgr.applyLBMs(this, block, stamp, (float)dtime_s);
        if (block->isOrphan())

I could not reproduce the issue afterwards which confirms it.

[ryvnf]

Do you think it would be feasible to initialize all generated mapblocks with an activation timestamp, before they have actually been activated? And then run all the lbms for mapblocks which do not have a timestamp?

It feels like lbms should be able to update blocks from generated terrain even if that terrain has not been "activated".

[sfan5]

All newly generated blocks are activated so this should not be the case:

luanti/src/emerge.cpp

Line 625 in 4b85062

m_server->m_env->activateBlock(block, 0);

luanti/src/serverenvironment.cpp

Lines 1074 to 1075 in 4b85062

	// Set current time as timestamp
	block->setTimestampNoChangedFlag(m_game_time);

---

Either way I could reproduce this bug by:

1. creating a new devtest world
2. exiting the game, add the code from ¹ to an active mod, change mapgen to singlenode so we only inspect old blocks
3. rejoin game and see

Most grass is converted correctly, but some is not and going near the affected blocks won't convert it either.

¹

core.register_lbm({
	name = ":lab:reliability_test",
	nodenames = {"basenodes:dirt_with_grass"},
	action = function(pos, node)
		node.name = "basenodes:stone"
		minetest.swap_node(pos, node)
	end,
})

[sfan5]

After a lengthy debug session I can say that there's no bug in the LBM code itself, but indeed not every generated block has its timestamp set, which causes this problem:

% grep '[-]46,0,25' debug.txt   
saved (-46,0,25) with ts 4294967295
# lbm is introduced here
activated (-46,0,25) with ts 4294967295
# ^ it won't be run
saved (-46,0,25) with ts 637

so the fix is either:

• make sure all generated blocks are activated (do we really need to?)
• make sure all generated blocks have a timestamp set (easier)

generally the ServerEnvironment::activateBlock API needs to go because such blocks are also not properly included in the active block list. they're just faux active for a single moment.
question: do people expect that LBMs immediately run for generated blocks? (they eventually will or it would be broken, but the point is immediately)

P.S.: This also means it's not a regression. and there won't be a retroactive fix.