Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#1686 closed enhancement (fixed)

[PATCH] "Slaughter" attack type needs actor variation and animation name support

Reported by: michael Owned by: leper
Priority: Must Have Milestone: Alpha 13
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description (last modified by michael)

Now that we have a "slaughter" attack element for ranged units to use a melee attack to kill domestic animals, there needs to be support for actor variations and animation names for this type of attack, so that the ranged units may use the right kind of animation and weapon variation.

Examples in the actor:

<animation event="0.5" file="biped/rider_spear_shield_atk_a.psa" name="Slaughter" speed="300"/>

and...

<variant name="slaughter">

  <props>
    <prop actor="props/units/weapons/spartan_sword.xml" attachpoint="r_hand"/>
  </props>

</variant>

Attached is a test actor.

Attachments (3)

cavalry_javelinist_b_r.xml (2.1 KB ) - added by michael 12 years ago.
update-attack-prefix.patch (1.4 KB ) - added by Doménique 12 years ago.
Adjust UnitAI to use attack_name animation and ActorViewer animation list
update-attack-animations.sh (5.0 KB ) - added by Doménique 11 years ago.
Sets up actors to use attack_type animation labels instead of just melee

Download all attachments as: .zip

Change History (15)

by michael, 12 years ago

Attachment: cavalry_javelinist_b_r.xml added

comment:1 by michael, 12 years ago

Description: modified (diff)

comment:2 by michael, 12 years ago

Description: modified (diff)

comment:3 by Doménique, 12 years ago

I would like to propose a change to how the attack animations are labelled within the actor files. This should make it easier to add and differentiate between different attack types within one actor file. Instead of using only the melee designation I altered the animation name to attack_melee (similar to how (gather_|carry_)food|wood|stone|ore is implemented), so we gain the ability to use attack_ranged, attack_charge, and attack_slaughter. This way the slaughter animation can be different from the usual ranged or melee animation.

I have attached a small patch that sets up the UnitAI to correctly select the attack_type animation, plus adjustments to the Atlas ActorViewer animation list. More importantly I've made a shell script that alters all actors to set the correct animation type (currently all are set to melee, even ranged units). Instead of a massive patch with diffs on all relevant actors, I think the shell script is easier to evaluate and alter. Plus, if someone has a mod running the script on their mod is an easy way to adjust all actors to this new style. Therefore I included some extra info on usage and caveats in the shell script. It should all be a painless and quick affair to do :)

To conclude, having done these changes should simplify the process of setting slaughter animations (no matter the type of the unit's main attack style, the (non-)presence of a slaughter animation tells it all).

I have tested this patch and not found issues so far. If any would pop up, it would be a ranged unit not having attack_ranged set but rather the default attack_melee. The Indian war elephant does suffer from this (a no projectile set error is the tell-tale sign), but that actor is borrowed from the melee Persian elephant. Differentiating the attack animations exposes such issues.

by Doménique, 12 years ago

Attachment: update-attack-prefix.patch added

Adjust UnitAI to use attack_name animation and ActorViewer animation list

comment:4 by leper, 11 years ago

Component: Core engineUI & Simulation
Keywords: patch review added
Summary: "Slaughter" attack type needs actor variation and animation name support[PATCH] "Slaughter" attack type needs actor variation and animation name support

Thanks for the patch, someone will take a look at it soon. For future contributions please read SubmittingPatches.

comment:5 by leper, 11 years ago

Milestone: Alpha 12Alpha 13

I tested this today and there are still some issues and as this isn't really urgent (and we want to release Alpha 12 soon) I'm pushing this to Alpha 13.

(Just noting this here for future reference)
The assumption that all entities in structures have a ranged attack is wrong as there are some siege_rams there.
I think we need to add attack_slaughter to the female_citizen actors instead of replacing attack_melee (see #1699).
We should do the same with all citizen soldier actors.

in reply to:  5 ; comment:6 by Doménique, 11 years ago

Replying to leper:

(Just noting this here for future reference)
The assumption that all entities in structures have a ranged attack is wrong as there are some siege_rams there.
I think we need to add attack_slaughter to the female_citizen actors instead of replacing attack_melee (see #1699).
We should do the same with all citizen soldier actors.

The main reason I proposed changing all actor files is that some recent additions have two kinds of attack (both ranged and melee). I forget which unit it was? A Mauryan one? Without the ability to differentiate such a unit would have only one kind of animation, either melee or ranged.

As for the structure adaptations: you are right, I forgot about the siege units. I may have to update the script somewhat anyway, as I noticed Boudicca now has two versions: a melee one, and the ranged archer chariot. There may be other additions and changes as well. Since this is time intensive (having to check actor files by hand, and sometimes cross-referencing it with the simulation files) I would prefer to only update it, if the team sees something in my proposed solution of differentiating the attack animations.

in reply to:  6 comment:7 by leper, 11 years ago

Replying to dvangennip:

The main reason I proposed changing all actor files is that some recent additions have two kinds of attack (both ranged and melee). I forget which unit it was? A Mauryan one? Without the ability to differentiate such a unit would have only one kind of animation, either melee or ranged.

I don't know if we have a unit with two different attacks (ranged and melee) currently (so that not only one of them is chosen), but having more than one attack for a unit is planned.

As for the structure adaptations: you are right, I forgot about the siege units. I may have to update the script somewhat anyway, as I noticed Boudicca now has two versions: a melee one, and the ranged archer chariot. There may be other additions and changes as well. Since this is time intensive (having to check actor files by hand, and sometimes cross-referencing it with the simulation files) I would prefer to only update it, if the team sees something in my proposed solution of differentiating the attack animations.

As already noted above I do think we want to have more than one attack so that needs changing, and your approach seems like it will require some work, but we only have to do this once.

I'd suggest we wait until after the release and then try to get this in as quick as possible.

comment:8 by leper, 11 years ago

Should we get this done soon? Any updates to the script?

in reply to:  8 comment:9 by Doménique, 11 years ago

Replying to leper:

Should we get this done soon? Any updates to the script?

I have updated the update-attack-animations script to reflect recent changes. The structures / siege rams issue is taken care of by making sure those rams remain with attack_melee, not a ranged type.

One thing that is not solved, or rather exposed by this change, is that with chariots horses and occupant units should have different labels for their animations, but horses are currently set to the same as the occupant unit. It may mean the horses should stick with the attack_melee animations (or even better a future attack_charge), while its occupant should get their appropriate attack_ranged or attack_melee. Currently the script does not account for that, but in the long term the best solution to this is to add charge animations to horses. Practically it is not really an issue for now, as the horse melee animation is (or reverts to) the idle animation.

If some more serious mislabelling occurs (e.g., a swordsman inadvertently getting set to attack_ranged) this would be easy to spot, because it simply won't show any attack animation. It can simply be fixed by hand in its actor file.

Finally I think it would be good to advertise this change on the forum (if it is applied), because anyone modding 0 A.D. should be made aware of how animations should be labelled from then on forward.

by Doménique, 11 years ago

Attachment: update-attack-animations.sh added

Sets up actors to use attack_type animation labels instead of just melee

comment:10 by leper, 11 years ago

Owner: set to leper
Resolution: fixed
Status: newclosed

In 13060:

Change attack animation naming to allow for multiple attacks. Patch/script by dvagennip. Fixes #1686.

comment:11 by leper, 11 years ago

Keywords: review removed

Thank you very much for the script and patch.

comment:12 by Kieran P, 11 years ago

Type: taskenhancement
Note: See TracTickets for help on using tickets.