Opened 9 years ago

Last modified 19 months ago

#3438 new enhancement

Split Autostart function into smaller pieces

Reported by: elexis Owned by:
Priority: Nice to Have Milestone: Work In Progress
Component: Core engine Keywords:
Cc: Patch: Phab:D1554

Description (last modified by Andy Alt)

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.

Change History (12)

comment:1 by elexis, 9 years ago

If we are touching that code already, we should attempt to move the related functions including AutostartVisualReplay() to a new file Autostart.cpp, since Gamesetup.cpp already has more than 1500 lines.

comment:2 by elexis, 8 years ago

Keywords: simple added

comment:3 by Andy Alt, 6 years ago

Description: modified (diff)
Owner: set to Andy Alt
Status: newassigned

in reply to:  1 comment:4 by Andy Alt, 6 years ago

Replying to elexis:

If we are touching that code already, we should attempt to move the related functions including AutostartVisualReplay() to a new file Autostart.cpp, since Gamesetup.cpp already has more than 1500 lines.

Before breaking off those if statements into functions, are you sure you don't want the entire Autostart() function moved to Autostart.cpp as a first step?

comment:5 by elexis, 6 years ago

You can try that in a branch and link it if it succeeds and shows an advantage over the current code.

(This ticket should only be a simple cleanup - in the long run / ideally we would have some kind of mechanism shared by gamesetup.js, Gamesetup.cpp and Atlas, so that we don't have to keep all 3 in sync when adding or chaning gamesettings.)

comment:6 by Andy Alt, 6 years ago

@elexis, thanks for the response.

I got sidetracked and not sure when I'll be getting to this ticket. I'll unassign myself for now.

I hope I didn't cause any inconvenience in my delayed reply.

comment:7 by Andy Alt, 6 years ago

Owner: Andy Alt removed
Status: assignednew

comment:8 by elexis, 6 years ago

No inconveniences :-)

comment:9 by Anil Eapen, 6 years ago

Owner: set to Anil Eapen
Status: newassigned

comment:10 by Anil Eapen, 6 years ago

Please find for review 1st change at Phab:D1554

Started with

if (mapDirectory == L"random")

as separate function LoadRandomMap

Version 1, edited 6 years ago by Anil Eapen (previous) (next) (diff)

comment:11 by elexis, 6 years ago

Keywords: simple removed
Milestone: BacklogWork In Progress
Patch: Phab:D1554

comment:12 by Stan, 19 months ago

Owner: Anil Eapen removed
Status: assignednew

Backlogging due to lack of progress, lack of review, or users going MIA.

Note: See TracTickets for help on using tickets.