[bkury]

First of all, I want to thank everyone who is working on KODI to make it better (core/addons/scripts/skins/forum or other things). Now that Leia is out, I really hope that there is a developer who has some spare time to further improve KODI, in my case the skinning engine.

If anyone is interested, here is a summary of skinning enhancements which would be nice to have, improve performance or add new possibilities:

DialogNotification.xml
Please add an option to get notification information (ID 400,401,402) via skinning engine, e.g. make something like this work would be great:

Code:

<onload>Skin.SetString(notification_icon,$INFO[Control.GetLabel(400)])</onload>
<onload>Skin.SetString(notification_label1,$INFO[Control.GetLabel(401)])</onload>
<onload>Skin.SetString(notification_label2,$INFO[Control.GetLabel(402)])</onload>

(if that isn't possible, maybe set a window property?)
Unfortunately, "onload" method does not work with modeless dialogs like DialogNotification.xml

Reason:
This enhancement would allow skin developers to create a "Notification Center", Home automation triggers on certain notifications (e.g. Incoming Call --> Pause Video --> Turn on Light) and more ...
(most useful with apps like yatse, ..)

New boolean condition

Code:

File.Exist(/test)
File.Exist($INFO[ListItem.Path]/extrafanart.jpg)
!File.Exist(/test/theme.mp3)

Returns true if a specific file or directory exist

Reason:
Check if a file/dir exist before calling "PlayMedia()", this is especially useful for playing "theme" files or showing extra fanart while browsing your library, can also be useful to check user input "e.g. The directory/file you entered does not exist!"

<includes>
Allow $INFO for <include> and <include content="">, e.g.:

Code:

<include>Transparency_$INFO[Skin.String(Transparency_Value)]</include>

In this case KODI should first get the $INFO value (e.g. 10) and then include the content, e.g.: <include>Transparency_10</include>.  This would save hundreds or even thousand lines of code in most skins because you don't need to add a condition for every possible value like this:

Code:

<include condition="String.isEqual(Skin.String(Transparency_Value),10)">Transparency_10</include>
<include condition="String.isEqual(Skin.String(Transparency_Value),20)">Transparency_20</include>
<include condition="String.isEqual(Skin.String(Transparency_Value),30)">Transparency_30</include>
...
<include condition="String.isEqual(Skin.String(Transparency_Value),100)">Transparency_100</include>

Extreme case: If there are 100+ possible user values you would also need 100+ conditions instead of 1 <include>, this would also improve performance on low-end hardware because KODI doesn't need to go through 100 conditions ...

Container (Content)
Option to fill a container with static items from an URL e.g.:

Code:

<control type="list">
        ...
        <content>http://kodi.tv/items.xml</content>
</control>

items.xml:
----------
<item>
        <label>test</label>
        ...
</item>

Useful for containers with frequently changing content. Normally you have to update the skin .xml file to add/remove or change a static item, with this feature you could just edit the .xml on the server and users will automatically get the new content.

Thanks for considering these enhancements!

[jurialmunkey]

RE: Notification Dialog:
That's what EventLog is for.
System Settings > Logging > Enable notification event logging.

RE: File Exists.
If it was a skin condition, that means Kodi would need to be constantly checking whether the file exists. Constant I/O reads to disk are taxing on system resources (and degrade hardware).
It would be better to create a simple python addon that can be run to evaluate whether the file exists.
Skin Helper already has such a function, but perhaps it would be nice to move that out into it's own addon.

Code:

RunScript(script.skin.helper.service,action=fileexists,file=[filenamepath],skinstring=[skinstring to store the result],winprop=[windowprop to store the result])

RE: Includes
Having variables in the include name opens a whole can of worms about missing includes. These cause error logging (more I/O writes to disk!)
There are far better approaches to this type of problem, and that would also give more useful functionality - namely, allowing variables and infolabels to act as values in animations.
e.g. A far better solution would be to allow something like:

Code:

<animation effect="fade" start="100" end="$INFO[Skin.String(Transparency_Level)]" condition="true">Conditional</animation>

RE: Container Content
Again, this could be done with a fairly simple python addon. The functionality you are asking for is essentially what every video addon does.
I think this type of thing is better to be done in an addon because there is more transparency about where the info is coming from and it gives the user more control over what is access the internet on their system.
Also, I think you would have more chance of getting something like this done because there are far more people around that are well versed in creating video addons than there are people who know their way around guilib.

[bkury]

RE: Notification Dialog:
That's what EventLog is for.
System Settings > Logging > Enable notification event logging.

I'm aware of that feature, unfortunately it doesn't log 3rd party or JSON notifications. You can test it yourself by adding "<F4>Notification(Testing 123,Hello world)</F4>" in your keymaps file, these kind of notifications won't show up in your eventlog. 
 

There are far better approaches to this type of problem, and that would also give more useful functionality - namely, allowing variables and infolabels to act as values in animations.
e.g. A far better solution would be to allow something like:

Code:

<animation effect="fade" start="100" end="$INFO[Skin.String(Transparency_Level)]" condition="true">Conditional</animation>

Well, maybe transparency was a bad example. I'm talking about <tags> which doesn't work with Vars or Infolabels to begin with like positional tags <left>,<top>,..
Currently there is no way to get a skin.string value into a <left> tag without using <include> with 1 condition for every possible skin.string value, <include> with $INFO would solve this and many other problems.
 

Having variables in the include name opens a whole can of worms about missing includes. These cause error logging (more I/O writes to disk!)

Why? Obviously you want to have a <include name> section for every possible skin.string value, nothing different than using "normal" <includes>, and in case skin.string is empty:

Code:

<include condition="!String.isEmpty(Skin.String(Transparency_Value))">Transparency_$INFO[Skin.String(Transparency_Value)]</include>

No error logging, no I/O, nothing ... in fact this feature could save a lot of cpu cycles because you can get rid of condition checks.


Anyway, thanks for your reply!
No need to talk about my other suggestions, it seems like there is no way around a "helper" python addon for this kind of functionality.

[jurialmunkey]

RE: Helper addons.
Yes, I think that is a better solution for those issues because it keeps things modular (which has been the general approach to development in order to reduce core complexity).

RE: Event log
Not logging JSON or other notifications in Event Log seems like a bug to me. I would expect logging of all notifications with the option enabled - the whole point of event log is so that you can go back and review notifications that you missed whilst you were away.
There might be a reason why it doesn't log these things, but I would still think it is worth lodging a bug report about this issue at https://github.com/xbmc/xbmc/issues

RE: includes
Don't get me wrong, I can certainly see the value in having variable include names.
My point is that it is a bit of an XY problem - the underlying issue is the lack of variable positional and animation tags, not variable includes.
Also, there is the problem of not being able to use localized strings in the includes - e.g.:

Code:

<onclick>Skin.SetString(MyString,$LOCALIZE[123])</onclick>
...
<include>MyInclude_$INFO[Skin.String(MyString)]</include>

[bkury]

RE: Helper addons.
Yes, I think that is a better solution for those issues because it keeps things modular (which has been the general approach to development in order to reduce core complexity).

I still think that File.Exist() is one of the most basic and non complex built-in functions you could have in any program and should work without a python addon.
 

RE: Event log
Not logging JSON or other notifications in Event Log seems like a bug to me. I would expect logging of all notifications with the option enabled - the whole point of event log is so that you can go back and review notifications that you missed whilst you were away.
There might be a reason why it doesn't log these things, but I would still think it is worth lodging a bug report about this issue at https://github.com/xbmc/xbmc/issues

I don't think that it's a bug, it's just not implemented yet.
 

RE: includes
Don't get me wrong, I can certainly see the value in having variable include names.
My point is that it is a bit of an XY problem - the underlying issue is the lack of variable positional and animation tags, not variable includes.

Well, let's be real here. There is absolutely no way that any KODI developer is going to add $VAR/$INFO support for every <tag>. This would be a ridiculous amount of work for a rather niche feature, not to mention the testing required because there is a good chance that you are going to break a tag.

In contrast, allowing $INFO/$VAR <includes> / <include content=""> would be:

• rather easy to implement
  
• would work with every <tag> or code piece
  
• very little risk that you break anything else
  
• very little testing required
  

 

Also, there is the problem of not being able to use localized strings in the includes - e.g.:

Come on, that's nitpicking There is no valid reason to use $LOCALIZE in <include>, in this case (onclick) you would have a button/container and you would localize the label not the <include>, e.g.:

Code:

<control type="button"> OR <item>
   <label>$LOCALIZE[123]</label> <!-- Localized string for yellow -->
   <onclick>Skin.SetString(MyString,yellow)</onclick>
   ...
</control>

<include>Color_$INFO[Skin.String(MyString)]</include>

[jurialmunkey]

These are all fair points.

For File.Exists() I agree it feels like a basic function, but so do basic integer math operations and we don't have those either.
A file check function in the skinning engine gets asked for all the time. All I'm doing is relaying the reasons why it isn't implemented:
https://forum.kodi.tv/showthread.php?tid...#pid357853

For positional/animation tags, all I was suggesting is the ability to use a skinstring instead of a constant value - it would only get evaluated when the window loads and take the value from the skinstring (which is the same as your includes approach, because anything beyond a skinstring is going to be unfeasible). If the skinstring is empty or is not an integer, then the control gets a zero value.

- You make good/valid points about the includes. My main objection is that it feels fundamentally incorrect to use a variable as a class name rather than passing through the variable as a value for the object. The goal should be to get the skinning engine closer to normal programming logic rather than adding band-aid solutions that move us further away simply because it is easier to do in the interim.

[bkury]

I agree, that would work too. I just think that no KODI developer is going to do that because it's just too much work and trouble. Also, <include> method has one big advantage compared to your suggestion, you are not limited to one value/tag because <includes> can have multiple lines of skin code.
 

My main objection is that it feels fundamentally incorrect to use a variable as a class name rather than passing through the variable as a value for the object. The goal should be to get the skinning engine closer to normal programming logic rather than adding band-aid solutions that move us further away simply because it is easier to do in the interim.

Yes, I understand your concern and you are right. What about using a new include param tag ? e.g.
 

Code:

<include content="myinclude">
   <param name="label">$INFO[ListItem.Label]</param>           <!-- like it's working now -->
   <info name="left">$INFO[Skin.String(MyLeftValue)]</info>    <!-- will get $INFO value before include -->
</include>

<include name="myinclude">
   <label>$PARAM[label]</label>
   <left>$PARAM[left]</left>
</include>

or even a dedicated <skinstring name="left">MyLeftValue</skinstring> and then <left>$SKINSTRING[left]</left>