Page MenuHomeWildfire Games

Move GUIUtil GetSetting to IGUIObject
ClosedPublic

Authored by elexis on Aug 26 2019, 2:03 PM.

Details

Summary

Motivation of this diff is to further the cleaning of the GUIUtil.h file, which contained a lot of random things that were removed already.
The following steps are to move GetSetting, SetSetting to AddSetting and SettingExists.
Since the patch doing that is already quite big, and SetSetting does benefit from further changes, this more simple diff is split from the rest of the logic changes.

Test Plan

Examine if the existing IGUIObject::AddSetting and IGUIObject::HasSetting functions are in the right or wrong place
I propose they are correct, because the Settings are members of IGUIObject, and they are owned by the IGUIObject conceptually (the settings set up the object).
So if AddSetting and HasSetting is there, GetSetting and SetSetting should be there as well, for the same argument.

Notice that moving these two function types means that the GUI<> class with static functions accessing foreign members will be gone.

That same GUI class and GUIutil.h file housed random other functions:
rP22744 RecurseObject
rP22689 GUI<>::FallBackSprite and GUI<>::FallBackColor
rP22662 GUI<int>::ParseColor
rP22605 GetDefaultGuiMatrix

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.Aug 26 2019, 2:03 PM

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

Linter detected issues:
Executing section Source...

source/gui/IGUIObject.h
|  44| template·<typename·T>·class·GUI;
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'template<...' is invalid C code. Use --std or --language to configure the language.

source/gui/GUIutil.h
|  31| template<typename·T>·class·GUI;
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'template<...' 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/500/display/redirect

elexis added inline comments.Aug 26 2019, 2:16 PM
source/gui/GUIutil.h
113 ↗(On Diff #9520)

safeguard line is now apparent because GetSetting appears right below SettingExists.
So the one who read the comment and 3 lines of surrounding context will notice.

115 ↗(On Diff #9520)

Keeping this comment line. The one below should be redundant.

source/gui/IGUIObject.cpp
143 ↗(On Diff #9520)

This function is necessary because of the const on the right side, because there are some functions, for example CChart::DrawAxes that are marked as const as well, so they aren't allowed to call the first getter.

elexis edited the test plan for this revision. (Show Details)Aug 26 2019, 2:18 PM
This revision was not accepted when it landed; it landed in state Needs Review.Aug 26 2019, 2:25 PM
This revision was automatically updated to reflect the committed changes.
elexis updated the Trac tickets for this revision.Aug 29 2019, 11:20 AM