clientmap, clientmedia: code modernization

* use range-based for loops
* simplify some tests
* various code style fixes
* remove debugprint in ClientMap::getBackgroundBrightness, debug code was not intended to be there
* remove unused fields in MapDrawControl
* use emplace_back instead of push_back when necessary

src/clientmap.cpp

@@ -22,9 +22,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "mapblock_mesh.h"
 #include <IMaterialRenderer.h>
 #include <matrix4.h>
-#include "log.h"
 #include "mapsector.h"
-#include "nodedef.h"
 #include "mapblock.h"
 #include "profiler.h"
 #include "settings.h"
@@ -42,10 +40,7 @@ ClientMap::ClientMap(
 	scene::ISceneNode(RenderingEngine::get_scene_manager()->getRootSceneNode(),
 		RenderingEngine::get_scene_manager(), id),
 	m_client(client),
-	m_control(control),
+	m_control(control)
-	m_camera_position(0,0,0),
-	m_camera_direction(0,0,1),
-	m_camera_fov(M_PI)
 {
 	m_box = aabb3f(-BS*1000000,-BS*1000000,-BS*1000000,
 			BS*1000000,BS*1000000,BS*1000000);
@@ -65,10 +60,6 @@ ClientMap::ClientMap(
 
 }
 
-ClientMap::~ClientMap()
-{
-}
-
 MapSector * ClientMap::emergeSector(v2s16 p2d)
 {
 	DSTACK(FUNCTION_NAME);
@@ -129,14 +120,13 @@ void ClientMap::getBlocksInViewRange(v3s16 cam_pos_nodes,
 			p_nodes_max.Z / MAP_BLOCKSIZE + 1);
 }
 
-void ClientMap::updateDrawList(video::IVideoDriver* driver)
+void ClientMap::updateDrawList()
 {
 	ScopeProfiler sp(g_profiler, "CM::updateDrawList()", SPT_AVG);
 	g_profiler->add("CM::updateDrawList() count", 1);
 
-	for (std::map<v3s16, MapBlock*>::iterator i = m_drawlist.begin();
+	for (auto &i : m_drawlist) {
-			i != m_drawlist.end(); ++i) {
+		MapBlock *block = i.second;
-		MapBlock *block = i->second;
 		block->refDrop();
 	}
 	m_drawlist.clear();
@@ -183,12 +173,11 @@ void ClientMap::updateDrawList(video::IVideoDriver* driver)
 			occlusion_culling_enabled = false;
 	}
 
-	for (std::map<v2s16, MapSector*>::iterator si = m_sectors.begin();
+	for (const auto &sector_it : m_sectors) {
-			si != m_sectors.end(); ++si) {
+		MapSector *sector = sector_it.second;
-		MapSector *sector = si->second;
 		v2s16 sp = sector->getPos();
 
-		if (m_control.range_all == false) {
+		if (!m_control.range_all) {
 			if (sp.X < p_blocks_min.X || sp.X > p_blocks_max.X ||
 					sp.Y < p_blocks_min.Z || sp.Y > p_blocks_max.Z)
 				continue;
@@ -203,14 +192,11 @@ void ClientMap::updateDrawList(video::IVideoDriver* driver)
 
 		u32 sector_blocks_drawn = 0;
 
-		for (MapBlockVect::iterator i = sectorblocks.begin();
+		for (auto block : sectorblocks) {
-				i != sectorblocks.end(); ++i) {
-			MapBlock *block = *i;
-
 			/*
-				Compare block position to camera position, skip
+			Compare block position to camera position, skip
-				if not seen on display
+			if not seen on display
-			*/
+		*/
 
 			if (block->mesh)
 				block->mesh->updateCameraOffset(m_camera_offset);
@@ -267,10 +253,6 @@ void ClientMap::updateDrawList(video::IVideoDriver* driver)
 			m_last_drawn_sectors.insert(sp);
 	}
 
-	m_control.blocks_would_have_drawn = blocks_would_have_drawn;
-	m_control.blocks_drawn = blocks_drawn;
-	m_control.farthest_drawn = farthest_drawn;
-
 	g_profiler->avg("CM: blocks in range", blocks_in_range);
 	g_profiler->avg("CM: blocks occlusion culled", blocks_occlusion_culled);
 	if (blocks_in_range != 0)
@@ -389,9 +371,8 @@ void ClientMap::renderMap(video::IVideoDriver* driver, s32 pass)
 
 	MeshBufListList drawbufs;
 
-	for (std::map<v3s16, MapBlock*>::iterator i = m_drawlist.begin();
+	for (auto &i : m_drawlist) {
-			i != m_drawlist.end(); ++i) {
+		MapBlock *block = i.second;
-		MapBlock *block = i->second;
 
 		// If the mesh of the block happened to get deleted, ignore it
 		if (!block->mesh)
@@ -507,12 +488,9 @@ void ClientMap::renderMap(video::IVideoDriver* driver, s32 pass)
 	if (blocks_drawn != 0)
 		g_profiler->avg(prefix + "empty blocks (frac)",
 			(float)blocks_without_stuff / blocks_drawn);
-
-	/*infostream<<"renderMap(): is_transparent_pass="<<is_transparent_pass
-			<<", rendered "<<vertex_count<<" vertices."<<std::endl;*/
 }
 
-static bool getVisibleBrightness(Map *map, v3f p0, v3f dir, float step,
+static bool getVisibleBrightness(Map *map, const v3f &p0, v3f dir, float step,
 		float step_multiplier, float start_distance, float end_distance,
 		INodeDefManager *ndef, u32 daylight_factor, float sunlight_min_d,
 		int *result, bool *sunlight_seen)
@@ -546,19 +524,20 @@ static bool getVisibleBrightness(Map *map, v3f p0, v3f dir, float step,
 			sunlight_min_d = 0;
 		}
 	}
-	for(int i=0; distance < end_distance; i++){
+	for (int i=0; distance < end_distance; i++) {
 		pf += dir * step;
 		distance += step;
 		step *= step_multiplier;
 
 		v3s16 p = floatToInt(pf, BS);
 		MapNode n = map->getNodeNoEx(p);
-		if(allow_allowing_non_sunlight_propagates && i == 0 &&
+		if (allow_allowing_non_sunlight_propagates && i == 0 &&
 				ndef->get(n).param_type == CPT_LIGHT &&
-				!ndef->get(n).sunlight_propagates){
+				!ndef->get(n).sunlight_propagates) {
 			allow_non_sunlight_propagates = true;
 		}
-		if(ndef->get(n).param_type != CPT_LIGHT ||
+
+		if (ndef->get(n).param_type != CPT_LIGHT ||
 				(!ndef->get(n).sunlight_propagates &&
 					!allow_non_sunlight_propagates)){
 			nonlight_seen = true;
@@ -567,9 +546,9 @@ static bool getVisibleBrightness(Map *map, v3f p0, v3f dir, float step,
 				break;
 			continue;
 		}
-		if(distance >= sunlight_min_d && *sunlight_seen == false
+
-				&& nonlight_seen == false)
+		if (distance >= sunlight_min_d && !*sunlight_seen && !nonlight_seen)
-			if(n.getLight(LIGHTBANK_DAY, ndef) == LIGHT_SUN)
+			if (n.getLight(LIGHTBANK_DAY, ndef) == LIGHT_SUN)
 				*sunlight_seen = true;
 		noncount = 0;
 		brightness_sum += decode_light(n.getLightBlend(daylight_factor, ndef));
@@ -587,13 +566,13 @@ static bool getVisibleBrightness(Map *map, v3f p0, v3f dir, float step,
 int ClientMap::getBackgroundBrightness(float max_d, u32 daylight_factor,
 		int oldvalue, bool *sunlight_seen_result)
 {
-	const bool debugprint = false;
 	static v3f z_directions[50] = {
 		v3f(-100, 0, 0)
 	};
 	static f32 z_offsets[sizeof(z_directions)/sizeof(*z_directions)] = {
 		-1000,
 	};
+
 	if(z_directions[0].X < -99){
 		for(u32 i=0; i<sizeof(z_directions)/sizeof(*z_directions); i++){
 			// Assumes FOV of 72 and 16/9 aspect ratio
@@ -605,8 +584,7 @@ int ClientMap::getBackgroundBrightness(float max_d, u32 daylight_factor,
 			z_offsets[i] = 0.01 * myrand_range(0,100);
 		}
 	}
-	if(debugprint)
+
-		std::cerr<<"In goes "<<PP(m_camera_direction)<<", out comes ";
 	int sunlight_seen_count = 0;
 	float sunlight_min_d = max_d*0.8;
 	if(sunlight_min_d > 35*BS)
@@ -646,22 +624,12 @@ int ClientMap::getBackgroundBrightness(float max_d, u32 daylight_factor,
 	else if(num_values_to_use >= 7)
 		num_values_to_use -= num_values_to_use/3;
 	u32 first_value_i = (values.size() - num_values_to_use) / 2;
-	if(debugprint){
+
-		for(u32 i=0; i < first_value_i; i++)
+	for (u32 i=first_value_i; i < first_value_i + num_values_to_use; i++) {
-			std::cerr<<values[i]<<" ";
-		std::cerr<<"[";
-	}
-	for(u32 i=first_value_i; i < first_value_i+num_values_to_use; i++){
-		if(debugprint)
-			std::cerr<<values[i]<<" ";
 		brightness_sum += values[i];
 		brightness_count++;
 	}
-	if(debugprint){
+
-		std::cerr<<"]";
-		for(u32 i=first_value_i+num_values_to_use; i < values.size(); i++)
-			std::cerr<<values[i]<<" ";
-	}
 	int ret = 0;
 	if(brightness_count == 0){
 		MapNode n = getNodeNoEx(floatToInt(m_camera_position, BS));
@@ -671,18 +639,9 @@ int ClientMap::getBackgroundBrightness(float max_d, u32 daylight_factor,
 			ret = oldvalue;
 		}
 	} else {
-		/*float pre = (float)brightness_sum / (float)brightness_count;
-		float tmp = pre;
-		const float d = 0.2;
-		pre *= 1.0 + d*2;
-		pre -= tmp * d;
-		int preint = pre;
-		ret = MYMAX(0, MYMIN(255, preint));*/
 		ret = brightness_sum / brightness_count;
 	}
-	if(debugprint)
+
-		std::cerr<<"Result: "<<ret<<" sunlight_seen_count="
-				<<sunlight_seen_count<<std::endl;
 	*sunlight_seen_result = (sunlight_seen_count > 0);
 	return ret;
 }

src/clientmap.h

@@ -36,12 +36,6 @@ struct MapDrawControl
 	u32 wanted_max_blocks = 0;
 	// show a wire frame for debugging
 	bool show_wireframe = false;
-	// Number of blocks rendered is written here by the renderer
-	u32 blocks_drawn = 0;
-	// Number of blocks that would have been drawn in wanted_range
-	u32 blocks_would_have_drawn = 0;
-	// Distance to the farthest block drawn
-	float farthest_drawn = 0;
 };
 
 class Client;
@@ -62,7 +56,7 @@ public:
 			s32 id
 	);
 
-	~ClientMap();
+	virtual ~ClientMap() = default;
 
 	s32 mapType() const
 	{
@@ -74,7 +68,7 @@ public:
 		ISceneNode::drop();
 	}
 
-	void updateCamera(v3f pos, v3f dir, f32 fov, v3s16 offset)
+	void updateCamera(const v3f &pos, const v3f &dir, f32 fov, const v3s16 &offset)
 	{
 		m_camera_position = pos;
 		m_camera_direction = dir;
@@ -109,7 +103,7 @@ public:
 
 	void getBlocksInViewRange(v3s16 cam_pos_nodes,
 		v3s16 *p_blocks_min, v3s16 *p_blocks_max);
-	void updateDrawList(video::IVideoDriver* driver);
+	void updateDrawList();
 	void renderMap(video::IVideoDriver* driver, s32 pass);
 
 	int getBackgroundBrightness(float max_d, u32 daylight_factor,
@@ -130,7 +124,7 @@ private:
 
 	MapDrawControl &m_control;
 
-	v3f m_camera_position;
+	v3f m_camera_position = v3f(0,0,0);
 	v3f m_camera_direction = v3f(0,0,1);
 	f32 m_camera_fov = M_PI;
 	v3s16 m_camera_offset;

src/clientmedia.cpp

@@ -22,11 +22,9 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "client.h"
 #include "filecache.h"
 #include "filesys.h"
-#include "debug.h"
 #include "log.h"
 #include "porting.h"
 #include "settings.h"
-#include "network/networkprotocol.h"
 #include "util/hex.h"
 #include "util/serialize.h"
 #include "util/sha1.h"
@@ -52,12 +50,11 @@ ClientMediaDownloader::~ClientMediaDownloader()
 	if (m_httpfetch_caller != HTTPFETCH_DISCARD)
 		httpfetch_caller_free(m_httpfetch_caller);
 
-	for (std::map<std::string, FileStatus*>::iterator it = m_files.begin();
+	for (auto &file_it : m_files)
-			it != m_files.end(); ++it)
+		delete file_it.second;
-		delete it->second;
 
-	for (u32 i = 0; i < m_remotes.size(); ++i)
+	for (auto &remote : m_remotes)
-		delete m_remotes[i];
+		delete remote;
 }
 
 void ClientMediaDownloader::addFile(const std::string &name, const std::string &sha1)
@@ -88,7 +85,7 @@ void ClientMediaDownloader::addFile(const std::string &name, const std::string &
 		return;
 	}
 
-	FileStatus *filestatus = new FileStatus;
+	FileStatus *filestatus = new FileStatus();
 	filestatus->received = false;
 	filestatus->sha1 = sha1;
 	filestatus->current_remote = -1;
@@ -105,7 +102,7 @@ void ClientMediaDownloader::addRemoteServer(const std::string &baseurl)
 		infostream << "Client: Adding remote server \""
 			<< baseurl << "\" for media download" << std::endl;
 
-		RemoteServerStatus *remote = new RemoteServerStatus;
+		RemoteServerStatus *remote = new RemoteServerStatus();
 		remote->baseurl = baseurl;
 		remote->active_count = 0;
 		remote->request_by_filename = false;
@@ -166,11 +163,9 @@ void ClientMediaDownloader::initialStep(Client *client)
 {
 	// Check media cache
 	m_uncached_count = m_files.size();
-	for (std::map<std::string, FileStatus*>::iterator
+	for (auto &file_it : m_files) {
-			it = m_files.begin();
+		std::string name = file_it.first;
-			it != m_files.end(); ++it) {
+		FileStatus *filestatus = file_it.second;
-		std::string name = it->first;
-		FileStatus *filestatus = it->second;
 		const std::string &sha1 = filestatus->sha1;
 
 		std::ostringstream tmp_os(std::ios_base::binary);
@@ -258,7 +253,7 @@ void ClientMediaDownloader::initialStep(Client *client)
 			fetch_request.timeout = m_httpfetch_timeout;
 			fetch_request.connect_timeout = m_httpfetch_timeout;
 			fetch_request.post_data = required_hash_set;
-			fetch_request.extra_headers.push_back(
+			fetch_request.extra_headers.emplace_back(
 				"Content-Type: application/octet-stream");
 			httpfetch_async(fetch_request);
 
@@ -379,30 +374,30 @@ s32 ClientMediaDownloader::selectRemoteServer(FileStatus *filestatus)
 
 	if (filestatus->available_remotes.empty())
 		return -1;
-	else {
-		// Of all servers that claim to provide the file (and haven't
-		// been unsuccessfully tried before), find the one with the
-		// smallest number of currently active transfers
 
-		s32 best = 0;
+	// Of all servers that claim to provide the file (and haven't
-		s32 best_remote_id = filestatus->available_remotes[best];
+	// been unsuccessfully tried before), find the one with the
-		s32 best_active_count = m_remotes[best_remote_id]->active_count;
+	// smallest number of currently active transfers
 
-		for (u32 i = 1; i < filestatus->available_remotes.size(); ++i) {
+	s32 best = 0;
-			s32 remote_id = filestatus->available_remotes[i];
+	s32 best_remote_id = filestatus->available_remotes[best];
-			s32 active_count = m_remotes[remote_id]->active_count;
+	s32 best_active_count = m_remotes[best_remote_id]->active_count;
-			if (active_count < best_active_count) {
+
-				best = i;
+	for (u32 i = 1; i < filestatus->available_remotes.size(); ++i) {
-				best_remote_id = remote_id;
+		s32 remote_id = filestatus->available_remotes[i];
-				best_active_count = active_count;
+		s32 active_count = m_remotes[remote_id]->active_count;
-			}
+		if (active_count < best_active_count) {
+			best = i;
+			best_remote_id = remote_id;
+			best_active_count = active_count;
 		}
-
-		filestatus->available_remotes.erase(
-				filestatus->available_remotes.begin() + best);
-
-		return best_remote_id;
 	}
+
+	filestatus->available_remotes.erase(
+			filestatus->available_remotes.begin() + best);
+
+	return best_remote_id;
+
 }
 
 void ClientMediaDownloader::startRemoteMediaTransfers()
@@ -480,11 +475,9 @@ void ClientMediaDownloader::startConventionalTransfers(Client *client)
 		// Some media files have not been received yet, use the
 		// conventional slow method (minetest protocol) to get them
 		std::vector<std::string> file_requests;
-		for (std::map<std::string, FileStatus*>::iterator
+		for (auto &file : m_files) {
-				it = m_files.begin();
+			if (!file.second->received)
-				it != m_files.end(); ++it) {
+				file_requests.push_back(file.first);
-			if (!it->second->received)
-				file_requests.push_back(it->first);
 		}
 		assert((s32) file_requests.size() ==
 				m_uncached_count - m_uncached_received_count);

src/clientmedia.h

@@ -42,10 +42,10 @@ public:
 
 	float getProgress() const {
 		if (m_uncached_count >= 1)
-			return 1.0 * m_uncached_received_count /
+			return 1.0f * m_uncached_received_count /
 				m_uncached_count;
-		else
+
-			return 0.0;
+		return 0.0f;
 	}
 
 	bool isStarted() const {

src/game.cpp

@@ -4217,7 +4217,7 @@ void Game::updateFrame(ProfilerGraph *graph, RunStats *stats, f32 dtime,
 			|| runData.update_draw_list_last_cam_dir.getDistanceFrom(camera_direction) > 0.2
 			|| m_camera_offset_changed) {
 		runData.update_draw_list_timer = 0;
-		client->getEnv().getClientMap().updateDrawList(driver);
+		client->getEnv().getClientMap().updateDrawList();
 		runData.update_draw_list_last_cam_dir = camera_direction;
 	}