move attack animations out of [attack] and into [unit], code+wmllint

RELEASE_NOTES

@@ -0,0 +1,14 @@
+This file is here to allow devs to easily add stuff in the release notes for the next release,
+it allows easy syncing with the release team, since you don't have to be around when the release takes place...
+
+just dump whatever you want to have mentionned in the release notes here.
+
+The release team can empty this file after each Release..
+
+
+
+Boucman:
+The old [animation] in [attack] that was used to describe attack animations has been deprecated...
+you can now use an [attack_anim] block within the [unit] block... 
+this means that attack animations work in a similar fashion to all other anims, and should be easier to edit/add via WML
+

changelog

@@ -47,6 +47,8 @@
    * a minus sign in front of a cardinal direction now reverses it ("-s"="n")
    * now radius expansion is handled last in Standard Location Filters;
      previously it was handled last except before [and], [or], and [not]
+   * the WML for attack animations has been moved from the [attack] block to
+   the [unit] block...
    * fix a bug with array.length side-effects causing empty arrays to increase
      to size 1
  * miscellaneous and bug fixes:

data/tools/wmllint

@@ -541,144 +541,143 @@ def hack_syntax(filename, lines):
                 modcount += 1        
             need_image = False
     # Boucman's transformation of animation syntax
-    if future:
+    class anim_frame:
-        class anim_frame:
+        def __init__(self, attackline, attackname, lineno, female):
-            def __init__(self, attackline, attackname, lineno, female):
+            self.attackstart = attackline
-                self.attackstart = attackline
+            self.name = attackname
-                self.name = attackname
+            self.animstart = lineno
-                self.animstart = lineno
+            self.female = female
-                self.female = female
+            self.animend = None
-                self.animend = None
+            self.attackend = None
-                self.attackend = None
+        def __repr__(self):
-            def __repr__(self):
+            return `self.__dict__`
-                return `self.__dict__`
+    in_attack = in_animation = in_female = False
-        in_attack = in_animation = in_female = False
+    animations = []
-        animations = []
+    attackname = None
-        attackname = None
+    attackline = None
-        attackline = None
+    for i in range(len(lines)):
+        if "no-syntax-rewrite" in lines[i]:
+            break
+        elif "[female]" in lines[i]:
+            in_female = True
+        elif "[/female]" in lines[i]:
+            in_female = False
+        elif "[unit]" in lines[i]:
+            in_attack = in_animation = in_female = False
+            female_attack_index = -1
+            male_attack_start = len(animations)
+        elif "[attack]" in lines[i]:
+            in_attack = True;
+            attackname = None
+            attackline = i
+            if in_female:
+                female_attack_index += 1
+        elif "[animation]" in lines[i] and in_attack:
+            #if verbose:
+            #    print '"%s", line %d: [animation] within [attack]' \
+            #          % (filename, i+1)
+            # This weird piece of code is because attacks for female
+            # variants don't have names.  Instead, they're supposed
+            # to pick up the name of the corresponding male attack,
+            # where correspondence is by order of declaration.  The
+            # male_attack_start variable copes with the possibility
+            # of multiple units per file.
+            if attackname == None and in_female:
+                attackname = animations[male_attack_start + female_attack_index].name
+            if not attackname:
+                print '"%s", line %d: cannot deduce attack name'%(filename, i+1)
+                
+            animations.append(anim_frame(attackline, attackname, i, in_female))
+            in_animation = True
+        elif "[/animation]" in lines[i] and in_attack:
+            in_animation = False
+            if animations and animations[-1].animstart != None and animations[-1].animend == None:
+                animations[-1].animend = i
+            else:
+                print '"%s", line %d: [animation] ending here may be ill-formed'%(filename, i+1)
+        elif "[/attack]" in lines[i]:
+            inattack = False;
+            attackname = None
+            if animations and (animations[-1].attackstart == None or animations[-1].attackend != None):
+                print '"%s", line %d: [attack] ending here may be ill-formed'%(filename, i+1)
+            elif animations:
+                # This loop is needed because a single attack tag may
+                # enclose both hit and miss animations.
+                j = len(animations)-1
+                while True:
+                    animations[j].attackend = i
+                    j -= 1
+                    if j < 0 or animations[j].attackend != None:
+                        break
+        # Only pick up the *first* name field in an attack block;
+        # by convention, it will be right after the opening [attack] tag
+        elif in_attack and not in_animation and not attackname:
+            #print filename + ":" + `i+1` + ";" + `lines[i]` 
+            fields = lines[i].strip().split('#')
+            syntactic = fields[0]
+            comment = ""
+            if len(fields) > 1:
+                comment = fields[1]
+            if syntactic.strip().startswith("name"):
+                attackname = syntactic.split("=")[1].strip()
+    # All animation ranges have been gathered,  We have a list of objects
+    # containing the attack information.  Reverse it, because we're
+    # going to process them back to front to avoid invalidating the
+    # already-collected line numbers.  Then pull out the animation
+    # WML and stash it in the frame objects.
+    animations.reverse()
+    for aframe in animations:
+        if verbose:
+            print '"%s", line %d: lifting animation block at %d:%d for %s attack (%d:%d)' % (filename, aframe.animstart+1, aframe.animstart+1, aframe.animend+1, aframe.name, aframe.attackstart+1, aframe.attackend+1)
+        # Make a copy of the animation block, change its enclosing tags,
+        # outdent it, and add the needed filter clause.
+        animation = lines[aframe.animstart:aframe.animend+1]
+        animation[0] = animation[0].replace("[animation]", "[attack_anim]")
+        animation[-1] = animation[-1].replace("[/animation]","[/attack_anim]")
+        for i in range(len(animation)):
+            animation[i] = outdent(animation[i])
+        indent = leader(animation[1])
+        animation.insert(1, indent + "[/attack_filter]\n")
+        animation.insert(1, indent + baseindent + "name="+aframe.name+"\n")
+        animation.insert(1, indent + "[attack_filter]\n")
+        # Save it and delete it from its original location
+        aframe.wml = "".join(animation)
+        lines = lines[:aframe.animstart] + lines[aframe.animend+1:]
+        modcount += 1
+    # Insert attacks where they belong
+    female_attacks = filter(lambda a: a.female, animations)
+    if female_attacks:
+        female_end = -1
         for i in range(len(lines)):
