Page MenuHomeWildfire Games

Move CGUI object types to own file, CGUI constructor to the top of CGUI.cpp
ClosedPublic

Authored by elexis on Nov 24 2019, 11:00 AM.

Details

Summary

The convention is that the constructor and destructor of a C++ class occur first. This is not the case here, so it should be fixed to avoid distracting developers with unique oddities.
The confusing location has been the case since introduction of CGUI in rP9.

Notice that it would be worth to achieve removing ConstructObject, GUI_OBJECT macro and those #include "gui/ObjectTypes/ includes in CGUI.cpp.
However everytime I attempt to do that, I fail to achieve it, or fail to acheive it in a way that is actually different from what it is now, since
(1) either the IGUIObject inheriting classes need to inform the CGUI class in a static way that they exist or
(2) the CGUI class needs to keep a hardcoded list of IGUIObjects that exist.

The latter is whats being done currently and has the disadvantage that it makes CGUI require to include all headers of IGUIObject inheriting classes despite never using them anywhere except for ConstructObject.
The former doesnt seem feasible, since the map would be created at compiletime but we can't do ObjectTypes["button"] = &CButton::ConstructObject; at compiletime.
Nor can we do it at CGUI construction time since we dont have a place to hook in unless its a CGUI member, which defeats the purpose of decoupling CGUI from IGUIObjects again.

Hence in this diff Im moving it to an own file at least, so that the headers and the users are right above each other, so that it becomes visible that these are the only users, and for similarity with the GUISettingTypes.h file.

Test Plan

See the removed comments not having added anything and that the code is the same.
Notice defining CGUI::AddObjectTypes() in an external file is odd too, but that making that a global function consuming a CGUI argument is not less weird.
Consider the summary and code whether it's possible to define those object types in a cleaner way.

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.Nov 24 2019, 11:00 AM
elexis edited the test plan for this revision. (Show Details)Nov 24 2019, 11:04 AM

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

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

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

Linter detected issues:
Executing section Source...

source/gui/CGUI.h
|  51| class·CGUI
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCGUI{' 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/1151/display/redirect

This revision was not accepted when it landed; it landed in state Needs Review.Nov 24 2019, 11:27 AM
This revision was automatically updated to reflect the committed changes.