Bulk LBMs (#14954)

builtin/game/features.lua

@@ -43,6 +43,7 @@ core.features = {
 	moveresult_new_pos = true,
 	override_item_remove_fields = true,
 	hotbar_hud_element = true,
+	bulk_lbms = true,
 }
 
 function core.has_feature(arg)

builtin/game/misc.lua

@@ -298,3 +298,28 @@ do
 		return valid_object_iterator(core.get_objects_in_area(min_pos, max_pos))
 	end
 end
+
+--
+-- Helper for LBM execution, called from C++
+--
+
+function core.run_lbm(id, pos_list, dtime_s)
+	local lbm = core.registered_lbms[id]
+	assert(lbm, "Entry with given id not found in registered_lbms table")
+	core.set_last_run_mod(lbm.mod_origin)
+	if lbm.bulk_action then
+		return lbm.bulk_action(pos_list, dtime_s)
+	end
+	-- emulate non-bulk LBMs
+	local expect = core.get_node(pos_list[1]).name
+	-- engine guarantees that
+	-- 1) all nodes are the same content type
+	-- 2) the list is up-to-date when we're called
+	assert(expect ~= "ignore")
+	for _, pos in ipairs(pos_list) do
+		local n = core.get_node(pos)
+		if n.name == expect then -- might have been changed by previous call
+			lbm.action(pos, n, dtime_s)
+		end
+	end
+end

builtin/game/register.lua

@@ -105,7 +105,12 @@ function core.register_lbm(spec)
 	-- Add to core.registered_lbms
 	check_modname_prefix(spec.name)
 	check_node_list(spec.nodenames, "nodenames")
