Intersect observer sets along parent chain

Co-authored-by: sfan5 <sfan5@live.de>

doc/lua_api.md

@@ -7927,13 +7927,20 @@ child will follow movement and rotation of that bone.
     * `observers` is a "set" of player names: `{[player name] = true, [other player name] = true, ...}`
         * Since this is a set, the values need to be `true`.
     * If players are managed, they always need to have themselves as observers.
-    * Attachments:
-        * If an object's observers are managed, the observers of all children need to be managed too.
-        * The observers of children need to be a subset of the observers of parents.
-	* Object activation and deactivation are unaffected by observability.
+    * Attachments: The "final observers" are the intersection of all observer sets
+      of all ancestors of an object and the object itself.
+      The intersection of an unmanaged observer set with a managed one is the managed one,
+      the intersection of two unmanaged observer sets is unmanaged.
+    * Object activation and deactivation are unaffected by observability.
 * `get_observers()`:
+    * throws an error if the object is invalid
     * returns `nil` if the observers are unmanaged
     * returns a table with all observer names as keys and `true` values (a "set") otherwise
+* `get_effective_observers()`:
+    * Like `get_observers()`, but returns the "effective" observers, taking into account attachments
+    * Time complexity: O(nm)
+        * n: number of observers of the involved entities
+        * m: number of ancestors along the attachment chain
 * `is_player()`: returns true for players, false otherwise
 * `get_nametag_attributes()`
     * returns a table with the attributes of the nametag of an object

src/player.cpp

@@ -30,7 +30,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "porting.h"  // strlcpy
 
 
-Player::Player(const std::string name, IItemDefManager *idef):
+Player::Player(const std::string &name, IItemDefManager *idef):
 	inventory(idef)
 {
 	m_name = name;

src/script/lua_api/l_object.cpp

@@ -27,6 +27,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "common/c_content.h"
 #include "log.h"
 #include "player.h"
+#include "server/serveractiveobject.h"
 #include "tool.h"
 #include "remoteplayer.h"
 #include "server.h"
@@ -847,10 +848,7 @@ int ObjectRef::l_set_observers(lua_State *L)
 
 	// Reset object to "unmanaged" (sent to everyone)?
 	if (lua_isnoneornil(L, 2)) {
-		ServerActiveObject *parent = sao->getParent();
-		if (parent != nullptr && parent->m_observer_names.has_value())
-			throw LuaError("Child observers need to be managed if parent observers are managed");
-		sao->m_observer_names.reset();
+		sao->m_observers.reset();
 		return 0;
 	}
 
@@ -876,51 +874,47 @@ int ObjectRef::l_set_observers(lua_State *L)
 			throw LuaError("Players need to observe themselves");
 	}
 
-	// Check attachments
-	ServerActiveObject *parent = sao->getParent();
-	if (parent != nullptr) {
-		for (auto &name : observer_names) {
-			if (!parent->isObservedBy(name))
-				throw LuaError("Child observers not a subset of parent observers");
-		}
-	}
-	auto child_ids = sao->getAttachmentChildIds();
-	for (auto child_id : child_ids) {
-		ServerActiveObject *child = env->getActiveObject(child_id);
-		assert(child);
-		if (!child->m_observer_names.has_value())
-			throw LuaError("Child observers need to be managed if parent observers are managed");
-		for (auto &name : child->m_observer_names.value()) {
-			if (!sao->isObservedBy(name))
-				throw LuaError("Child observers not a subset of parent observers");
-		}
-	}
-	sao->m_observer_names = observer_names;
+	sao->m_observers = observer_names;
 	return 0;
 }
 
