widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

* I assume that "BOT" was intended for chat or game Bots (we have/had some on irc, do we?)

* A first review of the diffs shows me no obvious Issues, some nits in the comments.

* I will now compile the code an do some review on the complete internet_gaming* files.
  (More for me to get familiar with that code)

* Notabalis: I will try to be online with that version starting at perhaps 16:00 CEST,
  so we can try you changes "live" in chat and on IRC. 
  Maybe we can hunt down these IRC Bots, too?

* GunCheloc: FYI with latest XCode updates Apple has pushed clang to a newer version,
  so a lot of newer options finally work here and some warnigns are finally gone:
  Apple LLVM version 9.1.0 (clang-902.0.39.1)
  Target: x86_64-apple-darwin17.5.0
  Thread model: posix
  InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

Diff comments:

> 
> === 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
> @@ -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;

Could we have an enum here like?:
    case kIRC_client_type: // No icon for IRC users

>  			default:
>  				continue;
>  			}
> 
> === 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
> @@ -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()) {

msgs.size() is an invariant and should be calculated only once

> +		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();


-- 
https://code.launchpad.net/~widelands-dev/widelands/net-user-type/+merge/343754
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/net-user-type.