Allow upto 512 images on a WPS

[kahuna_999]

I have found a problem with the ‘overloading’ of the %xp tag ( %xpa|srcx|srcy|w|h and %xpa|srcx|srcy|w|h|x|y ).  
Because of the optional x,y parameters,  it becomes very difficult to check for when used in conditionals.

%?pv<%xpr|0|0|15|15|%xpr|15|0|15|15>  - this will fail as it expects to find a trailing '|'. (coding assumption)
%?pv<%xpr|0|0|15|15|%xpr|15|0|15|15|>  - this wont fail, but will have three conditional options
%?pv<%xpr|0|0|15|15|10|10|%xpr|15|0|15|15|>  - this wont fail, but is it two conditionals( first %xp command with x,y cords), or is it 4, an %xp, 10, 10, %xp..

The only safe way I can see around this is to have two new tags, one with the x,y coords, one without..

%xp|srcx|srcy|w|h
%xx|srcx|srcy|w|h|x|y

Any suggestions on the second tag, could be %xx or %xP, or %xpxy

Alternatively, could the syntax for tag parms be changed to use ',' (comma) rather than '|' (or).  The 'or' would still make sense for separating the options in conditionals..  ie %?pv<%xpr,0,0,15,15,10,10|%xpr,15,0,15,15>  

Hopefully that made sense

[kahuna_999]

I decided to go with two tags for now...

%xp|srcx|srcy|w|h
%xx|srcx|srcy|w|h|x|y

Patch updated in tracker

Below is an example showing a sixteen step volume knob...
%xl|r|vol_knob.bmp|150|100|
%?pv<%xpr|0|0|15|15| %xpr|15|0|15|15| %xpr|30|0|15|15| %xpr|45|0|15|15| %xpr|0|15|15|15| %xpr|15|15|15|15| %xpr|30|15|15|15| %xpr|45|15|15|15| %xpr|0|30|15|15| %xpr|15|30|15|15| %xpr|30|30|15|15| %xpr|45|30|15|15| %xpr|0|45|15|15| %xpr|15|45|15|15| %xpr|30|45|15|15| %xpr|45|45|15|15>

[EDIT by NicolasP: Added a few spaces to allow wrapping]

Here is the bitmap used...

[Lear]

I have found a problem with the ‘overloading’ of the %xp tag ( %xpa|srcx|srcy|w|h and %xpa|srcx|srcy|w|h|x|y ).  
Because of the optional x,y parameters,  it becomes very difficult to check for when used in conditionals.

%?pv<%xpr|0|0|15|15|%xpr|15|0|15|15>  - this will fail as it expects to find a trailing '|'. (coding assumption)
%?pv<%xpr|0|0|15|15|%xpr|15|0|15|15|>  - this wont fail, but will have three conditional options
%?pv<%xpr|0|0|15|15|10|10|%xpr|15|0|15|15|>  - this wont fail, but is it two conditionals( first %xp command with x,y cords), or is it 4, an %xp, 10, 10, %xp..

The only safe way I can see around this is to have two new tags, one with the x,y coords, one without..

%xp|srcx|srcy|w|h
%xx|srcx|srcy|w|h|x|y

Any suggestions on the second tag, could be %xx or %xP, or %xpxy

Alternatively, could the syntax for tag parms be changed to use ',' (comma) rather than '|' (or).  The 'or' would still make sense for separating the options in conditionals..  ie %?pv<%xpr,0,0,15,15,10,10|%xpr,15,0,15,15>  

Yes, it is unfortunate that '|' has been used as tag parameter separator, but as it was only used for tags that didn't really need to be in conditionals, it hasn't been much of a problem until now.

But even if separate tags are used, as suggested above, will that really work for dynamic enums (like the volume and battery level)? At least before the tokenizing re-write, the code would just parse the enum parts to figure out how many options were used; '|' as a part of a non-enum tag would confuse it.

So I'd definitely suggest you use another separator char. If you can find one that isn't used by conditionals, and is safe for use with the other parameter tags (including stuff like album art), that'd be nice. I'm not suggesting that we should change the other tags now, but making it possible would be good.

[pixelma]

I agree from a wps writer point of view.

The example line not only is "confusing" the wps parser but also people who read or write such a line.
It's really hard to find out where the next conditional case starts and what still belongs to the bitmap part tag, very error-prone.

[Llorean]

I'd prefer xp and xP personally, as opposed to xp and xx. Since they're essentially two variants of the same tag.

