clientobject, clouds, collision, clientsimpleobject: code modernization (#6260)

nerzhul · web-flow · commit c738d1eeabbb · 2017-08-17T20:23:54.000+02:00
* clientobject, clouds, collision, clientsimpleobject: code modernization

* use range-based for loops
* simplify some tests
* various code style fixes
* use emplace_back instead of push_back when necessary
* use auto on some iterators
* use default operator when needed
* unroll v3s16 creation on collisionMoveSimple
diff --git a/src/clientobject.cpp b/src/clientobject.cpp
@@ -42,7 +42,7 @@ ClientActiveObject* ClientActiveObject::create(ActiveObjectType type,
 		Client *client, ClientEnvironment *env)
 {
 	// Find factory function
-	std::unordered_map<u16, Factory>::iterator n = m_types.find(type);
+	auto n = m_types.find(type);
 	if (n == m_types.end()) {
 		// If factory is not found, just return.
 		warningstream << "ClientActiveObject: No factory for type="
@@ -57,8 +57,8 @@ ClientActiveObject* ClientActiveObject::create(ActiveObjectType type,
 
 void ClientActiveObject::registerType(u16 type, Factory f)
 {
-	std::unordered_map<u16, Factory>::iterator n = m_types.find(type);
-	if(n != m_types.end())
+	auto n = m_types.find(type);
+	if (n != m_types.end())
 		return;
 	m_types[type] = f;
 }
diff --git a/src/clientsimpleobject.h b/src/clientsimpleobject.h
@@ -29,8 +29,9 @@ class ClientSimpleObject
 public:
 	bool m_to_be_removed = false;
 
-	ClientSimpleObject() {}
-	virtual ~ClientSimpleObject() {}
+	ClientSimpleObject() = default;
+	virtual ~ClientSimpleObject() = default;
+
 	virtual void step(float dtime) {}
 };
 
diff --git a/src/clouds.cpp b/src/clouds.cpp
@@ -208,7 +208,7 @@ void Clouds::render()
 
 		u32 i = GETINDEX(xi, zi, m_cloud_radius_i);
 
-		if(grid[i] == false)
+		if (!grid[i])
 			continue;
 
 		v2f p0 = v2f(xi,zi)*cloud_size + world_center_of_drawing_in_noise_f;
diff --git a/src/clouds.h b/src/clouds.h
@@ -74,7 +74,7 @@ class Clouds : public scene::ISceneNode
 
 	void update(const v3f &camera_p, const video::SColorf &color);
 
-	void updateCameraOffset(v3s16 camera_offset)
+	void updateCameraOffset(const v3s16 &camera_offset)
 	{
 		m_camera_offset = camera_offset;
 		updateBox();
diff --git a/src/collision.cpp b/src/collision.cpp
@@ -188,9 +188,8 @@ bool wouldCollideWithCeiling(
 
 	assert(y_increase >= 0);	// pre-condition
 
-	for (std::vector<NearbyCollisionInfo>::const_iterator it = cinfo.begin();
-			it != cinfo.end(); ++it) {
-		const aabb3f &staticbox = it->box;
+	for (const auto &it : cinfo) {
+		const aabb3f &staticbox = it.box;
 		if ((movingbox.MaxEdge.Y - d <= staticbox.MinEdge.Y) &&
 				(movingbox.MaxEdge.Y + y_increase > staticbox.MinEdge.Y) &&
 				(movingbox.MinEdge.X < staticbox.MaxEdge.X) &&
@@ -203,7 +202,7 @@ bool wouldCollideWithCeiling(
 	return false;
 }
 
-static inline void getNeighborConnectingFace(v3s16 p, INodeDefManager *nodedef,
+static inline void getNeighborConnectingFace(const v3s16 &p, INodeDefManager *nodedef,
 		Map *map, MapNode n, int v, int *neighbors)
 {
 	MapNode n2 = map->getNodeNoEx(p);
@@ -255,7 +254,7 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
 	std::vector<NearbyCollisionInfo> cinfo;
 	{
 	//TimeTaker tt2("collisionMoveSimple collect boxes");
-	ScopeProfiler sp(g_profiler, "collisionMoveSimple collect boxes avg", SPT_AVG);
+	ScopeProfiler sp2(g_profiler, "collisionMoveSimple collect boxes avg", SPT_AVG);
 
 	v3f newpos_f = *pos_f + *speed_f * dtime;
 	v3f minpos_f(
@@ -273,12 +272,10 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
 
 	bool any_position_valid = false;
 
-	for(s16 x = min.X; x <= max.X; x++)
-	for(s16 y = min.Y; y <= max.Y; y++)
-	for(s16 z = min.Z; z <= max.Z; z++)
-	{
-		v3s16 p(x,y,z);
-
+	v3s16 p;
+	for (p.X = min.X; p.X <= max.X; p.X++)
+	for (p.Y = min.Y; p.Y <= max.Y; p.Y++)
+	for (p.Z = min.Z; p.Z <= max.Z; p.Z++) {
 		bool is_position_valid;
 		MapNode n = map->getNodeNoEx(p, &is_position_valid);
 
@@ -288,12 +285,15 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
 			any_position_valid = true;
 			INodeDefManager *nodedef = gamedef->getNodeDefManager();
 			const ContentFeatures &f = nodedef->get(n);
-			if(f.walkable == false)
+
+			if (!f.walkable)
 				continue;
+
 			int n_bouncy_value = itemgroup_get(f.groups, "bouncy");
 
 			int neighbors = 0;
-			if (f.drawtype == NDT_NODEBOX && f.node_box.type == NODEBOX_CONNECTED) {
+			if (f.drawtype == NDT_NODEBOX &&
+				f.node_box.type == NODEBOX_CONNECTED) {
 				v3s16 p2 = p;
 
 				p2.Y++;
@@ -321,20 +321,15 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
 			}
 			std::vector<aabb3f> nodeboxes;
 			n.getCollisionBoxes(gamedef->ndef(), &nodeboxes, neighbors);
-			for(std::vector<aabb3f>::iterator
-					i = nodeboxes.begin();
-					i != nodeboxes.end(); ++i)
-			{
-				aabb3f box = *i;
-				box.MinEdge += v3f(x, y, z)*BS;
-				box.MaxEdge += v3f(x, y, z)*BS;
-				cinfo.push_back(NearbyCollisionInfo(false,
-					false, n_bouncy_value, p, box));
+			for (auto box : nodeboxes) {
+				box.MinEdge += intToFloat(p, BS);
+				box.MaxEdge += intToFloat(p, BS);
+				cinfo.emplace_back(false, false, n_bouncy_value, p, box);
 			}
 		} else {
 			// Collide with unloaded nodes
 			aabb3f box = getNodeBox(p, BS);
-			cinfo.push_back(NearbyCollisionInfo(true, false, 0, p, box));
+			cinfo.emplace_back(true, false, 0, p, box);
 		}
 	}
 
@@ -349,7 +344,7 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
 
 	if(collideWithObjects)
 	{
-		ScopeProfiler sp(g_profiler, "collisionMoveSimple objects avg", SPT_AVG);
+		ScopeProfiler sp2(g_profiler, "collisionMoveSimple objects avg", SPT_AVG);
 		//TimeTaker tt3("collisionMoveSimple collect object boxes");
 
 		/* add object boxes to cinfo */
@@ -361,9 +356,9 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
 			f32 distance = speed_f->getLength();
 			std::vector<DistanceSortedActiveObject> clientobjects;
 			c_env->getActiveObjects(*pos_f, distance * 1.5, clientobjects);
-			for (size_t i=0; i < clientobjects.size(); i++) {
-				if ((self == 0) || (self != clientobjects[i].obj)) {
-					objects.push_back((ActiveObject*)clientobjects[i].obj);
+			for (auto &clientobject : clientobjects) {
+				if (!self || (self != clientobject.obj)) {
+					objects.push_back((ActiveObject*) clientobject.obj);
 				}
 			}
 		}
@@ -375,9 +370,9 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
 				f32 distance = speed_f->getLength();
 				std::vector<u16> s_objects;
 				s_env->getObjectsInsideRadius(s_objects, *pos_f, distance * 1.5);
-				for (std::vector<u16>::iterator iter = s_objects.begin(); iter != s_objects.end(); ++iter) {
-					ServerActiveObject *current = s_env->getActiveObject(*iter);
-					if ((self == 0) || (self != current)) {
+				for (u16 obj_id : s_objects) {
+					ServerActiveObject *current = s_env->getActiveObject(obj_id);
+					if (!self || (self != current)) {
 						objects.push_back((ActiveObject*)current);
 					}
 				}
@@ -388,11 +383,11 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
 				iter != objects.end(); ++iter) {
 			ActiveObject *object = *iter;
 
-			if (object != NULL) {
+			if (object) {
 				aabb3f object_collisionbox;
 				if (object->getCollisionBox(&object_collisionbox) &&
 						object->collideWithObjects()) {
-					cinfo.push_back(NearbyCollisionInfo(false, true, 0, v3s16(), object_collisionbox));
+					cinfo.emplace_back(false, true, 0, v3s16(), object_collisionbox);
 				}
 			}
 		}
@@ -417,7 +412,7 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
 
 	while(dtime > BS * 1e-10) {
 		//TimeTaker tt3("collisionMoveSimple dtime loop");
-        	ScopeProfiler sp(g_profiler, "collisionMoveSimple dtime loop avg", SPT_AVG);
+        	ScopeProfiler sp2(g_profiler, "collisionMoveSimple dtime loop avg", SPT_AVG);
 
 		// Avoid infinite loop
 		loopcount++;
@@ -545,8 +540,7 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
 	aabb3f box = box_0;
 	box.MinEdge += *pos_f;
 	box.MaxEdge += *pos_f;
-	for (u32 boxindex = 0; boxindex < cinfo.size(); boxindex++) {
-		NearbyCollisionInfo &box_info = cinfo[boxindex];
+	for (const auto &box_info : cinfo) {
 		const aabb3f &cbox = box_info.box;
 
 		/*
diff --git a/src/collision.h b/src/collision.h
@@ -36,7 +36,8 @@ enum CollisionType
 
 struct CollisionInfo
 {
-	CollisionInfo() {}
+	CollisionInfo() = default;
+
 	CollisionType type = COLLISION_NODE;
 	v3s16 node_p = v3s16(-32768,-32768,-32768); // COLLISION_NODE
 	v3f old_speed;
@@ -45,7 +46,8 @@ struct CollisionInfo
 
 struct collisionMoveResult
 {
-	collisionMoveResult() {}
+	collisionMoveResult() = default;
+
 	bool touching_ground = false;
 	bool collides = false;
 	bool standing_on_object = false;

Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ ClientActiveObject* ClientActiveObject::create(ActiveObjectType type,`
`42`	`42`	`Client client, ClientEnvironment env)`
`43`	`43`	`{`
`44`	`44`	`// Find factory function`
`45`		`- std::unordered_map<u16, Factory>::iterator n = m_types.find(type);`
	`45`	`+ auto n = m_types.find(type);`
`46`	`46`	`if (n == m_types.end()) {`
`47`	`47`	`// If factory is not found, just return.`
`48`	`48`	`warningstream << "ClientActiveObject: No factory for type="`
`@@ -57,8 +57,8 @@ ClientActiveObject* ClientActiveObject::create(ActiveObjectType type,`
`57`	`57`
`58`	`58`	`void ClientActiveObject::registerType(u16 type, Factory f)`
`59`	`59`	`{`
`60`		`- std::unordered_map<u16, Factory>::iterator n = m_types.find(type);`
`61`		`- if(n != m_types.end())`
	`60`	`+ auto n = m_types.find(type);`
	`61`	`+ if (n != m_types.end())`
`62`	`62`	`return;`
`63`	`63`	`m_types[type] = f;`
`64`	`64`	`}`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ class Clouds : public scene::ISceneNode`
`74`	`74`
`75`	`75`	`void update(const v3f &camera_p, const video::SColorf &color);`
`76`	`76`
`77`		`- void updateCameraOffset(v3s16 camera_offset)`
	`77`	`+ void updateCameraOffset(const v3s16 &camera_offset)`
`78`	`78`	`{`
`79`	`79`	`m_camera_offset = camera_offset;`
`80`	`80`	`updateBox();`