CNetClient::PushGuiMessage script template to remove remaining Eval calls
ClosedPublic
Actions

Authored by elexis on Sep 6 2019, 12:54 PM.

Details

Reviewers: None
Commits: rP22867: Implement NetClient PushGuiMessage using parameter pack and ScriptInterface…

Summary

It seems I missed those Eval calls in D2080. While at it use static_cast and remove more duplication.

Test Plan

Go through every line and compare that the constructed JS Objects are exactly the same.
Confirm whether the tests are dead and not broken.
Notice the AIManager stuff is only on construction once.
Notice that the performance impact is hard to measure, see the days of experiments in D2127 and D2128.
Supposedly its half a percent faster according to the last test (that is only the previous vs new call, not taking the relation to all the other code or taking into account how unoften this is called to begin with).

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.Sep 6 2019, 12:54 PM

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

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

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

Linter detected issues:
Executing section Source...

source/network/tests/test_Net.h
|  37| class·TestNetComms·:·public·CxxTest::TestSuite
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classTestNetComms:' is invalid C code. Use --std or --language to configure the language.

source/network/tests/test_NetMessage.h
|   1| /*·Copyright·(C)·2010·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2010"

source/network/tests/test_NetMessage.h
|  24| class·TestNetMessage·:·public·CxxTest::TestSuite
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classTestNetMessage:' is invalid C code. Use --std or --language to configure the language.

source/network/NetClient.h
|  60| class·CNetClient·:·public·CFsm
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCNetClient:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

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

Harbormaster completed remote builds in B9244: Diff 9647.Sep 6 2019, 1:03 PM

Stan added a subscriber: Stan.Sep 6 2019, 1:08 PM

Stan added inline comments.

source/network/NetClient.h
158 ↗	(On Diff #9647)	Why is the function definition in the h file and not the cpp file ?

elexis added inline comments.Sep 7 2019, 2:43 AM

source/network/NetClient.h
158 ↗	(On Diff #9647)	If its in the cpp file, then the function is specialized for all calls in that cpp file. If it is in the h file, then the function is specialized for every call in files that include the h file (i.e. including NetClientTurnManager). That's a common downside to template functions used in different translation units. An alternative is to instantiate the specializations needed elsewhere in the cpp file. You can see that example in IGUIObject.cpp at the end for instance.

Stan added inline comments.Sep 7 2019, 9:14 AM

source/network/NetClient.h
158 ↗	(On Diff #9647)	Thanks !

Implement char literal ToJSVal to remove explicit std::string ctor.

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

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

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

Linter detected issues:
Executing section Source...

source/network/tests/test_Net.h
|  37| class·TestNetComms·:·public·CxxTest::TestSuite
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classTestNetComms:' is invalid C code. Use --std or --language to configure the language.

source/network/tests/test_NetMessage.h
|   1| /*·Copyright·(C)·2010·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2010"

source/network/tests/test_NetMessage.h
|  24| class·TestNetMessage·:·public·CxxTest::TestSuite
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classTestNetMessage:' is invalid C code. Use --std or --language to configure the language.

source/network/NetClient.h
|  60| class·CNetClient·:·public·CFsm
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCNetClient:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

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

Harbormaster completed remote builds in B9258: Diff 9661.Sep 7 2019, 6:23 PM

This revision was not accepted when it landed; it landed in state Needs Review.Sep 7 2019, 6:52 PM

Closed by commit rP22867: Implement NetClient PushGuiMessage using parameter pack and ScriptInterface… (authored by elexis). · Explain Why

This revision was automatically updated to reflect the committed changes.

elexis added a commit: rP22867: Implement NetClient PushGuiMessage using parameter pack and ScriptInterface….

Owners added a subscriber: Restricted Owners Package.Sep 7 2019, 6:52 PM