widelands-dev team mailing list archive

[Notabilis <notabilis27@xxxxxx>]

Notabilis has proposed merging lp:~widelands-dev/widelands/net-user-type into lp:widelands.

Commit message:
Cleaner transmission of client status

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/net-user-type/+merge/343754

Some small changes regarding transmitted client status and IRC clients.

- That a metaserver client is an IRC user is now transmitted as permission instead of as build-id. Avoids the theoretical problem that a Widelands client could have a build-id of "IRC".

- No longer block private messages to IRC users client side. This is now blocked on the metaserver.

- No longer sending (not existing) "points" of a user when transmitting the client list. If someone has an objection against removing these "points" (e.g., there are plans to use them), please say so.

- cleaning up code for "messages beeps" when receiving a non-system message.


Note: Don't merge/deploy this, I should do that myself since the protocol version has not been changed yet.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/net-user-type into lp:widelands.

=== modified file 'src/network/internet_gaming.cc'
--- src/network/internet_gaming.cc	2018-04-07 16:59:00 +0000
+++ src/network/internet_gaming.cc	2018-04-21 15:19:34 +0000
@@ -606,8 +606,7 @@
 				inc.build_id = packet.string();
 				inc.game = packet.string();
 				inc.type = packet.string();
-				inc.points = packet.string();
-				if (inc.build_id == "IRC") {
+				if (inc.type == INTERNET_CLIENT_IRC) {
 					irc.push_back(inc);
 					// No "join" or "left" messages for IRC users
 					continue;
@@ -631,7 +630,7 @@
 			clientlist_.insert(clientlist_.end(), irc.begin(), irc.end());
 
 			for (InternetClient& client : old) {
-				if (client.name.size() && client.build_id != "IRC") {
+				if (client.name.size() && client.type != INTERNET_CLIENT_IRC) {
 					format_and_add_chat(
 					   "", "", true, (boost::format(_("%s left the lobby")) % client.name).str());
 				}
@@ -867,17 +866,9 @@
 			   _("Message could not be sent: Was this supposed to be a private message?"));
 			return;
 		}
-		std::string recipient = msg.substr(1, space - 1);
-		for (const InternetClient& client : clientlist_) {
-			if (recipient == client.name && client.build_id == "IRC") {
-				format_and_add_chat(
-				   "", "", true, _("Private messages to IRC users are not supported."));
-				return;
-			}
-		}
 
 		s.string(trimmed);    // message
-		s.string(recipient);  // recipient
+		s.string(msg.substr(1, space - 1));  // recipient
 
 		format_and_add_chat(clientname_, msg.substr(1, space - 1), false, msg.substr(space + 1));
 

=== modified file 'src/network/internet_gaming.h'
--- src/network/internet_gaming.h	2018-04-07 16:59:00 +0000
+++ src/network/internet_gaming.h	2018-04-21 15:19:34 +0000
@@ -37,7 +37,6 @@
 	std::string build_id;
 	std::string game;
 	std::string type;
-	std::string points;  // Currently unused
 };
 
 /// A simple network game struct

=== modified file 'src/network/internet_gaming_protocol.h'
--- src/network/internet_gaming_protocol.h	2018-04-07 16:59:00 +0000
+++ src/network/internet_gaming_protocol.h	2018-04-21 15:19:34 +0000
@@ -71,7 +71,7 @@
 static const std::string INTERNET_CLIENT_UNREGISTERED = "UNREGISTERED";
 static const std::string INTERNET_CLIENT_REGISTERED = "REGISTERED";
 static const std::string INTERNET_CLIENT_SUPERUSER = "SUPERUSER";
-static const std::string INTERNET_CLIENT_BOT = "BOT";
+static const std::string INTERNET_CLIENT_IRC = "IRC";
 
 /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * COMMUNICATION PROTOCOL BETWEEN CLIENT AND METASERVER                    *
@@ -322,10 +322,8 @@
 
 /**
  * Sent by the metaserver to inform the client, that the list of clients was changed. No payload is
- * sent,
- * as e.g. clients in a game are not really interested about other clients and we want to keep
- * traffic
- * as low as possible.
+ * sent, as e.g. clients in a game are not really interested about other clients and we want to
+ * keep traffic as low as possible.
  *
  * To get the new list of clients, the client must send \ref IGPCMD_CLIENT
  */
@@ -338,9 +336,8 @@
  * \li string:    Number of client packages and for uint8_t i = 0; i < num; ++i {:
  * \li string:    Name of the client
  * \li string:    Widelands version
- * \li string:    Game the player is connected to, else empty.
+ * \li string:    Game the player is connected to, else empty
  * \li string:    Clients rights (see client rights section above)
- * \li string:    Points of the client
  * }
  */
 static const std::string IGPCMD_CLIENTS = "CLIENTS";

=== modified file 'src/ui_fsmenu/internet_lobby.cc'
--- src/ui_fsmenu/internet_lobby.cc	2018-04-07 16:59:00 +0000
+++ src/ui_fsmenu/internet_lobby.cc	2018-04-21 15:19:34 +0000
@@ -235,8 +235,9 @@
 		return 1;
 	if (type == INTERNET_CLIENT_SUPERUSER)
 		return 2;
-	if (type == INTERNET_CLIENT_BOT)
-		return 3;
+	// 3 was INTERNET_CLIENT_BOT which is not used
+	if (type == INTERNET_CLIENT_IRC)
+		return 4;
 	// if (type == INTERNET_CLIENT_UNREGISTERED)
 	return 0;
 }