-            if "no-syntax-rewrite" in lines[i]:
+            if lines[i].endswith("[/female]\n"):
+                female_end = i
                 break
-            elif "[female]" in lines[i]:
+        assert female_end != -1
-                in_female = True
+        female_wml = "".join(map(lambda x: x.wml, female_attacks))
-            elif "[/female]" in lines[i]:
+        lines = lines[:female_end] + [female_wml] + lines[female_end:]
-                in_female = False
+    male_attacks = filter(lambda a: not a.female, animations)
-            elif "[unit]" in lines[i]:
+    if male_attacks:
-                in_attack = in_animation = in_female = False
+        male_end = -1
-                female_attack_index = -1
+        for i in range(len(lines)):
-                male_attack_start = len(animations)
+            # Male attacks go either before the [female] tag or just
-            elif "[attack]" in lines[i]:
+            # before the closing [/unit]
-                in_attack = True;
+            if lines[i].endswith("[/unit]\n") or lines[i].endswith("[female]\n"):
-                attackname = None
+                male_end = i
-                attackline = i
+                break
-                if in_female:
+        assert male_end != -1
-                    female_attack_index += 1
+        male_wml = "".join(map(lambda x: x.wml, male_attacks))
-            elif "[animation]" in lines[i] and in_attack:
+        lines = lines[:male_end] + [male_wml] + lines[male_end:]
-                #if verbose:
+    # Garbage-collect any empty [attack] scopes left behind;
-                #    print '"%s", line %d: [animation] within [attack]' \
+    # this is likely to happen with female-variant units.
-                #          % (filename, i+1)
+    nullattack = True
-                # This weird piece of code is because attacks for female
+    while nullattack:
-                # variants don't have names.  Instead, they're supposed
+        nullattack = False
-                # to pick up the name of the corresponding male attack,
+        for i in range(len(lines)-1):
-                # where correspondence is by order of declaration.  The
+            if lines[i].strip() == "[attack]" and lines[i+1].strip() == "[/attack]":
-                # male_attack_start variable copes with the possibility
+                nullattack = True
-                # of multiple units per file.
+                break
-                if attackname == None and in_female:
+        if nullattack:
-                    attackname = animations[male_attack_start + female_attack_index].name
+            lines = lines[:i] + lines[i+2:]
-                if not attackname:
-                    print '"%s", line %d: cannot deduce attack name'%(filename, i+1)
-                    
-                animations.append(anim_frame(attackline, attackname, i, in_female))
-                in_animation = True
-            elif "[/animation]" in lines[i] and in_attack:
-                in_animation = False
-                if animations and animations[-1].animstart != None and animations[-1].animend == None:
-                    animations[-1].animend = i
-                else:
-                    print '"%s", line %d: [animation] ending here may be ill-formed'%(filename, i+1)
-            elif "[/attack]" in lines[i]:
-                inattack = False;
-                attackname = None
-                if animations and (animations[-1].attackstart == None or animations[-1].attackend != None):
-                    print '"%s", line %d: [attack] ending here may be ill-formed'%(filename, i+1)
-                elif animations:
-                    # This loop is needed because a single attack tag may
-                    # enclose both hit and miss animations.
-                    j = len(animations)-1
-                    while True:
-                        animations[j].attackend = i
-                        j -= 1
-                        if j < 0 or animations[j].attackend != None:
-                            break
-            # Only pick up the *first* name field in an attack block;
-            # by convention, it will be right after the opening [attack] tag
-            elif in_attack and not in_animation and not attackname:
-                #print filename + ":" + `i+1` + ";" + `lines[i]` 
-                fields = lines[i].strip().split('#')
-                syntactic = fields[0]
-                comment = ""
-                if len(fields) > 1:
-                    comment = fields[1]
-                if syntactic.strip().startswith("name"):
-                    attackname = syntactic.split("=")[1].strip()
-        # All animation ranges have been gathered,  We have a list of objects
-        # containing the attack information.  Reverse it, because we're
-        # going to process them back to front to avoid invalidating the
-        # already-collected line numbers.  Then pull out the animation
-        # WML and stash it in the frame objects.
-        animations.reverse()
-        for aframe in animations:
-            if verbose:
-                print '"%s", line %d: lifting animation block at %d:%d for %s attack (%d:%d)' % (filename, aframe.animstart+1, aframe.animstart+1, aframe.animend+1, aframe.name, aframe.attackstart+1, aframe.attackend+1)
-            # Make a copy of the animation block, change its enclosing tags,
-            # outdent it, and add the needed filter clause.
-            animation = lines[aframe.animstart:aframe.animend+1]
-            animation[0] = animation[0].replace("[animation]", "[attack_anim]")
-            animation[-1] = animation[-1].replace("[/animation]","[/attack_anim]")
-            for i in range(len(animation)):
-                animation[i] = outdent(animation[i])
-            indent = leader(animation[1])
-            animation.insert(1, indent + "[/attack_filter]\n")
-            animation.insert(1, indent + baseindent + "name="+aframe.name+"\n")
-            animation.insert(1, indent + "[attack_filter]\n")
-            # Save it and delete it from its original location
-            aframe.wml = "".join(animation)
-            lines = lines[:aframe.animstart] + lines[aframe.animend+1:]
-            modcount += 1
-        # Insert attacks where they belong
-        female_attacks = filter(lambda a: a.female, animations)
-        if female_attacks:
-            female_end = -1
-            for i in range(len(lines)):
-                if lines[i].endswith("[/female]\n"):
-                    female_end = i
-                    break
-            assert female_end != -1
-            female_wml = "".join(map(lambda x: x.wml, female_attacks))
-            lines = lines[:female_end] + [female_wml] + lines[female_end:]
-        male_attacks = filter(lambda a: not a.female, animations)
-        if male_attacks:
-            male_end = -1
-            for i in range(len(lines)):
-                # Male attacks go either before the [female] tag or just
-                # before the closing [/unit]
-                if lines[i].endswith("[/unit]\n") or lines[i].endswith("[female]\n"):
-                    male_end = i
-                    break
-            assert male_end != -1
-            male_wml = "".join(map(lambda x: x.wml, male_attacks))
-            lines = lines[:male_end] + [male_wml] + lines[male_end:]
-        # Garbage-collect any empty [attack] scopes left behind;
-        # this is likely to happen with female-variant units.
-        nullattack = True
-        while nullattack:
-            nullattack = False
-            for i in range(len(lines)-1):
-                if lines[i].strip() == "[attack]" and lines[i+1].strip() == "[/attack]":
-                    nullattack = True
-                    break
-            if nullattack:
-                lines = lines[:i] + lines[i+2:]
     # Upconvert old radius usage
     if "1.3.7" in versions and future:
         radius_pos = wmlfind("radius=", WmlIterator(lines))

