Simplifying animation frame wml

Discussion of all aspects of the game engine, including development of new and existing features.

Moderator: Forum Moderators

User avatar
8680
Moderator Emeritus
Posts: 742
Joined: March 20th, 2011, 11:45 pm
Location: The past

Re: Simplifying animation frame wml

Post by 8680 »

Boucman wrote:slightly favor ~ because - might lead to ambiguities with file names
Filenames, and also negative numbers, esp. if the string-formats of numbers are being preserved (image-[08-10].png becoming image-08.png,image-09.png,image-10.png rather than image-8.png,image-9.png,image-10.png).
Boucman wrote:image="units/human-loyalists/spearman.png,units/human-loyalists/spearman-stand-[1:400,2:150,3:150,4:300,5~6:100].png"
I too prefer this syntax, with related expansions confined to the same bracket-block, rather than being spread across multiple bracket-blocks or multiple attributes.
Boucman wrote:not coupling the different brackets within one line [...] expansion shouldn't spread between blocks
I concur, and I note that this allows recursive expansion with multiple bracket-blocks, thus:

Code: Select all

$ lua -i ExpandProgressiveFrames.lua 
Lua 5.2.1  Copyright (C) 1994-2012 Lua.org, PUC-Rio
> return expandProgressiveFrames("image-[1~5].png")
image-1.png,image-2.png,image-3.png,image-4.png,image-5.png
> return expandProgressiveFrames("image-[1~2][3~4].png")
image-13.png,image-14.png,image-23.png,image-24.png
> return expandProgressiveFrames("image-[1~2][3~4][5~6].png")
image-135.png,image-136.png,image-145.png,image-146.png,image-235.png,image-236.png,image-245.png,image-246.png
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Simplifying animation frame wml

Post by Coffee »

Reading all the latest replies I am relieved that others are starting to see the difficulties in getting proper syntax for frames, etc. :)

Oh, appologies in advance for the long post ;)
Boucman wrote:~ vs - : I don't mind as long as it's compatible, slightly favor ~ because - might lead to ambiguities with file names
Well. To get a sizeable reduction in complexity, I think we need to combine "[bowman,bowman-attack-1] and similar occurrences, as they are very common throughout all the default units. With the '~' character as the range separator this is possible and you can't get a filename with a '~' in it, leaving the widest range of possibilities open. This is why I think it necessary to go with '~' over '-'
Boucman wrote:duration : i don't like where you're going... duration is the duration of the frame. it should be split evenly (or depending on the : parameter) between progressive parameters. you are changing the philosophy with this syntax

basically

Code: Select all

[frame]
  duration=250
  image=image1:50,image2,image3
should display the images with a specified duration for the specified amount and the ones that arn't specified by dividing whatever time is left so here :

image1 : 50ms
image2 : 100ms
image3 : 100ms

that's how it works and I like that design
With the duration, halos, and images, I think we need to be very clear what we are aiming for or risk getting something that will need to be cleaned up at a later date. I'll try to elaborate on what I meant to say last post, but didn't quite make it there. You guys have convinced me that we should ditch the duration list idea BTW.

From looking at the code there are few instances where the animation blocks are not well ordered. One example of a badly ordered animation block (actually the only default one that must be completely rewritten due to 'begin' being depreciated and gaps employed) is the FAERIE_FIRE macro and its use for Elvish Lord line and Elvish Sorceress line. However, most have easy to discern patterns that follow in a logical time order.

I know Boucman and others may not initially like what I'd like to propose for the syntax, but I think you may come around if I explain what I would like to achieve more clearly and why it will be easier to read with only one block for each 'frame.'

Currently the unit_frame (or missile_frame) is split into 2 distinctly differently processed parts -- the image/image_diagonal/image_mod/duration and halo/offset. The image part is single entry only and takes the duration as its total, whilst the halo part specifies its own durations or divides evenly the duration to fit into the single 'frame.' If the 2 do not match up, then the main duration parameter takes precedence and cuts the halo short. My expansion patches so far have not changed this basic functioning and my planned patch would of course not change backwards compatibility.

One main thing I think is a worthy goal is to get rid of most of the duration parameters around the place unless they convey meaning that is kept when expansion is used (i.e. when it makes sense to explicitly say how long a series of frames should last and divide the internal time chunks evenly). This would actually load faster if we set the duration dynamically as the progressive_string is loaded (but that could come later).

Technically all I would do is read a image/image_diagonal/image_mod list as a virtual series of frames and match the halos to these frames (possibly cutting or extending them across multiple frames to match). Actually, after thinking about it, none of the image lists or halo lists would require matching of size to work.
Boucman wrote:taking your final example, and changing to the syntax I want

Code: Select all • Expand
#spearman standing animation
[frame]
#duration is optional since all frames have a time, should be the total
duration=1200
image="units/human-loyalists/spearman.png,units/human-loyalists/spearman-stand-[1:400,2:150,3:150,4:300,5~6:100].png"
#I'm open to change on that syntax, but that's the idea, if you want complicated timings then the [] block will be a bit complicated
#another syntax, I prefer tke first one, but it's also a possiblity
image="units/human-loyalists/spearman.png,units/human-loyalists/spearman-stand-[1~6]:[400,150,150,300,100,100].png"
[/frame]



the basic idea for the main syntax is that you first split the string at comma frontier, then do [] expension within the resulting blocks. expansion shouldn't spread between blocks and certainly not to neighbouring frames
By far the vast majority of cases has the duration for a series of frames set to a constant. For the few cases where this is not true doesn't it make more sense to keep the timings with the timings and the file numbers with the file numbers? Each 'block' or more rarely multiple block would be self-contained between the commas anyway, keeping the parts that logically are in sequence together.

