Simplifying animation frame wml

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

Moderator: Forum Moderators

User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Simplifying animation frame wml

Post by Coffee »

EDIT:
New syntax changes have made it in for wesnoth version 1.11.2. If there are problems or more discussion needed, now seems like a good time in the wesnoth development cycle to do so. Below is the history of these changes, you might want to scroll to near the end of the thread.


Hello

After playing around with some animations, I noticed that there is a lot of redundant WML that makes it hard to create new animations or follow what is happening easily.

So I suggest a possible approach to combat this that is backwards compatible with what there is now, along the same line that x and y positioning can be done for placing items/time of day spots/etc. (i.e. x=1-3,6 and y=1,4,5,6 for coords (1,1) (2,4) (3,5) (6,6))

So, how would this work for animations?

Well say you have a macro for MISSILE_FRAME_FIRE_BREATH_N (in animation_utils.cfg). Currently it is:

Code: Select all

#define MISSILE_FRAME_FIRE_BREATH_N OFFSET_POSITION
    # Animate a projectile for a fire-breath attack.
    [missile_frame]
        begin=-400
        end=100
        halo=projectiles/fire-breath-n-1.png:80,projectiles/fire-breath-n-2.png:80,projectiles/fire-breath-n-3.png:80,projectiles/fire-breath-n-4.png:80,projectiles/fire-breath-n-5.png:80
        halo_x,halo_y={OFFSET_POSITION}
    [/missile_frame]
#enddef
I suggest using square brackets where there is a sequence or comma separated file numbers in file names possible to simplify. This gives:

Code: Select all

#define MISSILE_FRAME_FIRE_BREATH_N OFFSET_POSITION
    # Animate a projectile for a fire-breath attack.
    [missile_frame]
        begin=-400
        end=100
        halo=projectiles/fire-breath-n-[1-5].png:80
        halo_x,halo_y={OFFSET_POSITION}
    [/missile_frame]
#enddef
See, much better and less prone to error.

I would suggest using square brackets with dash separated sequences or commas for not in sequence frames to simplify halos. This might even be quicker to load as it would just be a c++ expansion after reading the file to the long version we are already accustomed to (cutting out about half the disk reading time on average from looking at the macros). So in all, not much of a change and backwards compatible.

I would suggest that where the square brackets are used for the frame number, they might match to the duration and where the durations are short, just use the last specified one for all the remaining ones. i.e.

Code: Select all

halo=projectiles/frame[1-6]:[80,80,70,70,60,60]
So far so good. However, the image and image_diagonal frames could do with some simplifying too. Because the halos don't always match up this requires some effort to make work properly.

I suggest to do the same thing for image and image_diagonal frames and in the c++ code, just pretend that it is reading a series of missile_frame and such whole blocks of WML. Also, make the begin tag comma separatable, making the end the next begin in the list until the actual end which would remain specified. If not fully specified, could just average the remainder to get the proper begin and end times, ignoring any extra ones.

For exmaple the existing FIRE_BURST_SMALL

Code: Select all

#define FIRE_BURST_SMALL
    [missile_frame]
        begin=0
        end=75
        image="projectiles/fire-burst-small-1.png"
        image_diagonal="projectiles/fire-burst-small-1.png"
        offset=0.8
    [/missile_frame]
    [missile_frame]
        begin=75
        end=150
        image="projectiles/fire-burst-small-2.png"
        image_diagonal="projectiles/fire-burst-small-2.png"
        offset=0.83
    [/missile_frame]
    [missile_frame]
        begin=150
        end=225
        image="projectiles/fire-burst-small-3.png"
        image_diagonal="projectiles/fire-burst-small-3.png"
        offset=0.86
    [/missile_frame]
    [missile_frame]
        begin=225
        end=300
        image="projectiles/fire-burst-small-4.png"
        image_diagonal="projectiles/fire-burst-small-4.png"
        offset=0.89
    [/missile_frame]
    [missile_frame]
        begin=300
        end=375
        image="projectiles/fire-burst-small-5.png"
        image_diagonal="projectiles/fire-burst-small-5.png"
        offset=0.92
    [/missile_frame]
    [missile_frame]
        begin=375
        end=450
        image="projectiles/fire-burst-small-6.png"
        image_diagonal="projectiles/fire-burst-small-6.png"
        offset=0.95
    [/missile_frame]
    [missile_frame]
        begin=450
        end=525
        image="projectiles/fire-burst-small-7.png"
        image_diagonal="projectiles/fire-burst-small-7.png"
        offset=0.98
    [/missile_frame]
    [missile_frame]
        begin=525
        end=600
        image="projectiles/fire-burst-small-8.png"
        image_diagonal="projectiles/fire-burst-small-8.png"
        offset=1.0
    [/missile_frame]
#enddef
becomes

Code: Select all

[missile_frame]
        begin=0,75,150,225,300,375,450,525
        end=600
        image="projectiles/fire-burst-small-[1-8].png"
        image_diagonal="projectiles/fire-burst-small-[1-8].png"
        offset=0.8,0.83,0.86,0.89,0.92,0.95,0.98,1.0
[/missile_frame]
*note that I suggest that offset not have any square brackets, as this might be confusing with existing code

If a halo tag is present with square brackets as well as image/image_diagonal, then they are put into separate frames with seaprate halos. So to make this work (i.e. for FAERIE_FIRE) you would have separate frames as now and use the square brackets for the halo (as this is the faster changing of the 2). So if halo is present with square brackets it is expanded to a comma separated list as now, and if image/image_diagonal is present with square brackets, then the whole frame gets duplicated and sequenced.

I've attached an example animation-utils with the changes and it simplifies the code and actually removes half of the charaters in it. Of course this won't work without accompanying changes to the c++ code.
Attachments
animation-utils-new.cfg
(39.89 KiB) Downloaded 411 times
Last edited by Coffee on January 27th, 2013, 10:30 pm, edited 1 time in total.
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 »

Perhaps a tilde, rather than a hyphen, for consistency? AnimationWML uses the tilde to denote the range of progressive parameters; FilterWML uses the hyphen, as you mention, to denote the range of areas on the map.

Edit: Possible Lua function to process this. I may or may not convert it to C++ when I have the time.

Code: Select all

function expandProgressiveFrames(frameList)
    -- frameList = A string, e.g. "frame-[1~5].png".
    -- Returns expanded frameList.
    local q
    frameList, q = frameList: gsub("([^,%[%]%s]*)%[(%d+)~(%d+)%]([^,%s]*)",
        function(a, i, j, b)
            local r = a .. i .. b
            for n = i+1, j do
                r = ("%s,%s%d%s"): format(r, a, n, b)
            end
            return r
        end
    )
    return q ~= 0 and expandProgressiveFrames(frameList) or frameList
end
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Re: Simplifying animation frame wml

Post by Boucman »

i'm not against that syntax, but I think that the parsing of that area is already rather complicated,

