[Clean up] Use std::shared_ptr for Mng::New

Wargus · May 13, 2024 · fa19408 · fa19408
1 parent a895fb5
commit fa19408
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 65 deletions.
diff --git a/src/include/unittype.h b/src/include/unittype.h
@@ -563,7 +563,7 @@ class CUnitType
 {
 public:
 	CUnitType() = default;
-	~CUnitType();
+	~CUnitType() = default;
 
 	Vec2i GetHalfTileSize() const { return Vec2i(TileWidth / 2, TileHeight / 2); }
 	PixelSize GetPixelSize() const;
@@ -601,7 +601,7 @@ class CUnitType
 #ifdef USE_MNG
 	struct _portrait_ {
 		std::vector<std::string> Files;
-		std::vector<Mng *> Mngs;
+		std::vector<std::shared_ptr<Mng>> Mngs;
 		int Talking = 0; /// offset into portraits for talking portraits
 		mutable int CurrMng = 0;
 		mutable int NumIterations = 0;

diff --git a/src/include/video.h b/src/include/video.h
@@ -233,17 +233,14 @@ class CPlayerColorGraphic : public CGraphic
 
 class Mng : public gcn::SDLImage
 {
+public:
 	Mng() : gcn::SDLImage(nullptr, true) {}
 	~Mng();
-
 	Mng(const Mng &) = delete;
 	Mng &operator=(const Mng &) = delete;
 
-	uint32_t refcnt = 0;
+	static std::shared_ptr<Mng> New(const std::string &name);
 
-public:
-	static Mng *New(const std::string &name);
-	static void Free(Mng *mng);
 	bool Load();
 	void Reset();
 	void Draw(int x, int y);
@@ -270,11 +267,12 @@ class Mng : public gcn::SDLImage
 /// empty class for lua scripts
 class Mng : public gcn::Image
 {
+public:
 	Mng() {}
 	~Mng() {}
-public:
-	static Mng *New(const std::string &name) { return nullptr; }
-	static void Free(Mng *mng) {}
+
+	static std::shared_ptr<Mng> New(const std::string &name) { return nullptr; }
+
 	bool Load() { return false; }
 	void Reset() {}
 	void Draw(int x, int y) {}

diff --git a/src/include/widgets.h b/src/include/widgets.h
@@ -90,7 +90,7 @@ class LambdaActionListener : public gcn::ActionListener
 class ImageWidget : public gcn::Icon
 {
 public:
-	explicit ImageWidget(gcn::Image *img) : gcn::Icon(img) {} // TODO: Remove (used for Mng/Movie in ToLua++)
+	explicit ImageWidget(gcn::Image *img) : gcn::Icon(img) {} // TODO: Remove (used for Movie in ToLua++)
 	explicit ImageWidget(std::shared_ptr<gcn::Image> img) : gcn::Icon(img.get()), img(img) {}
 
 private:

diff --git a/src/tolua/ui.pkg b/src/tolua/ui.pkg
@@ -355,7 +355,7 @@ class ScrollArea : public BasicContainer
 class ImageWidget : public Widget
 {
 	ImageWidget(CGraphicPtr image);
-	ImageWidget(Mng *image);
+	ImageWidget(MngPtr image);
 	ImageWidget(Movie *image);
 };
 

diff --git a/src/tolua/video.pkg b/src/tolua/video.pkg
@@ -20,31 +20,31 @@ CVideo Video;
 void ToggleFullScreen(void);
 
 $using CGraphicPtr = std::shared_ptr<CGraphic>;
-$void Load(CGraphicPtr* p, bool grayscale) { (*p)->Load(grayscale); }
-$void Resize(CGraphicPtr* p, int w, int h) { (*p)->Resize(w, h); }
-$void SetPaletteColor(CGraphicPtr* p, int idx, int r, int g, int b) { (*p)->SetPaletteColor(idx, r, g, b); }
-$void OverlayGraphic(CGraphicPtr* p, CGraphicPtr g, bool mask) { (*p)->OverlayGraphic(g.get(), mask); }
+$void Load(CGraphicPtr* self, bool grayscale) { (*self)->Load(grayscale); }
+$void Resize(CGraphicPtr* self, int w, int h) { (*self)->Resize(w, h); }
+$void SetPaletteColor(CGraphicPtr* self, int idx, int r, int g, int b) { (*self)->SetPaletteColor(idx, r, g, b); }
+$void OverlayGraphic(CGraphicPtr* self, CGraphicPtr g, bool mask) { (*self)->OverlayGraphic(g.get(), mask); }
 
 $using CPlayerColorGraphicPtr = std::shared_ptr<CPlayerColorGraphic>;
-$void Load(CPlayerColorGraphicPtr* p, bool grayscale) { (*p)->Load(grayscale); }
-$void Resize(CPlayerColorGraphicPtr* p, int w, int h) { (*p)->Resize(w, h); }
-$void SetPaletteColor(CPlayerColorGraphicPtr* p, int idx, int r, int g, int b) { (*p)->SetPaletteColor(idx, r, g, b); }
-$void OverlayGraphic(CPlayerColorGraphicPtr* p, CGraphicPtr g, bool mask) { (*p)->OverlayGraphic(g.get(), mask); }
+$void Load(CPlayerColorGraphicPtr* self, bool grayscale) { (*self)->Load(grayscale); }
+$void Resize(CPlayerColorGraphicPtr* self, int w, int h) { (*self)->Resize(w, h); }
+$void SetPaletteColor(CPlayerColorGraphicPtr* self, int idx, int r, int g, int b) { (*self)->SetPaletteColor(idx, r, g, b); }
+$void OverlayGraphic(CPlayerColorGraphicPtr* self, CGraphicPtr g, bool mask) { (*self)->OverlayGraphic(g.get(), mask); }
 
 struct CGraphicPtr // std::shared_ptr<CGraphic>
 {
-	tolua_outside void Load @ Load(bool grayscale = false);
-	tolua_outside void Resize @ Resize(int w, int h);
-	tolua_outside void SetPaletteColor @ SetPaletteColor(int idx, int r, int g, int b);
-	tolua_outside void OverlayGraphic @ OverlayGraphic(CGraphicPtr g, bool mask = false);
+	tolua_outside void Load(bool grayscale = false);
+	tolua_outside void Resize(int w, int h);
+	tolua_outside void SetPaletteColor(int idx, int r, int g, int b);
+	tolua_outside void OverlayGraphic(CGraphicPtr g, bool mask = false);
 };
 
 struct CPlayerColorGraphicPtr // std::shared_ptr<CPlayerColorGraphic>
 {
-	tolua_outside void Load @ Load(bool grayscale = false);
-	tolua_outside void Resize @ Resize(int w, int h);
-	tolua_outside void SetPaletteColor @ SetPaletteColor(int idx, int r, int g, int b);
-	tolua_outside void OverlayGraphic @ OverlayGraphic(CGraphicPtr g, bool mask = false);
+	tolua_outside void Load(bool grayscale = false);
+	tolua_outside void Resize(int w, int h);
+	tolua_outside void SetPaletteColor(int idx, int r, int g, int b);
+	tolua_outside void OverlayGraphic(CGraphicPtr g, bool mask = false);
 };
 
 class CGraphic
@@ -78,16 +78,24 @@ void ClearAllColorCyclingRange();
 void AddColorCyclingRange(unsigned int startColorIndex, unsigned int endColorIndex);
 unsigned int SetColorCycleSpeed(unsigned int speed);
 
+$using MngPtr = std::shared_ptr<Mng>;
+$void Load(MngPtr* self) { (*self)->Load(); }
+$void Draw(MngPtr* self, int w, int h) { (*self)->Draw(w, h); }
+$void Reset(MngPtr* self) { (*self)->Reset(); }
+
+struct MngPtr // std::shared_ptr<Mng>
+{
+	tolua_outside void Load();
+	tolua_outside void Draw(int w, int h);
+	tolua_outside void Reset();
+};
+
 class Mng
 {
 public:
-		static Mng *New(const std::string name);
-		static void Free(Mng *mng);
-        bool Load();
-        void Draw(int x, int y);
-        void Reset();
+	static MngPtr New(const std::string name);
 
-		static int MaxFPS;
+	static int MaxFPS;
 }
 
 class Movie

diff --git a/src/unit/unittype.cpp b/src/unit/unittype.cpp
@@ -519,19 +519,6 @@ int GetResourceIdByName(lua_State *l, std::string_view resourceName)
 	return res;
 }
 
-CUnitType::~CUnitType()
-{
-#ifdef USE_MNG
-	if (!this->Portrait.Mngs.empty()) {
-		if (this->Portrait.Mngs[0]) {
-			for (auto *mng : this->Portrait.Mngs) {
-				Mng::Free(mng);
-			}
-		}
-	}
-#endif
-}
-
 PixelSize CUnitType::GetPixelSize() const
 {
 	return PixelSize(TileWidth * PixelTileSize.x, TileHeight * PixelTileSize.y);

diff --git a/src/video/mng.cpp b/src/video/mng.cpp
@@ -205,33 +205,21 @@ void Mng::Draw(int x, int y)
 	SDL_BlitSurface(mSurface, nullptr, TheScreen, &rect);
 }
 
-static std::map<std::string, Mng *> MngCache;
+static std::map<std::string, std::weak_ptr<Mng>> MngCache;
 
-Mng *Mng::New(const std::string &name)
+std::shared_ptr<Mng> Mng::New(const std::string &name)
 {
 	const std::string file = LibraryFileName(name);
-	Mng *&mng = MngCache[file];
+	auto& cache = MngCache[file];
+	auto mng = cache.lock();
 	if (mng == nullptr) {
-		mng = new Mng();
+		mng = std::make_shared<Mng>();
 		mng->name = LibraryFileName(name);
+		cache = mng;
 	}
-	mng->refcnt++;
 	return mng;
 }
 
-void Mng::Free(Mng *mng)
-{
-	// XXX: Weird free bug that I don't understand, just skip it if already nullptr
-	if ((intptr_t)mng < 40) {
-		return;
-	}
-	mng->refcnt--;
-	if (mng->refcnt == 0) {
-		MngCache.erase(mng->name);
-		delete mng;
-	}
-}
-
 /**
 **  Load a MNG
 **