I don't see that advantage in embedding the ':'s into the square bracket expansions, as this to me makes it less clear compared to the regular expressions which will occur far more frequently with the colon at the end. How does this play nicely with the standard format used now for halos or handle the case where a colon is accidentally specified in addition at the end and in the square bracket expansion? This is actually why I proposed initially to use a list for the duration (but I see that this would not reflect the duration of the frame properly).

What I was aiming for is a generic wesnoth (C++) function for WML that could be applied to sound file lists as well as unit animations and terrain animations. Embedding timing logic into it seems to me that it would detract from the general nature of the function and increase the complexity of the code? Maybe a duration list might solve this :P
8680 wrote:>return expandProgressiveFrames("image-[1~2][3~4].png")
image-13.png,image-14.png,image-23.png,image-24.png
wouldn't it make more sense to just do: expandProgressiveFrames("image-[13,14,23,24].png") for the same effect? I just don't see artists wanting to do much more than get a sequence of files going as an animation myself.

Examples of different approaches showing why what I suggest might be easiest to follow:

Code: Select all

#my suggestion of reduction for elvish enchantress magic attack
        [frame]
            image="units/elves-wood/enchantress-magic-[1,2,2,2,2].png:75"
            halo=halo/elven/nature-halo[1~5].png:75
            halo_x,halo_y=0,-28
        [/frame]
{SOUND:SLOW}
        [frame]
            image="units/elves-wood/enchantress-magic-2.png:75"
            halo=halo/elven/nature-halo6.png:75
            sound=entangle.wav
            halo_x,halo_y=0,-28
        [/frame]
        [frame]
            image="units/elves-wood/enchantress-magic-[2,1].png:75"
            halo=halo/elven/nature-halo[7,8].png:75
            halo_x,halo_y=0,-28
        [/frame]
#my suggestion of reduction of lich idle frame (*note not possible with '-' as range separator)
        [frame]
            image="units/undead-necromancers/lich[,-idle-1,,-idle-2,,-idle-2,,-idle-3,,-idle-3,,-idle-3,,-idle-2,,-idle-1,,-idle-2,,-idle-2,].png:110"
            halo=halo/undead/idle-flash-[1~21].png:110
        [/frame]
#other alternative proposed?
        [frame]
            image="units/undead-necromancers/lich[:110,-idle-1:110,:110,-idle-2:110,:110,-idle-2:110,:110,-idle-3:110,:110,-idle-3:110,:110,-idle-3:110,:110,-idle-2:110,:110,-idle-1:110,:110,-idle-2:110,:110,-idle-2:110,:110].png"
            halo=halo/undead/idle-flash-[1~21:110].png
        [/frame]
#my new suggestion for spearman idle
        [frame]
            image="units/human-loyalists/spearman-stand-[1~6].png:[150,150,300,100,100,100]"
        [/frame]
#my new suggestion for MISSILE_FRAME_FIREBALL
        [missile_frame]
            image="projectiles/fireball-n[-1,-2,,-2,-1].png:[40,100,125,55,30]"
            image_diagonal="projectiles/fireball-nw[-1,-2,,-2,-1].png:[40,100,125,55,30]"
            offset=0.0~0.10,0.1~0.30,0.3~0.60,0.7~0.8,0.8~0.9
        [/missile_frame]
            halo="projectiles/fireball-impact-[1~16].png:60"
            offset=1.0
        [/missile_frame]
User avatar
8680
Moderator Emeritus
Posts: 742
Joined: March 20th, 2011, 11:45 pm
Location: The past

Re: Simplifying animation frame wml

Post by 8680 »

Coffee wrote:wouldn't it make more sense to just do: expandProgressiveFrames("image-[13,14,23,24].png") for the same effect?
Yes, I suppose it would. Apologies for persistently not realizing the potential of your excellent comma-lists. I’m sure there are use cases where my recursive multiple-block expansion would be preferable, but none spring readily to mind.
Coffee wrote:

Code: Select all

#my suggestion of reduction of lich idle frame (*note not possible with '-' as range separator)
        [frame]
            image="units/undead-necromancers/lich[,-idle-1,,-idle-2,,-idle-2,,-idle-3,,-idle-3,,-idle-3,,-idle-2,,-idle-1,,-idle-2,,-idle-2,].png:110"
            halo=halo/undead/idle-flash-[1~21].png:110
        [/frame]
#other alternative proposed?
        [frame]
            image="units/undead-necromancers/lich[:110,-idle-1:110,:110,-idle-2:110,:110,-idle-2:110,:110,-idle-3:110,:110,-idle-3:110,:110,-idle-3:110,:110,-idle-2:110,:110,-idle-1:110,:110,-idle-2:110,:110,-idle-2:110,:110].png"
            halo=halo/undead/idle-flash-[1~21:110].png
        [/frame]
Where was it proposed to embed :durations into expansion blocks even if they’re all equal?
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Simplifying animation frame wml

Post by Coffee »

8680 wrote:Where was it proposed to embed :durations into expansion blocks even if they’re all equal?
I suppose it wasn't. I just wanted to show that with comma lists the embedded durations might look a bit strange.

Many different ways to solve this are of course possible, but I'd like to show the following example:
file list: unit-a-attack-1-ne.png:100,unit-b-attack-2-ne.png:100,unit-c-attack-3-ne.png:200,something-else.png
possible expansion using multiple brackets, self-contained between major comma separators:
unit-[a,b,c]-attack-[1~3]-ne.png:[100,100,200],something-else.png

With alternative expansions with embedded timings syntax, how do we resolve:
a[1:10,2:20]-[3:30,4:40].png:100
Would we throw an error if more than one expansion is used? Would we make it recursive (running into timing problems then as they could be defined multiple times)? Would we depreciate specifying timing with a colon at the end of a string?

I should probably add that we need to solve the problem of possible different total frame durations for image and image_diagonal. Sorry to go in circles here, but a slightly differently proposed duration list might solve all this easily.