src/unit.cpp

@@ -1310,6 +1310,7 @@ void unit::read(const config& cfg, bool use_traits)
 			const config::child_list& leading_anims = cfg_.get_children("leading_anim");
 
 			const config::child_list& defends = cfg_.get_children("defend");
+			const config::child_list& attack_anim = cfg_.get_children("attack_anim");
 			const config::child_list& teleports = cfg_.get_children("teleport_anim");
 			const config::child_list& extra_anims = cfg_.get_children("extra_anim");
 			const config::child_list& deaths = cfg_.get_children("death");
@@ -1408,6 +1409,22 @@ void unit::read(const config& cfg, bool use_traits)
 			}
 			animations_.push_back(unit_animation(0,unit_frame(absolute_image(),150),"movement",unit_animation::DEFAULT_ANIM));
 			// Always have a movement animation
+			bool found_attack = false;
+			for(config::child_list::const_iterator d = attack_anim.begin(); d != attack_anim.end(); ++d) {
+				animations_.push_back(unit_animation(**d));
+				found_attack = true;
+				//lg::wml_error<<"attack animations  are deprecate, support will be removed in 1.3.8 (in unit "<<id_<<")\n";
+				//lg::wml_error<<"please put it with an [animation] tag and apply_to=attack flag\n";
+			}
+			// TODO backward compat code, to be removed in 1.3.10, support for old attack format ([animation] in [attack] )
+			if(found_attack == false) {
+				std::vector<attack_type> tmp_attacks = type()->attacks();
+				for(std::vector<attack_type>::iterator attacks_itor = tmp_attacks.begin() ; attacks_itor!= tmp_attacks.end();attacks_itor++) {
+					animations_.insert(animations_.end(),attacks_itor->animation_.begin(),attacks_itor->animation_.end());
+				}
+			}
+			animations_.push_back(unit_animation(-150,unit_frame(absolute_image(),300),"attack",unit_animation::DEFAULT_ANIM));
+			// always have an attack animation
 			for(config::child_list::const_iterator d2 = defends.begin(); d2 != defends.end(); ++d2) {
 				(**d2)["apply_to"]="defend";
 				(**d2)["value"]=(**d2)["damage"];
@@ -1652,7 +1669,7 @@ void unit::set_extra_anim(const game_display &disp,const gamemap::location& loc,
 
 }
 
-const unit_animation & unit::set_attacking(const game_display &disp,const gamemap::location& loc,int damage,const attack_type& type,const attack_type* secondary_attack,int swing_num)
+void unit::set_attacking(const game_display &disp,const gamemap::location& loc,int damage,const attack_type& type,const attack_type* secondary_attack,int swing_num)
 {
 	state_ =  STATE_ATTACKING;
 	unit_animation::hit_type hit_type;
@@ -1663,7 +1680,7 @@ const unit_animation & unit::set_attacking(const game_display &disp,const gamema
 	}else {
 		hit_type = unit_animation::MISS;
 	}
-	return *start_animation(disp,loc,type.animation(disp,loc,this,hit_type,secondary_attack,swing_num,damage),true,true);
+	start_animation(disp,loc,choose_animation(disp,loc,"attack",damage,hit_type,&type,secondary_attack,swing_num),true);
 
 }
 void unit::set_leading(const game_display &disp,const gamemap::location& loc)
@@ -1737,22 +1754,16 @@ void unit::set_idling(const game_display &disp,const gamemap::location& loc)
 	start_animation(disp,loc,choose_animation(disp,loc,"idling"),true);
 }
 
