BigNoid / script.skinshortcuts

GNU General Public License v2.0
23 stars 69 forks source link

Smart Playlists widgets issues #23

Closed Nessus85100 closed 9 years ago

Nessus85100 commented 9 years ago

I am using the <widgetPlaylists type="playlist">True</widgetPlaylists> in overrides.xml to use the smart playlists as widgets and i have some problems/questions.

First of all using this $INFO[Container(20).ListItem.Property(widgetPlaylist)] as a content tag it breaks the "Hidden" animation. The "Visible" works fine. If i replace it with a real string path (special://profile/playlists/video/MY-SMARTPLAYLIST.xsp) it works fine as the rest of the widgets.

This is the code that i am using for the list... http://xbmclogs.com/p5fjv83ls and this is the visible/hidden animation... http://xbmclogs.com/phwog9fd2

The second issue that i have is the way that script handles the playlists. There is no distinguish between video and music playlist that's why in the above list code, i am using a substring visible condition (playlists/video) to check if the list is video playlist. Using "video" or "music" as "type=" in widgetPlaylists tag is not working. IMHO skinners should have the option to choose what type of playlist they will use so the lists will have different layout in look and info.

Thanks Nessus

ghost commented 9 years ago

For animations, look into templates (though they may not work for you until your second issue is addressed...) - one of the big reasons I wrote them is that they allow animation possibilities just not doable with a traditional skin shortcuts implementation. Specifically in your case, I've no idea why the visible animation wouldn't work - especially if the thing is actually becoming visible - unless you were moving from another menu item with a playlist widget.

As for the second issue, looking at the code the type= property just adds an extra property widgetType - set to whatever type=, to the menu item. It has nothing to do with what type of playlist is actually selected. This behaviour exists so that playlists can have the same widgetType as a widget defined in overrides.xml - makes sense to me to extend it so that a different type can be specified for video and audio playlists, if that makes sense to you & would fix your issue.

Nessus85100 commented 9 years ago

This behaviour exists so that playlists can have the same widgetType as a widget defined in overrides.xml

There is already a property for that. Makes no sense to add another one with almost the same value: <property name="widget">Playlist</property> <property name="widgetType">playlist</property>

makes sense to me to extend it so that a different type can be specified for video and audio playlists, if that makes sense to you & would fix your issue.

Yes, that makes sense and it will be very helpful.

As for the animation issue, is the "Hidden" animation that is not working. Not the "Visible" one.

All the animations for all the other widgets are working properly and for the playlists they work also properly if i am using this as a content tag... <content>special://profile/playlists/video/MY-VIDEO-SMARTPLAYLIST.xsp</content>

and they are not working if i use this... <content>$INFO[Container(20).ListItem.Property(widgetPlaylist)]</content>

So, maybe is better to set the playlist path as a Skin.String and not as a property ?

Thanks Nessus

ghost commented 9 years ago

There is already a property for that. Makes no sense to add another one with almost the same value:

Playlist playlist

Only if <widgetPlaylists type="playlist" /> - if type="media" for example (what I use) then you'll get

<property name="widget">Playlist</property> <property name="widgetType">media</property>

Which I then use to group together all my media widgets.

So, maybe is better to set the playlist path as a Skin.String and not as a property ?

Moving them to skin strings would be a lot of work - not only in terms of code, but in thinking through how the script will manage which and when skin strings should be set/cleared. If anyone's interested in coding it, I'll be happy to give some pointers as to what code you'd need to modify.

Yes, that makes sense and it will be very helpful.

I'll do a PR for that shortly.

Nessus85100 commented 9 years ago

I don't understand why only the "Hidden" animation is not working properly with an $INFO label, and why is working fine if i use a simple string in the content tag ???.

@phil65 Do you think is a bug in the content tag listing way ? .

if type="media" for example (what I use) then you'll get <property name="widget">Playlist</property> <property name="widgetType">media</property> Which I then use to group together all my media widgets.

How that grouping is possible ?. I use the button 309 for selecting widgets and if i am not mistaking that's not possible via a skin code... or am i wrong ?

phil65 commented 9 years ago

did you try to debug by showing those InfoLabels on-screen? You will probably get a better picture then about what´s goin on when doing specific "workflows".

ghost commented 9 years ago

How that grouping is possible ?

Grouping is probably the wrong word. What I did before I switched to templates was have a single list for all of my media widgets. It's visibility was set based on widgetType equaling 'media', then I'd use a set of condition's to display the correct artwork, and a var to fill the content.

Nessus85100 commented 9 years ago

@phil65

did you try to debug by showing those InfoLabels on-screen? You will probably get a better picture then about what´s goin on when doing specific "workflows".

Yes i did and the InfoLabel was behaving the same way as the list. The "Hidden" animation was broken and the "Visible" was working fine. Is this means that the issue is not in content tag ?

phil65 commented 9 years ago

I could take a quick look if you would provide me an out-of-the-box testing case (working copy of the skin)

Nessus85100 commented 9 years ago

@Ignoble61

Grouping is probably the wrong word. What I did before I switched to templates was have a single list for all of my media widgets. It's visibility was set based on widgetType equaling 'media', then I'd use a set of condition's to display the correct artwork, and a var to fill the content.

Do you mind sharing the code please ?.

ghost commented 9 years ago

I don't still have the code - as I say I'm using templates now, but a very simple example with just one widget (and therefore just one image) would be something like...

<control type="list">
    <!-- Layout and options -->
    <visible>StringCompare(Container(9000).ListItem.Property(widgetType),media)</visible>
    <layout>
        <control type="image">
            <!-- Layout here -->
            <texture>$INFO[ListItem.Art(tvshow.poster)]</texture>
            <visible>StringCompare(Container(9000).ListItem.Property(Widget),RecentEpisodes)</visible>
        </control>
    </layout>
    <focusedlayout>
        <!-- yada -->
    </focusedlayout>
    <content>$VAR[mediawidget]</content>
</control>

<variable name="mediawidget">
    <value condition="StringCompare(Container(9000).ListItem.Property(Widget),RecommendedEpisodes)">plugin://service.library.data.provider?type=recommendedepisodes&amp;reload=$INFO[Window.Property(recommendedepisodes)]</value>
</variable>
Nessus85100 commented 9 years ago

@phil65

I could take a quick look if you would provide me an out-of-the-box testing case (working copy of the skin)

Unfortunately my skin is a mess full of testing and debug code and textures. Can you please test it in your skin by adding an InfoLabel in home screen ?

EDIT: You can testing by using this mod of Bello... https://github.com/sualfred/Bello-Kodi-15.x-Nightlies Add this label in the home.xml http://xbmclogs.com/pddz1z4ey and set a video playlist as widget.

Nessus85100 commented 9 years ago

@Ignoble61 Thanks. I will have it in mind when i find some time to give a deeper look in the templates future.

ghost commented 9 years ago

Just FYI, the template code I'm using (only for the smart(ish) widgets at the moment, it's on my to-do list to extend it) is...