We could just say that image and image_diagonal cannot specify within them the timing (as it is now) and that the duration must come from the duration parameter and as a bonus it would eliminate duplication of specifying timings.

Here the following rules would apply:
- image/image_diagonal/image_mod lists would not specify timing or an error would be thrown
- the image/image_diagonal/image_mod list sizes would all be the same or not present, else an error is thrown
- If the duration parameter is single, then the image/image_diagonal/image_mod lists and halo lists duration would be divided evenly (they could be different sizes)
- If not specified, would be calculated from the halo specified timings (mainly for halo only animations and items)

In examples:

Code: Select all

[frame]
duration=300
image=image[1~3]-n.png
image-diagonal=image[1~3]-ne.png
[/frame]
#would be equivalent to:
[frame]
duration=100,100,100
image=image[1~3]-n.png
image-diagonal=image[1~3]-ne.png
[/frame]

[frame]
duration=300
image=image[1~3]-n.png
image-diagonal=image[1~3]-ne.png
halo=halo[1~6].png
[/frame]
#would be equivalent to
[frame]
duration=100,100,100
image=image[1~3]-n.png
image-diagonal=image[1~3]-ne.png
halo=halo[1~6].png:50
[/frame]
Perhaps this would satisfy everyone?
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Re: Simplifying animation frame wml

Post by Boucman »

ok, answring a couple of points

~ vs - : ok i'm convinced i'll apply that part of the patch

now to be big debate...

there are all sorts of good stuff flying around but the general conversation is starting to be confusing... so i'll do my best to answer what is flying around

there are various proposals that discuss changing the duration of the frame. This changes the way frames work wich is a totally different thing from simply changing how progressive parameters work. I am not totally closed to changing that but it will be something that will be hard to sell.

The current way of calculating duration wasn't easy to come by and took into account all sorts of constraints. here are a couple of explanations to help people understand.

* an animation can't have holes, that's why the begin= end= syntax is deprecated it led to problems of holes and we didn't handle unordered frames at all. thus the duration is equivalent to end-begin, and the begining time of the animation is the begin= of the first frame is used if not explicitely provided.
* with that in mind, timing of animations is entirely defined by the animation's start_time= and each frame's duration. No need to deal with begin= and end=

* if an animation has a duration= then this is the duration of that frame. it will be "forced" on other parameters that would contradict it. it has priority. if a halo= image= or image_diagonal= has different timings, they will be cut short. that's fine. duration= has priority
* if there is no duration= we are in a "best effort" mod. We take the halo= progressive parameter and use it as a duration. this is a failsafe and i'm happy with that. we could do something smarter (search for a progressive parameter that has a completely defined time, take the longest progressive parameter and use it as duration...) but that's a different problem, and not a problem I want to discuss here. let's keep the discussion simple.

So, basically, unless it's reaaaaaly needed, I don't want to change how duration is calculated in this particular patch. this patch is about extending [] in strings, not about the frame block.

ok. So we are expanding a string, that's all we are doing, we are not expanding an animation string, we are expanding a string into a list of strings, with no knowledge of what the resulting strings mean...

Now, I think we all agree that the "topmost" split is at the comma character. i.e we have a list of comma separated strings. We also agree that a string from the comma separated list with a single [] within it will multiply using the comma separated list of string within it (and ranges using ~)

so basically

a,b,c[1~5]

is expanded to the list (I keep it comma separated but it's a vector<string>)

a,b,c1,c2,c3,c4,c5

this is already a very powerfull syntax, since you can mix an match ~ and non~ syntax to do stuff like

a,b,c[1~5,6,2,5~1]

again this simple string replacement, not animation expansion. in particular, there is no special treatement for the ":" character.

so

img[1:50,2:50,3:100]

comes for free with the rules we already have. moreover the most common case (all elements have the same fixed duration) becomse

img[1~3]:50

simple enough

At this point what we have is really powerfull and extremely simple. And I am wondering if doing something more complicated from a risk/use-case point of view.

looking at Coffee's proposed syntax examples the only two syntax that are not covered by this are

halo=halo/undead/idle-flash-[1~21:110].png

in that case, the parsing is "too smart" because it knows about the ":" and do smart stuff with it.
There are a couple of cases in the examples where there is the same idea, but all of theses are replacable with my simple rule.

in other word : the rules for expansion is simple and easy to explain. we don't recommand having the :duration part in or out the expansion because we don't care. both work, it depends on your use case


the second example that doesn't work with this proposal is the following (and a couple of similar syntax)

image="units/human-loyalists/spearman-stand-[1~6].png:[150,150,300,100,100,100]"

here the question that is asked (and that I havn't answered yet) is how do we expand when there are multiple [] blocks within a substring

so far there are multiple proposals floating around

1) all blocks must have the same size, and we expand by matching
[1~3][a,b,c] => 1a,2b,3c

1b) same but if they don't have the same size repeat the last element/do something smart

2) work recursively and produce all combinations
[1~3][a,b,c] => 1a,2a,3a,1b,2b,3b

3) that's too complicated, just refuse it.

I personally would tend to go to number 3 at this point, with maybe number 1 as a second best, though I am not sure how to handle the error case. I don't want to die in an error. probably print an error on stdout and repeat the last element for the shorter blocks.


ok, is the proposal clear ? I think it is, but i'm not completely sure, those things are hard to describe

secondly, does it cover the need ? other ideas ? did I miss something ?
Fight key loggers: write some perl using vim
User avatar
Sleepwalker
Art Contributor
Posts: 416
Joined: October 23rd, 2008, 6:34 am
Location: Sweden

Re: Simplifying animation frame wml

Post by Sleepwalker »

Reading this through multiple times to get it through my thick skull I now understand everything proposed here. I'm not sure what way would be the best. It would be nice to be able to put multiple brackets as Boucman put in the 1st suggestion.

