Some GUIObject classes have settings that are mirrored in a member variable.
It seems to come from possible misunderstanding that the saved type is identical to the member variable, or from the misunderstanding that the lookup is very cheap.
There are many GetSetting calls even in Draw functions, so I suppose to make it consistent one way or the other (currently the other way being predominant).
Details
- Reviewers
- None
Can the same be done for m_ElementHighlight in CInput.cpp?
Notice that there is a behavior change if the Slider receives a JS value outside of the min/max bounds.
Previously the slider setting "value" was not clamped but only the member, now the "value" is clamped as well.
Consider whether it matters (it doesn't afaics).
Notice that the GetSetting calls in the constructor are doing useful zero initialization work, but are still misleading, because the members could just be set to 0, so the reader is misled to believe that perhaps the value is different from zero.
Notice that removing the members makes the code less error prone, because if there is on unique value in multiple variables, the variables have to be kept in sync in various places of the code determined by the logic of the code; whereas having only one variable per value means the variable is always self-consistent without paying for it.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 9137 Build 14986: Vulcan Build Jenkins Build 14985: Vulcan Build (Windows) Jenkins Build 14984: arc lint + arc unit
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/20/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... source/gui/CChart.h | 36| » std::vector<CVector2D>·m_Points; | | [MAJOR] CPPCheckBear (syntaxError): | | Code 'std::vector' is invalid C code. Use --std or --language to configure the language. source/gui/CSlider.h | 23| | | [MAJOR] CPPCheckBear (syntaxError): | | Code 'classCGUIList{' is invalid C code. Use --std or --language to configure the language. source/gui/CInput.h | 23| | | [MAJOR] CPPCheckBear (syntaxError): | | Code 'classCGUIList{' 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/529/display/redirect