Opened 3 years ago

Closed 3 years ago

#5575 closed defect (fixed)

GUIutil.h rewrite

Reported by: elexis Owned by: elexis
Priority: Should Have Milestone: Alpha 24
Component: Core engine Keywords:
Cc: Patch:


The file GUIutil.h and GUIutil.cpp were ugly. It was writtem mostly in 2004 and rarely changed.

The last version of the file before the rewrite in this ticket can be seen here:;21379

See also:

The file is ugly for the following reasons:

  • Mixes different purposes: GUI Object code, GUI setting code, Render function, many string parsing functions, unique type functions, weird boolean helper
  • Mixes many class and function types
  • Stores obsolete exotic helper class
  • Stores static functions that operate on IGUIObject that should have been members there
  • Should have been a namespace, not a class
  • Static functions requiring friendships to operate on private members instead of being located correctly
  • Requires default template specialization T=int despite that type never being used because functions were put into the class for no reason
  • The few functions that were located semi-correctly were broken (copies instead of references) or ugly (pointers instead of references)

I wasn't the only to notice:


14:10 < V0idExp> Hi! Can someone explain me how GUI<T> class is used and what it represents?
14:11 < V0idExp> I've noticed the GUI<SGUIMessage>::RecurseObject() call and can't figure out what is its purpose
14:18 <@Philip-> I think it represents a random collection of GUI-related functions that either take a template parameter or are used by other functions in that class
14:19 < V0idExp> Aaah... I thought it was some very strange interface to all GUI elements :D
14:19 <@Philip-> RecurseObject calls a function of the form "void IGUIObject::foo(T&)" recursively on the tree of GUI objects
14:19 <@Philip-> so it's kind of an interface in that way
14:19 <@Philip-> It's just static functions, you never have an object of type GUI<T>
14:20 < V0idExp> so, it's just an utility, right?
14:20 <@Philip-> Yeah
14:20 <@Philip-> which is why it's in GUIutil.h
14:21 <@Philip-> (I think it's fairly ugly)
14:22 <@Philip-> (but not enough to motivate me to rewrite it)

While refactoring existing (#5322, Phab:D1684) and creating new JS/XML GUI pages (#2604, Phab:D1746) and the SpiderMonkey 45 update (Phab:D1510), I was forced to discover these atrocities. Focusing on this project at the cost of improvements in other areas of the code allowed me to address these points:

r22563 Jul 28 Delete CInternalCGUIAccessorBase class from rP31.
r22570 Jul 29 Move semantics SetSetting used for CGUISpriteInstance, SetSettingWrap helper class
r22604 Aug 4 CGUISettings template class, IGUIObject::AddSetting templates, delete setting type enum
r22605 Aug 4 Move GetDefaultGuiMatrix to a separate file.
r22638 Aug 10 NONCOPYABLE GUI classes.
r22662 Aug 13 Remove proxy GUI<int>::ParseColor, refs rP1625, rP15278.
r22663 Aug 13 Delete CGUIManager::GetPreDefinedColor from rP7214.

r22689 Aug 18 Move FallBackSprite and FallBackColor to CGUIColor and CGUISpriteInstace

r22693 Aug 19 Introduce GetSetting returning a reference, replace GetSettingPointer calls.
r22696 Aug 19 Move GUI string parsing specializations to a GUIStringConversions.
r22695 Aug 19 Remove IsBoolTrue helper function.
r22744 Aug 21 Use variadic template for RecurseObject and move to IGUIObject.
r22765 Aug 23 Replace copy-assigning GetSetting calls with reference GetSetting calls.
r22779 Aug 25 Remove GetSettingPointer.
r22789 Aug 26 Move GetSetting to IGUIObject.
r22796 Aug 28 Move SetSetting to IGUIObject, remove default arguments.

and other defect fixes or improvements in source/gui/.

Change History (1)

comment:1 by elexis, 3 years ago

Resolution: fixed
Status: assignedclosed

In 22801:

Solemnly delete class GUI and rename GUIUtil.h to CGUISetting.h.
Finishes GUIutil rewrite and fixes #5575.

Differential Revision:
Test on: clang 8.0.1, Jenkins

Note: See TracTickets for help on using tickets.