Could sounds like hit and miss in the case of attacks have a start time specified? Whether in the flame block or outside. It would prevent having to split the animation into several frame blocks.

It would be great to have the animations take less space/ have better overview, as long as it doesn't mean reduced control. :D
Sometimes we must be hurt in order to grow, fail in order to know, lose in order to gain, and sometimes we must have to be broken so we can be whole again...
- Nercy Masayon
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Simplifying animation frame wml

Post by Coffee »

Hi and apologies again for any brain hurt in reading all the frame spec posts -- but it seems to be an evolving actual spec coming out.
Boucman wrote:there are various proposals that discuss changing the duration of the frame. This changes the way frames work wich is a totally different thing from simply changing how progressive parameters work. I am not totally closed to changing that but it will be something that will be hard to sell.
Well, I am not a salesman -- which is a good thing too or I would be starving :P In fact I'd like to try the politicians tactic and try to convince you that what you want and what I'm proposing (albeit in evolving stages) is actually the same thing :)

I understand that it took a while to get to the current frame WML spec, and I am taking pains to make my proposal for frame expansion not interfere at all with the current WML -- i.e. any current WML will continue to work after the expansion spec is coded and the patch applied.
Boucman wrote:So, basically, unless it's reaaaaaly needed, I don't want to change how duration is calculated in this particular patch. this patch is about extending [] in strings, not about the frame block.
That was true for the halo patch square expansion, which already had support for coma lists and colon timings. Frame 'image' and 'image_diagonal' currently do not have support for comma lists or internal timings, so I suggest that we keep it that way when we expand to cut down on branching and duplication when the same timing have to be specified for 'image', 'image_diagonal' and 'halo' in the same frame block. So essentially this is a different type of patch we are going to need from the halo one.
Boucman wrote:img[1:50,2:50,3:100]

comes for free with the rules we already have. moreover the most common case (all elements have the same fixed duration) becomse

img[1~3]:50
I think you are mistaken a bit here. With the current patch any '~' character has to have only numbers either side and an 'atoi' c++ function is used on them. If the colon was preserved, then

image[1:50,20:50,3:100]-n.png becomes...
image1:50-n.png,image2:50-n.png,image3:50-n.png

unless extra processing put the colon stuff at the end of the string, making it an animation specific function and also requiring extra error checking to make sure that it is not doubly specified. i.e.
not "image[1:50,2:100].png:50" or "image[1:50,2:50]-[a:100,b:100].png"
Boucman wrote:So, basically, unless it's reaaaaaly needed, I don't want to change how duration is calculated in this particular patch. this patch is about extending [] in strings, not about the frame block.
And it won't. To this end I propose yet another (more refined) effort at tackling the expansion of the frame 'image' and co. parameters. That is:

"image[49,50*6,49].png" expands to:
"image49.png,image50.png,image50.png,image50.png,image50.png,image50.png,image50.png,image49.png"

as actually occurs in DRAKE_FIRE_ANIM macro:

Code: Select all

...
        impact_burst_start_time=-160
        [impact_burst_frame]
            duration=320
            halo=misc/blank-hex.png:1,projectiles/fire-burst-small-[1~8].png:[39,40*6,39],misc/blank-hex.png:1
            offset=1.0
            layer=1
        [/impact_burst_frame]

        flame_trail_1_start_time=-450
        flame_trail_2_start_time=-400
        flame_trail_3_start_time=-350
        flame_trail_4_start_time=-300
        flame_trail_5_start_time=-250
        [flame_trail_1_frame]
            duration=500
            halo=misc/blank-hex.png:1,projectiles/fire-breath-[1~10].png:[49,50*8,49],misc/blank-hex.png:1
            halo_x={OFFSET_X}~0:300,0
            halo_y={OFFSET_Y}~0:300,0
            offset=0.0~1.0:300,1.0~1.25:200
        [/flame_trail_1_frame]
        [flame_trail_2_frame]
            duration=500
            halo=misc/blank-hex.png:1,projectiles/fire-breath-[1~10].png:[49,50*8,49],misc/blank-hex.png:1
            halo_x={OFFSET_X}~0:300,0
            halo_y={OFFSET_Y}~0:300,0
            offset=0.0~1.0:300,1.0~1.20:200
        [/flame_trail_2_frame]
        [flame_trail_3_frame]
            duration=500
            halo=misc/blank-hex.png:1,projectiles/fire-breath-[1~10].png:[49,50*8,49],misc/blank-hex.png:1
            halo_x={OFFSET_X}~0:300,0
            halo_y={OFFSET_Y}~0:300,0
            offset=0.0~1.0:300,1.0~1.15:200
        [/flame_trail_3_frame]
        [flame_trail_4_frame]
            duration=500
            halo=misc/blank-hex.png:1,projectiles/fire-breath-[1~10].png:[49,50*8,49],misc/blank-hex.png:1
            halo_x={OFFSET_X}~0:300,0
            halo_y={OFFSET_Y}~0:300,0
            offset=0.0~1.0:300,1.0~1.10:200
        [/flame_trail_4_frame]
        [flame_trail_5_frame]
            duration=500
            halo=misc/blank-hex.png:1,projectiles/fire-breath-[1~10].png:[49,50*8,49],misc/blank-hex.png:1
            halo_x={OFFSET_X}~0:300,0
            halo_y={OFFSET_Y}~0:300,0
            offset=0.0~1.0:300,1.0~1.05:200
        [/flame_trail_5_frame]
        ...
How does this help with 'duration' and 'image' parameters you say? Well, if 'image' is in a list, in the code it actually should mean that you are expressing the equivalent of multiple frames. So, now you can see that, for example, the lich idle animation is 21 frames long with each frame having a duration of 110.

Below I have included the lich animation WML file with changes applied to see how it might work with a real unit

