jeroenpardon / skin.grid

Grid, a UI for Kodi. By using this code you agree with the license terms as included.
http://www.gridskin.net
Other
39 stars 25 forks source link

Public Release #98

Closed jeroenpardon closed 7 years ago

jeroenpardon commented 7 years ago

So, with the beta going public I have closed the previous thread and opened a new one for the official first release. I would like to keep a systematic approach on the road to the official release (meaning Grid being submitted to Kodi's repo). That means I would like for all of you to stick around :)

I will keep pushing fixes on a pretty much daily basis, and after a couple of fixes have been done, I will push them collected in a point update to the add-on repo (which has moved btw, because of this)

So you will be pre-testing so to speak ;)

Do not in any way feel obliged, I know you all have lives too :)

Ostrymiecz commented 7 years ago

Two more strings to ask Jeroen: 31201 31225

Where are they used?

KOKONUTCREME commented 7 years ago

Congratulations on the public beta Jeroen.

When you refresh a Movie there's a white block of colour appearing that fills about half the screen momentarily behind the dialog window.

Is there a trick to adding a custom item to the widget content menu? Noticed after refreshing the menu that some library items had this option but not all.

jeroenpardon commented 7 years ago

@Ostrymiecz both used here: http://i.imgur.com/lmD6blF.png

@KOKONUTCREME I'll have to check that refresh issue tomorrow. About the custom item, it looks like it's not available until after you have already already set it to something. I must admit though that I never noticed that. I think it's a skin shortcuts issue as I am not overriding any of the default configuration here. I'll do some checks anyway. Cheers!

Ostrymiecz commented 7 years ago

@jeroenpardon so… looks like they're duplicated? Check strings 31037 and 31038.

KOKONUTCREME commented 7 years ago

@jeroenpardon

Thanks, the custom widget items that appeared after I reset the home menu to default were

Popular TV Shows In-cinema movies Recently added episodes Popular Albums

Ostrymiecz commented 7 years ago

@jeroenpardon Ah I see: `

$LOCALIZE[31005]: $LOCALIZE[31034]
    <value condition="String.IsEqual(Container(211).ListItem.Property(widgetLayout),wide)">$LOCALIZE[31005]: $LOCALIZE[31035]</value>
    <value condition="String.IsEqual(Container(211).ListItem.Property(widgetLayout),landscape)">$LOCALIZE[31005]: $LOCALIZE[31036]</value>
    <value condition="String.IsEqual(Container(211).ListItem.Property(widgetLayout),portrait)">$LOCALIZE[31005]: $LOCALIZE[31037]</value>
    <value condition="String.IsEqual(Container(211).ListItem.Property(widgetLayout),square)">$LOCALIZE[31005]: $LOCALIZE[31038]</value>
    <value>$LOCALIZE[31032]</value>
</variable>`

`

    <value condition="String.IsEqual(Container(211).ListItem.Property(widgetLayout),$LOCALIZE[31053])">&#59133;</value>
    <value condition="String.IsEqual(Container(211).ListItem.Property(widgetLayout),$LOCALIZE[31202])">&#59141;</value>
    <value condition="String.IsEqual(Container(211).ListItem.Property(widgetLayout),$LOCALIZE[31201])">&#59154;</value>
    <value condition="String.IsEqual(Container(211).ListItem.Property(widgetLayout),$LOCALIZE[31225])">&#59112;</value>
    <value condition="String.IsEqual(Container(211).ListItem.Property(widgetLayout),$LOCALIZE[21371])">&#59141;</value>
    <value condition="String.IsEqual(Container(211).ListItem.Property(widgetLayout),$LOCALIZE[20020])">&#59137;</value>
</variable>`

Those strings are used here. Is it good or duplicated? The ones in "widget:Layout" uses "name (x:x)" scheme and the "widget:Layout.Symbol" uses just "name" without proportions.

jeroenpardon commented 7 years ago

Yes, because in widget:Layout.Symbol they aren't actually used to show any strings, but to evaluate what the variable is set to and then display the appropriate icon for it. Actually using the localized strings there isn't a a very good idea now that I think about it. But as the strings are used elsewhere, they should stay in the strings file.

