Send NetClient GUIMessages to subscribing GUIObjects instead of pulling them onTick
Changes PlannedPublic
Actions

Authored by elexis on Nov 7 2019, 3:24 PM.

Details

Reviewers: None

Summary

This diff changes the way GUI messages created within the NetClient are obtained by the GUI.

rP7653 introduced the Engine.PollNetworkClient() JSInterface method which is called onTick from GUI pages listening to the NetClient.
This however means that if you have 100 FPS and 1 network message per second, you will get 100 function calls where only one of them is relevant.
This means that this is an unscalable pattern, if every event-subscription system would check onTick, the code would become very ugly on top of performing badly.
Hence this diff changes the way of propagation to have C++ call the JS functions if and only if there is a NetClient message pending.

rP23138/D2408 and rP22515/D1754 introduced the GUIManager SendEventToAll proxies for the CGUI SendEventToAll function.

Test Plan

Notice that if one doesn't clone the paramData object, the gamesetup will crash.
One can perform clone even on the JS side, but that seems compensating for an engine defect.

Notice that SendEventToAll does recurse through all GUI objects of all GUI pages, so that should be fixed as well if the performance should be maximized.
This can be achieved by keeping a list in CGUI of IGUIObjects that have m_ScriptHandlers for that event and then only calling these, similar to m_pAllObjects.

Diff Detail

Repository

rP 0 A.D. Public Repository

Branch

/ps/trunk

Lint

Lint OK

Unit

No Unit Test Coverage

Build Status

Buildable 9966
Build 16821: Vulcan Build	Jenkins
Build 16820: Vulcan Build (Windows)	Jenkins
Build 16819: arc lint + arc unit

Event Timeline

elexis created this revision.Nov 7 2019, 3:24 PM

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

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

Harbormaster failed remote builds in B9966: Diff 10282!Nov 7 2019, 3:24 PM

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

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

Stan added a subscriber: Stan.Nov 7 2019, 3:40 PM

Stan added inline comments.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
2104	I always wondered if the logging was slow. In the end we only ask for Warnings and Errors so mainlog makes little sense in release versions.
source/gui/GUIManager.cpp
362	. Maybe you could also display which param failed ?

elexis mentioned this in D2412: Lobby class implementation.Nov 17 2019, 10:47 PM

Botherthings:

XmppClient (edit: and UserReporter) equally pulls onTick but only receives messages occasionally, so it should receive the same treatment
One needs to subscribe to a GUI Object, but the GUI object itself is not involved at all in the processing of the event. So it would seem nicer to subscribe the event globally, perhaps similar to D2260/rP22851. (The event handlers must be destroyed when the GUI page is destroyed, yet CGUI should remain agnostic of NetClient and XmppClient)
To trigger a ScriptEvent, one needs to call RecurseObject. But that means on a GUI object page with 10k objects and only 1 object using the event, it will iterate over 10k objects with 10k function calls for no reason. (However not iterating using RecurseObject means iteration order will not be hierarchical anymore, which is sad, but appears that it can be assumed to be negligible. Secondly it means having to copy the pointers to iterate since that container can change while iterating).