Build is green

Updating workspaces.
Build (release)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
../../../source/lib/tex/tex_png.cpp: In member function ‘virtual Status TexCodecPng::encode(Tex*, DynArray*) const’:
../../../source/lib/tex/tex_png.cpp:309:9: warning: variable ‘ret’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
  Status ret = ERR::FAIL;
         ^
Build (debug)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/92/ for more details.

The code looks good. I've tested several messages sent from ai (/team, /allies, /msg and nothing). While /msg does not work on svn, all of them does after the patch. So that's fine for the purpose of this ticket, so i validate it.

Nonetheless, as i said in #4431, there is still a problem with replays (and certainly observers in game). They do not receive the /team, /allies, ... messages even after switching to a player as these use Engine.PlayerID() to determine the team or the alliances. Changing Engine.GetPlayerID() by (Engine.GetPlayerID() != -1 ? Engine.GetPlayerID() : g_ViewedPlayer) in the definition of g_IsChatAddressee (lines 167-189 of messages.js) fixes the problem. So maybe adding addition function which would do that in another ticket?

In D38#1138, @mimo wrote:

The code looks good. I've tested several messages sent from ai (/team, /allies, /msg and nothing). While /msg does not work on svn, all of them does after the patch. So that's fine for the purpose of this ticket, so i validate it.

Tested in MP thrice to complete.

Nonetheless, as i said in #4431, there is still a problem with replays (and certainly observers in game). They do not receive the /team, /allies, ... messages even after switching to a player as these use Engine.PlayerID() to determine the team or the alliances. Changing Engine.GetPlayerID() by (Engine.GetPlayerID() != -1 ? Engine.GetPlayerID() : g_ViewedPlayer) in the definition of g_IsChatAddressee (lines 167-189 of messages.js) fixes the problem. So maybe adding addition function which would do that in another ticket?

As in displaying the simulation ally/private chat messages to observers while hiding the networked ones? That won't make the code easier to read, but possible. We could also distinguish observermode in singleplayer-replays from regular observermode (in which case we would need to store the player ID of the single player in the replay).

In D38#1149, @elexis wrote:

As in displaying the simulation ally/private chat messages to observers while hiding the networked ones? That won't make the code easier to read, but possible. We could also distinguish observermode in singleplayer-replays from regular observermode (in which case we would need to store the player ID of the single player in the replay).

Sorry, I don't get what you mean. Nothing would be hidden. It is only that currently, in replay, when you switch to a player, you do not receive the messages sent by its allies or its team while i believe you should. The reason is because the tests done in g_IsChatAddressee rely only on Engine.GetPlayerID() which is not set in replays (unless you open the developper overlay). So the proposition was only to also use g_ViewedPlayer when Engine.GetPlayerID() returns -1.
And also, i don't get why you would need the player ID of the single player in replays. With the modif I propose, you can either switch to the player or to any AIs and receive the messages that were sent to them.

In D38#1178, @mimo wrote:

In D38#1149, @elexis wrote:

As in displaying the simulation ally/private chat messages to observers while hiding the networked ones? That won't make the code easier to read, but possible. We could also distinguish observermode in singleplayer-replays from regular observermode (in which case we would need to store the player ID of the single player in the replay).

Sorry, I don't get what you mean. Nothing would be hidden. It is only that currently, in replay, when you switch to a player, you do not receive the messages sent by its allies or its team while i believe you should. The reason is because the tests done in g_IsChatAddressee rely only on Engine.GetPlayerID() which is not set in replays (unless you open the developper overlay). So the proposition was only to also use g_ViewedPlayer when Engine.GetPlayerID() returns -1.
And also, i don't get why you would need the player ID of the single player in replays. With the modif I propose, you can either switch to the player or to any AIs and receive the messages that were sent to them.

If g_IsChatAddressee returns true if the addressee of the chat message is the viewed player, then it observers will be able to read team and private chat message of players in multiplayer mode, which is not wanted afaik. One could treat private messages sent from network confidential and private messages sent from bots as openly readable though. Notice that players can also send chat simulation commands, which might or might not be used some day.