Ostrymiecz commented 7 years ago

OK, but should I translate them or just copy english ones?

Edit: Nevermind, I've also translated those strings. BTW. You can fetch fresh Polish translation from Transifex.

jeroenpardon commented 7 years ago

Sure, you can can translate them. I already adjusted the variables to use the non localized strings

Thanks, I'll be merging the translations later

jeroenpardon commented 7 years ago

@loggio, if you're around (or anyone else with a pvr setup), could you take a look at this comment on the forum: https://forum.kodi.tv/showthread.php?tid=318152&pid=2635822#pid2635822 Could you verify the first issue that is mentioned is correct? Should it show a preview of the programme here, or does it always show the tv channel logo? Could you compare and check with Estuary? Looking at the code it should be pretty straight forward, but in the case of PVR you can never be sure it seems.. ;)

Ostrymiecz commented 7 years ago

@jeroenpardon Polish language just updated against latest English strings.

loggio commented 7 years ago

@jeroenpardon

@loggio, if you're around (or anyone else with a pvr setup), could you take a look at this comment on the forum: https://forum.kodi.tv/showthread.php?tid=318152&pid=2635822#pid2635822 Could you verify the first issue that is mentioned is correct? Should it show a preview of the programme here, or does it always show the tv channel logo? Could you compare and check with Estuary? Looking at the code it should be pretty straight forward, but in the case of PVR you can never be sure it seems.. ;)

Sorry for the late response jeroen. I can confirm this is not working.... However if I comment out line 631 It works fine again. <!-- <value condition="String.IsEmpty(Container.Content) + !ListItem.IsFolder">$INFO[ListItem.Art(thumb)]</value>-->

It all depends on the order of which these lines of code are written. If I move that line of code down the list, after the Skin.Helper code then works fine also.

As for the timeline view, the comment from Fenderman says that there should be "picture preview on the bottom left" and circles the area described... I also think you should add this, it feels odd without it.

jeroenpardon commented 7 years ago

Thanks @loggio for checking that out. Moving lines here is tricky as it can break things elsewhere. To try and make a long story short: Because Kodi has been going back and fourth between how add-ons should or shouldn't behave over the years, without actually enforcing these "rules" we have ended up with the biggest mess it ever was in this regard. So that line is needed for some add-ons in some situations. If I move it down some addons will use the wrong value because they match another value in the variable conditions first.. It's frustrating as hell. Anyway, thanks I'll try some things out. :)

Slurrrp commented 7 years ago

@jeroenpardon hey.. can you pls add the now playing album art back in? ;)

And is it expected behavior when you select blur effect the animated backgrounds get disabled (and greyed out)?

jeroenpardon commented 7 years ago

@Slurrrp I didn't take it out. Seems to be a Kodi bug where the MusicPlayer.Cover infolabel doesn't show anything, but also doesn't "tell" the skin it's empty either, resulting in a black screen.

Player.Art(thumb) however still returns the album cover though, so I'll use that as a workaround.

And yes, it's expected behaviour, there's no way to combine these properly. Short explanation: for the blur effect a copy of the fanart is created and a blur effect is applied to it. Then that processed image is laid on top of the original fanart image with a diffuse mask applied to it. In other words, the blur is not a real time effect and animating it would also move the blurred area out of position.

Slurrrp commented 7 years ago

@jeroenpardon okay.. understood, but i liked how it was before.. blur and everywhere animated backgrounds except on the homescreen.

But yeah.. you cant satisfy everyone ;)

jeroenpardon commented 7 years ago

@Slurrrp tbh, the background not animating on the homescreen was more a bug than a conscious decision ;) I'll make the following skin setting:

KOKONUTCREME commented 7 years ago

@jeroenpardon

Installed latest release from your repo and very impressed with the changes and improvements you've made to the skin.

Scrolling through the poster and landscape views is so much smoother than before.