Code: Select all

#textdomain wesnoth-units
[unit_type]
    ...
    {DEFENSE_ANIM "units/undead-necromancers/lich-defend.png" "units/undead-necromancers/lich.png" {SOUND_LIST:LICH_HIT} }
   ...

    [idle_anim]
        {STANDARD_IDLE_FILTER}
        start_time=0
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
        [/frame]
        [frame]
			duration=[110*21]
            image="units/undead-necromancers/lich[,-idle-1,,-idle-2,,-idle-2,,-idle-3,,-idle-3,,-idle-3,,-idle-2,,-idle-1,,-idle-2,,-idle-2,].png"
            halo=halo/undead/idle-flash-[1~21].png
        [/frame]
    [/idle_anim]

    [recruiting_anim]
        [filter_second]
            race=undead
        [/filter_second]
        start_time=-300
        [frame]
            duration=75,75
            image="units/undead-necromancers/lich-magic-[1,2].png"
        [/frame]
        [frame]
            duration=75
            image="units/undead-necromancers/lich-magic-3.png"
            halo=halo/undead/black-magic-1.png
        [/frame]
        [frame]
            duration=75
            image="units/undead-necromancers/lich-magic-3.png"
            sound=magic-dark-big.ogg
            halo=halo/undead/black-magic-2.png
        [/frame]
        [frame]
            duration=200
            image="units/undead-necromancers/lich-magic-3.png"
            halo=halo/undead/black-magic-[3~5].png:[75,75,50]
        [/frame]
        [frame]
            duration=50,50
            image="units/undead-necromancers/lich-magic-[2,1].png"
        [/frame]
        [frame]
            duration=1
            image="units/undead-necromancers/lich.png"
        [/frame]
    [/recruiting_anim]
    [attack_anim]
        [filter_attack]
            range=ranged
        [/filter_attack]
        [missile_frame]
            begin=-200
            end=0
            image="projectiles/darkmissile-n.png"
            image_diagonal="projectiles/darkmissile-ne.png"
        [/missile_frame]

        start_time=-300
        [frame]
            duration=75,75
            image="units/undead-necromancers/lich-magic-[1,2].png"
        [/frame]
        [frame]
            duration=75
            image="units/undead-necromancers/lich-magic-3.png"
            halo=halo/undead/black-magic-1.png
        [/frame]
        [frame]
            duration=75
            image="units/undead-necromancers/lich-magic-3.png"
            sound_hit=magic-dark-big.ogg
            sound_miss=magic-dark-big-miss.ogg
            halo=halo/undead/black-magic-2.png
        [/frame]
        [frame]
            duration=200
            image="units/undead-necromancers/lich-magic-3.png"
            halo=halo/undead/black-magic-[3~5].png:[75,75,50]
        [/frame]
        [frame]
            duration=50
            image="units/undead-necromancers/lich-magic-[2,1].png"
        [/frame]
    [/attack_anim]
    [attack_anim]
        [filter_attack]
            name=touch
        [/filter_attack]
        start_time=-250
        [frame]
            duration=50,100
            image="units/undead-necromancers/[lich,lich-melee-1].png"
        [/frame]
        [frame]
            duration=200
            image="units/undead-necromancers/lich-melee-2.png"
            sound_hit=wail-sml.wav
            sound_miss={SOUND_LIST:MISS}
        [/frame]
        [frame]
            duration=75,75
            image="units/undead-necromancers/[lich-magic-1,lich].png"
        [/frame]
    [/attack_anim]
[/unit_type]
Sleepwalker wrote: Could sounds like hit and miss in the case of attacks have a start time specified? Whether in the flame block or outside. It would prevent having to split the animation into several frame blocks.
I think we should just depreciate the "[if] hits=yes [else]" blocks altogether. With macro expansion they are not needed. I suggest we of course keep them for a while for backwards compatibility.

To replace "[if] hits=yes [else]" animation blocks I suggest we introduce a "sound_miss" and "sound_hit" set of tags. Example usage in the lich code above.

For other cases we add a "hits=yes" or "hits=miss" tag to frames, and we already have a top-level "hits=yes" or "hits=miss" to whole animation blocks. This covers all possible usage cases and could make the "[if] hits=yes [else]" syntax redundant if applied.

To get a sound to start at any time you would just use a duration with the same frame without a sound for the delay for which you want for the sound and then the next frame you would have the same image and the sound. 'Begin' and 'end' style tags are supposed to be depreciated.

Lastly:
Sleepwalker wrote:Reading this through multiple times to get it through my thick skull I now understand everything proposed here. I'm not sure what way would be the best. It would be nice to be able to put multiple brackets as Boucman put in the 1st suggestion.
That is currently how the my applied patch works (by matching), and includes appropriate error handling. I would be happy to recode it if a consensus is reached for another method.
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Re: Simplifying animation frame wml

Post by Boucman »

Coffee wrote:
Boucman wrote:img[1:50,2:50,3:100]

comes for free with the rules we already have. moreover the most common case (all elements have the same fixed duration) becomse

img[1~3]:50
I think you are mistaken a bit here. With the current patch any '~' character has to have only numbers either side and an 'atoi' c++ function is used on them. If the colon was preserved, then

image[1:50,20:50,3:100]-n.png becomes...
image1:50-n.png,image2:50-n.png,image3:50-n.png
no, I think I understand what you mean, if you look at my example, it's correct wrt the current expansion, and I am fine with that

your example above would need to be coded as

image[1-n.png:50,2-n.png:50,3-n.png:100]

it's not much of a simplification, but the expansion rules are simple and I like that. no special cases for timings
Coffee wrote:
Boucman wrote:So, basically, unless it's reaaaaaly needed, I don't want to change how duration is calculated in this particular patch. this patch is about extending [] in strings, not about the frame block.
And it won't. To this end I propose yet another (more refined) effort at tackling the expansion of the frame 'image' and co. parameters. That is:

