← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/net-uuid into lp:widelands

 

Thanks for the review! And for the grammar errors... ehem.

After some testing I am no longer so sure about the trimming of chat messages. Currently it is possible to send space-only messages to everyone or as a whisper message to a single player. So we should probably remove trailing (and leading?) space from all messages. Shall I do so? (A slight disadvantage would be that I can no longer explain the @whisper syntax by prepending it with a space.)

The first NOCOM is related to the fact that the SendMessage m is written to but the SendMessage s is send. It looks pretty broken to me, but has no-one ever tried to send an announcement and discovered this bug? I would just like to have a short confirmation that this is a bug (looking at the code is good enough for me).

As far as I understand it (language-wise), sent_uuid is correct. The variable is a boolean specifying whether the uuid should be sent, not the uuid itself. (The checkbox in the login dialog.)

I can't reproduce the bug of the second NOCOM, probably it was something unrelated to the code. Maybe on layer 8.

All on my local computer: Starting a metaserver and three clients. All Clients try to login with the name "Notabilis", second and third clients receive the names "Notabilis1" and "Notabilis2" respectively. First client opens a game, other two join it. As far as I can tell, everything worked fine.
If this answers your test, fine. But feel free to do some tests on your own. If you want me to setup a metaserver for it, just say so. (Hint: There is a --metaserver=IP command line switch in the game. Took me much too long to notice it.)
-- 
https://code.launchpad.net/~widelands-dev/widelands/net-uuid/+merge/332264
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/net-uuid into lp:widelands.


References