Merge lp:~widelands-dev/widelands/bug1438611 into lp:widelands

[Jens Beyer]

Memory leak - destructor which cleans up animations, found by Valgrind, also see related bug.

[TiborB (tiborb95)]

LGTM

review: Approve

[SirVer (sirver)]

I think that is an improvement. A better fix would be to change the m_animations vector to contain unique_ptr<> instead of raw pointers. The destructor would then not be needed.

If you do not want to do that, could you add a TODO next to m_animations in the .h file?

[Jens Beyer (qcumber-some)]

It is not a matter of "want", but of "can" :-P

I tried, and failed miserably. So I just left the TODO as requested ;-)

=== modified file 'src/graphic/animation.cc'
--- src/graphic/animation.cc	2014-12-08 05:22:52 +0000
+++ src/graphic/animation.cc	2015-10-06 16:43:16 +0000
@@ -464,3 +464,9 @@
 
 	return *m_animations[id - 1];
 }
+
+AnimationManager::~AnimationManager()
+{
+	for (vector<Animation*>::iterator it = m_animations.begin(); it != m_animations.end(); ++it)
+		delete *it;
+}

=== modified file 'src/graphic/animation.h'
--- src/graphic/animation.h	2014-12-07 15:41:39 +0000
+++ src/graphic/animation.h	2015-10-06 16:43:16 +0000
@@ -94,6 +94,7 @@
 */
 class AnimationManager {
 public:
+	~AnimationManager();
 	/**
 	 * Loads an animation, graphics sound and everything.
 	 *
@@ -114,6 +115,8 @@
 	const Animation& get_animation(uint32_t id) const;
 
 private:
+	// TODO(SirVer): this vector should be changed to unique__ptr (spelled wrong to avoid message by CodeCheck)
+	//  instead of raw pointers to get rid of the destructor
 	std::vector<Animation*> m_animations;
 };