-const unit_animation* unit::start_animation(const game_display &disp, const gamemap::location &loc,const unit_animation * animation,bool with_bars,bool is_attack_anim)
+void unit::start_animation(const game_display &disp, const gamemap::location &loc,const unit_animation * animation,bool with_bars)
 {
 	if(!animation) {
 		set_standing(disp,loc,with_bars);
-		return NULL;
+		return ;
 	}
 	draw_bars_ =  with_bars;
 	offset_=0;
 	if(anim_) delete anim_;
-	if(!is_attack_anim) {
+	anim_ = new unit_animation(*animation);
-		anim_ = new unit_animation(*animation);
-	} else {
-		//! @todo TODO this, the is_attack_anim param and the return value are ugly hacks
-		// that need to be taken care of eventually
-		anim_ = new attack_animation(*(static_cast<const attack_animation*>(animation)));
-	}
 	anim_->start_animation(anim_->get_begin_time(), false, disp.turbo_speed());
 	frame_begin_time_ = anim_->get_begin_time() -1;
 	if (disp.idle_anim()) {
@@ -1761,11 +1772,6 @@ const unit_animation* unit::start_animation(const game_display &disp, const game
 	} else {
 		next_idling_ = INT_MAX;
 	}
-	if(is_attack_anim) {
-		return &(static_cast<attack_animation*>(anim_))->get_missile_anim();
-	} else {
-		return NULL;
-	}
 }
 
 void unit::restart_animation(const game_display& disp,int start_time) {

src/unit.hpp

@@ -184,7 +184,7 @@ public:
 	void set_extra_anim(const game_display& disp,const gamemap::location& loc, std::string flag);
 	void set_dying(const game_display &disp,const gamemap::location& loc,const attack_type* attack,const attack_type* secondary_attack);
 	void set_walking(const game_display& disp,const gamemap::location& loc);
-	const unit_animation & set_attacking( const game_display &disp,const gamemap::location& loc,int damage,const attack_type& type,const attack_type* secondary_attack,int swing_num);
+	void set_attacking( const game_display &disp,const gamemap::location& loc,int damage,const attack_type& type,const attack_type* secondary_attack,int swing_num);
 	void set_recruited(const game_display& disp,const gamemap::location& loc);
 	void set_healed(const game_display& disp,const gamemap::location& loc,int healing);
 	void set_victorious(const game_display &disp,const gamemap::location& loc,const attack_type* attack,const attack_type* secondary_attack);
@@ -249,7 +249,7 @@ public:
 				STATE_DYING, STATE_EXTRA, STATE_TELEPORT,
 				STATE_RECRUITED, STATE_HEALED, STATE_POISONED,
 				STATE_IDLEIN, STATE_IDLING, STATE_VICTORIOUS};
-	const unit_animation * start_animation(const game_display &disp, const gamemap::location &loc,const unit_animation* animation, bool with_bars,bool is_attack_anim =false);
+	void start_animation(const game_display &disp, const gamemap::location &loc,const unit_animation* animation, bool with_bars);
 
 	//! The name of the file to game_display (used in menus).
 	const std::string& absolute_image() const { return cfg_["image"]; }
@@ -266,7 +266,6 @@ public:
 
 	const unit_animation* choose_animation(const game_display& disp, const gamemap::location& loc,const std::string& event,const int damage=0,const unit_animation::hit_type hit_type = unit_animation::INVALID,const attack_type* attack=NULL,const attack_type* second_attack = NULL, int swing_num =0) const;
 
-
 	bool get_ability_bool(const std::string& ability, const gamemap::location& loc) const;
 	unit_ability_list get_abilities(const std::string& ability, const gamemap::location& loc) const;
 	std::vector<std::string> ability_tooltips(const gamemap::location& loc) const;

src/unit_animation.cpp

@@ -163,6 +163,13 @@ unit_animation::unit_animation(const config& cfg,const std::string frame_string
 	for(itor = cfg.child_range("secondary_attack_filter").first; itor <cfg.child_range("secondary_attack_filter").second;itor++) {
 		secondary_attack_filter_.push_back(**itor);
 	}
+	// this whole block is a temporary hack that will stay until proper multi-frame in anim is supported...
+	range = cfg.child_range("missile_frame");
+	for(; range.first != range.second; ++range.first) {
+		unit_frame tmp_frame(**range.first);
+		missile_anim_.add_frame(tmp_frame.duration(),tmp_frame,!tmp_frame.does_not_change());
+	}
+
 }
 
 int unit_animation::matches(const game_display &disp,const gamemap::location& loc, const unit* my_unit,const std::string & event,const int value,hit_type hit,const attack_type* attack,const attack_type* second_attack, int swing_num) const
@@ -259,3 +266,11 @@ int unit_animation::matches(const game_display &disp,const gamemap::location& lo
 
 }
 
+
+void unit_animation::back_compat_add_name(const std::string name)
+{
+	config tmp;
+	tmp["name"] = name;
+	event_.push_back("attack");
+	primary_attack_filter_.push_back(tmp);
+}

src/unit_animation.hpp

@@ -41,6 +41,9 @@ class unit_animation:public animated<unit_frame>
 		explicit unit_animation(int start_time,const unit_frame &frame,const std::string& even="",const int variation=0);
 		int matches(const game_display &disp,const gamemap::location& loc,const unit* my_unit,const std::string & event="",const int value=0,hit_type hit=INVALID,const attack_type* attack=NULL,const attack_type* second_attack = NULL, int swing_num =0) const;
 
+		// only to support all [attack_anim] format, to remove at 1.3.10 time
+		void back_compat_add_name(const std::string name);
+		const animated<unit_frame> &get_missile_anim() const {return missile_anim_;} 
 	private:
 		t_translation::t_list terrain_types_;
 		std::vector<config> unit_filter_;
@@ -54,20 +57,8 @@ class unit_animation:public animated<unit_frame>
 		std::vector<config> secondary_attack_filter_;
 		std::vector<hit_type> hits_;
 		std::vector<int> swing_num_;
+		animated<unit_frame> missile_anim_;
 };
 
 
