Fix serverlist code style, const-correctness, and types

src/server.cpp

@@ -676,9 +676,9 @@ void Server::AsyncRunStep(bool initial_step)
 	{
 		float &counter = m_masterserver_timer;
 		if(!isSingleplayer() && (!counter || counter >= 300.0) &&
-				g_settings->getBool("server_announce") == true)
+				g_settings->getBool("server_announce"))
 		{
-			ServerList::sendAnnounce(!counter ? "start" : "update",
+			ServerList::sendAnnounce(counter ? "update" : "start",
 					m_clients.getPlayerNames(),
 					m_uptime.get(),
 					m_env->getGameTime(),

src/serverlist.cpp

@@ -35,28 +35,25 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 namespace ServerList
 {
+
 std::string getFilePath()
 {
 	std::string serverlist_file = g_settings->get("serverlist_file");
 
-	std::string dir_path = std::string("client") + DIR_DELIM
+	std::string dir_path = "client" DIR_DELIM "serverlist" DIR_DELIM;
-		+ "serverlist" + DIR_DELIM;
+	fs::CreateDir(porting::path_user + DIR_DELIM  "client");
-	fs::CreateDir(porting::path_user + DIR_DELIM + "client");
 	fs::CreateDir(porting::path_user + DIR_DELIM + dir_path);
-	std::string rel_path = dir_path + serverlist_file;
+	return porting::path_user + DIR_DELIM + dir_path + serverlist_file;
-	std::string path = porting::path_user + DIR_DELIM + rel_path;
-	return path;
 }
 
+
 std::vector<ServerListSpec> getLocal()
 {
 	std::string path = ServerList::getFilePath();
 	std::string liststring;
-	if(fs::PathExists(path))
+	if (fs::PathExists(path)) {
-	{
 		std::ifstream istream(path.c_str());
-		if(istream.is_open())
+		if (istream.is_open()) {
-		{
 			std::ostringstream ostream;
 			ostream << istream.rdbuf();
 			liststring = ostream.str();
@@ -64,19 +61,19 @@ std::vector<ServerListSpec> getLocal()
 		}
 	}
 
-	return ServerList::deSerialize(liststring);
+	return deSerialize(liststring);
 }
 
 
 std::vector<ServerListSpec> getOnline()
 {
-	Json::Value root = fetchJsonValue((g_settings->get("serverlist_url")+"/list").c_str(), NULL);
+	Json::Value root = fetchJsonValue(
+			(g_settings->get("serverlist_url") + "/list").c_str(), NULL);
 
 	std::vector<ServerListSpec> serverlist;
 
 	if (root.isArray()) {
-		for (unsigned int i = 0; i < root.size(); i++)
+		for (unsigned int i = 0; i < root.size(); i++) {
-		{
 			if (root[i].isObject()) {
 				serverlist.push_back(root[i]);
 			}
@@ -86,18 +83,18 @@ std::vector<ServerListSpec> getOnline()
 	return serverlist;
 }
 
-/*
+
-	Delete a server fromt he local favorites list
+// Delete a server from the local favorites list
-*/
+bool deleteEntry(const ServerListSpec &server)
-bool deleteEntry (ServerListSpec server)
 {
 	std::vector<ServerListSpec> serverlist = ServerList::getLocal();
-	for(unsigned i = 0; i < serverlist.size(); i++)
+	for (std::vector<ServerListSpec>::iterator it = serverlist.begin();
-	{
+			it != serverlist.end();) {
-		if  (serverlist[i]["address"] == server["address"]
+		if ((*it)["address"] == server["address"] &&
-		&&   serverlist[i]["port"]    == server["port"])
+				(*it)["port"] == server["port"]) {
-		{
+			it = serverlist.erase(it);
-			serverlist.erase(serverlist.begin() + i);
+		} else {
+			++it;
 		}
 	}
 
@@ -109,10 +106,8 @@ bool deleteEntry (ServerListSpec server)
 	return true;
 }
 
-/*
+// Insert a server to the local favorites list
-	Insert a server to the local favorites list
+bool insert(const ServerListSpec &server)
-*/
-bool insert (ServerListSpec server)
 {
 	// Remove duplicates
 	ServerList::deleteEntry(server);
@@ -125,79 +120,81 @@ bool insert (ServerListSpec server)
 	std::string path = ServerList::getFilePath();
 	std::ostringstream ss(std::ios_base::binary);
 	ss << ServerList::serialize(serverlist);
-	fs::safeWriteToFile(path, ss.str());
+	if (!fs::safeWriteToFile(path, ss.str()))
-
 		return false;
+
+	return true;
 }
 
-std::vector<ServerListSpec> deSerialize(std::string liststring)
+std::vector<ServerListSpec> deSerialize(const std::string &liststring)
 {
 	std::vector<ServerListSpec> serverlist;
 	std::istringstream stream(liststring);
 	std::string line, tmp;
-	while (std::getline(stream, line))
+	while (std::getline(stream, line)) {
-	{
 		std::transform(line.begin(), line.end(), line.begin(), ::toupper);
-		if (line == "[SERVER]")
+		if (line == "[SERVER]") {
-		{
+			ServerListSpec server;
-			ServerListSpec thisserver;
 			std::getline(stream, tmp);
-			thisserver["name"] = tmp;
+			server["name"] = tmp;
 			std::getline(stream, tmp);
-			thisserver["address"] = tmp;
+			server["address"] = tmp;
 			std::getline(stream, tmp);
-			thisserver["port"] = tmp;
+			server["port"] = tmp;
 			std::getline(stream, tmp);
-			thisserver["description"] = tmp;
+			server["description"] = tmp;
-			serverlist.push_back(thisserver);
+			serverlist.push_back(server);
 		}
 	}
 	return serverlist;
 }
 
-std::string serialize(std::vector<ServerListSpec> serverlist)
+const std::string serialize(const std::vector<ServerListSpec> &serverlist)
 {
 	std::string liststring;
-	for(std::vector<ServerListSpec>::iterator i = serverlist.begin(); i != serverlist.end(); i++)
+	for (std::vector<ServerListSpec>::const_iterator it = serverlist.begin();
-	{
+			it != serverlist.end();
+			it++) {
 		liststring += "[server]\n";
-		liststring += (*i)["name"].asString() + "\n";
+		liststring += (*it)["name"].asString() + '\n';
-		liststring += (*i)["address"].asString() + "\n";
+		liststring += (*it)["address"].asString() + '\n';
-		liststring += (*i)["port"].asString() + "\n";
+		liststring += (*it)["port"].asString() + '\n';
-		liststring += (*i)["description"].asString() + "\n";
+		liststring += (*it)["description"].asString() + '\n';
-		liststring += "\n";
+		liststring += '\n';
 	}
 	return liststring;
 }
 
-std::string serializeJson(std::vector<ServerListSpec> serverlist)
+const std::string serializeJson(const std::vector<ServerListSpec> &serverlist)
 {
 	Json::Value root;
 	Json::Value list(Json::arrayValue);
-	for(std::vector<ServerListSpec>::iterator i = serverlist.begin(); i != serverlist.end(); i++)
+	for (std::vector<ServerListSpec>::const_iterator it = serverlist.begin();
-	{
+			it != serverlist.end();
-		list.append(*i);
+			it++) {
+		list.append(*it);
 	}
 	root["list"] = list;
-	Json::StyledWriter writer;
+	Json::FastWriter writer;
 	return writer.write(root);
 }
 
 
 #if USE_CURL
-void sendAnnounce(std::string action,
+void sendAnnounce(const std::string &action,
 		const std::vector<std::string> &clients_names,
-		double uptime,
+		const double uptime,
-		u32 game_time,
+		const u32 game_time,
-		float lag,
+		const float lag,
-		std::string gameid,
+		const std::string &gameid,
-		std::vector<ModSpec> mods)
+		const std::vector<ModSpec> &mods)
 {
 	Json::Value server;
-	if (action.size())
 	server["action"] = action;
 	server["port"]    = g_settings->getU16("port");
+	if (g_settings->exists("server_address")) {
 		server["address"] = g_settings->get("server_address");
+	}
 	if (action != "delete") {
 		server["name"]         = g_settings->get("server_name");
 		server["description"]  = g_settings->get("server_description");
@@ -210,7 +207,7 @@ void sendAnnounce(std::string action,
 		server["uptime"]       = (int) uptime;
 		server["game_time"]    = game_time;
 		server["clients"]      = (int) clients_names.size();
-		server["clients_max"]  = g_settings->get("max_users");
+		server["clients_max"]  = g_settings->getU16("max_users");
 		server["clients_list"] = Json::Value(Json::arrayValue);
 		for (std::vector<std::string>::const_iterator it = clients_names.begin();
 				it != clients_names.end();
@@ -220,16 +217,17 @@ void sendAnnounce(std::string action,
 		if (gameid != "") server["gameid"] = gameid;
 	}
 
-	if (server["action"] == "start") {
+	if (action == "start") {
 		server["dedicated"]         = g_settings->getBool("server_dedicated");
 		server["rollback"]          = g_settings->getBool("enable_rollback_recording");
 		server["mapgen"]            = g_settings->get("mg_name");
 		server["privs"]             = g_settings->get("default_privs");
 		server["can_see_far_names"] = g_settings->getBool("unlimited_player_transfer_distance");
 		server["mods"]              = Json::Value(Json::arrayValue);
-		for (std::vector<ModSpec>::iterator m = mods.begin();
+		for (std::vector<ModSpec>::const_iterator it = mods.begin();
-				m != mods.end(); ++m) {
+				it != mods.end();
-			server["mods"].append(m->name);
+				++it) {
+			server["mods"].append(it->name);
 		}
 		actionstream << "Announcing to " << g_settings->get("serverlist_url") << std::endl;
 	} else {

src/serverlist.h

@@ -31,16 +31,18 @@ namespace ServerList
 {
 	std::vector<ServerListSpec> getLocal();
 	std::vector<ServerListSpec> getOnline();
-	bool deleteEntry(ServerListSpec server);
+	bool deleteEntry(const ServerListSpec &server);
-	bool insert(ServerListSpec server);
+	bool insert(const ServerListSpec &server);
-	std::vector<ServerListSpec> deSerialize(std::string liststring);
+	std::vector<ServerListSpec> deSerialize(const std::string &liststring);
-	std::string serialize(std::vector<ServerListSpec>);
+	const std::string serialize(const std::vector<ServerListSpec> &serverlist);
-	std::vector<ServerListSpec> deSerializeJson(std::string liststring);
+	std::vector<ServerListSpec> deSerializeJson(const std::string &liststring);
-	std::string serializeJson(std::vector<ServerListSpec>);
+	const std::string serializeJson(const std::vector<ServerListSpec> &serverlist);
 	#if USE_CURL
-	void sendAnnounce(std::string action = "", const std::vector<std::string> & clients_names = std::vector<std::string>(), 
+	void sendAnnounce(const std::string &action,
-				double uptime = 0, u32 game_time = 0, float lag = 0, std::string gameid = "", 
+			const std::vector<std::string> &clients_names = std::vector<std::string>(),
-				std::vector<ModSpec> mods = std::vector<ModSpec>());
+			const double uptime = 0, const u32 game_time = 0,
+			const float lag = 0, const std::string &gameid = "",
+			const std::vector<ModSpec> &mods = std::vector<ModSpec>());
 	#endif
 } // ServerList namespace