widelands-dev team mailing list archive

[Benedikt Straub <benedikt-straub@xxxxxx>]

Benedikt Straub has proposed merging lp:~widelands-dev/widelands/bug-1830526-soldier-walk-anims into lp:widelands.

Commit message:
Cache soldiers´ directional animations as unique_ptr to prevent memory leaks

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1830526 in widelands: "Memory leak in get_right_walk_anims"
  https://bugs.launchpad.net/widelands/+bug/1830526

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1830526-soldier-walk-anims/+merge/367939
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1830526-soldier-walk-anims into lp:widelands.

=== modified file 'src/logic/map_objects/tribes/soldier.cc'
--- src/logic/map_objects/tribes/soldier.cc	2019-05-18 20:43:25 +0000
+++ src/logic/map_objects/tribes/soldier.cc	2019-05-26 11:50:38 +0000
@@ -280,18 +280,23 @@
 }
 
 const DirAnimations& SoldierDescr::get_right_walk_anims(bool const ware,
-                                                        const Worker* worker) const {
-	const Soldier* soldier = dynamic_cast<const Soldier*>(worker);
+                                                        Worker* worker) const {
+	Soldier* soldier = dynamic_cast<Soldier*>(worker);
 	if (!soldier) {
 		return WorkerDescr::get_right_walk_anims(ware, worker);
 	}
-	DirAnimations* anim = new DirAnimations();
+	auto& cache = soldier->get_walking_animations_cache();
+	if (cache.first && cache.first->matches(soldier)) {
+		return *cache.second;
+	}
 	for (const auto& pair : walk_name_) {
 		if (pair.first->matches(soldier)) {
+			cache.first.reset(new SoldierLevelRange(*pair.first));
+			cache.second.reset(new DirAnimations());
 			for (uint8_t dir = 1; dir <= 6; ++dir) {
-				anim->set_animation(dir, get_animation(pair.second.at(dir), worker));
+				cache.second->set_animation(dir, get_animation(pair.second.at(dir), worker));
 			}
-			return *anim;
+			return *cache.second;
 		}
 	}
 	throw GameDataError(

=== modified file 'src/logic/map_objects/tribes/soldier.h'
--- src/logic/map_objects/tribes/soldier.h	2019-05-18 20:43:25 +0000
+++ src/logic/map_objects/tribes/soldier.h	2019-05-26 11:50:38 +0000
@@ -133,7 +133,7 @@
 
 	uint32_t get_rand_anim(Game& game, const std::string& name, const Soldier* soldier) const;
 
-	const DirAnimations& get_right_walk_anims(bool const ware, const Worker* w) const override;
+	const DirAnimations& get_right_walk_anims(bool const ware, Worker* w) const override;
 	uint32_t get_animation(const std::string& anim, const MapObject* mo = nullptr) const override;
 
 protected:
@@ -312,6 +312,10 @@
 	void start_task_move_in_battle(Game&, CombatWalkingDir);
 	void start_task_die(Game&);
 
+	std::pair<std::unique_ptr<SoldierLevelRange>, std::unique_ptr<DirAnimations>>& get_walking_animations_cache() {
+		return cache_walking_animations_;
+	}
+
 private:
 	void attack_update(Game&, State&);
 	void attack_pop(Game&, State&);
@@ -363,6 +367,8 @@
 	 */
 	Battle* battle_;
 
+	std::pair<std::unique_ptr<SoldierLevelRange>, std::unique_ptr<DirAnimations>> cache_walking_animations_;
+
 	static constexpr uint8_t kSoldierHealthBarWidth = 13;
 
 	/// Number of consecutive blocked signals until the soldiers are considered permanently stuck

=== modified file 'src/logic/map_objects/tribes/worker_descr.h'
--- src/logic/map_objects/tribes/worker_descr.h	2019-05-19 09:21:11 +0000
+++ src/logic/map_objects/tribes/worker_descr.h	2019-05-26 11:50:38 +0000
@@ -87,7 +87,7 @@
 			default_target_quantity_ = 1;
 	}
 
-	virtual const DirAnimations& get_right_walk_anims(bool const carries_ware, const Worker*) const {
+	virtual const DirAnimations& get_right_walk_anims(bool const carries_ware, Worker*) const {
 		return carries_ware ? walkload_anims_ : walk_anims_;
 	}
 	WorkerProgram const* get_program(const std::string&) const;