From 811adf5d42844bbd40e98e07773db3d16e104b90 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Sun, 11 Aug 2024 14:54:02 +0200 Subject: [PATCH] Bulk LBMs (#14954) --- builtin/game/features.lua | 1 + builtin/game/misc.lua | 25 +++++++++++ builtin/game/register.lua | 7 ++- builtin/profiler/instrumentation.lua | 5 ++- doc/lua_api.md | 17 +++++++- src/script/cpp_api/s_env.cpp | 42 ++++++++---------- src/script/cpp_api/s_env.h | 4 +- src/serverenvironment.cpp | 65 +++++++++++++++++++++------- src/serverenvironment.h | 9 +++- 9 files changed, 126 insertions(+), 49 deletions(-) diff --git a/builtin/game/features.lua b/builtin/game/features.lua index 358ee8ff0..81b291e6c 100644 --- a/builtin/game/features.lua +++ b/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) diff --git a/builtin/game/misc.lua b/builtin/game/misc.lua index 91ca738a4..25e19ddb0 100644 --- a/builtin/game/misc.lua +++ b/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 diff --git a/builtin/game/register.lua b/builtin/game/register.lua index 76cb72f87..3d99a6925 100644 --- a/builtin/game/register.lua +++ b/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 diff --git a/builtin/profiler/instrumentation.lua b/builtin/profiler/instrumentation.lua index 0ffd9e6a9..c4feda7b4 100644 --- a/builtin/profiler/instrumentation.lua +++ b/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 { - func = spec.action, + local k = spec.bulk_action ~= nil and "bulk_action" or "action" + spec[k] = instrument { + func = spec[k], class = "LBM", label = spec.label or spec.name, } diff --git a/doc/lua_api.md b/doc/lua_api.md index 83f186804..b06809aee 100644 --- a/doc/lua_api.md +++ b/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 } ``` diff --git a/src/script/cpp_api/s_env.cpp b/src/script/cpp_api/s_env.cpp index 5068e4f4d..deac90f3c 100644 --- a/src/script/cpp_api/s_env.cpp +++ b/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 &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, - const MapNode n, const float dtime_s) +void ScriptApiEnv::triggerLBM(int id, MapBlock *block, + const std::unordered_set &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"); - luaL_checktype(L, -1, LUA_TTABLE); + lua_getfield(L, -1, "run_lbm"); + 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); - FATAL_ERROR_IF(lua_isnil(L, -1), "Entry with given id not found in registered_lbms table"); - lua_remove(L, -2); // Remove registered_lbms - - setOriginFromTable(-1); - - // 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_createtable(L, positions.size(), 0); + int i = 1; + for (auto &p : positions) { + push_v3s16(L, pos_of_block + p); + lua_rawseti(L, -2, i++); + } lua_pushnumber(L, dtime_s); int result = lua_pcall(L, 3, 0, error_handler); diff --git a/src/script/cpp_api/s_env.h b/src/script/cpp_api/s_env.h index 8a88749fa..4722cb522 100644 --- a/src/script/cpp_api/s_env.h +++ b/src/script/cpp_api/s_env.h @@ -26,6 +26,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include 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 &positions, float dtime_s); private: void readABMs(); diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp index f79102e74..ac627dd50 100644 --- a/src/serverenvironment.cpp +++ b/src/serverenvironment.cpp @@ -257,40 +257,73 @@ 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(); - v3s16 pos; - MapNode n; - content_t c; - auto it = getLBMsIntroducedAfter(stamp); + // Collect a list of all LBMs and associated positions + struct LBMToRun { + std::unordered_set p; // node positions + std::unordered_set l; + }; + std::unordered_map 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) { - // Cache previous lookup result since it has a high performance penalty. + for (auto it = getLBMsIntroducedAfter(stamp); it != m_lbm_lookup.end(); ++it) { + 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 = n.getContent(); + c = block->getNodeNoCheck(pos).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) { - lbmdef->trigger(env, pos + pos_of_block, n, 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 + batch->p.insert(pos); + 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; + } + } } /* diff --git a/src/serverenvironment.h b/src/serverenvironment.h index 51d699cab..d20cc0b3f 100644 --- a/src/serverenvironment.h +++ b/src/serverenvironment.h @@ -96,8 +96,13 @@ struct LoadingBlockModifierDef virtual ~LoadingBlockModifierDef() = default; - virtual void trigger(ServerEnvironment *env, v3s16 p, - MapNode n, float dtime_s) {}; + /// @brief Called to invoke LBM + /// @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 &positions, float dtime_s) {}; }; class LBMContentMapping