Page MenuHomeWildfire Games

Change CGUIManager::SendEventToAll to send to all GUI pages
ClosedPublic

Authored by elexis on Nov 6 2019, 1:11 AM.

Details

Summary

As learnt in previous commits, the GUIManager methods that only operate on the topmost page are typically conceptually flawed as the topmost page might not be the intended page to work on by the event handling code.
This was the case for rP22200/D1701, rP22663/D2108, rP22922/D2302 for example.

Another example would be changing the NetClient GUIMessage code to push the GUI messages to all GUI pages having GUI objects subscribing to that event.
Another example use case would be the session GUI page and the summary dialog both subscribing to SimulationUpdate messages.

Test Plan

Notice that this patch only changes the following events:

simulation2/system/ReplayTurnManager.cpp:		g_GUI->SendEventToAll("ReplayFinished");
simulation2/system/ReplayTurnManager.cpp:	g_GUI->SendEventToAll("ReplayOutOfSync", paramData);
simulation2/system/ReplayTurnManager.cpp:		g_GUI->SendEventToAll("ReplayFinished");
simulation2/system/TurnManager.cpp:	g_GUI->SendEventToAll("SavegameLoaded", paramData);
ps/Game.cpp:				g_GUI->SendEventToAll("SimulationUpdate");
ps/GameSetup/GameSetup.cpp:	g_GUI->SendEventToAll("GameLoadProgress", paramData);

Notice SavegameLoaded and GameLoadProgress are only processed for the topmost page, so changing it is harmless.

The replay and OOS messages are currently not processed if a dialog (messsagebox, summary screen) is opened.
So these events are actually lost without the patch. With the patch, the event handlers may open session message boxes on top of the messagebox / summary screen, which is either less wrong or correct.

SimulationUpdate messages are currently not processed if theres an a GUI page stacked.
This is not relevant for unmodded singleplayer, since the game is paused when opening dialogs.
For multiplayer, the messages are not processed while the game progresses when a dialog is opened.
This has the effect that past messages are processed when the page is closed.
That means that the chat messages are not as sequential anymore (one may get chat messages from minutes ago that appear like recent messages).
It may also be considered a feature, as the player will get a summary of what happened since then when closing the dialog.
But then it's questionable whether the implementation is correct, since one can't assume that this delay effect is desirable for all events a priori.
Instead the event could itself decide whether to delay or not (by for instance subscribing to page close events).
Notice that this behavior change may be avoided for individual messages like the SimulationUpdate one by changing
g_GUI->SendEventToAll("SimulationUpdate"); to g_GUI->top()->....
Fact of the matter is that only sending to the top page means that the other GUI pages have no chance to process an event they may have legitimate interest in processing in,
and that if one doesnt wish to receive session popups when having opened the summary, lobby or options dialog, one can implement that without making it
impossible for non-topmost pages to handle these events.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Nov 6 2019, 1:11 AM
Vulcan added a comment.Nov 6 2019, 1:12 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1054/display/redirect

Vulcan added a comment.Nov 6 2019, 1:13 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/541/display/redirect

Vulcan added a comment.Nov 6 2019, 1:27 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1055/display/redirect

This revision was not accepted when it landed; it landed in state Needs Review.Nov 6 2019, 1:30 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
nani awarded a token.Nov 6 2019, 1:49 AM