8680 if you ever implement that, PM me (since i'm not really active with patches anymore) and I'll review it

also PM me if you hae code question with that area of code, I can help...
Fight key loggers: write some perl using vim
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 »

I was thinking that wherever the relevant attributes (image=, halo=, etc.) are read into the relevant structures, their values could be passed through the expand[acronym=or whatever this is going to be called]ProgressiveFrames[/acronym] function. Is this feasible? If so, how are the values represented — strings, vectors of strings? Should the function update the values in-place, or return modified values? If the function returns a modified value, should it be by copy or by move? And perhaps most importantly, where should I look to learn this myself?

(My apologies if this really needs to be private for some reason I overlooked.)
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Simplifying animation frame wml

Post by Coffee »

Boucman wrote:i'm not against that syntax, but I think that the parsing of that area is already rather complicated,

8680 if you ever implement that, PM me (since i'm not really active with patches anymore) and I'll review it
Um. I have actually implimented it already for fun and can post you a patch? It was easy to get the halo stuff working. :D
8680 wrote:I was thinking that wherever the relevant attributes (image=, halo=, etc.) are read into the relevant structures, their values could be passed through the expandProgressiveFrames function. Is this feasible? If so, how are the values represented — strings, vectors of strings? Should the function update the values in-place, or return modified values? If the function returns a modified value, should it be by copy or by move? And perhaps most importantly, where should I look to learn this myself?
I think this needs some thought. I just replaced the ProgressiveString entirely with ProgressiveStringExpand. Looking at the code there is a bucket load of redundant processing code, which is probably is the result of different people expanding things over time and not removing redundant code that they then replace with higher level (and more efficient) code.

I will follow this up with PM, maybe?
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:Um. I have actually implimented it already for fun [...]
My apologies, I assumed you were only at the idea stage. I’ll leave you to it, seeing as your knowledge of the Wesnoth codebase is far superior to mine (what’s a ProgressiveString?).
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Simplifying animation frame wml

Post by Coffee »

8680 wrote:
Coffee wrote:Um. I have actually implimented it already for fun [...]
My apologies, I assumed you were only at the idea stage.
Actually, I was and am on mobile Internet, so I had done the patch only to find that you posted some LUA code.
I’ll leave you to it, seeing as your knowledge of the Wesnoth codebase is far superior to mine (what’s a ProgressiveString?).
Lol. I could do with some help actually to get it right. I only looked at this code recently, but sat staring at it for so long that I believe I now understand the code paths :P

Attached is a patch (for SVN 56058M) that shows a working demo of all the halo functions converted into the new form for data/macros/animation-utils.cfg. It goes about half of the way to making it human readable IMO.

What I have learned:
- halo.cpp has an add function that is called all the time to add halos onto the screen graphics
- this is called by add_overlay and similar functions in game_display.cpp (and some other places) - not so important
- where the interesting stuff is in is the unit_frame.cpp, because the initialization of missile_frame/item/etc. with the 'halo' tag initializes the halo, which is a progressive_string function (the initialization is near the top of the file)
- this progressive_string initialization divies up the halo by comma separators and by time chunks
-- if duration is specified, the start of the frame is the missile_start_time and it goes until duration is up (this then divides up neatly any halos in a list into appropriate time segments if not specified uniquely after the colons)
-- otherwise a begin and end specify the begin and end frames
-- otherwise if no 'end' and no 'duration' tags are set, then an 'auto_duration' flag is set to true (from the patch) -- although currently this is unimplemented
- the splitting into time chunks and halo parts is done twice for some reason (again in halo.cpp), although the second time there is no need as it is already done
- the parenthetical_split function in src/serialization/string_utils.cpp I have extended to create a square_parenthetical_split function that also expands the '[]'s

So far so good. It even works for items, so you can do in WML:

Code: Select all

[item]
  x,y={X},{Y}
  halo=items/brazier-lit[1-8].png:100
[/item]
and it will work :)

But this leaves the frames still individually defined. Because I am not an artist, please tell me if I am on the right track??

I suggest that the following be adopted (slightly different than my initial proposal):
- frames would duplicate all content other than 'image' and 'image_diagonal', which must be the same length
- any halo/other square expansions would be for all frames like that instead of expanded 1 for 1 with the 'image'/'image_diagonal'

This should be easy to code, but I could do with a pointer as to what I might be intruding on by accident if anything -- as I don't really now how to fully test if I get this right or not??
Attachments
animation_halo_patch_with_macro_changes.patch
(55.81 KiB) Downloaded 415 times
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 »

It may be useful to have an optional step specifier, such that (e.g.) "frame-[1-5:2].png" would be equivalent to "frame-1.png,frame-3.png,frame-5.png". This could also be used to go backward, with "frame-[3-1:-1].png" being equivalent to "frame-3.png,frame-2.png,frame-1.png".
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Simplifying animation frame wml

Post by Coffee »

8680 wrote:It may be useful to have an optional step specifier, such that (e.g.) "frame-[1-5:2].png" would be equivalent to "frame-1.png,frame-3.png,frame-5.png". This could also be used to go backward, with "frame-[3-1:-1].png" being equivalent to "frame-3.png,frame-2.png,frame-1.png".
I thought about hard and, after converting all the instances in default units and macros, found it more useful to just have it so that [1-3] does 1,2,3 and [3-1] does 3,2,1. It seems that 1,3,5 would be rare and can be done by [1,3,5] anyway. Whether or not this goes against what is typically thought I don't know, as it would be easy to change.

I've put a full working patch for the halos, with auto_duration calculations when 'end' and 'duration' are unspecified, and also with all changes to default unit halos and macros here:
https://gna.org/patch/index.php?3604

I figure I should wait to see the feedback on that before doing the other half (autoexpanding the frames without halos).
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 »

Apologies again, I hadn’t thought of [1,3,5], which I concur is better.
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Re: Simplifying animation frame wml

Post by Boucman »

just to make things clearer since there seems to be a bit of confusion...

I'm all for discussing the syntax and the details openly here, the "send me a PM" was more a "wake me up when you decided what you want and you have a patch"

but even that is progressing well, i'll comment the patch detail in the patch itself, the syntax itself is better discussed here
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 »

Well, the halo half is committed to SVN and should be available for 1.11.2 if I am not mistaken.

I've now done a bit of thinking for the frames part, which will be harder to get right I think. Turns out 8680 was right about using '~' instead of '-', which I only figured out after looking at implementing the frames part :P *sigh*

This change is so that different image frames can be specified by comma lists, like so for the lich:

Code: Select all

    [idle_anim]
        {STANDARD_IDLE_FILTER}
        start_time=0
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-1.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich-idle-1.png"
            halo=halo/undead/idle-flash-2.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-3.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich-idle-2.png"
            halo=halo/undead/idle-flash-4.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-5.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich-idle-2.png"
            halo=halo/undead/idle-flash-6.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-7.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich-idle-3.png"
            halo=halo/undead/idle-flash-8.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-9.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich-idle-3.png"
            halo=halo/undead/idle-flash-10.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-11.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich-idle-3.png"
            halo=halo/undead/idle-flash-12.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-13.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich-idle-2.png"
            halo=halo/undead/idle-flash-14.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-15.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich-idle-1.png"
            halo=halo/undead/idle-flash-16.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-17.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich-idle-2.png"
            halo=halo/undead/idle-flash-18.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-19.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich-idle-2.png"
            halo=halo/undead/idle-flash-20.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
            halo=halo/undead/idle-flash-21.png
        [/frame]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
        [/frame]
    [/idle_anim]
Becomes:

Code: Select all

    [idle_anim]
        {STANDARD_IDLE_FILTER}
        start_time=0
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
        [/frame]
        [frame]
            duration=110
            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]
        [frame]
            duration=110
            image="units/undead-necromancers/lich.png"
        [/frame]
    [/idle_anim]