"image[49,50*6,49].png" expands to:
"image49.png,image50.png,image50.png,image50.png,image50.png,image50.png,image50.png,image49.png"
ok, using * to multiply frames could be usefull,
Coffee wrote:
How does this help with 'duration' and 'image' parameters you say? Well, if 'image' is in a list, in the code it actually should mean that you are expressing the equivalent of multiple frames. So, now you can see that, for example, the lich idle animation is 21 frames long with each frame having a duration of 110.

Below I have included the lich animation WML file with changes applied to see how it might work with a real unit

Code: Select all

#textdomain wesnoth-units
[unit_type]
    ...
    {DEFENSE_ANIM "units/undead-necromancers/lich-defend.png" "units/undead-necromancers/lich.png" {SOUND_LIST:LICH_HIT} }
   ...

    [idle_anim]
        {STANDARD_IDLE_FILTER}
        start_time=0
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
        [/frame]
        [frame]
			duration=[110*21]
            image="units/undead-necromancers/lich[,-idle-1,,-idle-2,,-idle-2,,-idle-3,,-idle-3,,-idle-3,,-idle-2,,-idle-1,,-idle-2,,-idle-2,].png"
            halo=halo/undead/idle-flash-[1~21].png
        [/frame]
    [/idle_anim]

[/unit_type]
so duration expands to

duration=110,110,110,110....

Again, I don't like that. I think you point a real problem with the animation syntax, and that limitation is that you can't add timings in image= and image_diagonal= That's a suprise to me but I just checked and you are indeed right...

so let's change the unit_frame to use a new progressive_image_locator for images, so the syntax is compatible

as I said, I don't like changing the duration
Coffee wrote:
Sleepwalker wrote: Could sounds like hit and miss in the case of attacks have a start time specified? Whether in the flame block or outside. It would prevent having to split the animation into several frame blocks.
I think we should just depreciate the "[if] hits=yes [else]" blocks altogether. With macro expansion they are not needed. I suggest we of course keep them for a while for backwards compatibility.
Ok... you miss 90% of the reason why we have a [if] the point is that it is not limited to hit, so let's not touch at [if] for the moment

and I hate that syntax too, but it's reallly very complicated to make evole. To make that syntax sane we need to have a clean way to separate conditions from animation instructions, but we don't have that

Coffee wrote: Lastly:
Sleepwalker wrote:Reading this through multiple times to get it through my thick skull I now understand everything proposed here. I'm not sure what way would be the best. It would be nice to be able to put multiple brackets as Boucman put in the 1st suggestion.
That is currently how the my applied patch works (by matching), and includes appropriate error handling. I would be happy to recode it if a consensus is reached for another method.
yes, let's focus of if/how we want to expand the multiple brackets...

more seriously, I really think that having progressive image is the way to go, rather than expanding durations... duration is frame-wide, it shouldn't have multiple fields...
Fight key loggers: write some perl using vim
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Simplifying animation frame wml

Post by Coffee »

Boucman wrote:no, I think I understand what you mean, if you look at my example, it's correct wrt the current expansion, and I am fine with that

your example above would need to be coded as

image[1-n.png:50,2-n.png:50,3-n.png:100]

it's not much of a simplification, but the expansion rules are simple and I like that. no special cases for timings
I see what you are saying, and would agree wholeheartedly if there weren't cases like this:

Code: Select all

#define MISSILE_FRAME_MUZZLE_FLARE_HIT_DIAG_SOUTH START_X START_Y
    missile_start_time=-250
    missile_offset=0.5~1.1:250,1.1~1.5:200,1.5~1.7:360
    missile_halo_x={START_X}~0
    missile_halo_y={START_Y}~0
    [missile_frame]
        halo="projectiles/muzzle-flash-se-[1~3].png~FL(vertical):[100,80,70],projectiles/muzzle-flash-[4~14].png:60"
    [/missile_frame]
#enddef
With matching expansion it is easy to follow. This is impossible to do with the expansion you suggest because of the "~FL(vertical)" containing the '~' character. If we, say, replaced this by a '>' character it would look like:
halo="projectiles/muzzle-flash-se-[1.png~FL(vertical):100,2.png~FL(vertical):80,3.png~FL(vertical):70],projectiles/muzzle-flash-[4>14].png:60"
To me this is getting a bit crowded and any color shifting functions, etc. would make it hard to tell the timings from the frame numbers/etc.
Boucman wrote:so duration expands to

duration=110,110,110,110....

Again, I don't like that. I think you point a real problem with the animation syntax, and that limitation is that you can't add timings in image= and image_diagonal= That's a suprise to me but I just checked and you are indeed right...

so let's change the unit_frame to use a new progressive_image_locator for images, so the syntax is compatible
I suppose this could be done. In a coding sense, how I see this being implemented either way is by tricking the c++ code to thinking that one is really putting in mutiple frames if multiple 'image' tags are seen. This would be the easiest to implement and would require no real specialized knowledge of the inner workings of the functions. We wouldn't see any reduced RAM, but we would see a substantially smaller cache and slightly faster intial loading speed. I'd need some help later to optimize this properly as internally single frames if you wanted to help me do this.

The approach and the fact that we would be doubling up on most timing paramters makes me want to suggest the duration lists instead. But really, we could do it both ways. I just don't want to end up writing 2 patches after anyone changes their mind.

I suppose we should leave the "if" stuff for now, but with a bit of thought I think we can do away with the animation "if, else" syntax and not lose any functionality without much effort from a coding point of view in the same way that you could specify a different sound_frame to start a sound at any time. This perhaps for a later discussion, with popcorn :D

