Skip to content

Commit 0df0a4b

Browse files
authored
Fix #4029: Use shared pointer in CClientDisplay(Manager) (#4108)
* Fixes quit crash in CClientDisplayManager::RemoveFromList * Implement shared_ptr & weak_ptr references for CClientDisplay(Manager) * Remove redundant / unused code
1 parent d919eab commit 0df0a4b

14 files changed

+85
-133
lines changed

Client/mods/deathmatch/logic/CClientDisplay.cpp

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,14 @@
1010

1111
#include <StdInc.h>
1212

13-
CClientDisplay::CClientDisplay(CClientDisplayManager* pDisplayManager, unsigned long ulID)
13+
CClientDisplay::CClientDisplay(unsigned long ulID)
1414
{
15-
m_pDisplayManager = pDisplayManager;
1615
m_ulID = ulID;
17-
18-
m_ulExpirationTime = 0;
1916
m_bVisible = true;
2017
m_Color = SColorRGBA(255, 255, 255, 255);
21-
22-
m_pDisplayManager->AddToList(this);
23-
}
24-
25-
CClientDisplay::~CClientDisplay()
26-
{
27-
// Remove us from the manager
28-
m_pDisplayManager->RemoveFromList(this);
2918
}
3019

3120
void CClientDisplay::SetColorAlpha(unsigned char ucAlpha)
3221
{
3322
m_Color.A = ucAlpha;
34-
}
23+
}

Client/mods/deathmatch/logic/CClientDisplay.h

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,13 @@ enum eDisplayType
2222