Addition of support for weather fanart is a nice touch, still prefer the vertical menu for aesthetics and ease of use, but the blur and fanart visual effects employed for the horizontal menu are very cool.

Well done, very proud to have been involved with everyone else in the testing.

jeroenpardon commented 7 years ago

thanks @KOKONUTCREME that's good to hear 👍

jeroenpardon commented 7 years ago

We're getting closer to an official release. There are't a lot of (big) issues that I am aware of, just wishes basically ;) With that also comes this repository going public., that will be happening pretty soon I think. Next week maybe, or even this week, not sure yet.

I am also contemplating moving this repo to Gitlab, where my repository add-on is located already. Should I ever want to temporarily move the repo private again, Gitlab offers private repos for free to open source projects whereas I am paying a monthly fee for that with Github. Gitlab also allows you to restrict issue creation to collaborators, which is something I am interested in. Then again, I don't believe add-on submission to the Kodi repo can be done without Github yet, so I guess I would have to at least have two locations.

Anyway, I just want to thank you all again for your contributions and participation. It has been a great ride :) 👍

Ostrymiecz commented 7 years ago

Thanks Jeroen. It's always a big pleasure to work with You. 😊

For the Grid! ⚔️🍺

Ostrymiecz commented 7 years ago

Jeroen how about updating changelog.txt on each release? In some skins I'm able to actually see changelog via button click on add-on info screen. Maybe You could add such button?

Actually I don't remember if he was in reFocus 🤔

jeroenpardon commented 7 years ago

I will only update the changelog on each major release that will be pushed to the Kodi repo. For everything in between releases, Github is your changelog :) Don't think I will add a button for it, but use the description area in the addon info screen for it, as that has plenty of room for it.

Ostrymiecz commented 7 years ago

Jeroen I don't see two new strings included in any XML right now. Those are: #31004 Skin shortcut

#31072 Common menu item

Are they for some near future?

jeroenpardon commented 7 years ago

No, these are in use already in overrides.xml

Ostrymiecz commented 7 years ago

In which lines? I don't find any strings macthing $LOCALIZE[31004] or $LOCALIZE[31072]. Am I missing something? 🤔

jeroenpardon commented 7 years ago

https://github.com/jeroenpardon/skin.grid/blob/a1764d437208a020bf07c272b2c0a31d59a73ffb/shortcuts/overrides.xml#L92

https://github.com/jeroenpardon/skin.grid/blob/a1764d437208a020bf07c272b2c0a31d59a73ffb/shortcuts/overrides.xml#L145

among others, but I don't think that's very relevant for translating?

Slurrrp commented 7 years ago

hey @jeroenpardon, the full screen album art isn't working again :P and i hardly ever do it like this, but when you use 'z' to change the view mode, the text doesn't show.

jeroenpardon commented 7 years ago

@Slurrrp Well, at this point I'm totally convinced this is an issue with Kodi.

The variable that determines what type of artwork to show is really simple:

<variable name="player:Art">
    <value condition="!String.IsEmpty(MusicPlayer.Property(Fanart_Image))">$INFO[MusicPlayer.Property(Fanart_Image)]</value>
    <value condition="!String.IsEmpty(MusicPlayer.Cover)">$INFO[MusicPlayer.Cover]</value>
    <value>$INFO[Player.Art(thumb)]</value>
</variable>

This says: if there's fanart show that, else if there's album art show that, else show a thumbnail.

If I take away all the other options, MusicPlayer.Cover doesn't show anything for me on any track I tried, just a black screen. Even if the track would have no cover art, it should move on to the next and last option, which is the thumbnail. In the case of songs the cover and thumbnail are the same image, but with other media it is not. I will file a bug report to Team Kodi for this.

By the way, how do you set your album art? Do you have like a folder.jpg in a folder or do you embed the artwork in the ID3 tags?

Slurrrp commented 7 years ago

I use a cover.jpg in almost all folders, but embedded, for example, doesnt work either.

The full screen album art works in (most) older builds of grid though?

jeroenpardon commented 7 years ago

Yes, but the changes made there caused issues with fanart images for others (people were getting album covers instead of fanart)