While removing copies in source/gui/, I found a number of places where setting values are copied using GetSetting rather than using the GetSettingPointer. That is all strings and colors are copied, but using a reference for those would be faster.
After writing the patch to switching the copies to GetSettingPointer, I found this pointer code too ugly when this could use references directly, and without the weird custom exceptions that are redundant to std exceptions that noone catches but relies on them being defined by the code (also none of the users check whether the setting type is correct either but rely on that).
Hence this patch introduces a GetSetting reference function that doesn't copy the value nor returns a pointer to avoid both of these issues when just parsing the std::map<CStr, IGUISetting*>.
This way the patches that will remove the GetSetting copies can use GetSetting by reference rather than having to deal with pointers.
Details
- Reviewers
- None
- Commits
- rP22693: Use a new GetSetting returning a reference instead of GetSettingPointer when…
- Trac Tickets
- #5575
Ensure there is no behavior change in this patch, only shorter code and references instead of pointers. Notice that there are no more users of GetSettingPointer except one internally that may be addressed later.
Notice there are a sufficient number of users that mutate the setting (for example every CGUISpriteInstance due to the draw cache), so const ref is not sufficient.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 8922 Build 14641: Vulcan Build Jenkins Build 14640: arc lint + arc unit
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... source/gui/CInput.h | 23| | | [MAJOR] CPPCheckBear (syntaxError): | | Code 'classCGUIList{' is invalid C code. Use --std or --language to configure the language. source/gui/GUIutil.h | 44| 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/426/display/redirect
source/gui/GUIutil.cpp | ||
---|---|---|
421 | If I add a flimsy unused HasSetting to please the ones objecting to missing setting existence checks, I should at least register it for all types. |
Mostly uploading to test against Jenkins, since committing a patch that Jenkins doesnt digest means it wont build revision proposals anymore, so it wouldnt even test the fix to Jenkins (as learnt by experience).
wut a fetish patch
but the tls in a23b is broken DEFAULT since ever half a year+ or wut
but no a23c release? or wut
Test it very good fix crash on Mac http://www.mediafire.com/file/00lxxtdmsu64nrs/0ad-0.0.23b-alpha-osx64.dmg/file