-	assert(type(spec.action) == "function", "Required field 'action' of type function")
+	local have = spec.action ~= nil
+	local have_bulk = spec.bulk_action ~= nil
+	assert(not have or type(spec.action) == "function", "Field 'action' must be a function")
+	assert(not have_bulk or type(spec.bulk_action) == "function", "Field 'bulk_action' must be a function")
+	assert(have ~= have_bulk, "Either 'action' or 'bulk_action' must be present")
+
 	core.registered_lbms[#core.registered_lbms + 1] = spec
 	spec.mod_origin = core.get_current_modname() or "??"
 end

builtin/profiler/instrumentation.lua

@@ -217,8 +217,9 @@ local function init()
 		-- Wrap register_lbm() to automatically instrument lbms.
 		local orig_register_lbm = core.register_lbm
 		core.register_lbm = function(spec)
-			spec.action = instrument {
+			local k = spec.bulk_action ~= nil and "bulk_action" or "action"
-				func = spec.action,
+			spec[k] = instrument {
+				func = spec[k],
 				class = "LBM",
 				label = spec.label or spec.name,
 			}

doc/lua_api.md

@@ -5522,6 +5522,8 @@ Utilities
       override_item_remove_fields = true,
       -- The predefined hotbar is a Lua HUD element of type `hotbar` (5.10.0)
       hotbar_hud_element = true,
+      -- Bulk LBM support (5.10.0)
+      bulk_lbms = true,
   }
   ```
 
@@ -9124,7 +9126,12 @@ Used by `minetest.register_lbm`.
 
 A loading block modifier (LBM) is used to define a function that is called for
 specific nodes (defined by `nodenames`) when a mapblock which contains such nodes
-gets activated (not loaded!)
+gets activated (not loaded!).
+
+Note: LBMs operate on a "snapshot" of node positions taken once before they are triggered.
+That means if an LBM callback adds a node, it won't be taken into account.
+However the engine guarantees that when the callback is called that all given position(s)
+contain a matching node.
 
 ```lua
 {
@@ -9148,7 +9155,13 @@ gets activated (not loaded!)
     action = function(pos, node, dtime_s) end,
     -- Function triggered for each qualifying node.
     -- `dtime_s` is the in-game time (in seconds) elapsed since the block
-    -- was last active
+    -- was last active.
+
+    bulk_action = function(pos_list, dtime_s) end,
+    -- Function triggered with a list of all applicable node positions at once.
+    -- This can be provided as an alternative to `action` (not both).
+    -- Available since `minetest.features.bulk_lbms` (5.10.0)
+    -- `dtime_s`: as above
 }
 ```
 

src/script/cpp_api/s_env.cpp

@@ -111,10 +111,11 @@ public:
 		this->name = name;
 	}
 
-	virtual void trigger(ServerEnvironment *env, v3s16 p, MapNode n, float dtime_s)
+	virtual void trigger(ServerEnvironment *env, MapBlock *block,
+		const std::unordered_set<v3s16> &positions, float dtime_s)
 	{
 		auto *script = env->getScriptIface();
-		script->triggerLBM(m_id, p, n, dtime_s);
+		script->triggerLBM(m_id, block, positions, dtime_s);
 	}
 };
 
@@ -291,10 +292,6 @@ void ScriptApiEnv::readLBMs()
 		bool run_at_every_load = getboolfield_default(L, current_lbm,
 			"run_at_every_load", false);
 
-		lua_getfield(L, current_lbm, "action");
-		luaL_checktype(L, current_lbm + 1, LUA_TFUNCTION);
-		lua_pop(L, 1);
-
 		LuaLBM *lbm = new LuaLBM(id, trigger_contents, name,
 			run_at_every_load);
 
@@ -462,34 +459,29 @@ void ScriptApiEnv::triggerABM(int id, v3s16 p, MapNode n,
 	lua_pop(L, 1); // Pop error handler
 }
 
-void ScriptApiEnv::triggerLBM(int id, v3s16 p,
+void ScriptApiEnv::triggerLBM(int id, MapBlock *block,
-	const MapNode n, const float dtime_s)
+		const std::unordered_set<v3s16> &positions, float dtime_s)
 {
 	SCRIPTAPI_PRECHECKHEADER
 
 	int error_handler = PUSH_ERROR_HANDLER(L);
 
-	// Get registered_lbms
+	const v3s16 pos_of_block = block->getPosRelative();
+
+	// Get core.run_lbm
 	lua_getglobal(L, "core");
-	lua_getfield(L, -1, "registered_lbms");
+	lua_getfield(L, -1, "run_lbm");
-	luaL_checktype(L, -1, LUA_TTABLE);
+	luaL_checktype(L, -1, LUA_TFUNCTION);
 	lua_remove(L, -2); // Remove core
 
-	// Get registered_lbms[m_id]
+	// Call it
 	lua_pushinteger(L, id);
-	lua_gettable(L, -2);
+	lua_createtable(L, positions.size(), 0);
-	FATAL_ERROR_IF(lua_isnil(L, -1), "Entry with given id not found in registered_lbms table");
+	int i = 1;
-	lua_remove(L, -2); // Remove registered_lbms
+	for (auto &p : positions) {
-
+		push_v3s16(L, pos_of_block + p);
-	setOriginFromTable(-1);
+		lua_rawseti(L, -2, i++);
-
+	}
-	// Call action
-	luaL_checktype(L, -1, LUA_TTABLE);
-	lua_getfield(L, -1, "action");
-	luaL_checktype(L, -1, LUA_TFUNCTION);
-	lua_remove(L, -2); // Remove registered_lbms[m_id]
-	push_v3s16(L, p);
-	pushnode(L, n);
 	lua_pushnumber(L, dtime_s);
 
 	int result = lua_pcall(L, 3, 0, error_handler);

src/script/cpp_api/s_env.h

@@ -26,6 +26,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <vector>
 
 class ServerEnvironment;
+class MapBlock;
 struct ScriptCallbackState;
 
 class ScriptApiEnv : virtual public ScriptApiBase
@@ -61,7 +62,8 @@ public:
 	void triggerABM(int id, v3s16 p, MapNode n,
 			u32 active_object_count, u32 active_object_count_wider);
 
-	void triggerLBM(int id, v3s16 p, MapNode n, float dtime_s);
+	void triggerLBM(int id, MapBlock *block,
+		const std::unordered_set<v3s16> &positions, float dtime_s);
 
 private:
 	void readABMs();

src/serverenvironment.cpp

@@ -257,38 +257,71 @@ void LBMManager::applyLBMs(ServerEnvironment *env, MapBlock *block,
 	FATAL_ERROR_IF(!m_query_mode,
 		"attempted to query on non fully set up LBMManager");
 
-	const v3s16 pos_of_block = block->getPosRelative();
+	// Collect a list of all LBMs and associated positions
-	v3s16 pos;
+	struct LBMToRun {
-	MapNode n;
+		std::unordered_set<v3s16> p; // node positions
-	content_t c;
+		std::unordered_set<LoadingBlockModifierDef*> l;
-	auto it = getLBMsIntroducedAfter(stamp);
+	};
+	std::unordered_map<content_t, LBMToRun> to_run;
+
 	// Note: the iteration count of this outer loop is typically very low, so it's ok.
-	for (; it != m_lbm_lookup.end(); ++it) {
+	for (auto it = getLBMsIntroducedAfter(stamp); it != m_lbm_lookup.end(); ++it) {
-		// Cache previous lookup result since it has a high performance penalty.
+		v3s16 pos;
+		content_t c;
+
+		// Cache previous lookups since it has a high performance penalty.
 		content_t previous_c = CONTENT_IGNORE;
 		const LBMContentMapping::lbm_vector *lbm_list = nullptr;
+		LBMToRun *batch = nullptr;
 
 		for (pos.Z = 0; pos.Z < MAP_BLOCKSIZE; pos.Z++)
 		for (pos.Y = 0; pos.Y < MAP_BLOCKSIZE; pos.Y++)
 		for (pos.X = 0; pos.X < MAP_BLOCKSIZE; pos.X++) {
-			n = block->getNodeNoCheck(pos);
+			c = block->getNodeNoCheck(pos).getContent();
-			c = n.getContent();
 
+			bool c_changed = false;
 			if (previous_c != c) {
+				c_changed = true;
 				lbm_list = it->second.lookup(c);
+				batch = &to_run[c];
 				previous_c = c;
 			}
 
 			if (!lbm_list)
 				continue;
-			for (auto lbmdef : *lbm_list) {
+			batch->p.insert(pos);
-				lbmdef->trigger(env, pos + pos_of_block, n, dtime_s);
+			if (c_changed) {
+				batch->l.insert(lbm_list->begin(), lbm_list->end());
+			} else {
+				// we were here before so the list must be filled
+				assert(!batch->l.empty());
+			}
+		}
+	}
+
+	// Actually run them
+	bool first = true;
+	for (auto &[c, batch] : to_run) {
+		for (auto &lbm_def : batch.l) {
+			if (!first) {
+				// The fun part: since any LBM call can change the nodes inside of he
+				// block, we have to recheck the positions to see if the wanted node
+				// is still there.
+				// Note that we don't rescan the whole block, we don't want to include new changes.
+				for (auto it2 = batch.p.begin(); it2 != batch.p.end(); ) {
+					if (block->getNodeNoCheck(*it2).getContent() != c)
+						it2 = batch.p.erase(it2);
+					else
+						++it2;
+				}
+			}
+			first = false;
+
+			if (batch.p.empty())
+				break;
+			lbm_def->trigger(env, block, batch.p, dtime_s);
 			if (block->isOrphan())
 				return;
-				n = block->getNodeNoCheck(pos);
-				if (n.getContent() != c)
-					break; // The node was changed and the LBMs no longer apply
-			}
 		}
 	}
 }

src/serverenvironment.h

@@ -96,8 +96,13 @@ struct LoadingBlockModifierDef
 
 	virtual ~LoadingBlockModifierDef() = default;
 
-	virtual void trigger(ServerEnvironment *env, v3s16 p,
+	/// @brief Called to invoke LBM
-			MapNode n, float dtime_s) {};
+	/// @param env environment
+	/// @param block the block in question
+	/// @param positions set of node positions (block-relative!)
+	/// @param dtime_s game time since last deactivation
+	virtual void trigger(ServerEnvironment *env, MapBlock *block,
+		const std::unordered_set<v3s16> &positions, float dtime_s) {};
 };
 
 class LBMContentMapping