[kahuna_999]

I am happy to go with the majority decision on this.

1. Keep the current separators ( | ) and have the two tags.
or
2. Switch to a comma separator, and have one overloaded tag.  

Would it be acceptable to change the tag syntax for this one tag?  This would go against the wps ‘standard’ already in place.  
This is a decision for the ‘core’ dev team to make, not for me and my first patch  

imho I think it would make sense to have tag parameters separated by a comma, and aim to change the syntax of all parameterised tags over time.

In the short, term there would be a non-consistent syntax for wps tags, but long term it would be easier to include more conditional options without the problems encountered here.

[Lear]

A comma separator is not so good, I think. Several tags include filenames, and a comma is perfectly valid in a filename. What about colon (':')?

The existing standard isn't good enough for general use, so something new is needed. I'd even vote against applying this patch with '|' as a separator.

[lowlight]

FWIW...

I have a similar concept kind of working that utilizes "icon" bitmaps, i.e. a vertical strip of images. This is the same format used for custom icons, and thus, reuses that code. My idea was to have a single iconset that could be used for common enumerations like volume, battery, codec, etc.

For instance, to custom volume graphic with 10 states needs:
-- 10 bmps
-- 10 %xl loads
-- use something like %pv<%xdA|%xdB|.....|%xdJ>

Using an iconset for the same thing, you need:
-- 1 bmp containing all the "icons" in a vertial strip
-- 1 %xl load (also specifing the number of icons in the file)
-- use 1 command like %evA (enumerate volume with image A)

I think this would greatly simplify the readibility of WPS's. Not to mention speed up the parsing of the wps and loading of images.

I've got it hacked into an older svn tree, so when I clean up my code, I'll post it on the tracker.

[Lear]

I like that idea. It isn't as flexible, but it still frees lots of images and can simplify the wps files noticeably.

[robin0800]

I am happy to go with the majority decision on this.

1. Keep the current separators ( | ) and have the two tags.
or
2. Switch to a comma separator, and have one overloaded tag.  

Would it be acceptable to change the tag syntax for this one tag?  This would go against the wps ‘standard’ already in place.  
This is a decision for the ‘core’ dev team to make, not for me and my first patch  

imho I think it would make sense to have tag parameters separated by a comma, and aim to change the syntax of all parameterised tags over time.

In the short, term there would be a non-consistent syntax for wps tags, but long term it would be easier to include more conditional options without the problems encountered here.

If You need an extra character why not use brackets ( ) these are not accepted by the wps parser at present but it should be a simple change for the paser to accept them even if at first it doesn't action them.
This would mean they would be to aid clarity in writing only.
Perhaps at a later date proper bracket handling could be done.

[NicolasP]

I don't see why we'd need brackets in the WPS syntax...
As of the different separator, I'm totally in favour of the idea and I like the semicolon.
I think the overloaded tag is better. It's not too hard to do and it feels better to use only one tag.
kahuna_999: I haven't had time to look at the patch, but I suggest you look at how the progressbar tag is parsed. IMHO it's quite a good way of treating this sort of tag that has a variable number of numerical parameters. Maybe we could even have a function dealing with it in a generic way...

[PaulJam]

As of the different separator, I'm totally in favour of the idea and I like the semicolon.

Wouldn't using a semicolon as separator potentialy cause problems when used with alternating sublines?

[Lear]

Wouldn't using a semicolon as separator potentialy cause problems when used with alternating sublines?

Indeed it would. Which is why I suggested ':'.

[NicolasP]

Oh I'm sorry, I meant the colon. In fact I just wanted to say I agreed with Lear but I was a bit sleepy

[kahuna_999]

patch re-sync'd in tracker.

I have changed the parm seperator to a colon as suggested, and re-implemented as an overloaded tag.

%xp<bitmap load id>:srcx:srcy:w:h
%xp<bitmap load id>:srcx:srcy:w:h:x:y

This no longer causes coding or readability issues.

kahuna_999: I haven't had time to look at the patch, but I suggest you look at how the progressbar tag is parsed. IMHO it's quite a good way of treating this sort of tag that has a variable number of numerical parameters. Maybe we could even have a function dealing with it in a generic way...

NichosP: Although the progressbar has optional parameters, it still uses a '|' (upright) as the parameter seperator, so if this was used in a conditional, the same issues would arise.
As for a generic function...  Each tag is parsed individually, so not a great deal of use for that.