Sorry if it looks like I am stomping all over well established turf. All I wanted to do initially was actually the minimal work to expand the current code (which has the advantage of being stable and mature) so that I could more easily work on my era for a UMC campaign that I am making :P Now I've gotten more into it and would really like to finish this properly.
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Re: Simplifying animation frame wml

Post by Boucman »

Coffee wrote: I see what you are saying, and would agree wholeheartedly if there weren't cases like this:

Code: Select all

#define MISSILE_FRAME_MUZZLE_FLARE_HIT_DIAG_SOUTH START_X START_Y
    missile_start_time=-250
    missile_offset=0.5~1.1:250,1.1~1.5:200,1.5~1.7:360
    missile_halo_x={START_X}~0
    missile_halo_y={START_Y}~0
    [missile_frame]
        halo="projectiles/muzzle-flash-se-[1~3].png~FL(vertical):[100,80,70],projectiles/muzzle-flash-[4~14].png:60"
    [/missile_frame]
#enddef
With matching expansion it is easy to follow. This is impossible to do with the expansion you suggest because of the "~FL(vertical)" containing the '~' character. If we, say, replaced this by a '>' character it would look like:
halo="projectiles/muzzle-flash-se-[1.png~FL(vertical):100,2.png~FL(vertical):80,3.png~FL(vertical):70],projectiles/muzzle-flash-[4>14].png:60"
To me this is getting a bit crowded and any color shifting functions, etc. would make it hard to tell the timings from the frame numbers/etc.
ok, i'm convinced, let's go for parallel expansion within a "comma zone"
Coffee wrote:
Boucman wrote:
so let's change the unit_frame to use a new progressive_image_locator for images, so the syntax is compatible
I suppose this could be done. In a coding sense, how I see this being implemented either way is by tricking the c++ code to thinking that one is really putting in mutiple frames if multiple 'image' tags are seen.
huh ? no....

if images can be stored as progressive params, you can simply give the current time at animation time and get the right image

basically you copy the progressive_string class to a new progressive_image class that uses image::locator change the uses everywhere and bam...

either I am missing something obvious or it's trivial to do...
Coffee wrote:
The approach and the fact that we would be doubling up on most timing paramters makes me want to suggest the duration lists instead. But really, we could do it both ways. I just don't want to end up writing 2 patches after anyone changes their mind.
I don't see what you mean by doubling durations... yes each line has its own timing, rather than specifying them separately, but I think that's simpler to understant (but longer to write)

reducing WML size is not a goal per-se, it's anice thing to have but having a list of duration would be more confusing than usefull, I think...
Coffee wrote:
I suppose we should leave the "if" stuff for now, but with a bit of thought I think we can do away with the animation "if, else" syntax and not lose any functionality without much effort from a coding point of view in the same way that you could specify a different sound_frame to start a sound at any time. This perhaps for a later discussion, with popcorn :D
yeah, please leave that out....

that syntax is awful and I would be more than happy to get rid of it, but it's really a complicated can of worm. I'd gladly discuss changes with you... heck, i'll give you my full support for making this syntax coherent, but one problem at a time
Coffee wrote:
Sorry if it looks like I am stomping all over well established turf. All I wanted to do initially was actually the minimal work to expand the current code (which has the advantage of being stable and mature) so that I could more easily work on my era for a UMC campaign that I am making :P Now I've gotten more into it and would really like to finish this properly.
no no no, i'm really happy that someone is looking into that area of code, that's a great thing, stomping on everything is a normal part of learning the code, especially for such an area of code that has lot of small details to know and took quite some time to mature. Please continue, and don't be suprise if I am a bit more cautious than you, I probably have a better vision of the big problem, and that's how a team work...
Fight key loggers: write some perl using vim
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Simplifying animation frame wml

Post by Coffee »

Boucman wrote:
Coffee wrote: I see what you are saying, and would agree wholeheartedly if there weren't cases like this:

Code: Select all

#define MISSILE_FRAME_MUZZLE_FLARE_HIT_DIAG_SOUTH START_X START_Y
    missile_start_time=-250
    missile_offset=0.5~1.1:250,1.1~1.5:200,1.5~1.7:360
    missile_halo_x={START_X}~0
    missile_halo_y={START_Y}~0
    [missile_frame]
        halo="projectiles/muzzle-flash-se-[1~3].png~FL(vertical):[100,80,70],projectiles/muzzle-flash-[4~14].png:60"
    [/missile_frame]
#enddef
With matching expansion it is easy to follow. This is impossible to do with the expansion you suggest because of the "~FL(vertical)" containing the '~' character. If we, say, replaced this by a '>' character it would look like:
halo="projectiles/muzzle-flash-se-[1.png~FL(vertical):100,2.png~FL(vertical):80,3.png~FL(vertical):70],projectiles/muzzle-flash-[4>14].png:60"
To me this is getting a bit crowded and any color shifting functions, etc. would make it hard to tell the timings from the frame numbers/etc.
ok, i'm convinced, let's go for parallel expansion within a "comma zone"
I knew the politicians tactic would work eventually :D

So what do you think about the syntax looking like this for the lich (please ignore the if optimizations for now as you are right that this is for a later patch):

Code: Select all

