Add an activeobject manager to hold active objects (#7939)

* Add an activeobject manager to hold active objects
* Add unittests

src/client/CMakeLists.txt

@@ -25,6 +25,7 @@ set(client_SRCS
 	${CMAKE_CURRENT_SOURCE_DIR}/render/plain.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/render/sidebyside.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/render/stereo.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/activeobjectmgr.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/camera.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/client.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/clientenvironment.cpp

src/client/activeobjectmgr.cpp

@@ -0,0 +1,106 @@
+/*
+Minetest
+Copyright (C) 2010-2018 nerzhul, Loic BLOT <loic.blot@unix-experience.fr>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation; either version 2.1 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License along
+with this program; if not, write to the Free Software Foundation, Inc.,
+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+#include <log.h>
+#include "profiler.h"
+#include "activeobjectmgr.h"
+
+namespace client
+{
+
+void ActiveObjectMgr::clear()
+{
+	// delete active objects
+	for (auto &active_object : m_active_objects) {
+		delete active_object.second;
+	}
+}
+
+void ActiveObjectMgr::step(
+		float dtime, const std::function<void(ClientActiveObject *)> &f)
+{
+	g_profiler->avg("Client::ActiveObjectMgr: num of objects",
+			m_active_objects.size());
+	for (auto &ao_it : m_active_objects) {
+		f(ao_it.second);
+	}
+}
+
+// clang-format off
+bool ActiveObjectMgr::registerObject(ClientActiveObject *obj)
+{
+	assert(obj); // Pre-condition
+	if (obj->getId() == 0) {
+		u16 new_id = getFreeId();
+		if (new_id == 0) {
+			infostream << "Client::ActiveObjectMgr::registerObject(): "
+					<< "no free id available" << std::endl;
+
+			delete obj;
+			return false;
+		}
+		obj->setId(new_id);
+	}
+
+	if (!isFreeId(obj->getId())) {
+		infostream << "Client::ActiveObjectMgr::registerObject(): "
+				<< "id is not free (" << obj->getId() << ")" << std::endl;
+		delete obj;
+		return false;
+	}
+	infostream << "Client::ActiveObjectMgr::registerObject(): "
+			<< "added (id=" << obj->getId() << ")" << std::endl;
+	m_active_objects[obj->getId()] = obj;
+	return true;
+}
+
+void ActiveObjectMgr::removeObject(u16 id)
+{
+	verbosestream << "Client::ActiveObjectMgr::removeObject(): "
+			<< "id=" << id << std::endl;
+	ClientActiveObject *obj = getActiveObject(id);
+	if (!obj) {
+		infostream << "Client::ActiveObjectMgr::removeObject(): "
+				<< "id=" << id << " not found" << std::endl;
+		return;
+	}
+
+	m_active_objects.erase(id);
+
+	obj->removeFromScene(true);
+	delete obj;
+}
+
+// clang-format on
+void ActiveObjectMgr::getActiveObjects(const v3f &origin, f32 max_d,
+		std::vector<DistanceSortedActiveObject> &dest)
+{
+	for (auto &ao_it : m_active_objects) {
+		ClientActiveObject *obj = ao_it.second;
+
+		f32 d = (obj->getPosition() - origin).getLength();
+
+		if (d > max_d)
+			continue;
+
+		dest.emplace_back(obj, d);
+	}
+}
+
+} // namespace client

src/client/activeobjectmgr.h

@@ -0,0 +1,41 @@
+/*
+Minetest
+Copyright (C) 2010-2018 nerzhul, Loic BLOT <loic.blot@unix-experience.fr>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation; either version 2.1 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License along
+with this program; if not, write to the Free Software Foundation, Inc.,
+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+#pragma once
+
+#include <functional>
+#include <vector>
+#include "../activeobjectmgr.h"
+#include "clientobject.h"
+
+namespace client
+{
+class ActiveObjectMgr : public ::ActiveObjectMgr<ClientActiveObject>
+{
+public:
+	void clear();
+	void step(float dtime,
+			const std::function<void(ClientActiveObject *)> &f) override;
+	bool registerObject(ClientActiveObject *obj) override;
+	void removeObject(u16 id) override;
+
+	void getActiveObjects(const v3f &origin, f32 max_d,
+			std::vector<DistanceSortedActiveObject> &dest);
+};
+} // namespace client

src/client/clientenvironment.cpp

@@ -53,10 +53,7 @@ ClientEnvironment::ClientEnvironment(ClientMap *map,
 
 ClientEnvironment::~ClientEnvironment()
 {
-	// delete active objects
-	for (auto &active_object : m_active_objects) {
-		delete active_object.second;
-	}
+	m_ao_manager.clear();
 
 	for (auto &simple_object : m_simple_objects) {
 		delete simple_object;
@@ -262,12 +259,10 @@ void ClientEnvironment::step(float dtime)
 		Step active objects and update lighting of them
 	*/
 
-	g_profiler->avg("CEnv: num of objects", m_active_objects.size());
 	bool update_lighting = m_active_object_light_update_interval.step(dtime, 0.21);
-	for (auto &ao_it : m_active_objects) {
-		ClientActiveObject* obj = ao_it.second;
+	auto cb_state = [this, dtime, update_lighting, day_night_ratio] (ClientActiveObject *cao) {
 		// Step object
-		obj->step(dtime, this);
+		cao->step(dtime, this);
 
 		if (update_lighting) {
 			// Update lighting
@@ -275,16 +270,18 @@ void ClientEnvironment::step(float dtime)
 			bool pos_ok;
 
 			// Get node at head
-			v3s16 p = obj->getLightPosition();
-			MapNode n = m_map->getNodeNoEx(p, &pos_ok);
+			v3s16 p = cao->getLightPosition();
+			MapNode n = this->m_map->getNodeNoEx(p, &pos_ok);
 			if (pos_ok)
 				light = n.getLightBlend(day_night_ratio, m_client->ndef());
 			else
 				light = blend_light(day_night_ratio, LIGHT_SUN, 0);
 
-			obj->updateLight(light);
+			cao->updateLight(light);
 		}
-	}
+	};
+
+	m_ao_manager.step(dtime, cb_state);
 
 	/*
 		Step and handle simple objects
@@ -319,14 +316,6 @@ GenericCAO* ClientEnvironment::getGenericCAO(u16 id)
 	return NULL;
 }
 
-ClientActiveObject* ClientEnvironment::getActiveObject(u16 id)
-{
-	auto n = m_active_objects.find(id);
-	if (n == m_active_objects.end())
-		return NULL;
-	return n->second;
-}
-
 bool isFreeClientActiveObjectId(const u16 id,
 	ClientActiveObjectMap &objects)
 {
@@ -336,7 +325,7 @@ bool isFreeClientActiveObjectId(const u16 id,
 
 u16 getFreeClientActiveObjectId(ClientActiveObjectMap &objects)
 {
-	//try to reuse id's as late as possible
+	// try to reuse id's as late as possible
 	static u16 last_used_id = 0;
 	u16 startid = last_used_id;
 	for(;;) {
@@ -351,43 +340,25 @@ u16 getFreeClientActiveObjectId(ClientActiveObjectMap &objects)
 
 u16 ClientEnvironment::addActiveObject(ClientActiveObject *object)
 {
-	assert(object); // Pre-condition
-	if(object->getId() == 0)
-	{
-		u16 new_id = getFreeClientActiveObjectId(m_active_objects);
-		if(new_id == 0)
-		{
-			infostream<<"ClientEnvironment::addActiveObject(): "
-				<<"no free ids available"<<std::endl;
-			delete object;
-			return 0;
-		}
-		object->setId(new_id);
-	}
-	if (!isFreeClientActiveObjectId(object->getId(), m_active_objects)) {
-		infostream<<"ClientEnvironment::addActiveObject(): "
-			<<"id is not free ("<<object->getId()<<")"<<std::endl;
-		delete object;
+	// Register object. If failed return zero id
+	if (!m_ao_manager.registerObject(object))
 		return 0;
-	}
-	infostream<<"ClientEnvironment::addActiveObject(): "
-		<<"added (id="<<object->getId()<<")"<<std::endl;
-	m_active_objects[object->getId()] = object;
+
 	object->addToScene(m_texturesource);
-	{ // Update lighting immediately
-		u8 light = 0;
-		bool pos_ok;
 
-		// Get node at head
-		v3s16 p = object->getLightPosition();
-		MapNode n = m_map->getNodeNoEx(p, &pos_ok);
-		if (pos_ok)
-			light = n.getLightBlend(getDayNightRatio(), m_client->ndef());
-		else
-			light = blend_light(getDayNightRatio(), LIGHT_SUN, 0);
+	// Update lighting immediately
+	u8 light = 0;
+	bool pos_ok;
 
-		object->updateLight(light);
-	}
+	// Get node at head
+	v3s16 p = object->getLightPosition();
+	MapNode n = m_map->getNodeNoEx(p, &pos_ok);
+	if (pos_ok)
+		light = n.getLightBlend(getDayNightRatio(), m_client->ndef());
+	else
+		light = blend_light(getDayNightRatio(), LIGHT_SUN, 0);
+
+	object->updateLight(light);
 	return object->getId();
 }
 
@@ -423,21 +394,6 @@ void ClientEnvironment::addActiveObject(u16 id, u8 type,
 	addActiveObject(obj);
 }
 
-void ClientEnvironment::removeActiveObject(u16 id)
-{
-	verbosestream<<"ClientEnvironment::removeActiveObject(): "
-		<<"id="<<id<<std::endl;
-	ClientActiveObject* obj = getActiveObject(id);
-	if (obj == NULL) {
-		infostream<<"ClientEnvironment::removeActiveObject(): "
-			<<"id="<<id<<" not found"<<std::endl;
-		return;
-	}
-	obj->removeFromScene(true);
-	delete obj;
-	m_active_objects.erase(id);
-}
-
 void ClientEnvironment::processActiveObjectMessage(u16 id, const std::string &data)
 {
 	ClientActiveObject *obj = getActiveObject(id);
@@ -485,21 +441,6 @@ void ClientEnvironment::damageLocalPlayer(u8 damage, bool handle_hp)
 	Client likes to call these
 */
 
-void ClientEnvironment::getActiveObjects(v3f origin, f32 max_d,
-	std::vector<DistanceSortedActiveObject> &dest)
-{
-	for (auto &ao_it : m_active_objects) {
-		ClientActiveObject* obj = ao_it.second;
-
-		f32 d = (obj->getPosition() - origin).getLength();
-
-		if (d > max_d)
-			continue;
-
-		dest.emplace_back(obj, d);
-	}
-}
-
 ClientEnvEvent ClientEnvironment::getClientEnvEvent()
 {
 	FATAL_ERROR_IF(m_client_event_queue.empty(),

src/client/clientenvironment.h

@@ -23,6 +23,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <ISceneManager.h>
 #include "clientobject.h"
 #include "util/numeric.h"
+#include "activeobjectmgr.h"
 
 class ClientSimpleObject;
 class ClientMap;
@@ -87,7 +88,10 @@ public:
 	*/
 
 	GenericCAO* getGenericCAO(u16 id);
-	ClientActiveObject* getActiveObject(u16 id);
+	ClientActiveObject* getActiveObject(u16 id)
+	{
+		return m_ao_manager.getActiveObject(id);
+	}
 
 	/*
 		Adds an active object to the environment.
@@ -100,7 +104,10 @@ public:
 	u16 addActiveObject(ClientActiveObject *object);
 
 	void addActiveObject(u16 id, u8 type, const std::string &init_data);
-	void removeActiveObject(u16 id);
+	void removeActiveObject(u16 id)
+	{
+		m_ao_manager.removeObject(id);
+	}
 
 	void processActiveObjectMessage(u16 id, const std::string &data);
 
@@ -115,8 +122,11 @@ public:
 	*/
 
 	// Get all nearby objects
-	void getActiveObjects(v3f origin, f32 max_d,
-		std::vector<DistanceSortedActiveObject> &dest);
+	void getActiveObjects(const v3f &origin, f32 max_d,
+		std::vector<DistanceSortedActiveObject> &dest)
+	{
+		return m_ao_manager.getActiveObjects(origin, max_d, dest);
+	}
 
 	bool hasClientEnvEvents() const { return !m_client_event_queue.empty(); }
 
@@ -142,7 +152,7 @@ private:
 	ITextureSource *m_texturesource;
 	Client *m_client;
 	ClientScripting *m_script = nullptr;
-	ClientActiveObjectMap m_active_objects;
+	client::ActiveObjectMgr m_ao_manager;
 	std::vector<ClientSimpleObject*> m_simple_objects;
 	std::queue<ClientEnvEvent> m_client_event_queue;
 	IntervalLimiter m_active_object_light_update_interval;