-
-
-class attack_animation: public unit_animation
-{
-	public:
-		explicit attack_animation(const config& cfg):unit_animation(cfg),missile_anim(cfg,"missile_frame"){};
-		explicit attack_animation(int start_time,const unit_frame &frame):unit_animation(start_time,frame) {};
-		const unit_animation &get_missile_anim() {return missile_anim;}
-	private:
-		unit_animation missile_anim;
-
-};
-
 #endif

src/unit_display.cpp

@@ -252,7 +252,8 @@ static void unit_attack_ranged(
 
 
 	// Start leader and attacker animation, wait for attacker animation to end
-	unit_animation missile_animation = attacker.set_attacking(*disp,a,damage,attack,secondary_attack,swing);
+	 attacker.set_attacking(*disp,a,damage,attack,secondary_attack,swing);
+	animated<unit_frame> missile_animation = attacker.get_animation()->get_missile_anim();
 	const gamemap::location leader_loc = under_leadership(units,a);
 	unit_map::iterator leader = units.end();
 	if(leader_loc.valid()){

src/unit_types.cpp

@@ -65,20 +65,20 @@ attack_type::attack_type(const config& cfg,const std::string& id, const std::str
 		const config expanded_cfg = unit_animation::prepare_animation(cfg,"animation");
 		const config::child_list& animations = expanded_cfg.get_children("animation");
 		for(config::child_list::const_iterator d = animations.begin(); d != animations.end(); ++d) {
-			animation_.push_back(attack_animation(**d));
+			lg::wml_error<<"attack animation  directly in attack is deprecated, support will be removed in 1.3.10 (in unit "<<id<<")\n";
+			lg::wml_error<<"please put it with an [attack_anim] tag in the [unit] and filter on the attack name\n";
+			animation_.push_back(unit_animation(**d));
+			animation_.back().back_compat_add_name(cfg["name"]);
 		}
-
 		if(cfg.child("frame") || cfg.child("missile_frame") || cfg.child("sound")) {
-			LOG_STREAM(err, config) << "the animation for " << cfg["name"] << "in unit " << id
+			lg::wml_error<<"using frame directly in attack is VERY deprecated, support will be removed in 1.3.10 (in unit "<<id<<")\n";
-									<< " is directly in the attack, please use [animation]\n" ;
 		}
 		if(animation_.empty()) {
-			animation_.push_back(attack_animation(cfg));
+			animation_.push_back(unit_animation(cfg));
+			animation_.back().back_compat_add_name(cfg["name"]);
 		}
-		if(animation_.empty()) {
+		animation_.push_back(unit_animation(-200,unit_frame(image_fighting,300),"attack",unit_animation::DEFAULT_ANIM));
-			animation_.push_back(attack_animation(-200,unit_frame(image_fighting,300)));
+		animation_.back().back_compat_add_name(cfg["name"]);
-		}
-		assert(!animation_.empty());
 	}
 
 	id_ = unit_id_test(cfg["name"]);
@@ -111,26 +111,6 @@ attack_type::attack_type(const config& cfg,const std::string& id, const std::str
 
 }
 
-const attack_animation* attack_type::animation(const game_display& disp, const gamemap::location& loc,const unit* my_unit,
-		const unit_animation::hit_type hit,const attack_type*secondary_attack,int swing_num,int damage) const
-{
-	// Select one of the matching animations at random
-	std::vector<const attack_animation*>  options;
-	int max_val = -3;
-	for(std::vector<attack_animation>::const_iterator i = animation_.begin(); i != animation_.end(); ++i) {
-		int matching = i->matches(disp,loc,my_unit,"attack",damage,hit,this,secondary_attack,swing_num);
-		if(matching == max_val) {
-			options.push_back(&*i);
-		} else if(matching > max_val) {
-			max_val = matching;
-			options.erase(options.begin(),options.end());
-			options.push_back(&*i);
-		}
-	}
-
-	if(options.empty()) return NULL;
-	return options[rand()%options.size()];
-}
 
 bool attack_type::matches_filter(const config& cfg,bool self) const
 {
@@ -785,6 +765,22 @@ unit_type::unit_type(const config& cfg, const movement_type_map& mv_types,
 	}
 	animations_.push_back(unit_animation(-150,unit_frame(image(),300),"defend",unit_animation::DEFAULT_ANIM));
 	// Always have a defensive animation
+	expanded_cfg = unit_animation::prepare_animation(cfg,"attack_anim");
+	const config::child_list& attack_anims = expanded_cfg.get_children("attack_anim");
+	for(config::child_list::const_iterator d = attack_anims.begin(); d != attack_anims.end(); ++d) {
+		(**d)["apply_to"] ="attack";
+		animations_.push_back(unit_animation(**d));
+		//lg::wml_error<<"attack animations  are deprecate, support will be removed in 1.3.8 (in unit "<<id()<<")\n";
+		//lg::wml_error<<"please put it with an [animation] tag and apply_to=attack flag\n";
+	}
+	// get old animation format, to be removed circum 1.3.10
+	std::vector<attack_type> tmp_attacks = attacks(true);
+	for(std::vector<attack_type>::iterator attacks_itor = tmp_attacks.begin() ; attacks_itor!= tmp_attacks.end();attacks_itor++) {
+		animations_.insert(animations_.end(),attacks_itor->animation_.begin(),attacks_itor->animation_.end());
+		// this has been detected elsewhere, no deprecation message needed here
+	}
+	animations_.push_back(unit_animation(-150,unit_frame(image(),300),"attack",unit_animation::DEFAULT_ANIM));
+	// always have an attack animation
 	expanded_cfg = unit_animation::prepare_animation(cfg,"death");
 	const config::child_list& deaths = expanded_cfg.get_children("death");
 	for(anim_itor = deaths.begin(); anim_itor != deaths.end(); ++anim_itor) {

src/unit_types.hpp

@@ -90,9 +90,11 @@ public:
 	bool special_affects_self(const config& cfg) const;
 
 	config cfg_;
-	const attack_animation* animation(const game_display& disp, const gamemap::location& loc,const unit* my_unit,const unit_animation::hit_type hit,const attack_type* secondary_attack,int swing_num,int damage) const;
+	const unit_animation* animation(const game_display& disp, const gamemap::location& loc,const unit* my_unit,const unit_animation::hit_type hit,const attack_type* secondary_attack,int swing_num,int damage) const;
+	// made public to ease backward compatibility for WML syntax
+	// to be removed (with all corresponding code once 1.3.6 is reached
+	std::vector<unit_animation> animation_;
 private:
-	std::vector<attack_animation> animation_;
 	t_string description_;
 	std::string id_;
 	std::string type_;
@@ -274,7 +276,7 @@ private:
 	// animations
 	std::vector<unit_animation> animations_;
 
-    std::string flag_rgb_;
+	std::string flag_rgb_;
 };
 
 class game_data