chrisbevan / skin.pellucid

A skin for Kodi by theDeadMan
Other
46 stars 43 forks source link

Testing Krypton release candidate #13

Closed paulvt closed 7 years ago

paulvt commented 7 years ago

Hi, I have been testing Krypton, also with the Krypton branch of the skin (release candidate 2), and I currently have two issues:

The logs are constently spammed with: ERROR: CGUITextureManager::GetTexturePath: could not find texture 'DefaultAudio.png' ERROR: CGUITextureManager::GetTexturePath: could not find texture 'DefaultShortcut.png' However, this might be a result of running it directly from git.

Also, I cannot enter the Videos or Music main menu item at all, and there is no error logged whatsoever?

chrisbevan commented 7 years ago

Hi Paul, thanks for testing and reporting!

Both issues sound like teething problems with the skin.shortcuts addon to me. The texture issue is an easy fix, i'll add those two to the next update (neither image is ever used in the skin, but for the sake of a couple of .pngs I might as well keep the logs happy!)

The second issue is more worrying. I'm pretty sure there's an issue with skin shortcuts caused by an older version that doesn't accept controls with two onclick actions assigned to them as a default (it just goes ahead and rebuilds the menu xml on startup, replacing the first onclick action instead of adding another).

Short term fix: 1.) First, can you confirm which version of SS you are using now? 2.) Update SS to 1.0.13 if not already 3.) Delete pellucid/1080i/script-skinshortcuts-includes.xml 4.) reload the skin.

That should rebuild the menus to the correct default and fix music and videos. Please let me know if it doesn't.

I'm keen to bullet proof this before Krypton goes live. There's an override feature in Skin Shortcuts that ought to do the trick ;-)

paulvt commented 7 years ago

About the missing images, it actually logs about 30 images (or more) when moving around!

About the second issue, I did meddle with editing the menu (and thought I undid my change), maybe that has broken something, I'll investigate this weekend and try your fix.

paulvt commented 7 years ago

Removing 1080i/script-skinshortcuts-includes.xml seems to have resolved the problem for now, however now I probably lost what I got in there that broke the menu. Should've moved it aside instead.

Anyway, I actually installed script.skinshortcuts now, I didn't have it at all. I see no depend on it in addon.xml of Pellucid.

chrisbevan commented 7 years ago

Thanks Paul,

Those images are defaults that never actually get used in the skin so I'm loathe to include them all if it's not completely necessary.

With skin shortcuts, i'll need to give that a really good look. The cunning plan was to ship the skin with a default menu (script-skinshortcuts-includes.xml) that would work without needing to install skin shortcuts (hence no depend in addon.xml). The trigger to install is attached to the 'edit menu items' buttons in the sub navs. I'm wondering if there isn't some caching going on somewhere that i've missed...

paulvt commented 7 years ago

Yes, I noticed that installing it, didn't change anything at all. The curious thing is that if remove (git checkout) the file, and reload the skin, it works.. but after starting the file is modified according to git (quite a huge diff), and with next reload of the skin, it doesn't work anymore. So something is generated into the file that cannot be read anymore.

Does it help if I push the not-working, modified version to the repo as a branch somewhere?

paulvt commented 7 years ago

I switched from running some checkout to the 1.0.2 release via the official repository. I still cannot enter the Music and Video menu items; fortunately I remember that I have buttons for that on the remote. Is there any way to debug this? Or maybe I have to upgrade to 17.0 beta7 for this?

chrisbevan commented 7 years ago

Seems like skin shortcuts is still playing up..

To test the update, I removed everything related first:

[delete] addons/skin.pellucid addons/script.skinshortcuts addons/packages (everything) userdata/skin.pellucid userdata/script.skinshortcuts

reboot, then reinstall pellucid from the repo.

That should pull through the latest skin shortcuts, which will then rebuild the menu. Please let me know if it doesn't!

paulvt commented 7 years ago

Yes, that fixed it. I am quite sure it was userdata/addon_data/script.skinshortcuts that blew it. Maybe due to me fiddling with the menu in the earlier release. I have, for example, now been able to add Party mode to the Music menu.

paulvt commented 7 years ago

I want to add that if I add an item to the Music menu, the "Latest Content" menu item stops working for Music. For Video it kept working, until I added Youtube as a menu item. The shortcuts data contains:

        <shortcut>
                <defaultID>31005</defaultID>
                <label>$SKIN[31005|skin.pellucid|Latest Content]</label>
                <label2>Videos</label2>
                <icon>DefaultAddon.png</icon>
                <thumb />
                <action>Skin.SetString(recentitems,video)</action>
        </shortcut>

I am not sure if Skin.SetString() is enough of an action to show the latest content view?

chrisbevan commented 7 years ago

That latest content problem has the same cause as the videos / music thing. For some reason, it's not pulling through the extra action (in this case, open the damn window!)

This is just all too flaky for my liking. I'll find an alternative route.

chrisbevan commented 7 years ago

OK... The latest version should fix these menu problems. You'll need to reset the menu to default after install to pull the changes through.

N.B: I've now merged the Krypton branch into Master.

paulvt commented 7 years ago

I did some extensive testing and found the menu issues to be fixed. Nicely done man! It seems it is ready for Krypton indeed.

Remaining small qualms that I have:

Finally, a nitpick probably due to that I also work on user interfaces: Is there any consistency/rule in the casing of the labels.? I see "Now Playing" (so, title-cased), then "Update library" (ok, just capitalized then), and finally "Sort by: Name" (hmm, mixed?). Ok, so the latter may be logical :).

paulvt commented 7 years ago

Two additional things that came out of my extensive tests:

As extra feedback on a previous point, when switching to the OpenWeatherMap add-on, the weather window works fine, so it breaks only if the selected weather add-on is unable to retrieve data.

chrisbevan commented 7 years ago

Cheers, appreciated.

Yahoo Weather does seem to be playing up of late. I'll see what I can do to check the presence of data before display.

I agree that label casing needs looking at. Like you i'm a stickler for details like this!

Those fullscreen music exit bugs have been giving me a headache for a while.. Obviously I haven't yet found a bulletproof solution. Part of the problem is that i'm using a custom window so I can keep the player controls etc. I'll keep pushing at it.

paulvt commented 7 years ago

I gathered that it was an issue with the custom windows. I also have configured my keymap for my remote such that red goes to Home, green to Videos, yellow to Music and blue to Pictures, but that also seems to break sometimes for probably the same reason. (I generally use the arrows/back/enter anyway.)

Feel free to close this issue, if you think things meet your standards for Krypton that is going to be released soon! I think v1.0.3 is really good already.

chrisbevan commented 7 years ago

Cheers Paul, I appreciate all your input!