Kodi Community Forum
Important Skinners: Please test PR13754 - Printable Version

+- Kodi Community Forum (https://forum.kodi.tv)
+-- Forum: Development (https://forum.kodi.tv/forumdisplay.php?fid=32)
+--- Forum: Skinning (https://forum.kodi.tv/forumdisplay.php?fid=12)
+--- Thread: Important Skinners: Please test PR13754 (/showthread.php?tid=330696)

Pages: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15


RE: Skinners: Please test PR13754 - sualfred - 2018-09-14

Yep, accused you too early Smile Sorry. But thanks for taking care and making @garbear aware of it.


RE: Skinners: Please test PR13754 - scott967 - 2018-09-15

(2018-09-13, 22:41)ksooo Wrote: @scott967 could you please try this testbuild to check whether listitem.dbtype now works for music videos? => http://mirrors.kodi.tv/test-builds/windows/win64/KodiSetup-20180913-2ec6b0ae-guiinfo-fix-listitem-dbtype-x64.exe

It would be great if you could also do some deeper regression tests as well as I needed to change some important bits and pieces for the fix and want to make sure there are no bad side effects.
 Did some testing.  Good news is ListItem.DBType shows musicvideo correctly now in the music / musicplaylist windows and videos windows unaffected.

But I have a (sort-of) regression.  This is for music m3u playlists containing songs and / or musicvideos.  Before PR13754, ListItem.DBType was null for either songs or musicvideos in both music and musicplaylist windows.  With the test build we actually have this behavior restored.  With original PR13754, ListItem.DBType was set as songs for both songs and musicvideos.  So in some sense ListItem.DBType was improved by PR13754 (for songs) though in error for musicvideos.

In all three builds (Pre-PR13754, Post-PR13754 and this test build) Container.content infolabel is "songs" for all music m3u playlists.

I'm not sure how relevant m3u playlists containing music videos are, just reporting my test results.

From a skinner perspective, knowing the correct DBType for a listitem is valuable as the information available to display to user is different for songs compared to music videos.

scott s.
.


RE: Skinners: Please test PR13754 - ksooo - 2018-09-15

(2018-09-15, 04:18)scott967 Wrote:
(2018-09-13, 22:41)ksooo Wrote: @scott967 could you please try this testbuild to check whether listitem.dbtype now works for music videos? => http://mirrors.kodi.tv/test-builds/windows/win64/KodiSetup-20180913-2ec6b0ae-guiinfo-fix-listitem-dbtype-x64.exe

It would be great if you could also do some deeper regression tests as well as I needed to change some important bits and pieces for the fix and want to make sure there are no bad side effects.
 Did some testing.  Good news is ListItem.DBType shows musicvideo correctly now in the music / musicplaylist windows and videos windows unaffected.

But I have a (sort-of) regression.  This is for music m3u playlists containing songs and / or musicvideos.  Before PR13754, ListItem.DBType was null for either songs or musicvideos in both music and musicplaylist windows.  With the test build we actually have this behavior restored.  With original PR13754, ListItem.DBType was set as songs for both songs and musicvideos.  So in some sense ListItem.DBType was improved by PR13754 (for songs) though in error for musicvideos.

In all three builds (Pre-PR13754, Post-PR13754 and this test build) Container.content infolabel is "songs" for all music m3u playlists.

I'm not sure how relevant m3u playlists containing music videos are, just reporting my test results.

From a skinner perspective, knowing the correct DBType for a listitem is valuable as the information available to display to user is different for songs compared to music videos.

scott s.
Thanks for testing. If pre-PR13754 behavior is restored, I'm done. Fixing other bugs is not on my list. This should be done by the devs responsible for m3u playlists in this case as I cannot judge on what is correct.


RE: Skinners: Please test PR13754 - scott967 - 2018-09-18

(2018-09-15, 08:52)ksooo Wrote: Thanks for testing. If pre-PR13754 behavior is restored, I'm done. Fixing other bugs is not on my list. This should be done by the devs responsible for m3u playlists in this case as I cannot judge on what is correct. 

Fair enough. Don't see any problem with that.

scott s.
.


RE: Skinners: Please test PR13754 - bkury - 2018-10-04

I may have found a bug related to this PR, see:
https://forum.kodi.tv/showthread.php?tid=336117

It's working in Krypton but not in Leia (18.0+git20181002.0201-6fb44a8)


Skinners: Please test PR13754 - ksooo - 2018-10-04

Regardless the PR discussed here caused the change in behavior (i doubt this, btw) the new behavior seems correct to me.

The values returned in Krypton seem wrong to me as those are not paths at all. And parent (folder) path being equal to own path seems even more wrong


RE: Skinners: Please test PR13754 - bkury - 2018-10-04

Fair enough but the "new behavior" doesn't solve the problem that the correct path (favourites.xml) isn't available via infolabel. Honestly, I don't care if I get this information via ListItem.Path/FolderPath or a new ListItem.Property(path). The point is that this information should be available for skins (like it was in krypton), so that we can get rid of script.favourites and use the built-in favourites feature of KODI.

Anyway, I don't want to hijack this thread if you think that it's not related to this PR but can we please get this fixed? Adding a new ListItem.Property(path) would take 5 mins of coding ...

Thanks!


Skinners: Please test PR13754 - ksooo - 2018-10-04

(2018-10-04, 12:49)bkury Wrote: Fair enough but the "new behavior" doesn't solve the problem that the correct path (favourites.xml) isn't available via infolabel. Honestly, I don't care if I get this information via ListItem.Path/FolderPath or a new ListItem.Property(path). The point is that this information should be available for skins (like it was in krypton), so that we can get rid of script.favourites and use the built-in favourites feature of KODI.

Anyway, I don't want to hijack this thread if you think that it's not related to this PR but can we please get this fixed? Adding a new ListItem.Property(path) would take 5 mins of coding ...

Thanks!

Then go ahead, take 5 mins of your spare time and code it, test it and submit a PR, please.

Oh, you're not a dev? Then you never ever should tell devs how much time it will take to implement something. 5 mins is pure nonsense. Devs really don't like that kind of supervision, believe me.

If you want somebody else than yourself to fix it, open a trac ticket, please.


RE: Skinners: Please test PR13754 - bkury - 2018-10-04

You are right. I'm sorry, I should have phrased it differently ...
I just wanted to report a bug but I found it odd that an active/experienced dev like you replied with "new behavior seems correct to me", when it's clearly not the case.

Anyway, again sorry ...


RE: Skinners: Please test PR13754 - scott967 - 2018-10-17

I think I have another issue.

VideoPlayer.Title

When a video is played from file (not in library) prior to this PR the filename was returned in $INFO[Videoplayer.Title].  Now the infolabel is empty.

scott s.
.


Skinners: Please test PR13754 - ksooo - 2018-10-17

(2018-10-17, 03:57)scott967 Wrote: I think I have another issue.

VideoPlayer.Title

When a video is played from file (not in library) prior to this PR the filename was returned in $INFO[Videoplayer.Title].  Now the infolabel is empty.

scott s.
.


Are you using latest Kodi master? IIRC, there was an issue with this which I fixed recently.


RE: Skinners: Please test PR13754 - scott967 - 2018-10-17

(2018-10-17, 07:05)ksooo Wrote:
(2018-10-17, 03:57)scott967 Wrote: I think I have another issue.

VideoPlayer.Title

When a video is played from file (not in library) prior to this PR the filename was returned in $INFO[Videoplayer.Title].  Now the infolabel is empty.


Are you using latest Kodi master? IIRC, there was an issue with this which I fixed recently.  
  
OK Pulled the 1015 nightly and you are right, it's fixed.  Sorry to not check it before posting.

scott s.
.


RE: Skinners: Please test PR13754 - MacGyver - 2018-12-05

This might be related as the timeline fits, it stopped working correctly between Alpha1 and Alpha2.

Best way I can put it is that Container(9000).ListItem.Property() works everywhere but inside a list control control.

It works here
Code:
                <control type="image">
                    <visible>String.IsEqual(Container(9000).ListItem.Property(submenuVisibility),movies)</visible>
                </control>
but it doesn't work in any control inside this control
Code:
                <control type="list" id="8000">
                    <control
                        <visible>String.IsEqual(Container(9000).ListItem.Property(submenuVisibility),movies)</visible>
                    </control>
                </control>

The second one is always false, it has no access to the content in control 9000, neither does anything else in a control, so variables that use a condition based on Container(9000).ListItem.Property() are always false.

Thread where I originally posted this. It shows that even a label with that as the string is empty.


RE: Skinners: Please test PR13754 - MacGyver - 2019-01-25

So it's just me?
Seems like you may be real close to putting out a final release so I'm a little worried that I'm going to have to hide a bunch of stuff to make look right.

You can see it in Estuary if you replace line 253 of Home.xml with:
Code:
<param name="widget_header" value=$INFO[Container(9000).ListItem.Property(widgetType)]/>

You will see that the Addons header is now blank too. Anything in 8000 is unable to see the properties from an object in 9000.


RE: Skinners: Please test PR13754 - jurialmunkey - 2019-01-25

@MacGyver - Are you sure this worked previously? I don't think that you've ever been able to reference another container from inside a list control - same as with window properties. But maybe I'm mistaken?