Add on_deactivate callback for luaentities (#10723)

12 files changed, 106 insertions, 27 deletions

diff --git a/builtin/profiler/instrumentation.lua b/builtin/profiler/instrumentation.lua
index 237f048fb..6b951a2c2 100644
--- a/builtin/profiler/instrumentation.lua
+++ b/builtin/profiler/instrumentation.lua
@@ -160,6 +160,7 @@ local function init()
 		-- Simple iteration would ignore lookup via __index.
 		local entity_instrumentation = {
 			"on_activate",
+			"on_deactivate",
 			"on_step",
 			"on_punch",
 			"on_rightclick",
diff --git a/doc/lua_api.txt b/doc/lua_api.txt
index 8bf6ade69..47c2776e6 100644
--- a/doc/lua_api.txt
+++ b/doc/lua_api.txt
@@ -4207,6 +4207,8 @@ Callbacks:
     * Called when the object is instantiated.
     * `dtime_s` is the time passed since the object was unloaded, which can be
       used for updating the entity state.
+* `on_deactivate(self)
+    * Called when the object is about to get removed or unloaded.
 * `on_step(self, dtime)`
     * Called on every server tick, after movement and collision processing.
       `dtime` is usually 0.1 seconds, as per the `dedicated_server_step` setting
diff --git a/games/devtest/mods/testentities/callbacks.lua b/games/devtest/mods/testentities/callbacks.lua
index 711079f87..320690b39 100644
--- a/games/devtest/mods/testentities/callbacks.lua
+++ b/games/devtest/mods/testentities/callbacks.lua
@@ -31,6 +31,9 @@ minetest.register_entity("testentities:callback", {
 	on_activate = function(self, staticdata, dtime_s)
 		message("Callback entity: on_activate! pos="..spos(self).."; dtime_s="..dtime_s)
 	end,
+	on_deactivate = function(self)
+		message("Callback entity: on_deactivate! pos="..spos(self))
+	end,
 	on_punch = function(self, puncher, time_from_last_punch, tool_capabilities, dir, damage)
 		local name = get_object_name(puncher)
 		message(
diff --git a/src/script/cpp_api/s_entity.cpp b/src/script/cpp_api/s_entity.cpp
index ea9320051..746f7013e 100644
--- a/src/script/cpp_api/s_entity.cpp
+++ b/src/script/cpp_api/s_entity.cpp
@@ -103,6 +103,32 @@ void ScriptApiEntity::luaentity_Activate(u16 id,
 	lua_pop(L, 2); // Pop object and error handler
 }
 
+void ScriptApiEntity::luaentity_Deactivate(u16 id)
+{
+	SCRIPTAPI_PRECHECKHEADER
+
+	verbosestream << "scriptapi_luaentity_deactivate: id=" << id << std::endl;
+
+	int error_handler = PUSH_ERROR_HANDLER(L);
+
+	// Get the entity
+	luaentity_get(L, id);
+	int object = lua_gettop(L);
+
+	// Get on_deactivate
+	lua_getfield(L, -1, "on_deactivate");
+	if (!lua_isnil(L, -1)) {
+		luaL_checktype(L, -1, LUA_TFUNCTION);
+		lua_pushvalue(L, object);
+
+		setOriginFromTable(object);
+		PCALL_RES(lua_pcall(L, 1, 0, error_handler));
+	} else {
+		lua_pop(L, 1);
+	}
+	lua_pop(L, 2); // Pop object and error handler
+}
+
 void ScriptApiEntity::luaentity_Remove(u16 id)
 {
 	SCRIPTAPI_PRECHECKHEADER
diff --git a/src/script/cpp_api/s_entity.h b/src/script/cpp_api/s_entity.h
index b5f7a6586..b52f6e447 100644
--- a/src/script/cpp_api/s_entity.h
+++ b/src/script/cpp_api/s_entity.h
@@ -33,6 +33,7 @@ public:
 	bool luaentity_Add(u16 id, const char *name);
 	void luaentity_Activate(u16 id,
 			const std::string &staticdata, u32 dtime_s);
+	void luaentity_Deactivate(u16 id);
 	void luaentity_Remove(u16 id);
 	std::string luaentity_GetStaticdata(u16 id);
 	void luaentity_GetProperties(u16 id,
diff --git a/src/script/lua_api/l_object.cpp b/src/script/lua_api/l_object.cpp
index 4d7a1bc41..f52e4892e 100644
--- a/src/script/lua_api/l_object.cpp
+++ b/src/script/lua_api/l_object.cpp
@@ -110,7 +110,7 @@ int ObjectRef::l_remove(lua_State *L)
 	sao->clearParentAttachment();
 
 	verbosestream << "ObjectRef::l_remove(): id=" << sao->getId() << std::endl;
-	sao->m_pending_removal = true;
+	sao->markForRemoval();
 	return 0;
 }
 
diff --git a/src/server/luaentity_sao.cpp b/src/server/luaentity_sao.cpp
index b39797531..c7277491a 100644
--- a/src/server/luaentity_sao.cpp
+++ b/src/server/luaentity_sao.cpp
@@ -112,6 +112,15 @@ void LuaEntitySAO::addedToEnvironment(u32 dtime_s)
 	}
 }
 
+void LuaEntitySAO::dispatchScriptDeactivate()
+{
+	// Ensure that this is in fact a registered entity,
+	// and that it isn't already gone.
+	// The latter also prevents this from ever being called twice.
+	if (m_registered && !isGone())
+		m_env->getScriptIface()->luaentity_Deactivate(m_id);
+}
+
 void LuaEntitySAO::step(float dtime, bool send_recommended)
 {
 	if(!m_properties_sent)
@@ -302,7 +311,7 @@ u16 LuaEntitySAO::punch(v3f dir,
 {
 	if (!m_registered) {
 		// Delete unknown LuaEntities when punched
-		m_pending_removal = true;
+		markForRemoval();
 		return 0;
 	}
 
@@ -335,7 +344,7 @@ u16 LuaEntitySAO::punch(v3f dir,
 		clearParentAttachment();
 		clearChildAttachments();
 		m_env->getScriptIface()->luaentity_on_death(m_id, puncher);
-		m_pending_removal = true;
+		markForRemoval();
 	}
 
 	actionstream << puncher->getDescription() << " (id=" << puncher->getId() <<
diff --git a/src/server/luaentity_sao.h b/src/server/luaentity_sao.h
index e060aa06d..6883ae1b9 100644
--- a/src/server/luaentity_sao.h
+++ b/src/server/luaentity_sao.h
@@ -71,6 +71,11 @@ public:
 	bool getSelectionBox(aabb3f *toset) const;
 	bool collideWithObjects() const;
 
+protected:
+	void dispatchScriptDeactivate();
+	virtual void onMarkedForDeactivation() { dispatchScriptDeactivate(); }
+	virtual void onMarkedForRemoval() { dispatchScriptDeactivate(); }
+
 private:
 	std::string getPropertyPacket();
 	void sendPosition(bool do_interpolate, bool is_movement_end);
diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp
index 62515d1c9..232c6a01d 100644
--- a/src/server/player_sao.cpp
+++ b/src/server/player_sao.cpp
@@ -531,7 +531,7 @@ bool PlayerSAO::setWieldedItem(const ItemStack &item)
 void PlayerSAO::disconnected()
 {
 	m_peer_id = PEER_ID_INEXISTENT;
-	m_pending_removal = true;
+	markForRemoval();
 }
 
 void PlayerSAO::unlinkPlayerSessionAndSave()
diff --git a/src/server/serveractiveobject.cpp b/src/server/serveractiveobject.cpp
index 8cb59b2d6..96b433d1d 100644
--- a/src/server/serveractiveobject.cpp
+++ b/src/server/serveractiveobject.cpp
@@ -73,3 +73,19 @@ void ServerActiveObject::dumpAOMessagesToQueue(std::queue<ActiveObjectMessage> &
 		m_messages_out.pop();
 	}
 }
+
+void ServerActiveObject::markForRemoval()
+{
+	if (!m_pending_removal) {
+		onMarkedForRemoval();
+		m_pending_removal = true;
+	}
+}
+
+void ServerActiveObject::markForDeactivation()
+{
+	if (!m_pending_deactivation) {
+		onMarkedForDeactivation();
+		m_pending_deactivation = true;
+	}
+}
diff --git a/src/server/serveractiveobject.h b/src/server/serveractiveobject.h
index 2764d159e..25653a1ad 100644
--- a/src/server/serveractiveobject.h
+++ b/src/server/serveractiveobject.h
@@ -70,6 +70,10 @@ public:
 	virtual bool environmentDeletes() const
 	{ return true; }
 
+	// Safely mark the object for removal or deactivation
+	void markForRemoval();
+	void markForDeactivation();
+
 	// Create a certain type of ServerActiveObject
 	static ServerActiveObject* create(ActiveObjectType type,
 			ServerEnvironment *env, u16 id, v3f pos,
@@ -214,31 +218,15 @@ public:
 	u16 m_known_by_count = 0;
 
 	/*
-		- Whether this object is to be removed when nobody knows about
-		  it anymore.
-		- Removal is delayed to preserve the id for the time during which
-		  it could be confused to some other object by some client.
-		- This is usually set to true by the step() method when the object wants
-		  to be deleted but can be set by anything else too.
-	*/
-	bool m_pending_removal = false;
-
-	/*
-		Same purpose as m_pending_removal but for deactivation.
-		deactvation = save static data in block, remove active object
-
-		If this is set alongside with m_pending_removal, removal takes
-		priority.
-	*/
-	bool m_pending_deactivation = false;
-
-	/*
 		A getter that unifies the above to answer the question:
 		"Can the environment still interact with this object?"
 	*/
 	inline bool isGone() const
 	{ return m_pending_removal || m_pending_deactivation; }
 
+	inline bool isPendingRemoval() const
+	{ return m_pending_removal; }
+
 	/*
 		Whether the object's static data has been stored to a block
 	*/
@@ -250,6 +238,9 @@ public:
 	v3s16 m_static_block = v3s16(1337,1337,1337);
 
 protected:
+	virtual void onMarkedForDeactivation() {}
+	virtual void onMarkedForRemoval() {}
+
 	virtual void onAttach(int parent_id) {}
 	virtual void onDetach(int parent_id) {}
 
@@ -258,6 +249,27 @@ protected:
 	std::unordered_set<u32> m_attached_particle_spawners;
 
 	/*
+		Same purpose as m_pending_removal but for deactivation.
+		deactvation = save static data in block, remove active object
+
+		If this is set alongside with m_pending_removal, removal takes
+		priority.
+		Note: Do not assign this directly, use markForDeactivation() instead.
+	*/
+	bool m_pending_deactivation = false;
+
+	/*
+		- Whether this object is to be removed when nobody knows about
+		  it anymore.
+		- Removal is delayed to preserve the id for the time during which
+		  it could be confused to some other object by some client.
+		- This is usually set to true by the step() method when the object wants
+		  to be deleted but can be set by anything else too.
+		Note: Do not assign this directly, use markForRemoval() instead.
+	*/
+	bool m_pending_removal = false;
+
+	/*
 		Queue of messages to be sent to the client
 	*/
 	std::queue<ActiveObjectMessage> m_messages_out;
diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp
index d044b003d..56dbb0632 100644
--- a/src/serverenvironment.cpp
+++ b/src/serverenvironment.cpp
@@ -1164,7 +1164,7 @@ void ServerEnvironment::clearObjects(ClearObjectsMode mode)
 
 		// If known by some client, don't delete immediately
 		if (obj->m_known_by_count > 0) {
-			obj->m_pending_removal = true;
+			obj->markForRemoval();
 			return false;
 		}
 
@@ -1792,7 +1792,7 @@ void ServerEnvironment::removeRemovedObjects()
 		/*
 			Delete static data from block if removed
 		*/
-		if (obj->m_pending_removal)
+		if (obj->isPendingRemoval())
 			deleteStaticFromBlock(obj, id, MOD_REASON_REMOVE_OBJECTS_REMOVE, false);
 
 		// If still known by clients, don't actually remove. On some future
@@ -1803,7 +1803,7 @@ void ServerEnvironment::removeRemovedObjects()
 		/*
 			Move static data from active to stored if deactivated
 		*/
-		if (!obj->m_pending_removal && obj->m_static_exists) {
+		if (!obj->isPendingRemoval() && obj->m_static_exists) {
 			MapBlock *block = m_map->emergeBlock(obj->m_static_block, false);
 			if (block) {
 				const auto i = block->m_static_objects.m_active.find(id);
@@ -1991,6 +1991,7 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 		if (!force_delete && obj->m_static_exists &&
 		   !m_active_blocks.contains(obj->m_static_block) &&
 		   m_active_blocks.contains(blockpos_o)) {
+
 			// Delete from block where object was located
 			deleteStaticFromBlock(obj, id, MOD_REASON_STATIC_DATA_REMOVED, false);
 
@@ -2068,6 +2069,10 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 				force_delete = true;
 		}
 
+		// Regardless of what happens to the object at this point, deactivate it first.
+		// This ensures that LuaEntity on_deactivate is always called.
+		obj->markForDeactivation();
+
 		/*
 			If known by some client, set pending deactivation.
 			Otherwise delete it immediately.
@@ -2077,7 +2082,6 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 						  << "object id=" << id << " is known by clients"
 						  << "; not deleting yet" << std::endl;
 
-			obj->m_pending_deactivation = true;
 			return false;
 		}