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 )
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)
follow-up: 4 comment:1 by , 9 years ago
comment:2 by , 8 years ago
Keywords: | simple added |
---|
comment:3 by , 6 years ago
Description: | modified (diff) |
---|---|
Owner: | set to |
Status: | new → assigned |
comment:4 by , 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 fileAutostart.cpp
, sinceGamesetup.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 , 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 , 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 , 6 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:9 by , 6 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:10 by , 6 years ago
Please find for review 1st change at Phab:D1554
Started with
if (mapDirectory == L"random")
as separate function LoadRandomMap
comment:11 by , 6 years ago
Keywords: | simple removed |
---|---|
Milestone: | Backlog → Work In Progress |
Patch: | → Phab:D1554 |
comment:12 by , 19 months ago
Owner: | removed |
---|---|
Status: | assigned → new |
Backlogging due to lack of progress, lack of review, or users going MIA.
If we are touching that code already, we should attempt to move the related functions including
AutostartVisualReplay()
to a new fileAutostart.cpp
, sinceGamesetup.cpp
already has more than 1500 lines.