[no squash] General fixes & improvements (season 2)

src/client/clientlauncher.cpp

@@ -108,13 +108,6 @@ bool ClientLauncher::run(GameStartData &start_data, const Settings &cmd_args)
 		return false;
 	}
 
-	// Speed tests (done after irrlicht is loaded to get timer)
-	if (cmd_args.getFlag("speedtests")) {
-		dstream << "Running speed tests" << std::endl;
-		speed_tests();
-		return true;
-	}
-
 	if (m_rendering_engine->get_video_driver() == NULL) {
 		errorstream << "Could not initialize video driver." << std::endl;
 		return false;
@@ -387,7 +380,7 @@ bool ClientLauncher::launch_game(std::string &error_message,
 	if (cmd_args.exists("password-file")) {
 		std::ifstream passfile(cmd_args.get("password-file"));
 		if (passfile.good()) {
-			getline(passfile, start_data.password);
+			std::getline(passfile, start_data.password);
 		} else {
 			error_message = gettext("Provided password file "
 					"failed to open: ")
@@ -444,8 +437,6 @@ bool ClientLauncher::launch_game(std::string &error_message,
 
 		int world_index = menudata.selected_world;
 		if (world_index >= 0 && world_index < (int)worldspecs.size()) {
-			g_settings->set("selected_world_path",
-					worldspecs[world_index].path);
 			start_data.world_spec = worldspecs[world_index];
 		}
 
@@ -517,15 +508,7 @@ bool ClientLauncher::launch_game(std::string &error_message,
 			return false;
 		}
 
-		if (porting::signal_handler_killstatus())
-			return true;
-
-		if (!start_data.game_spec.isValid()) {
-			error_message = gettext("Invalid gamespec.");
-			error_message += " (world.gameid=" + worldspec.gameid + ")";
-			errorstream << error_message << std::endl;
-			return false;
-		}
+		return true;
 	}
 
 	start_data.world_path = start_data.world_spec.path;
@@ -563,104 +546,3 @@ void ClientLauncher::main_menu(MainMenuData *menudata)
 	/* leave scene manager in a clean state */
 	m_rendering_engine->get_scene_manager()->clear();
 }
-
-void ClientLauncher::speed_tests()
-{
-	// volatile to avoid some potential compiler optimisations
-	volatile static s16 temp16;
-	volatile static f32 tempf;
-	// Silence compiler warning
-	(void)temp16;
-	static v3f tempv3f1;
-	static v3f tempv3f2;
-	static std::string tempstring;
-	static std::string tempstring2;
-
-	tempv3f1 = v3f();
-	tempv3f2 = v3f();
-	tempstring.clear();
-	tempstring2.clear();
-
-	{
-		infostream << "The following test should take around 20ms." << std::endl;
-		TimeTaker timer("Testing std::string speed");
-		const u32 jj = 10000;
-		for (u32 j = 0; j < jj; j++) {
-			tempstring.clear();
-			tempstring2.clear();
-			const u32 ii = 10;
-			for (u32 i = 0; i < ii; i++) {
-				tempstring2 += "asd";
-			}
-			for (u32 i = 0; i < ii+1; i++) {
-				tempstring += "asd";
-				if (tempstring == tempstring2)
-					break;
-			}
-		}
-	}
-
-	infostream << "All of the following tests should take around 100ms each."
-	           << std::endl;
-
-	{
-		TimeTaker timer("Testing floating-point conversion speed");
-		tempf = 0.001;
-		for (u32 i = 0; i < 4000000; i++) {
-			temp16 += tempf;
-			tempf += 0.001;
-		}
-	}
-
-	{
-		TimeTaker timer("Testing floating-point vector speed");
-
-		tempv3f1 = v3f(1, 2, 3);
-		tempv3f2 = v3f(4, 5, 6);
-		for (u32 i = 0; i < 10000000; i++) {
-			tempf += tempv3f1.dotProduct(tempv3f2);
-			tempv3f2 += v3f(7, 8, 9);
-		}
-	}
-
-	{
-		TimeTaker timer("Testing std::map speed");
-
-		std::map<v2s16, f32> map1;
-		tempf = -324;
-		const s16 ii = 300;
-		for (s16 y = 0; y < ii; y++) {
-			for (s16 x = 0; x < ii; x++) {
-				map1[v2s16(x, y)] =  tempf;
-				tempf += 1;
-			}
-		}
-		for (s16 y = ii - 1; y >= 0; y--) {
-			for (s16 x = 0; x < ii; x++) {
-				tempf = map1[v2s16(x, y)];
-			}
-		}
-	}
-
-	{
-		infostream << "Around 5000/ms should do well here." << std::endl;
-		TimeTaker timer("Testing mutex speed");
-
-		std::mutex m;
-		u32 n = 0;
-		u32 i = 0;
-		do {
-			n += 10000;
-			for (; i < n; i++) {
-				m.lock();
-				m.unlock();
-			}
-		}
-		// Do at least 10ms
-		while(timer.getTimerTime() < 10);
-
-		u32 dtime = timer.stop();
-		u32 per_ms = n / dtime;
-		infostream << "Done. " << dtime << "ms, " << per_ms << "/ms" << std::endl;
-	}
-}

src/client/clientlauncher.h

@@ -44,8 +44,6 @@ class ClientLauncher
 
 	void main_menu(MainMenuData *menudata);
 
-	void speed_tests();
-
 	bool skip_main_menu = false;
 	bool random_input = false;
 	RenderingEngine *m_rendering_engine = nullptr;

src/client/game.cpp

@@ -1750,7 +1750,7 @@ bool Game::getServerContent(bool *aborted)
 		// End condition
 		if (client->mediaReceived() && client->itemdefReceived() &&
 				client->nodedefReceived()) {
-			break;
+			return true;
 		}
 
 		// Error conditions
@@ -1809,7 +1809,9 @@ bool Game::getServerContent(bool *aborted)
 		}
 	}
 
-	return true;
+	*aborted = true;
+	infostream << "Connect aborted [device]" << std::endl;
+	return false;
 }
 
 

