Page MenuHomeWildfire Games

Split GUItext.h and CGUIString.h
ClosedPublic

Authored by elexis on Sat, Aug 10, 8:06 PM.

Details

Summary

The CGUIString and GUItext class already have some decent complexity / lines of code.
Currently GUItext.cpp only contains CGUIString functions.
So if one was to add (or move) few hundred lines of code to SGUIText struct in the cpp file, it would become unnecessarily intransparent in comparison to having one class per file.

Test Plan

Notice that now is a good opportunity to commit this, because the cpp file only contains one class information, sparing the split of the cpp file.
Notice that its a copied file with deletions, so no changes performed, hence it should be regression-free.
Ensure that the headers are correct, that forward declarations are preferred, that the front doesn't fall off on some exotic compiler, or without pch, or debug mode, or some weird unix, or VS2015 (but not VS2013), and Jenkins, and whomever else wants something.

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.Sat, Aug 10, 8:06 PM

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

Linter detected issues:
Executing section Source...

source/gui/CGUIString.h
|  52| class·CGUIString
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCGUIString{' is invalid C code. Use --std or --language to configure the language.

source/gui/CGUIList.h
|  23| class·CGUIList
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCGUIList{' is invalid C code. Use --std or --language to configure the language.

source/gui/GUItext.h
| 131| »   »   std::list<SSpriteCall>::pointer·m_pSpriteCall;
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'std::list' 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/352/display/redirect

Tested on VS2015, gcc 9.0.1. on arch

Why CGUIString, but _GUItext? Maybe rename that too?

source/gui/CGUIString.cpp
22 ↗(On Diff #9287)

Could you fix the order?

source/gui/GUItext.h
26 ↗(On Diff #9287)

Wrong order.

Rename _GUItext

I don't want to name it SGUIText.h because it might become CGUIText.h, in which case it should happen separately, as there are 49 occurrences of that classname, and probably less in that follow up.

Thanks for having taken a look.

elexis added inline comments.Sat, Aug 10, 9:05 PM
source/gui/CGUIString.cpp
22 ↗(On Diff #9287)

I propose to do it in a separate commit, so that this file has no hunks

vladislavbelov accepted this revision.Sat, Aug 10, 9:09 PM
This revision is now accepted and ready to land.Sat, Aug 10, 9:09 PM