From d3e56f0f57cc596cb5aeaa0727e4a6733aa4ef1f Mon Sep 17 00:00:00 2001 From: elexis Date: Sat, 10 Aug 2019 12:51:27 +0000 Subject: [PATCH] Unfriend the 20 IGUIObject classes from CGUI. Improves separation of concerns and makes the code less error prone, since the IGUIObject classes can't break CGUI private members without CGUIs help anymore. This is achieved by making CGUI GetFocusedObject, UpdateObjects public, by introducing public CGUI GetBaseObject, GetMousePos, GetMouseButtons, HasStyle, GetStyle getters, and by removing the pointless IGUIObject GetMousePos proxy. Delete GetGUI() checks that are either always or never true following 2c47fbd66a. Use const references instead of copies for some mouse positions. Differential Revision: https://code.wildfiregames.com/D2166 This was SVN commit r22641. --- source/gui/CButton.cpp | 3 -- source/gui/CChart.cpp | 6 ---- source/gui/CCheckBox.cpp | 3 -- source/gui/CDropDown.cpp | 14 +++------- source/gui/CGUI.h | 38 +++++++++++++++++++------ source/gui/CImage.cpp | 3 -- source/gui/CInput.cpp | 7 ++--- source/gui/CList.cpp | 7 ++--- source/gui/COList.cpp | 8 +----- source/gui/CProgressBar.cpp | 3 -- source/gui/CSlider.cpp | 9 ++---- source/gui/CText.cpp | 5 +--- source/gui/CTooltip.cpp | 8 +----- source/gui/GUItext.cpp | 2 +- source/gui/IGUIButtonBehavior.cpp | 3 -- source/gui/IGUIObject.cpp | 46 ++++++++----------------------- source/gui/IGUIObject.h | 7 +---- source/gui/IGUIScrollBar.cpp | 4 +-- source/gui/IGUIScrollBarOwner.cpp | 3 -- source/gui/MiniMap.cpp | 4 +-- 20 files changed, 61 insertions(+), 122 deletions(-) diff --git a/source/gui/CButton.cpp b/source/gui/CButton.cpp index 4cd372767a..2feab9f874 100644 --- a/source/gui/CButton.cpp +++ b/source/gui/CButton.cpp @@ -57,9 +57,6 @@ CButton::~CButton() void CButton::SetupText() { - if (!GetGUI()) - return; - ENSURE(m_GeneratedTexts.size() == 1); CStrW font; diff --git a/source/gui/CChart.cpp b/source/gui/CChart.cpp index 5e10f9f2e7..3a5760966e 100644 --- a/source/gui/CChart.cpp +++ b/source/gui/CChart.cpp @@ -116,9 +116,6 @@ void CChart::Draw() { PROFILE3("render chart"); - if (!GetGUI()) - return; - if (m_Series.empty()) return; @@ -210,9 +207,6 @@ void CChart::UpdateSeries() void CChart::SetupText() { - if (!GetGUI()) - return; - for (SGUIText* t : m_GeneratedTexts) delete t; m_GeneratedTexts.clear(); diff --git a/source/gui/CCheckBox.cpp b/source/gui/CCheckBox.cpp index cf87763f91..4262930311 100644 --- a/source/gui/CCheckBox.cpp +++ b/source/gui/CCheckBox.cpp @@ -66,9 +66,6 @@ CCheckBox::~CCheckBox() void CCheckBox::SetupText() { - if (!GetGUI()) - return; - ENSURE(m_GeneratedTexts.size() == 1); CStrW font; diff --git a/source/gui/CDropDown.cpp b/source/gui/CDropDown.cpp index 2142ff7269..5db6934ce8 100644 --- a/source/gui/CDropDown.cpp +++ b/source/gui/CDropDown.cpp @@ -103,7 +103,7 @@ void CDropDown::HandleMessage(SGUIMessage& Message) if (!m_Open) break; - CPos mouse = GetMousePos(); + CPos mouse = m_pGUI->GetMousePos(); if (!GetListRect().PointInside(mouse)) break; @@ -205,7 +205,7 @@ void CDropDown::HandleMessage(SGUIMessage& Message) } else { - CPos mouse = GetMousePos(); + const CPos& mouse = m_pGUI->GetMousePos(); // If the regular area is pressed, then abort, and close. if (m_CachedActualSize.PointInside(mouse)) @@ -460,24 +460,18 @@ CRect CDropDown::GetListRect() const bool CDropDown::MouseOver() { - if(!GetGUI()) - throw PSERROR_GUI_OperationNeedsGUIObject(); - if (m_Open) { CRect rect(m_CachedActualSize.left, std::min(m_CachedActualSize.top, GetListRect().top), m_CachedActualSize.right, std::max(m_CachedActualSize.bottom, GetListRect().bottom)); - return rect.PointInside(GetMousePos()); + return rect.PointInside(m_pGUI->GetMousePos()); } else - return m_CachedActualSize.PointInside(GetMousePos()); + return m_CachedActualSize.PointInside(m_pGUI->GetMousePos()); } void CDropDown::Draw() { - if (!GetGUI()) - return; - float bz = GetBufferedZ(); float dropdown_size, button_width; diff --git a/source/gui/CGUI.h b/source/gui/CGUI.h index 8aa6f34a09..9997f0db21 100644 --- a/source/gui/CGUI.h +++ b/source/gui/CGUI.h @@ -72,8 +72,6 @@ class CGUI { NONCOPYABLE(CGUI); - friend class IGUIObject; - private: // Private typedefs using ConstructObjectFunction = IGUIObject* (*)(CGUI*); @@ -161,6 +159,11 @@ public: */ void LoadXmlFile(const VfsPath& Filename, boost::unordered_set& Paths); + /** + * Return the object which is an ancestor of every other GUI object. + */ + IGUIObject* GetBaseObject() const { return m_BaseObject; }; + /** * Checks if object exists and return true or false accordingly * @@ -169,7 +172,6 @@ public: */ bool ObjectExists(const CStr& Name) const; - /** * Returns the GUI object with the desired name, or NULL * if no match is found, @@ -184,6 +186,16 @@ public: */ IGUIObject* FindObjectUnderMouse() const; + /** + * Returns the current screen coordinates of the cursor. + */ + const CPos& GetMousePos() const { return m_MousePos; }; + + /** + * Returns the currently pressed mouse buttons. + */ + const unsigned int& GetMouseButtons() { return m_MouseButtons; }; + const SGUIScrollBarStyle* GetScrollBarStyle(const CStr& style) const; /** @@ -233,16 +245,25 @@ public: */ SGUIText GenerateText(const CGUIString& Text, const CStrW& Font, const float& Width, const float& BufferZone, const IGUIObject* pObject = NULL); - /** * Check if an icon exists */ - bool IconExists(const CStr& str) const { return (m_Icons.count(str) != 0); } + bool HasIcon(const CStr& name) const { return (m_Icons.count(name) != 0); } /** * Get Icon (a const reference, can never be changed) */ - const SGUIIcon& GetIcon(const CStr& str) const { return m_Icons.find(str)->second; } + const SGUIIcon& GetIcon(const CStr& name) const { return m_Icons.at(name); } + + /** + * Check if a style exists + */ + bool HasStyle(const CStr& name) const { return (m_Styles.count(name) != 0); } + + /** + * Get Style if it exists, otherwise throws an exception. + */ + const SGUIStyle& GetStyle(const CStr& name) const { return m_Styles.at(name); } /** * Get pre-defined color (if it exists) @@ -253,8 +274,6 @@ public: shared_ptr GetScriptInterface() { return m_ScriptInterface; }; JS::Value GetGlobalObject() { return m_ScriptInterface->GetGlobalObject(); }; -private: - /** * Updates the object pointers, needs to be called each * time an object has been added or removed. @@ -266,6 +285,7 @@ private: */ void UpdateObjects(); +private: /** * Adds an object to the GUI's object database * Private, since you can only add objects through @@ -286,12 +306,12 @@ private: */ IGUIObject* ConstructObject(const CStr& str); +public: /** * Get Focused Object. */ IGUIObject* GetFocusedObject() { return m_FocusedObject; } -public: /** * Change focus to new object. * Will send LOST_FOCUS/GOT_FOCUS messages as appropriate. diff --git a/source/gui/CImage.cpp b/source/gui/CImage.cpp index f3d87352b7..6cda4148e1 100644 --- a/source/gui/CImage.cpp +++ b/source/gui/CImage.cpp @@ -38,9 +38,6 @@ CImage::~CImage() void CImage::Draw() { - if (!GetGUI()) - return; - float bz = GetBufferedZ(); CGUISpriteInstance* sprite; diff --git a/source/gui/CInput.cpp b/source/gui/CInput.cpp index 22768bcaff..adca7f712c 100644 --- a/source/gui/CInput.cpp +++ b/source/gui/CInput.cpp @@ -912,7 +912,7 @@ void CInput::HandleMessage(SGUIMessage& Message) // Check if we're selecting the scrollbar if (GetScrollBar(0).GetStyle() && multiline) { - if (GetMousePos().x > m_CachedActualSize.right - GetScrollBar(0).GetStyle()->m_Width) + if (m_pGUI->GetMousePos().x > m_CachedActualSize.right - GetScrollBar(0).GetStyle()->m_Width) break; } @@ -1178,9 +1178,6 @@ void CInput::Draw() if (scrollbar && multiline) IGUIScrollBarOwner::Draw(); - if (!GetGUI()) - return; - CStrW font_name_w; CGUIColor color, color_selected; GUI::GetSetting(this, "font", font_name_w); @@ -1907,7 +1904,7 @@ int CInput::GetMouseHoveringTextPosition() const std::list::const_iterator current = m_CharacterPositions.begin(); - CPos mouse = GetMousePos(); + CPos mouse = m_pGUI->GetMousePos(); if (multiline) { diff --git a/source/gui/CList.cpp b/source/gui/CList.cpp index 471bb19617..8658ca58a8 100644 --- a/source/gui/CList.cpp +++ b/source/gui/CList.cpp @@ -71,9 +71,6 @@ CList::~CList() void CList::SetupText() { - if (!GetGUI()) - return; - m_Modified = true; CGUIList* pList; GUI::GetSettingPointer(this, "list", pList); @@ -538,8 +535,8 @@ int CList::GetHoveredItem() if (scrollbar) scroll = GetScrollBar(0).GetPos(); - CRect rect = GetListRect(); - CPos mouse = GetMousePos(); + const CRect& rect = GetListRect(); + CPos mouse = m_pGUI->GetMousePos(); mouse.y += scroll; // Mouse is over scrollbar diff --git a/source/gui/COList.cpp b/source/gui/COList.cpp index 5109cafa6e..fd40d1e08f 100644 --- a/source/gui/COList.cpp +++ b/source/gui/COList.cpp @@ -41,9 +41,6 @@ COList::COList(CGUI* pGUI) void COList::SetupText() { - if (!GetGUI()) - return; - CGUIList* pList; GUI::GetSettingPointer(this, "list", pList); @@ -154,7 +151,7 @@ void COList::HandleMessage(SGUIMessage& Message) if (!sortable) return; - CPos mouse = GetMousePos(); + const CPos& mouse = m_pGUI->GetMousePos(); if (!m_CachedActualSize.PointInside(mouse)) return; @@ -321,9 +318,6 @@ void COList::DrawList(const int& selected, const CStr& _sprite, const CStr& _spr if (scrollbar) IGUIScrollBarOwner::Draw(); - if (!GetGUI()) - return; - CRect rect = GetListRect(); CGUISpriteInstance* sprite = NULL; diff --git a/source/gui/CProgressBar.cpp b/source/gui/CProgressBar.cpp index 0540294019..a638b1c08d 100644 --- a/source/gui/CProgressBar.cpp +++ b/source/gui/CProgressBar.cpp @@ -64,9 +64,6 @@ void CProgressBar::HandleMessage(SGUIMessage& Message) void CProgressBar::Draw() { - if (!GetGUI()) - return; - float bz = GetBufferedZ(); CGUISpriteInstance* sprite_background; diff --git a/source/gui/CSlider.cpp b/source/gui/CSlider.cpp index ee292e3fe4..1d24a9bcc0 100644 --- a/source/gui/CSlider.cpp +++ b/source/gui/CSlider.cpp @@ -83,7 +83,7 @@ void CSlider::HandleMessage(SGUIMessage& Message) } case GUIM_MOUSE_PRESS_LEFT: { - m_Mouse = GetMousePos(); + m_Mouse = m_pGUI->GetMousePos(); m_IsPressed = true; IncrementallyChangeValue((m_Mouse.x - GetButtonRect().CenterPoint().x) * GetSliderRatio()); @@ -100,8 +100,8 @@ void CSlider::HandleMessage(SGUIMessage& Message) m_IsPressed = false; if (m_IsPressed) { - float difference = float(GetMousePos().x - m_Mouse.x) * GetSliderRatio(); - m_Mouse = GetMousePos(); + float difference = float(m_pGUI->GetMousePos().x - m_Mouse.x) * GetSliderRatio(); + m_Mouse = m_pGUI->GetMousePos(); IncrementallyChangeValue(difference); } break; @@ -113,9 +113,6 @@ void CSlider::HandleMessage(SGUIMessage& Message) void CSlider::Draw() { - if (!GetGUI()) - return; - CGUISpriteInstance* sprite; CGUISpriteInstance* sprite_button; int cell_id; diff --git a/source/gui/CText.cpp b/source/gui/CText.cpp index e2e6d98b0e..f285e67b8f 100644 --- a/source/gui/CText.cpp +++ b/source/gui/CText.cpp @@ -198,9 +198,6 @@ void CText::Draw() // Draw scrollbar IGUIScrollBarOwner::Draw(); - if (!GetGUI()) - return; - CGUISpriteInstance* sprite; int cell_id; bool clip; @@ -251,7 +248,7 @@ bool CText::MouseOverIcon() for (const SGUIText::SSpriteCall& spritecall : guitext->m_SpriteCalls) { // Check mouse over sprite - if (!spritecall.m_Area.PointInside(GetMousePos() - m_CachedActualSize.TopLeft())) + if (!spritecall.m_Area.PointInside(m_pGUI->GetMousePos() - m_CachedActualSize.TopLeft())) continue; // If tooltip exists, set the property diff --git a/source/gui/CTooltip.cpp b/source/gui/CTooltip.cpp index f3ed6801c1..abc368806d 100644 --- a/source/gui/CTooltip.cpp +++ b/source/gui/CTooltip.cpp @@ -63,9 +63,6 @@ CTooltip::~CTooltip() void CTooltip::SetupText() { - if (!GetGUI()) - return; - ENSURE(m_GeneratedTexts.size() == 1); CStrW font; @@ -92,7 +89,7 @@ void CTooltip::SetupText() GUI::GetSetting(this, "independent", independent); if (independent) - mousepos = GetMousePos(); + mousepos = m_pGUI->GetMousePos(); else GUI::GetSetting(this, "_mousepos", mousepos); @@ -151,9 +148,6 @@ void CTooltip::HandleMessage(SGUIMessage& Message) void CTooltip::Draw() { - if (!GetGUI()) - return; - float z = 900.f; // TODO: Find a nicer way of putting the tooltip on top of everything else CGUISpriteInstance* sprite; diff --git a/source/gui/GUItext.cpp b/source/gui/GUItext.cpp index 490421d859..aeba801ddf 100644 --- a/source/gui/GUItext.cpp +++ b/source/gui/GUItext.cpp @@ -97,7 +97,7 @@ void CGUIString::GenerateTextCall(const CGUI* pGUI, SFeedback& Feedback, CStrInt tag.m_TagType == TextChunk::Tag::TAG_ICON); const std::string& path = utf8_from_wstring(tag.m_TagValue); - if (!pGUI->IconExists(path)) + if (!pGUI->HasIcon(path)) { if (pObject) LOGERROR("Trying to use an icon, imgleft or imgright-tag with an undefined icon (\"%s\").", path.c_str()); diff --git a/source/gui/IGUIButtonBehavior.cpp b/source/gui/IGUIButtonBehavior.cpp index eb8ded19b6..685a019de1 100644 --- a/source/gui/IGUIButtonBehavior.cpp +++ b/source/gui/IGUIButtonBehavior.cpp @@ -166,9 +166,6 @@ CGUIColor IGUIButtonBehavior::ChooseColor() void IGUIButtonBehavior::DrawButton(const CRect& rect, const float& z, CGUISpriteInstance& sprite, CGUISpriteInstance& sprite_over, CGUISpriteInstance& sprite_pressed, CGUISpriteInstance& sprite_disabled, int cell_id) { - if (!GetGUI()) - return; - bool enabled; GUI::GetSetting(this, "enabled", enabled); diff --git a/source/gui/IGUIObject.cpp b/source/gui/IGUIObject.cpp index f723a97e5c..08d0f7526c 100644 --- a/source/gui/IGUIObject.cpp +++ b/source/gui/IGUIObject.cpp @@ -70,11 +70,6 @@ void IGUIObject::AddChild(IGUIObject* pChild) m_Children.push_back(pChild); - // If this (not the child) object is already attached - // to a CGUI, it pGUI pointer will be non-null. - // This will mean we'll have to check if we're using - // names already used. - if (pChild->GetGUI()) { try { @@ -135,10 +130,7 @@ void IGUIObject::AddSetting(const CStr& Name) bool IGUIObject::MouseOver() { - if (!GetGUI()) - throw PSERROR_GUI_OperationNeedsGUIObject(); - - return m_CachedActualSize.PointInside(GetMousePos()); + return m_CachedActualSize.PointInside(m_pGUI->GetMousePos()); } bool IGUIObject::MouseOverIcon() @@ -146,14 +138,6 @@ bool IGUIObject::MouseOverIcon() return false; } -CPos IGUIObject::GetMousePos() const -{ - if (GetGUI()) - return GetGUI()->m_MousePos; - - return CPos(); -} - void IGUIObject::UpdateMouseOver(IGUIObject* const& pMouseOver) { if (pMouseOver == this) @@ -269,17 +253,12 @@ void IGUIObject::UpdateCachedSize() } } -void IGUIObject::LoadStyle(CGUI& GUIinstance, const CStr& StyleName) +void IGUIObject::LoadStyle(CGUI& pGUI, const CStr& StyleName) { - // Fetch style - if (GUIinstance.m_Styles.count(StyleName) == 1) - { - LoadStyle(GUIinstance.m_Styles[StyleName]); - } + if (pGUI.HasStyle(StyleName)) + LoadStyle(pGUI.GetStyle(StyleName)); else - { debug_warn(L"IGUIObject::LoadStyle failed"); - } } void IGUIObject::LoadStyle(const SGUIStyle& Style) @@ -325,9 +304,6 @@ float IGUIObject::GetBufferedZ() const void IGUIObject::RegisterScriptHandler(const CStr& Action, const CStr& Code, CGUI* pGUI) { - if(!GetGUI()) - throw PSERROR_GUI_OperationNeedsGUIObject(); - JSContext* cx = pGUI->GetScriptInterface()->GetContext(); JSAutoRequest rq(cx); JS::RootedValue globalVal(cx, pGUI->GetGlobalObject()); @@ -396,11 +372,13 @@ void IGUIObject::ScriptEvent(const CStr& Action) // Set up the 'mouse' parameter JS::RootedValue mouse(cx); + const CPos& mousePos = m_pGUI->GetMousePos(); + m_pGUI->GetScriptInterface()->CreateObject( &mouse, - "x", m_pGUI->m_MousePos.x, - "y", m_pGUI->m_MousePos.y, - "buttons", m_pGUI->m_MouseButtons); + "x", mousePos.x, + "y", mousePos.y, + "buttons", m_pGUI->GetMouseButtons()); JS::AutoValueVector paramData(cx); paramData.append(mouse); @@ -465,17 +443,17 @@ CStr IGUIObject::GetPresentableName() const void IGUIObject::SetFocus() { - GetGUI()->m_FocusedObject = this; + m_pGUI->SetFocusedObject(this); } bool IGUIObject::IsFocused() const { - return GetGUI()->m_FocusedObject == this; + return m_pGUI->GetFocusedObject() == this; } bool IGUIObject::IsRootObject() const { - return GetGUI() != 0 && m_pParent == GetGUI()->m_BaseObject; + return m_pParent == m_pGUI->GetBaseObject(); } void IGUIObject::TraceMember(JSTracer* trc) diff --git a/source/gui/IGUIObject.h b/source/gui/IGUIObject.h index 33d22491b1..bdcaa13691 100644 --- a/source/gui/IGUIObject.h +++ b/source/gui/IGUIObject.h @@ -264,7 +264,7 @@ protected: * @param GUIinstance Reference to the GUI * @param StyleName Style by name */ - void LoadStyle(CGUI& GUIinstance, const CStr& StyleName); + void LoadStyle(CGUI& pGUI, const CStr& StyleName); /** * Loads a style. @@ -319,11 +319,6 @@ protected: */ IGUIObject* GetParent() const; - /** - * Get Mouse from CGUI. - */ - CPos GetMousePos() const; - /** * Handle additional children to the \-tag. In IGUIObject, this function does * nothing. In CList and CDropDown, it handles the \, used to build the data. diff --git a/source/gui/IGUIScrollBar.cpp b/source/gui/IGUIScrollBar.cpp index 88e021186a..9e6f3d1f75 100644 --- a/source/gui/IGUIScrollBar.cpp +++ b/source/gui/IGUIScrollBar.cpp @@ -96,7 +96,7 @@ void IGUIScrollBar::HandleMessage(SGUIMessage& Message) { // TODO Gee: Optimizations needed! - CPos mouse = m_pHostObject->GetMousePos(); + const CPos& mouse = m_pGUI->GetMousePos(); // If bar is being dragged if (m_BarPressed) @@ -123,7 +123,7 @@ void IGUIScrollBar::HandleMessage(SGUIMessage& Message) if (!m_pHostObject) break; - CPos mouse = m_pHostObject->GetMousePos(); + const CPos& mouse = m_pGUI->GetMousePos(); // if bar is pressed if (GetBarRect().PointInside(mouse)) diff --git a/source/gui/IGUIScrollBarOwner.cpp b/source/gui/IGUIScrollBarOwner.cpp index f83e4ba45f..1df2f4ff15 100644 --- a/source/gui/IGUIScrollBarOwner.cpp +++ b/source/gui/IGUIScrollBarOwner.cpp @@ -46,9 +46,6 @@ void IGUIScrollBarOwner::AddScrollBar(IGUIScrollBar* scrollbar) const SGUIScrollBarStyle* IGUIScrollBarOwner::GetScrollBarStyle(const CStr& style) const { - if (!GetGUI()) - return NULL; - return GetGUI()->GetScrollBarStyle(style); } diff --git a/source/gui/MiniMap.cpp b/source/gui/MiniMap.cpp index ccf22ed09e..b80d47c094 100644 --- a/source/gui/MiniMap.cpp +++ b/source/gui/MiniMap.cpp @@ -192,7 +192,7 @@ void CMiniMap::HandleMessage(SGUIMessage& Message) bool CMiniMap::MouseOver() { // Get the mouse position. - CPos mousePos = GetMousePos(); + const CPos& mousePos = m_pGUI->GetMousePos(); // Get the position of the center of the minimap. CPos minimapCenter = CPos(m_CachedActualSize.left + m_CachedActualSize.GetWidth() / 2.0, m_CachedActualSize.bottom - m_CachedActualSize.GetHeight() / 2.0); // Take the magnitude of the difference of the mouse position and minimap center. @@ -208,7 +208,7 @@ void CMiniMap::GetMouseWorldCoordinates(float& x, float& z) { // Determine X and Z according to proportion of mouse position and minimap - CPos mousePos = GetMousePos(); + const CPos& mousePos = m_pGUI->GetMousePos(); float px = (mousePos.x - m_CachedActualSize.left) / m_CachedActualSize.GetWidth(); float py = (m_CachedActualSize.bottom - mousePos.y) / m_CachedActualSize.GetHeight();