phil65 / skin.estuary

Public repository for the Kodi default skin "Estuary"
Other
26 stars 82 forks source link

Allow toggling of DialogSeekBar in Music Visualisation window #194

Closed jjd-uk closed 7 years ago

jjd-uk commented 7 years ago

An alternative to https://github.com/phil65/skin.estuary/pull/191 as that doesn't work using Master. This is PR'd against Krypton as this version also doesn't work in Master.

This allows the seek bar and other OSD elements to be dismissed, they will then only reappear on a track change for 10 seconds or if explicitly asked for. This changes Estuary to match the behaviour of how the Music Visualisation OSD elements operated in Confluence, and also to align behaviour with full screen video.

I've not been able to figure out why this doesn't work when applied in Master, seems to due to a difference somewhere else other than DialogSeekBar.xml.

jjd-uk commented 7 years ago

Fix for this issue https://github.com/phil65/skin.estuary/issues/189

host505 commented 7 years ago

What isn't working on master? On my end it works, as well as #191.

jjd-uk commented 7 years ago

What version of Kodi have you tested Master with? It doesn't work for me when used in conjunction with Kodi V18 which is what Master branch is for, I've not tested what happens when using Master for V17.

host505 commented 7 years ago

Ah, sorry you're right, I'm testing with 17.

host505 commented 7 years ago

Btw I think fullscreeninfo window only applies to video playback, the equivalent for visualisation is Player.ShowInfo, no?

jjd-uk commented 7 years ago

fullscreeninfo and Player.ShowInfo can both apply in video and music, although how they are used in Estuary does appear to different between Video and Music.

phil65 commented 7 years ago

fullscreeninfo and Player.ShowInfo can both apply in video and music, although how they are used in Estuary does appear to different between Video and Music.

not correct, fullscreeninfo only applies to video, Player.ShowInfo can apply to both, but doesnt have to.

phil65 commented 7 years ago

The propopsed implementation here is spagetthi-ish, please test https://github.com/phil65/skin.estuary/tree/seekbar .

host505 commented 7 years ago

I can't install this on latest Krypton nightly for windows (KodiSetup-20170123-138845b-Krypton), it says it requires xbmc.gui version 5.13.0. Is this not intended for Krypton (xbmc.gui=5.12.0)?

jjd-uk commented 7 years ago

@phil65 Thanks for taking the time to look at this. I've testing on both Krypton and Leia and all seems to be working as expected. Observed behaviour:

Info initially appears for 10 seconds then disappears, then reappears for 10 seconds on each track change.

Pressing I key (for Info) with no Info on display will then permanently display the Info for those that want it, until I is pressed again to dismiss it.

@host505 I guess you used the install zip function, I used the backdoor and simply erased contents of skin.estuary folder then copied the contents of the zip from phil's branch into the folder. There were some oddities on Krypton because the branch is probably based on Master.

jjd-uk commented 7 years ago

And it seems I've a lot more to learn when it comes to skinning.

host505 commented 7 years ago

@host505 I guess you used the install zip function, I used the backdoor and simply erased contents of skin.estuary folder then copied the contents of the zip from phil's branch into the folder. There were some oddities on Krypton because the branch is probably based on Master.

Yes, I figured that's based on master-branch. What I did to test on Krypton is to apply just this commit (just the +2,-1 lines of DialogSeekBar.xml) on this github's krypton-branch. It worked. Then I applied the commit on current Krypton nightly's version of the skin (as I don't know which one is ahead -this krypton-branch or the one that comes with kodi's krypton nightlies) and worked there too.

So, if that change is intended for krypton too, it works and I guess can be backported.

PS I only checked visualisation and fullscreenvideo, I'm not sure if other elements can be affected by this and need more testing.