Merge lp:~widelands-dev/widelands/bug-1830526-soldier-walk-anims into lp:widelands

[Benedikt Straub]

Cache soldiers´ directional animations as unique_ptr to prevent memory leaks

[GunChleoc (gunchleoc)]

Code LGTM - just 1 nit for a variable name

review: Approve

[GunChleoc (gunchleoc)]

Done some testing - it's gone :)

@bunnybot merge

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5066. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/537427196.
Appveyor build 4846. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1830526_soldier_walk_anims-4846.

=== 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 12:22:03 +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 12:22:03 +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 walking_animations_cache_;
+	}
+
 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>> walking_animations_cache_;
+
 	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 12:22:03 +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;