Opened 2 years ago

Closed 19 months ago

Last modified 19 months ago

#3286 closed enhancement (fixed)

[PATCH] File inclusion in actor files

Reported by: sanderd17 Owned by:
Priority: Should Have Milestone: Alpha 21
Component: Core engine Keywords: patch
Cc: Patch:

Description

With the capture stuff, there's a lot of discussion about what animation should be used. And I don't really want to set an animation for 300-ish units, only to have to change it again a week later. There's currently no inheritance or file inclusion to make this easier in any way.

That's why I propose variant inclusion. Variants are the building blocks of the actors. An actor is formed by picking a variant from every group (either randomly or by name), and adding those variants together to form a variation. Every task for a unit is a variant with its own animations and props. And for many tasks, these definitions are rather equal (f.e. cutting a tree has the same animation for about every unit, with the same props).

So the separate variant file could contain the animations and the props, like

<variant name="attack_capture">
  <animations>
	<animation event="0.4" file="infantry/sword/attack/isw_s_def_01.psa" name="attack_capture" speed="100"/>
	<animation event="0.4" file="infantry/sword/attack/isw_s_def_06.psa" name="attack_capture" speed="100"/>
	<animation event="0.2" file="infantry/sword/attack/isw_s_em_04.psa" name="attack_capture" speed="100"/>
  </animations>
  <props>
    <prop actor="props/units/weapons/gladus.xml" attachpoint="r_hand"/>
  </props>
</variant>

While the actor only has the reference to that file

<variant file="units/variants/biped_attack_sword.xml"/>

For common animations, but where the prop depends on the unit (f.e. a more fancy sword for higher-ranked units), the general variant and a specification can be mixed:

<variant file="units/variants/biped_attack_sword.xml">
  <props>
    <prop actor="props/units/weapons/gladus.xml" attachpoint="r_hand"/>
  </props>
</variant>

In attachment, there's a patch that implements this. The actor editor isn't changed so far (I don't know if it needs changes), and the XML grammar should be changed for sure.

Attachments (3)

easy_switch_animations.diff (18.3 KB) - added by sanderd17 2 years ago.
easy_switch_animations.2.diff (27.3 KB) - added by sanderd17 2 years ago.
easy_switch_animations.3.diff (28.6 KB) - added by sanderd17 20 months ago.

Download all attachments as: .zip

Change History (17)

Changed 2 years ago by sanderd17

Attachment: easy_switch_animations.diff added

comment:1 Changed 2 years ago by stanislas69

comment:2 Changed 2 years ago by sanderd17

#2324 is not really related to this (though this is also an update of the actor format, but for different reasons). And #2324 won't even conflict very much with this (if we ignore all the whitespace changes in there).

comment:3 in reply to:  2 Changed 2 years ago by Vladislav Belov

Replying to sanderd17: Is not better to use switch in ObjectBase?.cpp:149?

Last edited 2 years ago by Vladislav Belov (previous) (diff)

Changed 2 years ago by sanderd17

comment:4 Changed 2 years ago by sanderd17

New file that also supports the Actor editor to a certain degree. The variant files are taken out of the actor directory (as they didn't follow the same schema).

A schema is also added for the variants (both the rnc and rng are hand-written (or hand-copied) though, it seemed to be easier than finding a working conversion tool).

comment:5 in reply to:  description Changed 2 years ago by Ben Brian

Replying to sanderd17:

With the capture stuff, there's a lot of discussion about what animation should be used. And I don't really want to set an animation for 300-ish units, only to have to change it again a week later. There's currently no inheritance or file inclusion to make this easier in any way.

You could copy existing animations and give them a name that likely won't change. I'm afraid all this proposal will do is make the actor format harder to understand and deal with.

comment:6 Changed 2 years ago by sanderd17

And the prop files too? And not everything I'd defined in the animation files, like currently, there's a random selection between 3 animations with a certain distribution for capturing. It's possible this is brought to 1 animation, or more than 3. Our we might want to change the speed. Same for props, we might want to have it in the left hand instead of the right hand suddenly.

comment:7 Changed 2 years ago by stanislas69

Milestone: BacklogAlpha 20

comment:8 Changed 21 months ago by Itms

Milestone: Alpha 20Alpha 21

comment:9 Changed 20 months ago by Itms

In this patch you make some changes to some actors, and it looks like the changes are really few compared to all the new code: is it just a subset of the "300-ish units" you talk about in the ticket description, or is the patch just a convenience for hypothetical bigger changes?

comment:10 Changed 20 months ago by sanderd17

The patch only includes actor changes for the basic citizen soldiers of the Athenian civ. That were the only units I gave initial animations.

So if this patch gets approved, all other units (at least those similar enough to use the same animation), could use that single xml line. If the patch (or at least the idea around it) doesn't get approved, then all units would need the extra xml you see these Athenian soldiers have already.

Of course, it's useful for other animations too, but I don't think it's a good idea to update them all immediately (as its an awful lot of work). But it's useful to have the code for when we want new units, or replace animations.

Last edited 19 months ago by sanderd17 (previous) (diff)

Changed 20 months ago by sanderd17

comment:11 Changed 20 months ago by sanderd17

So, updated patch with removed newline and updated years.

comment:12 Changed 20 months ago by fatherbushido

I tested the patch but i didn't look at the code. It works nice. I tried adding <variant file="biped/attack_capture.xml"/> to other infantry unit and it works as expected.

comment:13 Changed 19 months ago by elexis

Keywords: review removed
Resolution: fixed
Status: newclosed

Fixed by r18054 and r18056

comment:14 Changed 19 months ago by Palaxin

Yaaaaay, I was also getting mad when I added/changed animations for all infantry units in my mod. This is what I have been dreaming about :)

Note: See TracTickets for help on using tickets.