+template<typename F>
+static int get_observers(lua_State *L, F get_observers)
+{
+	ObjectRef *ref = ObjectRef::checkObject<ObjectRef>(L, 1);
+	ServerActiveObject *sao = ObjectRef::getobject(ref);
+	if (sao == nullptr)
+		throw LuaError("invalid ObjectRef");
+
+	const auto observers = get_observers(sao);
+	if (!observers) {
+		lua_pushnil(L);
+		return 1;
+	}
+	// Push set of observers {[name] = true}
+	lua_createtable(L, 0, observers->size());
+	for (auto &name : *observers) {
+		lua_pushboolean(L, true);
+		lua_setfield(L, -2, name.c_str());
+	}
+	return 1;
+}
+
 // get_observers(self)
 int ObjectRef::l_get_observers(lua_State *L)
 {
 	NO_MAP_LOCK_REQUIRED;
-	ObjectRef *ref = checkObject<ObjectRef>(L, 1);
-	ServerActiveObject *sao = getobject(ref);
-	if (sao == nullptr)
-		throw LuaError("invalid ObjectRef");
+	return get_observers(L, [](auto sao) { return sao->m_observers; });
+}
 
-	if (!sao->m_observer_names.has_value()) {
-		lua_pushnil(L);
-		return 1;
-	}
-
-	const auto &observer_names = sao->m_observer_names.value();
-	// Push set of observers {[name] = true}
-	lua_createtable(L, 0, observer_names.size());
-	for (auto &name : observer_names) {
-		lua_pushboolean(L, true);
-		lua_setfield(L, -2, name.c_str());
-	}
-	return 1;
+// get_effective_observers(self)
+int ObjectRef::l_get_effective_observers(lua_State *L)
+{
+	NO_MAP_LOCK_REQUIRED;
+	return get_observers(L, [](auto sao) {
+		// The cache may be outdated, so we always have to recalculate.
+		return sao->recalculateEffectiveObservers();
+	});
 }
 
 // is_player(self)

src/script/lua_api/l_object.h

@@ -166,6 +166,9 @@ private:
 	// get_observers(self)
 	static int l_get_observers(lua_State *L);
 
+	// get_effective_observers(self)
+	static int l_get_effective_observers(lua_State *L);
+
 	// is_player(self)
 	static int l_is_player(lua_State *L);
 

src/server.cpp

@@ -785,6 +785,8 @@ void Server::AsyncRunStep(float dtime, bool initial_step)
 		//infostream<<"Server: Checking added and deleted active objects"<<std::endl;
 		MutexAutoLock envlock(m_env_mutex);
 
+		m_env->invalidateActiveObjectObserverCaches();
+
 		{
 			ClientInterface::AutoLock clientlock(m_clients);
 			const RemoteClientMap &clients = m_clients.getClientList();

src/server/activeobjectmgr.cpp

@@ -118,6 +118,16 @@ void ActiveObjectMgr::removeObject(u16 id)
 	}
 }
 
+void ActiveObjectMgr::invalidateActiveObjectObserverCaches()
+{
+	for (auto &activeObject : m_active_objects.iter()) {
+		ServerActiveObject *obj = activeObject.second.get();
+		if (!obj)
+			continue;
+		obj->invalidateEffectiveObservers();
+	}
+}
+
 void ActiveObjectMgr::getObjectsInsideRadius(const v3f &pos, float radius,
 		std::vector<ServerActiveObject *> &result,
 		std::function<bool(ServerActiveObject *obj)> include_obj_cb)
@@ -185,7 +195,7 @@ void ActiveObjectMgr::getAddedActiveObjectsAroundPos(
 		} else if (distance_f > radius)
 			continue;
 
-		if (!object->isObservedBy(player_name))
+		if (!object->isEffectivelyObservedBy(player_name))
 			continue;
 
 		// Discard if already on current_objects

src/server/activeobjectmgr.h

@@ -38,6 +38,8 @@ public:
 	bool registerObject(std::unique_ptr<ServerActiveObject> obj) override;
 	void removeObject(u16 id) override;
 
+	void invalidateActiveObjectObserverCaches();
+
 	void getObjectsInsideRadius(const v3f &pos, float radius,
 			std::vector<ServerActiveObject *> &result,
 			std::function<bool(ServerActiveObject *obj)> include_obj_cb);

src/server/serveractiveobject.cpp

