Skin Variables / Conditional Labels

  Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Post Reply
pieh Offline
Team-Kodi Member
Posts: 672
Joined: Aug 2010
Reputation: 13
Location: Poland
Post: #1
It was often mentioned - now it's about time to get it done.

This is now just discussion - it's not done yet

Skin variables are meant to get rid of multiples controls differing only by value of <texture> / <label> depending on <visible> condition.
Good example of its use are media flags (I'll use video type based on filename - bluray / hddvd / dvd / tv / vhs):

Currently this is done:
Code:
<include name="VideoTypeHackFlaggingConditions">
    <control type="image">
        <description>Bluray Image</description>
        <width>80</width>
        <height>35</height>
        <aspectratio align="left">keep</aspectratio>
        <texture>flagging/video/bluray.png</texture>
        <visible>[substring(ListItem.FilenameAndPath,bluray) | substring(ListItem.FilenameAndPath,bdrip)] + !Skin.HasSetting(HideFilenameFlagging)</visible>
    </control>
    <control type="image">
        <description>HDDVD Image</description>
        <width>80</width>
        <height>35</height>
        <aspectratio align="left">keep</aspectratio>
        <texture>flagging/video/hddvd.png</texture>
        <visible>substring(ListItem.FilenameAndPath,hddvd) + !Skin.HasSetting(HideFilenameFlagging)</visible>
    </control>
    <control type="image">
        <description>DVD Image</description>
        <width>80</width>
        <height>35</height>
        <aspectratio align="left">keep</aspectratio>
        <texture>flagging/video/dvd.png</texture>
        <visible>[substring(ListItem.FilenameAndPath,dvd) + ![substring(ListItem.FilenameAndPath,hddvd) | substring(ListItem.FilenameAndPath,bluray) | substring(ListItem.FilenameAndPath,bdrip)]] + !Skin.HasSetting(HideFilenameFlagging)</visible>
    </control>
    <control type="image">
        <description>TV Types Image</description>
        <width>80</width>
        <height>35</height>
        <aspectratio align="left">keep</aspectratio>
        <texture>flagging/video/TV.png</texture>
        <visible>[substring(ListItem.FilenameAndPath,pdtv) | substring(ListItem.FilenameAndPath,hdtv) | substring(ListItem.FilenameAndPath,dsr)] + !Skin.HasSetting(HideFilenameFlagging)</visible>
    </control>
    <control type="image">
        <description>VHS Image</description>
        <width>80</width>
        <height>35</height>
        <aspectratio align="left">keep</aspectratio>
        <texture>flagging/video/vhs.png</texture>
        <visible>substring(ListItem.FilenameAndPath,vhs) + !Skin.HasSetting(HideFilenameFlagging)</visible>
    </control>
</include>

With skin variables it could be:

Code:
<variable name="filename_video_type">
    <value condition="substring(ListItem.FilenameAndPath,bluray) | substring(ListItem.FilenameAndPath,bdrip)">bluray</value>
    <value condition="substring(ListItem.FilenameAndPath,hddvd)">hddvd</value>
    <value condition="substring(ListItem.FilenameAndPath,dvd)">dvd</value>
    <value condition="substring(ListItem.FilenameAndPath,pdtv) | substring(ListItem.FilenameAndPath,hdtv) | substring(ListItem.FilenameAndPath,dsr)">TV</value>
    <value condition="substring(ListItem.FilenameAndPath,vhs)">vhs</value>
</variable>

-- note difference with "dvd" checking: in variables condition I don't check against "hddvd" because if path would contain "hddvd" previous condition would be met and variable would already have value hddvd (and further checking would be aborted)

and then single image control with

Code:
<control type="image">
    <description>Video type image</description>
    <width>80</width>
    <height>35</height>
    <aspectratio align="left">keep</aspectratio>
    <texture>$VAR[filename_video_type,flagging/video/,.png]</texture>
    <!-- alternatively You could omit $VAR and use <texture>filename_video_type</texture> (along with slightly changed variable values) -->
    <visible>!Skin.HasSetting(HideFilenameFlagging)</visible>
</control>

Of course variables could contain $INFO[] or any ether element normal label / texture could have.

Skin variables could be contained in variables.xml

Code:
<variables>
  <variable name="x">
    <value="foo">bar</value>
    <value>default</value> <!-- if default not set then default value would be empty "" -->
  </variable>
</variables>

Do You have ideas how to make it better? Let's start discussion.

Always read the XBMC online-manual, FAQ and search the forums before posting.
Do NOT e-mail Team-XBMC members asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting, make sure you read this first

My previous forum/trac nickname: grajen3
(This post was last modified: 2011-08-18 08:13 by pieh.)
find quote
wyrm Offline
Skilled Skinner
Posts: 980
Joined: Nov 2007
Reputation: 10
Location: Perth, Australia (GMT +8)
Post: #2
pieh Wrote:It was often mentioned - now it's about time to get it done.

This is now just discussion - it's not done yet

Do You have ideas how to make it better? Let's start discussion.
pieh,

Hey I'm happy to kick this one off, I like what you have suggested but why limit this to <texture>? Could you also include <onclick> in this party? For example I currently use this in my home.xml

PHP Code:
<item>
                    <
icon>icon-tv.png</icon>
                    <
thumb>icon-tv-blur1.png</thumb>
                    <
label>31027</label>
                    <include 
condition="!Skin.String(tv-libraryentry)">TVLibraryTitle</include>
                    <include 
condition="StringCompare(Skin.String(tv-libraryentry),$LOCALIZE[369])">TVLibraryTitle</include>
                    <include 
condition="StringCompare(Skin.String(tv-libraryentry),$LOCALIZE[20108])">TVLibraryRoot</include>
                    <include 
condition="StringCompare(Skin.String(tv-libraryentry),$LOCALIZE[20387])">TVLibraryRecentlyadded</include>
                    <include 
condition="StringCompare(Skin.String(tv-libraryentry),$LOCALIZE[135])">TVLibraryGenres</include>
                    <include 
condition="StringCompare(Skin.String(tv-libraryentry),$LOCALIZE[652])">TVLibraryYears</include>
                    <include 
condition="StringCompare(Skin.String(tv-libraryentry),$LOCALIZE[344])">TVLibraryActors</include>
                    <include 
condition="StringCompare(Skin.String(tv-libraryentry),$LOCALIZE[20388])">TVLibraryStudios</include>
                    <include 
condition="StringCompare(Skin.String(tv-libraryentry),$LOCALIZE[744])">TVLibraryFiles</include>
                    <include 
condition="Skin.HasSetting(updatetventry)">RestartSkinTV</include>
                    <
visible>!Skin.HasSetting(home-tv)</visible>
                </
item
and each one of the includes is just a <onclick> like this
PHP Code:
<!-- TVLibraryTitle Include -->
    <include 
name="TVLibraryTitle">
        <
onclick>ActivateWindow(Videos,tvshowtitles,return)</onclick>
    </include> 
if you included <onclick> in the skin variable party I could rewrite this as
PHP Code:
<item>
                    <
icon>icon-tv.png</icon>
                    <
thumb>icon-tv-blur1.png</thumb>
                    <
label>31027</label>
                    <
onclick>$VAR[tv-libraryentry]</onclick>
                    <include 
condition="Skin.HasSetting(updatetventry)">RestartSkinTV</include>
                    <
visible>!Skin.HasSetting(home-tv)</visible>
                </
item
then I could set the skin variable to the contents of the <onclick> in my settings menu. The benefit would be that home.xml would run a Sh!t load quicker and be a lot more readable. Yes, my settings menu would become slower but who cares as the limiting factor there is the user interaction and besides this only happens rarely so not a major problem.

Wyrm (xTV-SAF)
find quote
jmarshall Offline
Team-XBMC Developer
Posts: 26,221
Joined: Oct 2003
Reputation: 178
Post: #3
It looks fine to me, though I'd be tempted to at least consider dropping $VAR in favour of just using <texture>filename_video_type</texture>. Do we need the ability to combine $VAR with prefix/postfix stuff? If we do, then it makes sense to leave it that way. If not they're more like constants.

As for where to place in the XML my first thought was that the variables would be global to the skin, but I guess there may be some niceness to be had from limiting scope? Am not really sure what you plan is on that, but it seems to me that global is probably all we really need.

Cheers,
Jonathan

Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


[Image: badge.gif]
find quote
wyrm Offline
Skilled Skinner
Posts: 980
Joined: Nov 2007
Reputation: 10
Location: Perth, Australia (GMT +8)
Post: #4
jmarshall Wrote:It looks fine to me, though I'd be tempted to at least consider dropping $VAR in favour of just using <texture>filename_video_type</texture>. Do we need the ability to combine $VAR with prefix/postfix stuff? If we do, then it makes sense to leave it that way. If not they're more like constants.
Jonathan,

Don't think that is such a good idea. How would you tell if it was a skin variable or (as in your example) if that was a file in the .xbt file called "filename_video_type". The $VAR makes it very clear that it is a variable and not an actual name of an image file.

Just my 2c worth.
Wyrm (xTV-SAF)
find quote
Hitcher Offline
Skilled Skinner
Posts: 11,195
Joined: Aug 2007
Reputation: 106
Location: Eastleigh, UK
Post: #5
wyrm Wrote:pieh,

Hey I'm happy to kick this one off, I like what you have suggested but why limit this to <texture>? Could you also include <onclick> in this party?

Wyrm (xTV-SAF)

I think this covers that -

https://github.com/xbmc/xbmc/pull/352
find quote
wyrm Offline
Skilled Skinner
Posts: 980
Joined: Nov 2007
Reputation: 10
Location: Perth, Australia (GMT +8)
Post: #6
Hitcher Wrote:I think this covers that -

https://github.com/xbmc/xbmc/pull/352
Hitcher,

Thanks was not aware of this. Will go have a look see. Reply to Jonathan's message still applies. Think unless there is a really good reason should still be $VAR[foo].

Wyrm (xTV-SAF)
find quote
wyrm Offline
Skilled Skinner
Posts: 980
Joined: Nov 2007
Reputation: 10
Location: Perth, Australia (GMT +8)
Post: #7
Hitcher Wrote:I think this covers that -

https://github.com/xbmc/xbmc/pull/352
Hitcher,

Just checked it out. It's close (will do away with the multiple includes) but to my way of thinking <onclick>$VAR[foo]</onclick> where foo is an action for <onclick> to perform will do away with multiple stringcompares (very slow) to include the one action that the user wants the button to do.

That is unless I have completely misread what pull/352 does. Still looks better than what I currently have so will recode to this later tonight.

Wyrm (xTV-SAF)

BTW, is there a limit on the number of <onclick> lines I can have in a <item>. I always thought it was three. That alone would make <onclick>$VAR[foo]</onclick> a much better way of doing things
find quote
pieh Offline
Team-Kodi Member
Posts: 672
Joined: Aug 2010
Reputation: 13
Location: Poland
Post: #8
wyrm Wrote:pieh,

Hey I'm happy to kick this one off, I like what you have suggested but why limit this to <texture>? Could you also include <onclick> in this party? For example I currently use this in my home.xml
This will apply to anything that You can use $INFO tags in (so onclicks, onfocus, visible, texture, label, altlabel, etc, I just mentioned most used tags I think)

jmarshall Wrote:It looks fine to me, though I'd be tempted to at least consider dropping $VAR in favour of just using <texture>filename_video_type</texture>. Do we need the ability to combine $VAR with prefix/postfix stuff? If we do, then it makes sense to leave it that way. If not they're more like constants.
I think If skinner want to use just variable as texture then we can ommit $VAR, but I think there are situations when he will want to use it in middle of static text/other infolabel so propably both ways should be supported

jmarshall Wrote:As for where to place in the XML my first thought was that the variables would be global to the skin, but I guess there may be some niceness to be had from limiting scope? Am not really sure what you plan is on that, but it seems to me that global is probably all we really need.
so, introduce variables.xml?
I didn't think too much on this (my concern was that all vars would be evaluated each frame, but after look at infomanager it seems that they will be evaluated only if needed/used?).

wyrm Wrote:Just checked it out. It's close (will do away with the multiple includes) but to my way of thinking <onclick>$VAR[foo]</onclick> where foo is an action for <onclick> to perform will do away with multiple stringcompares (very slow) to include the one action that the user wants the button to do.

That is unless I have completely misread what pull/352 does. Still looks better than what I currently have so will recode to this later tonight.
If You want to have just single action based on condition then Skin Vars is the way to go.

PR/352 will iterate through entire list of actions and will execute all actions that condition is met

wyrm Wrote:BTW, is there a limit on the number of <onclick> lines I can have in a <item>. I always thought it was three. That alone would make <onclick>$VAR[foo]</onclick> a much better way of doing things
there's no hardcoded limit of <onclick>, you can have as many as You want

Always read the XBMC online-manual, FAQ and search the forums before posting.
Do NOT e-mail Team-XBMC members asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting, make sure you read this first

My previous forum/trac nickname: grajen3
find quote
pieh Offline
Team-Kodi Member
Posts: 672
Joined: Aug 2010
Reputation: 13
Location: Poland
Post: #9
another thought regarding wyrm example: as Your conditions need to be evaluated just once (on window load) there might be optional flag to say that this variable don't need to be evaluated each frame (<variable name="x" evaluate_just_on_window_load="true">, ofcourse better naming here would be needed ;P) - this would save comparing const strings every frame

--edit perhaps just using <constant name="x"> would be proper here?
--edit2 scratch that <constant> - it's already used in different purpose

Always read the XBMC online-manual, FAQ and search the forums before posting.
Do NOT e-mail Team-XBMC members asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting, make sure you read this first

My previous forum/trac nickname: grajen3
(This post was last modified: 2011-08-16 10:52 by pieh.)
find quote
wyrm Offline
Skilled Skinner
Posts: 980
Joined: Nov 2007
Reputation: 10
Location: Perth, Australia (GMT +8)
Post: #10
pieh Wrote:another thought regarding wyrm example: as Your conditions need to be evaluated just once (on window load) there might be optional flag to say that this variable don't need to be evaluated each frame (<variable name="x" evaluate_just_on_window_load="true">, ofcourse better naming here would be needed ;P) - this would save comparing const strings every frame
pieh,

You got me on that one sport, had not even thought about that. First of all maybe <variable name="x" onload="true"> could be used. I was assuming that this would only apply on window load, so default would probably be onload="true", but if others want to jump in here on what they think should happen that would be good.

On previous post, great that you are thinking of applying this to more than just textures. Sound like it's going to be a LOT of work, so a virtual beer for that man.
Quote:--edit perhaps just using <constant name="x"> would be proper here?
Not sure what you mean here, does this relate to above thoughts on $VAR or are you suggesting that I use constants as provided to solve my issue with posted example code? As was pointed out by Hitcher and yourself, there is a better way to code my example currently available (althought $VAR would be even better way to code problem). My posted code was more an example of how $VAR could be used to improve things (and to throw some ideas out to the forum). Will take provided hints to improve my code.

Thanks,
Wyrm (xTV-SAF)
find quote
pieh Offline
Team-Kodi Member
Posts: 672
Joined: Aug 2010
Reputation: 13
Location: Poland
Post: #11
I meant that instead of having <variable name="x" onload="true"> we could have <constant name="x"> - none of this is implemented yet. Another idea (I think most self-explanatory till now) would be to have <variable name="x" evaluate="always/onload/oncontentchange"> (where "oncontentchange" would kick in when we navigate through directories/library nodes)

Main idea is that this would be evaluated every frame (see my example in 1 post - this won't work if we check only when we load window) - but we can see that there are cases we don't need to evaluate it every frame and doing it just when window is loading will suffice.

--edit scratch that <constant> thing - it's already used in different purpose

Always read the XBMC online-manual, FAQ and search the forums before posting.
Do NOT e-mail Team-XBMC members asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting, make sure you read this first

My previous forum/trac nickname: grajen3
(This post was last modified: 2011-08-16 10:52 by pieh.)
find quote
Jezz_X Online
Team-XBMC Skinner
Posts: 5,289
Joined: Jun 2006
Reputation: 57
Location: Earth
Post: #12
we already have <constant name="x"> and have had for a long time I use this in my touched skin
PHP Code:
    <constant name="ScreenWidth">1280</constant>
    <
constant name="ScreenHeight">960</constant>
    <
constant name="FanartCrossfadeTime">500</constant>
    <
constant name="IconCrossfadeTime">400</constant

and confluence uses them too

my JX720 skin uses

PHP Code:
    <constant name="HomeBladeAnimationSpeed">150</constant>
    <
constant name="HomeVisibleFadeAnimationSpeed">100</constant>
    <
constant name="DialogVisibleFadeAnimationSpeed">300</constant>
    <
constant name="SubBladeFadeInDelay">250</constant> <!-- HomeBladeAnimationSpeed 100 -->
    <
constant name="ButtonSlideBackDistance">-5</constant>
    <
constant name="ButtonSlideBackTime">275</constant>
    <
constant name="FanartCrossfadeTime">600</constant>
    <
constant name="IconCrossfadeTime">400</constant>
    <
constant name="SideMenuSlideOutTime">200</constant
(This post was last modified: 2011-08-16 07:51 by Jezz_X.)
find quote
pieh Offline
Team-Kodi Member
Posts: 672
Joined: Aug 2010
Reputation: 13
Location: Poland
Post: #13
Jezz_X Wrote:we already have <constant name="x"> and have had for a long time I use this in my touched skin

oh ... everyday something new :O

Always read the XBMC online-manual, FAQ and search the forums before posting.
Do NOT e-mail Team-XBMC members asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting, make sure you read this first

My previous forum/trac nickname: grajen3
find quote
jmarshall Offline
Team-XBMC Developer
Posts: 26,221
Joined: Oct 2003
Reputation: 178
Post: #14
Note that it's only because <constant> is restricted to numbers (floats) currently that it can't be used in strings - no reason for that to be the case, though I'm not sure I see a good reason of extending them to strings as the idea of them is to have a single place to define something, making it easy to change it across the entire skin. This doesn't seem to apply well to string arguments.

Personally I would just keep constants and variables.

Variables would be evaluated on demand only either way, so IMO there's no point allowing the evaluation to be skipped in some cases: XBMC should work that out automatically (push based stuff) in the future anyway.

For most cases where this is used (eg figuring out textures, labels, onclicks) they'll just be evaluated once per frame and ALREADY they're being evaluated once (or more) per frame due to the shitload of multiple controls with slightly varying visible conditions we have in play because we don't have the variables.

Thus, it should be if anything a win for CPU without the skinner needing to care about things.

As for placement, I think there's two options:

1. Make it global so you can access it via $VAR[]. This seems the easiest way to go, and is the same as constants and includes. Skinners can dump them in any includes file as they'd be read by the same code that loads includes.

2. Make them contextual to the control, so they'd become a property of the control, accessible via something like $INFO[control(id).<name_of_variable>]. This doesn't make sense to me as not all controls have unique IDs. It would also mean defining the variable within the control and then having to use the control's id to reference it within that same control (as the info manager needs context) which doesn't make any sense.

Thus, I'd go with your original plan. $VAR[] seems like a simple way to handle it and I don't really see all that much advantage of trying to parse the strings some other way (which would incur a map<string, variable>::find() hit for every string parsed, regardless of whether it was a variable or not).

Lastly, if we want to consider aligning <onclick> stuff with this, then we'd have a "multiple variable" approach which instead of returning the first item returns all items that match. That's something to consider after the single return is implemented.

Cheers,
Jonathan

Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


[Image: badge.gif]
find quote
wyrm Offline
Skilled Skinner
Posts: 980
Joined: Nov 2007
Reputation: 10
Location: Perth, Australia (GMT +8)
Post: #15
pieh,

While I know the implementation side of things is important, I can't help thinking your are overthinking this just a bit. Really what I am after (and most other skinners I would think) is a way to assign some value to a "variable" that we can then use elsewhere in the skin. Basically what Jonathan said
Quote:Thus, I'd go with your original plan. $VAR[] seems like a simple way to handle it and I don't really see all that much advantage of trying to parse the strings some other way (which would incur a map<string, variable>::find() hit for every string parsed, regardless of whether it was a variable or not).

Lastly, if we want to consider aligning <onclick> stuff with this, then we'd have a "multiple variable" approach which instead of returning the first item returns all items that match.
The way I thought this would work was something like this : Skin.SetVariable(variable-name,variable-value) and then access the the variable elsewhere in the skin with $VAR[variable-name]. Probably we would need to expand the checks against the variable as well, so something like IsEqual($VAR[variable-name],some-value). The other checks that would be handy would be IsNotEqual, IsGreaterThan, IsLessThan. Once again, just throwing the idea out there.

Wyrm(xTV-SAF)
find quote
Post Reply