patchBattle for Wesnoth - Patches: patch #3604, Simplify halos in units/macros...

 
 
Show feedback again

patch #3604: Simplify halos in units/macros with square bracket expansion

Submitted by:  David Mikos <coffee>
Submitted on:  Sat 19 Jan 2013 11:55:30 PM UTC  
 
Priority: 5 - NormalStatus: Done
Privacy: PublicAssigned to: Jérémy Rosen <boucman>
Open/Closed: Closed

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sun 27 Jan 2013 12:44:13 PM UTC, comment #6:

and done...

note that I moved you in about.cfg from coders to contributors to follow our guidelines

i'm looking forward to your next patch

Jérémy Rosen <boucman>
Project MemberIn charge of this item.
Sun 27 Jan 2013 09:15:10 AM UTC, comment #5:

Thanks for the quick reply and applying the patch.

I've gone and updated the documentation for the AnimationWML, TerrainGraphicsWML, and itemWML to include examples for the square bracket usage.

In updating the documentation I noticed that the sting_utils.hpp examples I should indicate more clearly that it is a vector being returned rather than a string.

I've included a minor change to nicen up the paladdin and wose shaman that somehow slipped through in the previous patch.

You are very welcome to close this if you think it's done and I'll get to the unit frame amalgamation in a separate thread.

(file #17030)

David Mikos <coffee>
Project Member
Sat 26 Jan 2013 10:34:54 PM UTC, comment #4:

Ok, I have reworded your changelog entry and moved you to the correct section in about.cfg

but appart from that it's good, and I commited it, thanks a lot

the last step before I close this patch is to update the documentation for the different places where you changed the syntax

drop a comment when you're done and i'll close this

thx a lot for your work

Jérémy Rosen <boucman>
Project MemberIn charge of this item.
Sat 26 Jan 2013 09:20:50 AM UTC, comment #3:

I've attached an ammended patch and included myself this time in the about.cfg.

This ammended patch adds several examples to the square_parenthetical_split function as well as the capacity to take into account leading zeros: i.e. "[07-10]" expands to "07,08,09,10"

I've deleted the custom calculate_duration function in unit_frame.cpp and gone with what boucman had suggested.

I have expanded the use of the square_parenthetical split function to all animations that can be described by comma separated lists. This now includes all terrain animations, items, and side flag animations, etc. The builder.cfg file is much simpler now and I think the terrain animations load faster (although this may be subjective) -- certainly they are easier to follow after this patch.

Because of the use for not just units, but terrain, flags, and item animations, the square_parenthetical_split function probably needs to stay in the string_utils.cpp file I think.

As the patch is already getting pretty large (mostly macro simplifications), I don't think I should do the next stage until you core devs are happy with my approach and apply the patch.

(file #17028)

David Mikos <coffee>
Project Member
Tue 22 Jan 2013 07:54:58 PM UTC, comment #2:

Thanks for the reply, I will supply an ammended patch. However, there are some things you may have overlooked:

Perhaps putting the square_parenthetical split somwhere else would make sense, but not in unit_frame.cpp I don't think as it is not always running on a progressive_string. This also works for placed items and terrain image lists (called in halo.cpp "add" function with large overhead for each frame currently if many are placed on a map as it is called each visible frame).

Also, the progressive_string::duration function appears to be never used. I suggest that just deleting it and using the calculation function I provided in the patch instead would be the way to go -- this is also because the progressive_string functions takes in duration as a parameter to start with and we do not know that yet.

What do you think?

David Mikos <coffee>
Project Member
Sun 20 Jan 2013 10:01:34 PM UTC, comment #1:

ok, your whole plan (including consolidating frames and optimisations sounds good

and yes, let's work one step at a time

  • you didn't add yourself in about.cfg (assuming you're not there yet, I didn't find you)
  • your square_parenthetical_split function : it seems to be similar from the normal parenthetical split function but is actually totally different, please expand the description and examples and rename it to something like expand_parenthetical_list since it's about looking at the content, not really finding and splitting parenthesis like the other function
  • your change to duration seems OK but I don't like how you reproduce code from progressive string... at this stage halo_ is fundamentally an unparsed progressive string, i'd rather have you create a temporary progressive_string and use its duration() method. it might be slower, but it's more error-proof
  • I have done only a quick glance but the square_parenthetical_split function seems to not reuse the parenthetical split, which seems suprising, I would expect it to use it to do the splitting itself...

Actually that function is very animation specific, you probably should movin within unit_frame.cpp (not visible to the world)

overall I like what I see, pleas pursue the good work

Jérémy Rosen <boucman>
Project MemberIn charge of this item.
Sat 19 Jan 2013 11:55:30 PM UTC, original submission:

The attached patch simplifies the WML 'halo' tag to allow for square bracket expansions as per:
http://forums.wesnoth.org/viewtopic.php?f=10&t=38198

This attached patch also converts all the default unit WML and macros (in /data/macros/animation-utils) to the new, shorter, style. The expansion is in the same style as the map coordinate expansion, etc. that is used elsewhere in wesnoth WML already.

To summarize:

halo="halo/holy/light-beam-[1-6].png:30,halo/holy/light-beam-7.png:130,halo/holy/light-beam-[6-1].png:70"

is all that is needed in place of the old multi-frame halo for the white mage attack. When neither the 'end' nor 'duration' tags are present, the calculation is done automatically based on the total halo duration. This all acts to simplify the WML, shorten file sizes for faster loading from disk, and make more human readable/less error prone.

A separate patch for doing something similar to amalgamate the frames is planned, but I think should be for a separate thread.

Although this is I believe faster to run than before (due to lower loading times from disk), several code optimizations can be made to speed this up further for frequently run code in halo.cpp and sting_utils.cpp. This is also planned as a separate patch.

David Mikos <coffee>
Project Member

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by shadowmaster (Updated the item)
  • -unavailable- added by boucman (Posted a comment)
  • -unavailable- added by lipk (Updated the item)
  • -unavailable- added by coffee (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 06 Aug 2013 12:44:02 AM UTCshadowmasterOpen/ClosedOpen=>Closed
    Sun 27 Jan 2013 12:44:13 PM UTCboucmanStatusIn Progress=>Done
    Sun 27 Jan 2013 09:15:10 AM UTCcoffeeAttached File-=>Added animationWML_halo_nicen_up.patch, #17030
    Sat 26 Jan 2013 10:34:54 PM UTCboucmanStatusNone=>In Progress
    Sat 26 Jan 2013 09:20:50 AM UTCcoffeeAttached File-=>Added animation_WML_patch+unit+terrain+item_changes_v2.patch, #17028
    Sun 20 Jan 2013 08:50:32 AM UTClipkAssigned toNone=>boucman
    Sat 19 Jan 2013 11:55:30 PM UTCcoffeeAttached File-=>Added animationWML_patch+macro_and_unit_changes.patch, #16978
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup