[jmarshall]

As discussed earlier in this thread:

http://forum.xbmc.org/showthread.php?tid=104870

I've been doing some work on cleaning up the XBMC side of the condition parsing. I've got to the point now where I've reviewed most of them and some are in need of change.

This will likely be a break of backwards compatibility, and hence I want to give you a heads up.

Conditions that are already removed:

bar.* (these used to be used in the LCD and system info but were always just wrappers for the corresponding system.* infolabel/bool).
system.freespace©
system.usedspace©
system.totalspace©
system.usedspacepercent©
system.freespacepercent©
lcd.freespace©

Conditions that are proposed to be removed:
Note that these will require manual intervention by you if you use these
lcd.*
fanart.image Only once the usage of it is taken care of by Listitem.property(fanart_image)
fanart.color1 .. color3
musicpartymode.* except musicpartymode.enabled
system.hasdrivef
system.hasdriveg
system.setting (seems to only work with "hidewatched")

Conditions that will be changed:
library.isscanningvideo -> library.isscanning(video)
library.isscanningmusic -> library.isscanning(music)
musicplayer.duration -> player.duration
musicplayer.time -> player.time
musicplayer.timeremaining -> player.timeremaining
musicplayer.timespeed -> player.timespeed
videoplayer.duration -> player.duration
videoplayer.time -> player.time
videoplayer.timeremaining -> player.timeremaining
videoplayer.timespeed -> player.timespeed

Suggested changes for others (comments welcome)
container.sort(rating) -> isequal(container.sort,rating)
container.sortdirection(ascending) -> isequal(container.sortdirection,ascending)
listitemnowrap(##) -> listitem(##,nowrap)
listitemposition(##) -> listitem(##,absolute) Really not sure about this one - container.position might be better - I need to check what it does!
skin.string(blah,foo) -> isequal(skin.string(blah),foo)
skin.currenttheme -> skin.theme
skin.currentcolortheme -> skin.colortheme
skin.hastheme(blah) -> isequal(skin.theme,blah)
window.isactive(id) -> window(id).isactive
window.istopmost(id) -> window(id).istopmost
window.isvisible(id) -> window(id).isvisible
window.previous(id) -> isequal(window.previous,id) OR isequal(gui.previouswindow,id)
window.next(id) -> isequal(window.next,id) OR isequal(gui.nextwindow,id)
control.hasfocus(id) -> control(id).hasfocus
control.isvisible(id) -> control(id).isvisible
control.isenabled(id) -> control(id).isenabled
control.getlabel(id) -> control(id).getlabel
system.platform.linux -> isequal(system.platform,linux) NOTE: introduce new label system.platform
system.platform.win32 -> isequal(system.platform,win32)
system.platform.osx -> isequal(system.platform,osx)
musicplayer.offset(##).* -> playlist.offset(##).*
musicplayer.position(##).* -> playlist.position(##).*
player.rewinding2x -> player.rewindspeed NOTE: These ones will involve manual changes - can't automate that
player.forwarding2x -> player.forwardingspeed NOTE: These ones will involve manual changes - can't automate that

It would be nice if there was a script to update skins - perhaps one of the python guys could write something?

Cheers,
Jonathan

[ZombieRobot]

It would be nice if there was a script to update skins - perhaps one of the python guys could write something?

Now that would be awesome

[pecinko]

As discussed earlier in this thread:

http://forum.xbmc.org/showthread.php?tid=104870

I've been doing some work on cleaning up the XBMC side of the condition parsing. I've got to the point now where I've reviewed most of them and some are in need of change.

This will likely be a break of backwards compatibility, and hence I want to give you a heads up.

I would propose to wait a bit, until Eden is more mature.

Lots of people are still using Dharma and it would be more difficult to maintain both skin versions.

[spiff]

uhm, that's a horrible way to look at it. this is precisely the time to do these kind of intrusive changes.. to maximize the time you guys have to adapt before eden is released.

[ronie]

Conditions that are proposed to be removed:

the only one that would still be useful to me is fanart.image,
as it returns the fanart image in season and episode nodes when the '..' (up) item is selected,
whereas listitem.property(fanart_image) does not.

if this functionality can be moved to listitem.property(fanart_image),
i have no objections to fanart.image being removed.

Conditions that will be changed:

is there some clear logic (code-wise or other) as to which booleans
will be changed to the isequal(foo,bar) way?

for instance, isequal(window.next,id)....
why not use window(id).isnext as it's quite similar to
window(id).isactive and so on...

this part is still a bit fuzzy to me.

[Hitcher]

is there some clear logic (code-wise or other) as to which booleans
will be changed to the isequal(foo,bar) way?

for instance, isequal(window.next,id)....
why not use window(id).isnext as it's quite similar to
window(id).isactive and so on...

this part is still a bit fuzzy to me.

I must admit isequal has me confused as well.

[pecinko]

I must admit isequal has me confused as well.

StringCompare, I suppose, i.e. is A = B - isequal (A,B)

[`Black]

1:
The whole isequal thing does not make it better to me. On some things it even confuses me, e.g. skin.hastheme(blah) -> isequal(skin.theme).

2:
system.platform.os should be changed to system.platform(os) imo.

3:
Does listitemposition exist? It's listitem(##) and listitemnowrap(##), isn't it? So I would suggest to have listitem(##) and listitem(##,nowrap). What about container.position(##) and container.row(##), is that going to merge somehow with listitem(##)?

[mcborzu]

I agree with Ronie on Fanart.image, that property only has that one purpose...

Why get rid of:
videoplayer.duration
videoplayer.time
videoplayer.timeremaining
musicplayer.duration
musicplayer.time
musicplayer.timeremaining

Unless those are being superseded by something else as those are quite popular...

[Hitcher]

MusicPlayer and VideoPlayer will simply be Player I assume.

[BigNoid]

Most of the time I welcome change, change is good. But that whole list beneath Suggested changes for others (comments welcome) bit, I feel these changes are purely cosmetic and do not benefit skinners at all. I'm all for cleaning up code when it's not used like the first list of proposed removals and don't mind a few changes. But I feel this is going too far. And I say this because I really doubt a script could be made to make these changes in the skin and I absolutely don't feel like changing every single xml there is by hand.

[Jeroen]

I welcome the changes, even if it may require some work on the skinner's side. Some of the inconsistencies confuse me even today, and with Eden already having some major work done to the skinning engine I feel this is the right time.

I do agree with Ronie about keeping Fanart.Image or providing it's functionality through the ListItem.Property

[BigNoid]

I would prefer an occasional look in the wiki over rewriting thousands lines of code. Off course if a script can be made I have no problem with adapting to the changes, it's just that I don't think it's possible to script this.

[pieh]

the only one that would still be useful to me is fanart.image,
as it returns the fanart image in season and episode nodes when the '..' (up) item is selected,
whereas listitem.property(fanart_image) does not.

if this functionality can be moved to listitem.property(fanart_image),
i have no objections to fanart.image being removed.

if this is sane I'll update '..' items to have fanart_image property. I don't know if '..' on seasons node should have current tvshow fanart as after selecting it we're going to tvshows node. I see no problem with episode level.

for instance, isequal(window.next,id)....
why not use window(id).isnext as it's quite similar to
window(id).isactive and so on...

This reflect how it's done in xbmc - xbmc just set id of next window and not set 'isactive' property on each window. Having 'window(id).isactive' require additional checking on xbmc side.

Apart from that this has also advantage for skinners - having integer value allow You to use more sophisticated checks (f.e. comparisons). I realise that checking (window.next > 50) isn't really something worth anything, but in future we may extend IsEqual / add other method IsOneOf to compare against multiple values: IsOneOf(window.next,30,40,50).

1:
The whole isequal thing does not make it better to me. On some things it even confuses me, e.g. skin.hastheme(blah) -> isequal(skin.theme).

This is propably mistake and should be isequal(skin.theme,blah)

2:
system.platform.os should be changed to system.platform(os) imo.

it's proposed to be system.platform that can have linux/win32/osx values

---

it may look like unnecessary changes, but it has advantages over current model:

1. syntax is more sane:
(in programming) syntax foo(bar) is widely used to
a) execute method foo with parameter bar
b) access object/property identified by bar in object foo

from logic/syntax point of view "creatures" like Control.HasFocus(id) are some weird abominations - it's like we ask some undefined control if it has focus identified by id (whatever that could mean)

2. xbmc code will have real benefit of cleaning this up
3. this is next step on push-based processing that will benefit xbmc with decreased CPU usage thus decreased heat/noise production and lower power consumption

PS. if noone will be willing too I can prepare python script that will do conversion (I think it's simple regexp replacing as we will get 1:1 replacements)

[BigNoid]

To me it indeed sounded like unnecessary changes, but if you put it like that, if there's an advantage to be made it would be a good change and then I won't mind so much to put in some extra work.

I'm by no means a gifted scripter, but there are so many variables to account for, it would seem like an impossible task to do.
How would you make a script to change Container(id).ListItemNoWrap(%d).Icon or Container(id).ListItemNoWrap(%d).Label,path,.extension
in <info> or <texture> tags to the new format?