<template>
    <other include="widget">
        <condition tag="property" attribute="name|widgetType">media</condition>
        <property name="content" tag="property" attribute="name|widget" value="SmartMovies">plugin://service.smartish.widgets?type=movies&amp;reload=$INFO[Window.Property(smartish.movies)]</property>
        <property name="content" tag="property" attribute="name|widget" value="SmartEpisodes">plugin://service.smartish.widgets?type=episodes&amp;reload=$INFO[Window.Property(smartish.episodes)]</property>
        <property name="content" tag="property" attribute="name|widget" value="SmartAlbums">plugin://service.smartish.widgets?type=albums&amp;reload=$INFO[Window.Property(smartish.albums)]</property>
        <property name="content" tag="property" attribute="name|widget" value="SmartPVR">plugin://service.smartish.widgets?type=pvr&amp;reload=$INFO[Window.Property(smartish.pvr)]</property>

        <property name="art" tag="property" attribute="name|widget" value="SmartMovies">$INFO[ListItem.Art(poster)]</property>
        <property name="art" tag="property" attribute="name|widget" value="SmartEpisodes">$INFO[ListItem.Art(tvshow.poster)]</property>
        <property name="art" tag="property" attribute="name|widget" value="SmartAlbums">$INFO[ListItem.Icon]</property>
        <property name="art" tag="property" attribute="name|widget" value="SmartPVR">$INFO[ListItem.Icon]</property>

        <controls>
            <control type="list" id="9002">
                <skinshortcuts>visibility</skinshortcuts>
                <animation effect="fade" start="100" end="60" time="0" condition="!Control.HasFocus(9002)">Conditional</animation>
                <animation effect="fade" start="60" end="100" time="0" condition="Control.HasFocus(9002)">Conditional</animation>
                <animation effect="fade" start="0" end="100" time="200">Visible</animation>
                <animation effect="fade" start="100" end="0" time="200">Hidden</animation>
                <left>60</left>
                <top>505</top>
                <width>959</width>
                <height>200</height>
                <ondown>9000</ondown>
                <onup>9010</onup>
                <orientation>horizontal</orientation>
                <itemlayout width="137" height="200">
                    <control type="image">
                        <left>1</left>
                        <top>1</top>
                        <width>136</width>
                        <height>200</height>
                        <aspectratio aligny="center">keep</aspectratio>
                        <texture>$skinshortcuts[art]</texture>
                        <animation effect="fade" start="30" end="30" time="0" condition="true">Conditional</animation>
                    </control>
                </itemlayout>
                <focusedlayout width="137" height="200">
                    <control type="image">
                        <width>136</width>
                        <height>200</height>
                        <left>1</left>
                        <top>1</top>
                        <aspectratio aligny="center">keep</aspectratio>
                        <texture>$skinshortcuts[art]</texture>
                        <animation type="Focus">
                            <effect type="fade" start="30" end="100" time="200" />
                        </animation>
                        <animation type="UnFocus">
                            <effect type="fade" start="100" end="30" time="200" />
                        </animation>
                    </control>
                </focusedlayout>
                <content>$skinshortcuts[content]</content>
            </control>
        </controls>
    </other>
