3438 - Split Autostart function into smaller pieces
AbandonedPublic
Actions

Authored by anileapen05 on Jun 2 2018, 2:24 PM.

Details

Reviewers

elexis

Trac Tickets

#3438

Summary

The autostart() function in GameSetup.cpp has 282 lines of code. To improve the readability of that function, we should move the contents of the following if-statements into separate functions:

if (mapDirectory == L"random")
if (mapDirectory == L"scenarios" || mapDirectory == L"skirmishes")
if (args.Has("autostart-ai"))
if (args.Has("autostart-aidiff"))
if (args.Has("autostart-civ"))
if (args.Has("autostart-playername"))
if (args.Has("autostart-host"))
if (args.Has("autostart-client"))

Maybe moving this one from Init() would also make sense:

if (!args.Has("mod") && (flags & INIT_MODS) == INIT_MODS)

To allow easy reviewing, we could make one patch / commit per move.

Test Plan

Start a new game using Single Player -> Matches

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Lint Skipped

Unit

Unit Tests Skipped

Build Status

Buildable 6233
Build 10353: Vulcan Build	Jenkins

Event Timeline

anileapen05 created this revision.Jun 2 2018, 2:24 PM

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

Link to build: https://jenkins.wildfiregames.com/job/differential/618/display/redirect

Harbormaster failed remote builds in B6204: Diff 6707!Jun 2 2018, 2:25 PM

I think you have some unnoticed merge conflicts in there. Update svn to the most recent version.

(There is also a ticket about reducing the autostart code and reuse other existing code. But we didn't design an implementation however and doing so may be easier if the autostart code is cleanup up as proposed here.)

source/ps/GameSetup/GameSetup.cpp
1178	Should we move this to `Autostart.cpp` and maybe pack it into a namespace rather than these unsorted globals?
1182–1183	JS values are passed as JS::Value rather than JS::RootedValue (so as to prevent rooting issues I suspect)
1185	use whitespace like we do in this file, directory and elsewhere (Coding_Conventions)
1323	missing spaces after comma
1677	dont need these newlines

vladislavbelov added a subscriber: vladislavbelov.Jun 3 2018, 1:54 AM

vladislavbelov added inline comments.

source/ps/GameSetup/GameSetup.cpp
97	Why wrong CC? There're wrong names already, but it's not the reason to add new.
1179	I think, it'd be better to have these lines with the same indentation as the `const CmdLineArgs& args`.
1180	`const ScriptInterface& scriptInterface` AFAIK by leper's changes.
1185	Why not as a return value?