@@ -262,11 +263,6 @@
 			er.set_string(2, client.build_id);
 			er.set_string(3, client.game);
 
-			if (client.build_id == "IRC") {
-				// No icon for IRC users
-				continue;
-			}
-
 			const Image* pic;
 			switch (convert_clienttype(client.type)) {
 			case 0:  // UNREGISTERED
@@ -278,11 +274,13 @@
 				er.set_picture(0, pic);
 				break;
 			case 2:  // SUPERUSER
-			case 3:  // BOT
 				pic = g_gr->images().get("images/wui/overlays/roadb_green.png");
 				er.set_color(RGBColor(0, 255, 0));
 				er.set_picture(0, pic);
 				break;
+			case 4:  // IRC
+				// No icon for IRC users
+				continue;
 			default:
 				continue;
 			}
@@ -301,11 +299,6 @@
 	if (clientsonline_list_.has_selection()) {
 		UI::Table<const InternetClient* const>::EntryRecord& er = clientsonline_list_.get_record(i);
 
-		if (er.get_string(2) == "IRC") {
-			// No PM to IRC users
-			return;
-		}
-
 		std::string temp("@");
 		temp += er.get_string(1);
 		std::string text(chat.get_edit_text());

=== modified file 'src/wui/gamechatpanel.cc'
--- src/wui/gamechatpanel.cc	2018-04-07 16:59:00 +0000
+++ src/wui/gamechatpanel.cc	2018-04-21 15:19:34 +0000
@@ -45,7 +45,7 @@
              g_gr->images().get("images/ui_basic/but1.png"),
              UI::MultilineTextarea::ScrollMode::kScrollLogForced),
      editbox(this, 0, h - 20, w, 20, 2),
-     chat_message_counter(std::numeric_limits<uint32_t>::max()) {
+     chat_message_counter(0) {
 	editbox.ok.connect(boost::bind(&GameChatPanel::key_enter, this));
 	editbox.cancel.connect(boost::bind(&GameChatPanel::key_escape, this));
 	editbox.activate_history(true);
@@ -74,19 +74,16 @@
 	chatbox.set_text(str);
 
 	// If there are new messages, play a sound
-	if (0 < msgs.size() && msgs.size() != chat_message_counter) {
-		// computer generated ones are ignored
-		// Note: if many messages arrive simultaneously,
-		// the latest is a system message and some others
-		// are not, then this act wrong!
-		if (!msgs.back().sender.empty() && !chat_.sound_off()) {
-			// The latest message is not a system message
-			if (std::string::npos == msgs.back().sender.find("(IRC)") &&
-			    chat_message_counter < msgs.size()) {
-				// The latest message was not relayed from IRC.
-				// The above built-in string constant should match
-				// that of the IRC bridge.
+	if (chat_message_counter < msgs.size()) {
+		if (!chat_.sound_off()) {
+			for (size_t i = chat_message_counter; i < msgs.size(); ++i) {
+				if (msgs[i].sender.empty()) {
+					// System message. Don't play a sound
+					continue;
+				}
+				// Got a message that is no system message. Beep
 				play_new_chat_message();
+				break;
 			}
 		}
 		chat_message_counter = msgs.size();

=== modified file 'src/wui/gamechatpanel.h'
--- src/wui/gamechatpanel.h	2018-04-07 16:59:00 +0000
+++ src/wui/gamechatpanel.h	2018-04-21 15:19:34 +0000
@@ -58,7 +58,7 @@
 	ChatProvider& chat_;
 	UI::MultilineTextarea chatbox;
 	UI::EditBox editbox;
-	uint32_t chat_message_counter;
+	size_t chat_message_counter;
 	std::unique_ptr<Notifications::Subscriber<ChatMessage>> chat_message_subscriber_;
 };