Merge lp:~widelands-dev/widelands/irc_users into lp:widelands

[Notabilis]

Sorting IRC users behind the players in the lobby.

When the next version of the metaserver is deployed [1], IRC users will also be listed in the lobby as players with a build-id of "IRC". Currently, this leads to a long list of IRC users followed by a few entries of players.

With this branch, IRC users are listed after the online players. Additionally, no "user status" (unregistered, ...) is displayed for them.

[1] https://github.com/widelands/widelands_metaserver/pull/19

[Notabilis (notabilis27)]

Additionally, I wanted to color the lines of the IRC users in gray. However, this did not work.
- A <rt><font ...> directive was printed as text.
- The existing Table::EntryRecord::set_color() method seems to be broken. At least, it does nothing and I can't find code that reads the set color.
I wouldn't mind leaving it as it is, though. Without the status symbols the lines stand out enough in my opinion.

When logging in to the metaserver, the user is greeted by the (obsolete) message "For hosting a game, ...". Maybe add a similar message "Users on IRC are unlikely to react to chat messages"? I think in the past we had some users on the forum which complained about not getting any answers.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3041. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/326955621.
Appveyor build 2849. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_irc_users-2849.

[GunChleoc (gunchleoc)]

I think a note is a good idea.

Don't worry about the colors - I already have a WIP branch where I have implemented fullscreen switching, so it would make more sense to implement it there to avoid merge conflicts, or wait until that branch has been merged. We could also open a bug for it.

Code LGTM, not tested.

review: Approve

[Notabilis (notabilis27)]

When joining the lobby a note is printed now. Additionally, trying to send a private message to IRC users is aborted with a messages, since it does not work and there would never be a response.

Thanks for the explanation regarding the colors. When no-one complains we can leave it as it is right now, otherwise we can still change it later.

The related metaserver branch has been deployed so this can be tested now. As far as I am concerned, this branch can go in.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3044. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/328332356.
Appveyor build 2852. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_irc_users-2852.

[GunChleoc (gunchleoc)]

Code still LGTM, 1 nit.

[GunChleoc (gunchleoc)]

I have now done some testing, looks good.

Merge any time you're ready after fixing the nit :)

[Notabilis (notabilis27)]

Was already done. ;)
Thanks!

@bunnybot merge

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3065. State: canceled. Details: https://travis-ci.org/widelands/widelands/builds/329644718.
Appveyor build 2872. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_irc_users-2872.

[bunnybot (widelandsofficial)]

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 3065. State: canceled. Details: https://travis-ci.org/widelands/widelands/builds/329644718.

[GunChleoc (gunchleoc)]

Can you please merge trunk again to make Travis go green?

[SirVer (sirver)]

I did the merge of master.

@bunnybot merge

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3107. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/334452896.
Appveyor build 2914. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_irc_users-2914.

=== modified file 'src/network/internet_gaming.cc'
--- src/network/internet_gaming.cc	2017-11-30 20:45:41 +0000
+++ src/network/internet_gaming.cc	2018-01-28 19:08:17 +0000
@@ -170,8 +170,7 @@
 			if (state_ == LOBBY) {
 				if (!should_relogin) {
 					format_and_add_chat(
-					   "", "", true, _("For hosting a game, please take a look at the notes at:"));
-					format_and_add_chat("", "", true, "http://wl.widelands.org/wiki/InternetGaming");
+					   "", "", true, _("Users marked with IRC will possibly not react to messages."));
 				}
 
 				// Try to establish a second connection to tell the metaserver about our IPv4 address
@@ -537,21 +536,29 @@
 			// Client received the new list of clients
 			uint8_t number = boost::lexical_cast<int>(packet.string()) & 0xff;
 			std::vector<InternetClient> old = clientlist_;
+			// Push IRC users to a second list and add them to the back
+			std::vector<InternetClient> irc;
 			clientlist_.clear();
 			log("InternetGaming: Received a client list update with %u items.\n", number);
+			InternetClient inc;
 			for (uint8_t i = 0; i < number; ++i) {
-				InternetClient* inc = new InternetClient();
-				inc->name = packet.string();
-				inc->build_id = packet.string();
-				inc->game = packet.string();
-				inc->type = packet.string();
-				inc->points = packet.string();
-				clientlist_.push_back(*inc);
+				inc.name = packet.string();
+				inc.build_id = packet.string();
+				inc.game = packet.string();
+				inc.type = packet.string();
+				inc.points = packet.string();
+				if (inc.build_id == "IRC") {
+					irc.push_back(inc);
+					// No "join" or "left" messages for IRC users
+					continue;
+				}
+				// No IRC client
+				clientlist_.push_back(inc);
 
 				bool found =
 				   old.empty();  // do not show all clients, if this instance is the actual change
 				for (InternetClient& client : old) {
-					if (client.name == inc->name) {
+					if (client.name == inc.name) {
 						found = true;
 						client.name = "";
 						break;
@@ -559,14 +566,12 @@
 				}
 				if (!found)
 					format_and_add_chat(
-					   "", "", true, (boost::format(_("%s joined the lobby")) % inc->name).str());
-
-				delete inc;
-				inc = nullptr;
+					   "", "", true, (boost::format(_("%s joined the lobby")) % inc.name).str());
 			}
+			clientlist_.insert(clientlist_.end(), irc.begin(), irc.end());
 
 			for (InternetClient& client : old) {
-				if (client.name.size()) {
+				if (client.name.size() && client.build_id != "IRC") {
 					format_and_add_chat(
 					   "", "", true, (boost::format(_("%s left the lobby")) % client.name).str());
 				}
@@ -800,8 +805,18 @@
 			   _("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(msg.substr(1, space - 1));  // recipient
+		s.string(recipient);  // recipient
 
 		format_and_add_chat(clientname_, msg.substr(1, space - 1), false, msg.substr(space + 1));
 
 
=== modified file 'src/ui_fsmenu/internet_lobby.cc'
--- src/ui_fsmenu/internet_lobby.cc	2017-12-19 07:17:15 +0000
+++ src/ui_fsmenu/internet_lobby.cc	2018-01-28 19:08:17 +0000
@@ -262,6 +262,11 @@
 			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
@@ -296,6 +301,11 @@
 	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());