2323
class CClientDisplay
2424
{
25-
friend class CClientDisplayManager;
26-
2725
public:
28-
CClientDisplay(class CClientDisplayManager* pDisplayManager, unsigned long ulID);
29-
virtual ~CClientDisplay();
26+
CClientDisplay(unsigned long ulID);
27+
virtual ~CClientDisplay() = default;
3028

31-
unsigned long GetID() { return m_ulID; }
29+
unsigned long GetID() const { return m_ulID; }
3230
virtual eDisplayType GetType() = 0;
3331

34-
unsigned long GetExpirationTime() { return m_ulExpirationTime; };
35-
void SetExpirationTime(unsigned long ulTime) { m_ulExpirationTime = ulTime; };
36-
unsigned long GetTimeTillExpiration() { return m_ulExpirationTime - CClientTime::GetTime(); };
37-
void SetTimeTillExpiration(unsigned long ulMs) { m_ulExpirationTime = CClientTime::GetTime() + ulMs; };
38-
3932
virtual const CVector& GetPosition() { return m_vecPosition; };
4033
virtual void SetPosition(const CVector& vecPosition) { m_vecPosition = vecPosition; };
4134

@@ -49,12 +42,7 @@ class CClientDisplay
4942
virtual void Render() = 0;
5043

5144
protected:
52-
bool IsExpired() { return (m_ulExpirationTime != 0 && (CClientTime::GetTime() > m_ulExpirationTime)); };
53-
54-
CClientDisplayManager* m_pDisplayManager;
55-
5645
unsigned long m_ulID;
57-
unsigned long m_ulExpirationTime;
5846
bool m_bVisible;
5947
CVector m_vecPosition;
6048
SColor m_Color;

Client/mods/deathmatch/logic/CClientDisplayManager.cpp

Lines changed: 30 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -12,27 +12,21 @@
1212

1313
using std::list;
1414

15-
CClientDisplayManager::CClientDisplayManager()
16-
{
17-
// Init
18-
m_bCanRemoveFromList = true;
19-
}
20-
21-
CClientDisplayManager::~CClientDisplayManager()
22-
{
23-
RemoveAll();
24-
}
25-
26-
CClientDisplay* CClientDisplayManager::Get(unsigned long ulID)
15+
std::shared_ptr<CClientDisplay> CClientDisplayManager::Get(unsigned long ulID)
2716
{
2817
// Find the display with the given id
29-
list<CClientDisplay*>::const_iterator iter = m_List.begin();
30-
for (; iter != m_List.end(); iter++)
18+
auto iter = m_List.begin();
19+
20+
for (; iter != m_List.end(); iter++) // Iterate weak_ptr list
3121
{
32-
if ((*iter)->GetID() == ulID)
22+
if (const auto& display = (*iter).lock()) // Make sure the shared_ptr still exists
3323
{
34-
return *iter;
24+
if (display->GetID() == ulID)
25+
{
26+
return display;
27+
}
3528
}
29+
3630
}
3731

3832
return NULL;
@@ -49,57 +43,38 @@ void CClientDisplayManager::DrawText2D(const char* szCaption, const CVector& vec
4943
static_cast<int>(fResHeight), rgbaColor, szCaption, fScale, fScale, 0);
5044
}
5145

52-
void CClientDisplayManager::AddToList(CClientDisplay* pDisplay)
46+
void CClientDisplayManager::AddToList(const std::shared_ptr<CClientDisplay>& display)
5347
{
54-
m_List.push_back(pDisplay);
48+
m_List.push_back(display);
5549
}
5650

57-
void CClientDisplayManager::RemoveAll()
51+
void CClientDisplayManager::DoPulse()
5852
{
59-
// Delete all the items in the list
60-
m_bCanRemoveFromList = false;
61-
list<CClientDisplay*>::iterator iter = m_List.begin();
62-
for (; iter != m_List.end(); iter++)
63-
{
64-
delete *iter;
65-
}
53+
// Render all our displays
54+
auto iter = m_List.begin();
6655

67-
// Clear the list
68-
m_List.clear();
69-
m_bCanRemoveFromList = true;
70-
}
56+
// Clean up expired weak_ptr
57+
m_List.remove_if([](const std::weak_ptr<CClientDisplay>& wp) { return wp.expired(); });
7158

72-
void CClientDisplayManager::RemoveFromList(CClientDisplay* pDisplay)
73-
{
74-
if (m_bCanRemoveFromList)
59+
for (; iter != m_List.end(); iter++) // Iterate weak_ptr list
7560
{
76-
if (!m_List.empty())
61+
if (const auto& display = (*iter).lock()) // Make sure the shared_ptr still exists
7762
{
78-
m_List.remove(pDisplay);
63+
display->Render();
7964
}
8065
}
8166
}
8267

83-
void CClientDisplayManager::DoPulse()
68+
std::shared_ptr<CClientVectorGraphicDisplay> CClientDisplayManager::CreateVectorGraphicDisplay(CClientVectorGraphic* svg)
8469
{
85-
// Render all our displays
86-
m_bCanRemoveFromList = false;
87-
list<CClientDisplay*>::iterator iter = m_List.begin();
88-
while (iter != m_List.end())
89-
{
90-
CClientDisplay* pObject = *iter;
91-
if (pObject->IsExpired())
92-
{
93-
// Delete it and remove it from the list
94-
delete pObject;
95-
iter = m_List.erase(iter);
96-
}
97-
else
98-
{
99-
++iter;
100-
pObject->Render();
101-
}
102-
}
70+
auto display = std::make_shared<CClientVectorGraphicDisplay>(svg);
71+
AddToList(display);
72+
return display;
73+
}
10374

104-
m_bCanRemoveFromList = true;
75+
std::shared_ptr<CClientTextDisplay> CClientDisplayManager::CreateTextDisplay(int ID)
76+
{
77+
auto display = std::make_shared<CClientTextDisplay>(ID);
78+
AddToList(display);
79+
return display;
10580
}

Client/mods/deathmatch/logic/CClientDisplayManager.h

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,31 +13,34 @@ class CClientDisplayManager;
1313
#pragma once
1414

1515
#include "CClientManager.h"
16-
#include <list>
1716

18-
class CClientDisplay;
17+
#include "CClientDisplay.h"
18+
#include "CClientVectorGraphicDisplay.h"
19+
#include "CClientTextDisplay.h"
20+
21+
#include <list>
1922

2023
class CClientDisplayManager
2124
{
2225
friend class CClientManager;
2326
friend class CClientDisplay;
2427

2528
public:
26-
CClientDisplayManager();
27-
~CClientDisplayManager();
29+
CClientDisplayManager() = default;
30+
~CClientDisplayManager() = default;
2831

2932
void DoPulse();
3033

3134
unsigned int Count() { return static_cast<unsigned int>(m_List.size()); };
32-
CClientDisplay* Get(unsigned long ulID);
35+
std::shared_ptr<CClientDisplay> Get(unsigned long ulID);
3336

3437
void DrawText2D(const char* szCaption, const CVector& vecPosition, float fScale = 1.0f, RGBA rgbaColor = 0xFFFFFFFF);
3538

36-
void RemoveAll();
39+
void AddToList(const std::shared_ptr<CClientDisplay>& display);
3740

38-
void AddToList(CClientDisplay* pDisplay);
39-
void RemoveFromList(CClientDisplay* pDisplay);
41+
std::shared_ptr<CClientVectorGraphicDisplay> CreateVectorGraphicDisplay(CClientVectorGraphic* svg);
42+
std::shared_ptr<CClientTextDisplay> CreateTextDisplay(int ID = 0xFFFFFFFF);
4043

41-
std::list<CClientDisplay*> m_List;
42-
bool m_bCanRemoveFromList;
44+
std::list<std::weak_ptr<CClientDisplay>> m_List;
4345
};
46+

Client/mods/deathmatch/logic/CClientTextDisplay.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,14 @@ using std::list;
1414

1515
float CClientTextDisplay::m_fGlobalScale = 1.0f;
1616

17-
CClientTextDisplay::CClientTextDisplay(CClientDisplayManager* pDisplayManager, int ID) : CClientDisplay(pDisplayManager, ID)
17+
CClientTextDisplay::CClientTextDisplay(int ID) : CClientDisplay(ID)
1818
{
1919
// Init
2020
m_fScale = 1;
2121
m_ulFormat = 0;
2222
m_bVisible = true;
2323
}
2424

25-
CClientTextDisplay::~CClientTextDisplay()
26-
{
27-
}
28-
2925
void CClientTextDisplay::SetCaption(const char* szCaption)
3026
{
3127
if (szCaption)

Client/mods/deathmatch/logic/CClientTextDisplay.h

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,11 @@ class CClientTextDisplay;
1616
#include "CClientDisplayManager.h"
1717
#include <gui/CGUI.h>
1818

19-
class CClientTextDisplay : public CClientDisplay
19+
class CClientTextDisplay final : public CClientDisplay
2020
{
21-
friend class CClientDisplayManager;
22-
2321
public:
24-
CClientTextDisplay(CClientDisplayManager* pDisplayManager, int ID = 0xFFFFFFFF);
25-
~CClientTextDisplay();
22+
CClientTextDisplay(int ID = 0xFFFFFFFF);
23+
~CClientTextDisplay() = default;
2624

2725
eDisplayType GetType() { return DISPLAY_TEXT; }
2826

@@ -34,24 +32,24 @@ class CClientTextDisplay : public CClientDisplay
3432
void SetColorAlpha(unsigned char ucAlpha);
3533
void SetShadowAlpha(unsigned char ucShadowAlpha);
3634

37-
float GetScale() { return m_fScale; };
35+
float GetScale() const { return m_fScale; };
3836
void SetScale(float fScale);
3937

40-
unsigned long GetFormat() { return m_ulFormat; };
38+
unsigned long GetFormat() const { return m_ulFormat; };
4139
void SetFormat(unsigned long ulFormat);
4240

4341
void SetVisible(bool bVisible);
4442

4543
void Render();
4644

47-
static void SetGlobalScale(float fScale) { m_fGlobalScale = fScale; }
45+
static void SetGlobalScale(float fScale) { m_fGlobalScale = fScale; };
4846

4947
private:
5048
SString m_strCaption;
5149
float m_fScale;
5250

5351
unsigned long m_ulFormat;
54-
unsigned char m_ucShadowAlpha;
52+
unsigned char m_ucShadowAlpha{};
5553

5654
static float m_fGlobalScale;
5755
};

Client/mods/deathmatch/logic/CClientVectorGraphic.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ CClientVectorGraphic::CClientVectorGraphic(CClientManager* pManager, ElementID I
1717
SetTypeName("svg");
1818

1919
m_pManager = pManager;
20-
21-
m_pVectorGraphicDisplay = std::make_unique<CClientVectorGraphicDisplay>(m_pManager->GetDisplayManager(), this);
20+
m_pVectorGraphicDisplay = m_pManager->GetDisplayManager()->CreateVectorGraphicDisplay(this);
2221

2322
// Generate the default XML document
2423
SString defaultXmlString = SString("<svg viewBox='0 0 %u %u'></svg>", pVectorGraphicItem->m_uiSizeX, pVectorGraphicItem->m_uiSizeY);
@@ -84,7 +83,7 @@ void CClientVectorGraphic::OnUpdate()
8483

8584
if (std::holds_alternative<CLuaFunctionRef>(m_updateCallbackRef))
8685
{
87-
auto func = std::get<CLuaFunctionRef>(m_updateCallbackRef);
86+
auto& func = std::get<CLuaFunctionRef>(m_updateCallbackRef);
8887
auto state = func.GetLuaVM();
8988

9089
if (VERIFY_FUNCTION(func) && state != NULL)

Client/mods/deathmatch/logic/CClientVectorGraphic.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class CClientVectorGraphic final : public CClientTexture
5757
std::unique_ptr<SXMLString> m_pXMLString = nullptr;
5858
CXMLNode* m_pXMLDocument = nullptr;
5959

60-
std::unique_ptr<CClientVectorGraphicDisplay> m_pVectorGraphicDisplay;
60+
std::shared_ptr<CClientVectorGraphicDisplay> m_pVectorGraphicDisplay;
6161

6262
std::variant<CLuaFunctionRef, bool> m_updateCallbackRef = false;
6363

Client/mods/deathmatch/logic/CClientVectorGraphicDisplay.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313

1414
using namespace lunasvg;
1515

16-
CClientVectorGraphicDisplay::CClientVectorGraphicDisplay(CClientDisplayManager* pDisplayManager, CClientVectorGraphic* pVectorGraphic, int ID)
17-
: CClientDisplay(pDisplayManager, ID)
16+
CClientVectorGraphicDisplay::CClientVectorGraphicDisplay(CClientVectorGraphic* pVectorGraphic, int ID)
17+
: CClientDisplay(ID)
1818
{
1919
m_pVectorGraphic = pVectorGraphic;
2020
m_bVisible = true;

Client/mods/deathmatch/logic/CClientVectorGraphicDisplay.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class CClientVectorGraphicDisplay final : public CClientDisplay
2020
friend class CClientDisplayManager;
2121

2222
public:
23-
CClientVectorGraphicDisplay(CClientDisplayManager* pDisplayManager, CClientVectorGraphic* pVectorGraphic, int ID = DISPLAY_VECTORGRAPHIC);
23+
CClientVectorGraphicDisplay(CClientVectorGraphic* pVectorGraphic, int ID = DISPLAY_VECTORGRAPHIC);
2424
~CClientVectorGraphicDisplay() = default;
2525

2626
eDisplayType GetType() { return DISPLAY_VECTORGRAPHIC; }
@@ -38,7 +38,6 @@ class CClientVectorGraphicDisplay final : public CClientDisplay
3838
private:
3939
void UnpremultiplyBitmap(lunasvg::Bitmap& bitmap);
4040

41-
private:
4241
CClientVectorGraphic* m_pVectorGraphic;
4342

4443
bool m_bIsCleared;

0 commit comments

Comments
 (0)