</template>

This does the same as the previous code, but each widget ends up in its own list rather than sharing one.

phil65 commented 9 years ago

@Ignoble61 I am not that much into this (only using the script myself for submenus), but this looks pretty similar to the new include type introduced in Isengard which supports parameters, perhaps they can save you some work?. You know about those?

ghost commented 9 years ago

@phil65 Yup I know about them, but have had only a quick play-around myself - I'll try again with them I'm working on that part of the skin again.

Nessus85100 commented 9 years ago

@phil65 have you had the time to look into this ?...

EDIT: You can testing by using this mod of Bello... https://github.com/sualfred/Bello-Kodi-15.x-Nightlies Add this label in the home.xml http://xbmclogs.com/pddz1z4ey and set a video playlist as widget.

phil65 commented 9 years ago

nope, no time yet.

Nessus85100 commented 9 years ago

@phil65 The broken "Hidden" animation issue in dynamic list's if the 'content' tag starts with an InfoLabel ($INFO) it also happens when using this future... https://github.com/BigNoid/script.skinshortcuts/commit/14b215dd081bad81b450356e209a2f0e53cd7634 and this time both ("Visible" and "Hidden") animation are broken.

Nessus85100 commented 9 years ago

@Ignoble61 Is it possible when adding the <widgetPlaylists> in overrides.xml, instead of using the audiotype check to distinguish between music and video playlists, to check the content type of the playlist the according to the way that Kodi GUI does when you create a smart playlist ?. That way we can have more detailed distinguish between playlist's. Example... <widgetPlaylists type="video" content="movies">True</widgetPlaylists> <widgetPlaylists type="video" content="tvshows">True</widgetPlaylists> <widgetPlaylists type="video" content="episodes">True</widgetPlaylists> <widgetPlaylists type="video" content="musicvideos">True</widgetPlaylists> <widgetPlaylists type="music" content="albums">True</widgetPlaylists> <widgetPlaylists type="music" content="songs ">True</widgetPlaylists> <widgetPlaylists type="mixed" content="mixed">True</widgetPlaylists>

I don't know if this detailed distinguish can be done with templates but i don't want to dig in templates future at the moment.

ghost commented 9 years ago

@Ignoble61 Is it possible when adding the in overrides.xml, instead of using the audiotype check to distinguish between music and video playlists, to check the content type of the playlist the according to the way that Kodi GUI does when you create a smart playlist ?

@Nessus85100 it's possible, but not something I'd add (not a feature that interests me, and I don't think the idea of multiple <widgetPlaylists /> in overrides.xml is a good one)

Nessus85100 commented 9 years ago

it's possible, but not something I'd add (not a feature that interests me

I understand that but judging from my experience from the forums users smart playlists are very popular especially for users that want to customize the home window. IMHO this would be a very useful feature for the skin shortcuts.

Maybe someone else can do it ?

I don't think the idea of multiple <widgetPlaylists /> in overrides.xml is a good one)

That was just a suggestion that i can think of to achieve this. It's not necessary the best way to do this.

ghost commented 9 years ago

If it helps I'm slowly writing an alternative way to select widgets - based on groupings as used when selecting a shortcut (so a library or add-on node can be selected as well). One thing I haven't fully thought through yet is where the widgetType property will come from, so I'll keep your request in mind as I go :)

Nessus85100 commented 9 years ago

Good to hear that. I remember talking about something like this (widgets grouping) with Unfledged some time a go. I guess the widgets provided by the skinner will still be an option... right ?

Anyway, until this is integrated, and the animations issues are fixed i will completely remove smart playlists from my widgets list.

Thank you all for the help and info.

Cheers Nessus

phil65 commented 9 years ago

@Nessus85100 in case you didnt know it: you´re talkin to Unfledged. ;)

Nessus85100 commented 9 years ago

Hehe... i've had a suspicion that was him (by seeing the details in his comments !), that's why i've drop him a PM in Kodi forums but since i didn't get an answer i thought i was wrong.

Glad we have you back mate !!!

ghost commented 9 years ago

Not sure who you PM'd, but I'm not on the forums :)

Nessus85100 commented 9 years ago

There is a new user in the forums with the same user name (Ignoble) without the 61 number in the end... http://forum.kodi.tv/member.php?action=profile&uid=272653.

I guess i've PM'd the wrong guy... lol !!!

ghost commented 9 years ago

Nothing to do with me, but I'm sure they appreciate the thought - or are incredibly confused by it!

marcelveldt commented 9 years ago

@Ignoble61, I think this one is also solved when that new method for selecting the widgets is ready ;-) That would be an all-in-one solution