This will not only help for the lich, but most units to amalgamate the frames. Pretty much all animated units should see a file reduction of about a third to a half their size and even some of the halos can be shrunk further with this approach.

To summarize how I plan to finish this animation simplification (perhaps it should extend to sounds as well):
-- the number of entries in the 'image' list speficies completely how many frames this represents
-- if duration is specified as a list, there should be a corresponding 'image' list or an error is thrown
-- if duration is single and there is an 'image' list, the duration is set for all images
-- begin lists can be used in place of duration, with a single end parameter, and must be as long as the size of the 'image' list
-- 'image' lists must be the same size as 'image-diagonal' or 'image-mod' or the field not present
-- if timing is not specified in halo list entries, the time is divided evenly as before, but now between the total time of the frames (because this is easy to code and I think makes most sense)
-- offset list size should equal halo size if both are present or an error is thrown

Or in examples:

Code: Select all

       #spearman standing animation
        [frame]
            duration=400,150,150,300,100,100,100
            image="units/human-loyalists/spearman.png,units/human-loyalists/spearman-stand-[1~6].png"
        [/frame]

       #end of lich melee attack
        [frame]
            duration=75,75
            image="units/undead-necromancers/[lich-magic-1,lich].png"
        [/frame]

       #part of MISSILE_FRAME_FIREBALL not covered by halo expansion
        [missile_frame]
            duration=40,100,125,55,30
            image="projectiles/fireball-n[-1,-2,,-2,-1].png"
            image_diagonal="projectiles/fireball-nw[-1,-2,,-2,-1].png"
            offset=0.0~0.10,0.1~0.30,0.3~0.60,0.7~0.8,0.8~0.9
        [/missile_frame]

       #part of elvish shyde thorn attack
        [frame]
            duration=75
            image="units/elves-wood/druid-magic-[1-4].png"
            halo="halo/elven/nature-halo[1-4].png"
            halo_x,halo_y=0,-12
        [/frame]
AI
Developer
Posts: 2396
Joined: January 31st, 2008, 8:38 pm

Re: Simplifying animation frame wml

Post by AI »

Actually, I prefer [1-5] to [1~5]. The 'progressive' syntax is for continuous ranges, while the ranges described here are quite discrete.
User avatar
Pentarctagon
Project Manager
Posts: 5526
Joined: March 22nd, 2009, 10:50 pm
Location: Earth (occasionally)

Re: Simplifying animation frame wml

Post by Pentarctagon »

I get 2 warnings in src/serialization/string_utils.cpp about comparing signed and unsigned integers when compiling. The variable 'padding' is declared as an int on line 166, then compared to s_begin.size() on line 167 and s_end.size() on line 174 which are unsigned.
99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Re: Simplifying animation frame wml

Post by Boucman »

ok, couple of replies here...

Pentarctagon : that depends on compiler version, which is why it wasn't caught in the original patch, fixed...

~ vs - : I don't mind as long as it's compatible, slightly favor ~ because - might lead to ambiguities with file names

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

(after rereading your last patch, this might be a problem in the example and not in the syntax you propose, please clarify)


begin : as stated elsewhere, begin is deprecated. frames should only contain durations. the begining time is a property of the animation, not of the frame. so don't bother with these syntax

I don't like how you force all lists to have the same size, you should expand per text line, that the only way it makes sense... maybe even not coupling the different brackets within one line (need to think about that some more)


taking your final example, and changing to the syntax I want

Code: Select all

       #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

frames are consistent entities and i'd like to keep it that way. (and it would be waaaay more complicated to implement, trust me :P )


I've seen your patch, I keep them in my todo list, but let's wait a couple of days to see where we are going with this. once there has been a round of comment on ~ vs - I will probably commit that patch or just the optimisation part, but let's leave a little time for the discussion


Overall I like where we are going, keep up the good stuff
Fight key loggers: write some perl using vim
Post Reply