@@ -18,25 +18,31 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 */
 
 #include "serveractiveobject.h"
+#include <algorithm>
+#include <cstddef>
+#include <cstdio>
 #include <fstream>
+#include <unordered_map>
 #include "inventory.h"
 #include "inventorymanager.h"
 #include "constants.h" // BS
 #include "log.h"
 
-ServerActiveObject::ServerActiveObject(ServerEnvironment *env, v3f pos):
+using SAO = ServerActiveObject;
+
+SAO::ServerActiveObject(ServerEnvironment *env, v3f pos):
 	ActiveObject(0),
 	m_env(env),
 	m_base_position(pos)
 {
 }
 
-float ServerActiveObject::getMinimumSavedMovement()
+float SAO::getMinimumSavedMovement()
 {
 	return 2.0*BS;
 }
 
-ItemStack ServerActiveObject::getWieldedItem(ItemStack *selected, ItemStack *hand) const
+ItemStack SAO::getWieldedItem(ItemStack *selected, ItemStack *hand) const
 {
 	*selected = ItemStack();
 	if (hand)
@@ -45,12 +51,12 @@ ItemStack ServerActiveObject::getWieldedItem(ItemStack *selected, ItemStack *han
 	return ItemStack();
 }
 
-bool ServerActiveObject::setWieldedItem(const ItemStack &item)
+bool SAO::setWieldedItem(const ItemStack &item)
 {
 	return false;
 }
 
-std::string ServerActiveObject::generateUpdateInfantCommand(u16 infant_id, u16 protocol_version)
+std::string SAO::generateUpdateInfantCommand(u16 infant_id, u16 protocol_version)
 {
 	std::ostringstream os(std::ios::binary);
 	// command
@@ -67,7 +73,7 @@ std::string ServerActiveObject::generateUpdateInfantCommand(u16 infant_id, u16 p
 	return os.str();
 }
 
-void ServerActiveObject::dumpAOMessagesToQueue(std::queue<ActiveObjectMessage> &queue)
+void SAO::dumpAOMessagesToQueue(std::queue<ActiveObjectMessage> &queue)
 {
 	while (!m_messages_out.empty()) {
 		queue.push(std::move(m_messages_out.front()));
@@ -75,7 +81,7 @@ void ServerActiveObject::dumpAOMessagesToQueue(std::queue<ActiveObjectMessage> &
 	}
 }
 
-void ServerActiveObject::markForRemoval()
+void SAO::markForRemoval()
 {
 	if (!m_pending_removal) {
 		onMarkedForRemoval();
@@ -83,7 +89,7 @@ void ServerActiveObject::markForRemoval()
 	}
 }
 
-void ServerActiveObject::markForDeactivation()
+void SAO::markForDeactivation()
 {
 	if (!m_pending_deactivation) {
 		onMarkedForDeactivation();
@@ -91,12 +97,49 @@ void ServerActiveObject::markForDeactivation()
 	}
 }
 
-InventoryLocation ServerActiveObject::getInventoryLocation() const
+InventoryLocation SAO::getInventoryLocation() const
 {
 	return InventoryLocation();
 }
 
-bool ServerActiveObject::isObservedBy(const std::string &player_name) const
+void SAO::invalidateEffectiveObservers()
 {
-	return !m_observer_names.has_value() || m_observer_names.value().count(player_name) > 0;
+	m_effective_observers.reset();
+}
+
+const SAO::Observers &SAO::getEffectiveObservers()
+{
+	if (m_effective_observers) // cached
+		return *m_effective_observers;
+
+	auto parent = getParent();
+	if (parent == nullptr)
+		return *(m_effective_observers = m_observers);
+	auto parent_observers = parent->getEffectiveObservers();
+	if (!parent_observers) // parent is unmanaged
+		return *(m_effective_observers = m_observers);
+	if (!m_observers) // we are unmanaged
+		return *(m_effective_observers = parent_observers);
+	// Set intersection between parent_observers and m_observers
+	m_effective_observers = std::unordered_set<std::string>();
+	for (const auto &observer_name : *m_observers) {
+		if (parent_observers->count(observer_name) > 0)
+			(*m_effective_observers)->insert(observer_name);
+	}
+	return *m_effective_observers;
+}
+
+const SAO::Observers &SAO::recalculateEffectiveObservers()
+{
+	// Invalidate final observers for this object and all of its parents.
+	for (auto obj = this; obj != nullptr; obj = obj->getParent())
+		obj->invalidateEffectiveObservers();
+	// getEffectiveObservers will now be forced to recalculate.
+	return getEffectiveObservers();
+}
+
+bool SAO::isEffectivelyObservedBy(const std::string &player_name)
+{
+	auto effective_observers = getEffectiveObservers();
+	return !effective_observers || effective_observers->count(player_name) > 0;
 }

src/server/serveractiveobject.h

@@ -242,14 +242,25 @@ public:
 	*/
 	v3s16 m_static_block = v3s16(1337,1337,1337);
 
-	bool isObservedBy(const std::string &player_name) const;
+	// Names of players to whom the object is to be sent, not considering parents.
+	using Observers = std::optional<std::unordered_set<std::string>>;
+	Observers m_observers;
 
-	/*
-		Names of players to whom the object is to be sent
-	*/
-	std::optional<std::unordered_set<std::string>> m_observer_names;
+	// Invalidate final observer cache. This needs to be done whenever
+	// the observers of this object or any of its ancestors may have changed.
+	void invalidateEffectiveObservers();
+	// Get from cache (or compute) final observers.
+	const Observers &getEffectiveObservers();
+	// Force a recalculation of final observers (including all parents).
+	const Observers &recalculateEffectiveObservers();
+	// Check whether an object is finally observed by a player.
+	// Requires the cached final observers to be up to date.
+	bool isEffectivelyObservedBy(const std::string &player_name);
 
 protected:
+	// Cached intersection of m_observers of this object and all its parents.
+	std::optional<Observers> m_effective_observers;
+
 	virtual void onMarkedForDeactivation() {}
 	virtual void onMarkedForRemoval() {}
 

src/serverenvironment.cpp

@@ -1711,6 +1711,11 @@ u16 ServerEnvironment::addActiveObject(std::unique_ptr<ServerActiveObject> objec
 	return id;
 }
 
+void ServerEnvironment::invalidateActiveObjectObserverCaches()
+{
+	m_ao_manager.invalidateActiveObjectObserverCaches();
+}
+
 /*
 	Finds out what new objects have been added to
 	inside a radius around a position
@@ -1747,7 +1752,7 @@ void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius
 	if (player_radius_f < 0)
 		player_radius_f = 0;
 
-	const std::string player_name = playersao->getPlayer()->getName();
+	const std::string &player_name = playersao->getPlayer()->getName();
 
 	/*
 		Go through current_objects; object is removed if:
@@ -1755,7 +1760,8 @@ void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius
 		  error condition; objects should be removed only after all clients
 		  have been informed about removal), or
 		- object is to be removed or deactivated, or
-		- object is too far away
+		- object is too far away, or
+		- object is marked as not observable by the client
 	*/
 	for (u16 id : current_objects) {
 		ServerActiveObject *object = getActiveObject(id);
@@ -1777,7 +1783,7 @@ void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius
 			? distance_f <= player_radius_f || player_radius_f == 0
 			: distance_f <= radius_f;
 
-		if (!inRange || !object->isObservedBy(player_name))
+		if (!inRange || !object->isEffectivelyObservedBy(player_name))
 			removed_objects.emplace_back(false, id); // out of range or not observed anymore
 	}
 }

src/serverenvironment.h

@@ -277,6 +277,8 @@ public:
 	*/
 	u16 addActiveObject(std::unique_ptr<ServerActiveObject> object);
 
+	void invalidateActiveObjectObserverCaches();
+
 	/*
 		Find out what new objects have been added to
 		inside a radius around a position