src/clientiface.h

@@ -284,10 +284,10 @@ class RemoteClient
 		return m_blocks_sent.find(p) != m_blocks_sent.end();
 	}
 
-	// Increments timeouts and removes timed-out blocks from list
-	// NOTE: This doesn't fix the server-not-sending-block bug
-	//       because it is related to emerging, not sending.
-	//void RunSendingTimeouts(float dtime, float timeout);
+	bool markMediaSent(const std::string &name) {
+		auto insert_result = m_media_sent.emplace(name);
+		return insert_result.second; // true = was inserted
+	}
 
 	void PrintInfo(std::ostream &o)
 	{
@@ -310,7 +310,7 @@ class RemoteClient
 
 	ClientState getState() const { return m_state; }
 
-	std::string getName() const { return m_name; }
+	const std::string &getName() const { return m_name; }
 
 	void setName(const std::string &name) { m_name = name; }
 
@@ -394,6 +394,12 @@ class RemoteClient
 	const s16 m_max_gen_distance;
 	const bool m_occ_cull;
 
+	/*
+		Set of media files the client has already requested
+		We won't send the same file twice to avoid bandwidth consumption attacks.
+	*/
+	std::unordered_set<std::string> m_media_sent;
+
 	/*
 		Blocks that are currently on the line.
 		This is used for throttling the sending of blocks.

src/main.cpp

@@ -379,8 +379,6 @@ static void set_allowed_options(OptionList *allowed_options)
 	allowed_options->insert(std::make_pair("recompress", ValueSpec(VALUETYPE_FLAG,
 			_("Recompress the blocks of the given map database."))));
 #ifndef SERVER
-	allowed_options->insert(std::make_pair("speedtests", ValueSpec(VALUETYPE_FLAG,
-			_("Run speed tests"))));
 	allowed_options->insert(std::make_pair("address", ValueSpec(VALUETYPE_STRING,
 			_("Address to connect to. ('' = local game)"))));
 	allowed_options->insert(std::make_pair("random-input", ValueSpec(VALUETYPE_FLAG,

src/mapblock.cpp

@@ -40,13 +40,11 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "util/basic_macros.h"
 
 static const char *modified_reason_strings[] = {
-	"initial",
-	"reallocate",
+	"reallocate or initial",
 	"setIsUnderground",
 	"setLightingExpired",
 	"setGenerated",
 	"setNode",
-	"setNodeNoCheck",
 	"setTimestamp",
 	"NodeMetaRef::reportMetadataChange",
 	"clearAllObjects",
@@ -73,6 +71,7 @@ MapBlock::MapBlock(v3s16 pos, IGameDef *gamedef):
 		m_gamedef(gamedef)
 {
 	reallocate();
+	assert(m_modified > MOD_STATE_CLEAN);
 }
 
 MapBlock::~MapBlock()

src/mapblock.h

@@ -43,27 +43,27 @@ class VoxelManipulator;
 //// MapBlock modified reason flags
 ////
 
-#define MOD_REASON_INITIAL                   (1 << 0)
-#define MOD_REASON_REALLOCATE                (1 << 1)
-#define MOD_REASON_SET_IS_UNDERGROUND        (1 << 2)
-#define MOD_REASON_SET_LIGHTING_COMPLETE     (1 << 3)
-#define MOD_REASON_SET_GENERATED             (1 << 4)
-#define MOD_REASON_SET_NODE                  (1 << 5)
-#define MOD_REASON_SET_NODE_NO_CHECK         (1 << 6)
-#define MOD_REASON_SET_TIMESTAMP             (1 << 7)
-#define MOD_REASON_REPORT_META_CHANGE        (1 << 8)
-#define MOD_REASON_CLEAR_ALL_OBJECTS         (1 << 9)
-#define MOD_REASON_BLOCK_EXPIRED             (1 << 10)
-#define MOD_REASON_ADD_ACTIVE_OBJECT_RAW     (1 << 11)
-#define MOD_REASON_REMOVE_OBJECTS_REMOVE     (1 << 12)
-#define MOD_REASON_REMOVE_OBJECTS_DEACTIVATE (1 << 13)
-#define MOD_REASON_TOO_MANY_OBJECTS          (1 << 14)
-#define MOD_REASON_STATIC_DATA_ADDED         (1 << 15)
-#define MOD_REASON_STATIC_DATA_REMOVED       (1 << 16)
-#define MOD_REASON_STATIC_DATA_CHANGED       (1 << 17)
-#define MOD_REASON_EXPIRE_DAYNIGHTDIFF       (1 << 18)
-#define MOD_REASON_VMANIP                    (1 << 19)
-#define MOD_REASON_UNKNOWN                   (1 << 20)
+enum ModReason : u32 {
+	MOD_REASON_REALLOCATE                 = 1 << 0,
+	MOD_REASON_SET_IS_UNDERGROUND         = 1 << 1,
+	MOD_REASON_SET_LIGHTING_COMPLETE      = 1 << 2,
+	MOD_REASON_SET_GENERATED              = 1 << 3,
+	MOD_REASON_SET_NODE                   = 1 << 4,
+	MOD_REASON_SET_TIMESTAMP              = 1 << 5,
+	MOD_REASON_REPORT_META_CHANGE         = 1 << 6,
+	MOD_REASON_CLEAR_ALL_OBJECTS          = 1 << 7,
+	MOD_REASON_BLOCK_EXPIRED              = 1 << 8,
+	MOD_REASON_ADD_ACTIVE_OBJECT_RAW      = 1 << 9,
+	MOD_REASON_REMOVE_OBJECTS_REMOVE      = 1 << 10,
+	MOD_REASON_REMOVE_OBJECTS_DEACTIVATE  = 1 << 11,
+	MOD_REASON_TOO_MANY_OBJECTS           = 1 << 12,
+	MOD_REASON_STATIC_DATA_ADDED          = 1 << 13,
+	MOD_REASON_STATIC_DATA_REMOVED        = 1 << 14,
+	MOD_REASON_STATIC_DATA_CHANGED        = 1 << 15,
+	MOD_REASON_EXPIRE_DAYNIGHTDIFF        = 1 << 16,
+	MOD_REASON_VMANIP                     = 1 << 17,
+	MOD_REASON_UNKNOWN                    = 1 << 18,
+};
 
 ////
 //// MapBlock itself
@@ -155,7 +155,7 @@ class MapBlock
 	{
 		if (newflags != m_lighting_complete) {
 			m_lighting_complete = newflags;
-			raiseModified(MOD_STATE_WRITE_NEEDED, MOD_REASON_SET_LIGHTING_COMPLETE);
+			raiseModified(MOD_STATE_WRITE_AT_UNLOAD, MOD_REASON_SET_LIGHTING_COMPLETE);
 		}
 	}
 
@@ -296,7 +296,7 @@ class MapBlock
 	inline void setNodeNoCheck(s16 x, s16 y, s16 z, MapNode n)
 	{
 		data[z * zstride + y * ystride + x] = n;
-		raiseModified(MOD_STATE_WRITE_NEEDED, MOD_REASON_SET_NODE_NO_CHECK);
+		raiseModified(MOD_STATE_WRITE_NEEDED, MOD_REASON_SET_NODE);
 	}
 
 	inline void setNodeNoCheck(v3s16 p, MapNode n)
@@ -525,8 +525,8 @@ class MapBlock
 		  block has been modified from the one on disk.
 		- On the client, this is used for nothing.
 	*/
-	u16 m_modified = MOD_STATE_WRITE_NEEDED;
-	u32 m_modified_reason = MOD_REASON_INITIAL;
+	u16 m_modified = MOD_STATE_CLEAN;
+	u32 m_modified_reason = 0;
 
 	/*
 		When block is removed from active blocks, this is set to gametime.

src/network/serverpackethandler.cpp

@@ -356,24 +356,24 @@ void Server::handleCommand_Init2(NetworkPacket* pkt)
 
 void Server::handleCommand_RequestMedia(NetworkPacket* pkt)
 {
-	std::vector<std::string> tosend;
+	std::unordered_set<std::string> tosend;
 	u16 numfiles;
 
 	*pkt >> numfiles;
 
 	session_t peer_id = pkt->getPeerId();
-	infostream << "Sending " << numfiles << " files to " <<
-		getPlayerName(peer_id) << std::endl;
-	verbosestream << "TOSERVER_REQUEST_MEDIA: requested file(s)" << std::endl;
+	verbosestream << "Client " << getPlayerName(peer_id)
+		<< " requested media file(s):\n";
 
 	for (u16 i = 0; i < numfiles; i++) {
 		std::string name;
 
 		*pkt >> name;
 
-		tosend.emplace_back(name);
-		verbosestream << "  " << name << std::endl;
+		tosend.emplace(name);
+		verbosestream << "  " << name << "\n";
 	}
+	verbosestream << std::flush;
 
 	sendRequestedMedia(peer_id, tosend);
 }