...
    [idle_anim]
        {STANDARD_IDLE_FILTER}
        start_time=0
        [frame]
            image="units/undead-necromancers/lich.png:110"
        [/frame]
        [frame]
            image="units/undead-necromancers/lich[,-idle-1,,-idle-2,,-idle-2,,-idle-3,,-idle-3,,-idle-3,,-idle-2,,-idle-1,,-idle-2,,-idle-2,].png:110"
            halo=halo/undead/idle-flash-[1~21].png
        [/frame]
    [/idle_anim]

    [recruiting_anim]
        [filter_second]
            race=undead
        [/filter_second]
        start_time=-300
        [frame]
            image="units/undead-necromancers/lich-magic-[1,2].png:75"
        [/frame]
        [frame]
            image="units/undead-necromancers/lich-magic-3.png:75"
            halo=halo/undead/black-magic-1.png
        [/frame]
        [frame]
            image="units/undead-necromancers/lich-magic-3.png:75"
            sound=magic-dark-big.ogg
            halo=halo/undead/black-magic-2.png
        [/frame]
        [frame]
            image="units/undead-necromancers/lich-magic-3.png:200"
            halo=halo/undead/black-magic-[3~5].png:[75,75,50]
        [/frame]
        [frame]
            image="units/undead-necromancers/lich-magic-[2,1].png:50"
        [/frame]
        [frame]
            image="units/undead-necromancers/lich.png:1"
        [/frame]
    [/recruiting_anim]
    [attack_anim]
        [filter_attack]
            range=ranged
        [/filter_attack]
        missile_start_time=-200
        [missile_frame]
            duration=200
            image="projectiles/darkmissile-n.png"
            image_diagonal="projectiles/darkmissile-ne.png"
        [/missile_frame]

        start_time=-300
        [frame]
            image="units/undead-necromancers/lich-magic-[1,2].png:75"
        [/frame]
        [frame]
            image="units/undead-necromancers/lich-magic-3.png:75"
            halo=halo/undead/black-magic-1.png:75
        [/frame]
        [frame]
            image="units/undead-necromancers/lich-magic-3.png:75"
            sound_hit=magic-dark-big.ogg
            sound_miss=magic-dark-big-miss.ogg
            halo=halo/undead/black-magic-2.png:75
        [/frame]
        [frame]
            image="units/undead-necromancers/lich-magic-3.png:200"
            halo=halo/undead/black-magic-[3~5].png:[75,75,50]
        [/frame]
        [frame]
            image="units/undead-necromancers/lich-magic-[2,1].png:50"
        [/frame]
    [/attack_anim]
    [attack_anim]
        [filter_attack]
            name=touch
        [/filter_attack]
        start_time=-250
        [frame]
            image="units/undead-necromancers/[lich,lich-melee-1].png:[50,100]"
        [/frame]
        [frame]
            image="units/undead-necromancers/lich-melee-2.png:200"
            sound_hit=wail-sml.wav
            sound_miss={SOUND_LIST:MISS}
        [/frame]
        [frame]
            image="units/undead-necromancers/[lich-magic-1,lich].png:75"
        [/frame]
    [/attack_anim]
[/unit_type]
Where the unspecified timings are implicit based on the rules we already have and should make it easy to follow. Are we good to go with this and similar for other units? *note that I have pretty much removed basically all the duration tags except in missle_frames (I am still keen on removing redundancy where it interferes with ease of maintenance).

From what else you are saying it seems as though a "frame" can be any sequence of files that serves a logical block of animations. I'm good with this and your suggestion to use image_locator makes sense as we should reduce overall Wesnoth RAM overhead by a good fraction too if we do it like that (important with all the new running/etc. animations coming in). I'll give it a go if we are good to start but may need some help if I get stuck.
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Re: Simplifying animation frame wml

Post by Boucman »

looks perfect, i'm looking forward to your code...

as for the frame, you've pinned what they are pretty well. I'd suggest having the progressive_image_locator as a separate patch so I can proofread one piece at a time, but I think we agree on what is to be done

let's go for it
Fight key loggers: write some perl using vim
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Simplifying animation frame wml

Post by Coffee »

Boucman wrote:looks perfect, i'm looking forward to your code...

as for the frame, you've pinned what they are pretty well. I'd suggest having the progressive_image_locator as a separate patch so I can proofread one piece at a time, but I think we agree on what is to be done

let's go for it
You didn't have to wait long -- I've uplodaed a new preliminary patch for this :D

It turns out that you were right about the image_locator stuff. This works a treat and was simple to code. I think the artists will be happy that they can get smooth offsets for their animations now for both 'halos' and 'images'. After converting some units I've found this makes it better than the duration list approach, because they have the most freedom with the progressive_image approach.

However, I have a problem with the idle or 'standing' animations (e.g. the fencer). They don't loop properly now unless there is more than one frame block. I've seen the Easy Coding ideas section for a 'cycle' paramter. Is this what we need to fix this?
Last edited by 8680 on February 9th, 2013, 11:37 pm, edited 1 time in total.
Reason: Fixed broken link.
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Re: Simplifying animation frame wml

Post by Boucman »

ok, i'll look into your patch... i'm not sure how looping works anymore so i'll hve to look into it. don't hold your breath.

we are speaking about standing animations I guess, since idle animations don't loop

i'm not sure how it works anymore but I remember it was tricky...
Fight key loggers: write some perl using vim
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Simplifying animation frame wml

Post by Coffee »

Thanks for the hint on the does_not_change parameter. Looks like we're pretty much there.

Hence, I'd like to suggest a last (I think) simplification:

Code: Select all

        [frame]
            image="units/nagas/fighter-melee-2.png:100"
            [if]
               hits=yes
               sound={SOUND_LIST:SWORD_SWISH}
            [/if]
            [else]
                hits=no
                sound={SOUND_LIST:MISS}
            [/else]
        [/frame]
to replace

Code: Select all

        [if]
            hits=yes
            [frame]
                image="units/nagas/fighter-melee-2.png:100"
                sound={SOUND_LIST:SWORD_SWISH}
            [/frame]
        [/if]
        [else]
            hits=no
            [frame]
                image="units/nagas/fighter-melee-2.png:100"
                sound={SOUND_LIST:MISS}
            [/frame]
        [/else]
etc.

The idea is that we move the animation "if" block inside the frame and cut down on duplication. This would have the benefit of fixing a problem with the current set up -- as far as I can tell you can only have one "if/else" block within one animation block (this is completely undocumented, but I have found this from testing).
Post Reply