Page MenuHomeWildfire Games
Differential D863

More ScriptInterface const.
ClosedPublic
Actions

Authored by • leper on Sep 2 2017, 2:43 AM.

Details

Reviewers
elexis
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Commits
rP20519: More ScriptInterface const.
Summary

Followup to D739. There are a few more in the network code, but these would require a rebase of echotangoecho's network rewrite.

Test Plan

Compile it.

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

• leper created this revision.Sep 2 2017, 2:43 AM
Vulcan added a subscriber: Vulcan.Sep 2 2017, 3:51 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/1953/ for more details.

Vulcan added a comment.Sep 2 2017, 3:52 AM
Executing section Default...
Executing section Source...

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
|  33| #include·"lib/tex/tex.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: ''.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
|  33| #include·"lib/tex/tex.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
|  33| #include·"lib/tex/tex.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/470/ for more details.

Harbormaster completed remote builds in B2981: Diff 3465.Sep 2 2017, 3:52 AM
• leper updated this revision to Diff 3690.Sep 17 2017, 7:40 AM

It seems one of those in the simulation code could be made const.
The others cannot be (unless my last check of was wrong) as they
are calling some registration functions for the JS interface code.

Vulcan added a comment.Sep 17 2017, 12:04 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2041/ for more details.

Vulcan added a comment.Sep 17 2017, 12:05 PM
Executing section Default...
Executing section Source...

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
|  33| #include·"lib/tex/tex.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: ''.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
|  33| #include·"lib/tex/tex.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
|  33| #include·"lib/tex/tex.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/530/ for more details.

Harbormaster completed remote builds in B3192: Diff 3690.Sep 17 2017, 12:05 PM
• leper mentioned this in D878: Switch from a placeable filter to an unplaceable filter..Sep 25 2017, 8:51 AM
bb added a subscriber: bb.Nov 12 2017, 1:05 PM

What about the ones in GameSetup/GameSetup.cpp?
All other are either const, or are init functions or are in network so further patch is complete.

FindPlaceableTemplates functions got removed, so needs a rebase, also is the scriptinterface inclusion in TemplateLoader.h still needed?

• leper added a comment.Nov 12 2017, 2:39 PM
In D863#40395, @bb wrote:

What about the ones in GameSetup/GameSetup.cpp?

Those end up in that one function that takes a pointer after some long and slightly confusing code. And that function needs to be able to take a nullptr in one case IIRC, which makes the whole thing somewhat ugly.

I tried changing that but it is more complicated (or rather annoying) than it looks, so at some point I decided that it isn't worth it and having it stick out like a sore thumb might cause someone to fix that.

All other are either const, or are init functions or are in network so further patch is complete.

FindPlaceableTemplates functions got removed, so needs a rebase, also is the scriptinterface inclusion in TemplateLoader.h still needed?

Nope, seems useless. Actually was useless before this change too, since it was a reference already.

Given that it needs a rebase, and that one network patch hasn't seen a lot of activity I'll just merge them.

• leper updated this revision to Diff 4354.Nov 25 2017, 3:41 AM

Remove now useless include. Also include network code changes.

elexis accepted this revision.Nov 25 2017, 4:46 AM
elexis added a subscriber: elexis.

Compiles, game starts and doesn't fall apart. Agreeing with bb's review.
Didn't test if those were call things that can be made const.
We can still add more afterwards if we find them.
(As usual better have this committed to prevent unneeded compile times, rebases and rereviews IMO)
And thanks for the followup patch.

source/ps/Game.cpp
188 ↗(On Diff #4354)

good move

source/ps/TemplateLoader.h
22 ↗(On Diff #4354)

ack unused (while paramnode is used)

source/scriptinterface/ScriptInterface.cpp
609 ↗(On Diff #4354)

Ack unused

This revision is now accepted and ready to land.Nov 25 2017, 4:46 AM
Vulcan added a comment.Nov 25 2017, 5:56 AM

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Vulcan added a comment.Nov 25 2017, 5:57 AM
Executing section Default...
Executing section Source...

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
|  33| #include·"lib/tex/tex.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: ''.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
|  33| #include·"lib/tex/tex.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
|  33| #include·"lib/tex/tex.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
Executing section JS...
Harbormaster completed remote builds in B3800: Diff 4354.Nov 25 2017, 5:57 AM
Closed by commit rP20519: More ScriptInterface const. (authored by • leper). · Explain WhyNov 25 2017, 7:50 AM
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Nov 25 2017, 7:50 AM

Revision Contents
Changeset List

PathSize
ps/
trunk/
source/
network/
4 lines
4 lines
2 lines
4 lines
6 lines
ps/
4 lines
1 line
scriptinterface/
4 lines
7 lines
simulation2/
4 lines
components/
10 lines
2 lines
system/
2 lines
tools/
atlas/
GameInterface/
Handlers/
8 lines

Diff 4357

View Options

ps/trunk/source/network/NetClient.h

Loading...
View Options

ps/trunk/source/network/NetClient.cpp

Loading...
View Options

ps/trunk/source/network/NetClientTurnManager.cpp

Loading...
View Options

ps/trunk/source/network/NetServer.h

Loading...
View Options

ps/trunk/source/network/NetServer.cpp

Loading...
View Options

ps/trunk/source/ps/Game.cpp

Loading...
View Options

ps/trunk/source/ps/TemplateLoader.h

Loading...
View Options

ps/trunk/source/scriptinterface/ScriptInterface.h

Loading...
View Options

ps/trunk/source/scriptinterface/ScriptInterface.cpp

Loading...
View Options

ps/trunk/source/simulation2/Simulation2.cpp

Loading...
View Options

ps/trunk/source/simulation2/components/CCmpAIManager.cpp

Loading...
View Options

ps/trunk/source/simulation2/components/CCmpCommandQueue.cpp

Loading...
View Options

ps/trunk/source/simulation2/system/ComponentTest.h

Loading...
View Options

ps/trunk/source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp

Loading...
Wildfire Games · Phabricator