Skin condition cleanup
#16
Thanks for the feedback guys, keep it coming Smile

Some responses:

fanart.image
I'll hold off on this one until the case it's being used for is eliminated. It's a tricky one as technically does the ".." item represent the parent? Not sure!

isequal(info.label,foo)

This will be used wherever there's an existing info label available to be compared to. A simple example is skin.hastheme(foo). We already have skin.currenttheme available, so you can already do isequal(skin.currenttheme,foo). Thus, skin.hastheme(foo) is unnecessary, we can drop it.

videoplayer.time etc.

These are all just wrappers (have been for a long time) to player.time etc. We're just cleaning up here. Long term I'd quite like to eliminate the videoplayer/musicplayer distinction as you shouldn't have to care about it.

window.isnext(id)

This one is tricky, I admit. It could be done a bunch of ways:

isequal(window.next, id)
window(id).isnext
isequal(gui.nextwindow, id)

etc. The key is that "next" isn't really a property of the window object: the window object doesn't know that it's the next window to be shown. I kinda prefer the last one to be honest because of this.

The idea is to keep things logically consistent from an API perspective - foo.bar should always mean that bar is a property of foo.

The advantage of the isequal technique here is, as pieh points out, it allows you to use integergreaterthan for comparison, and later on we could add more useful comparison techniques.

I definitely think a script can be written to take care of MOST this for you - don't worry, we won't be forcing this on you without it automatically being taken care of as much as possible. The ones that can't be done automatically are highlighted in the first post. Basically just the ones that are being deleted, as well as the change from player.rewinding2x -> player.rewindspeed. We'll see if we can arrange the script to detect and highlight these for you.

@pieh/ronie: would greatly appreciate if you could take some time to do a script for this Smile I'll TRY and add backward compatibility when we change things anyway, but note that the backward compat code will NOT be available in Eden.

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#17
jmarshall Wrote:Conditions that will be changed:
library.isscanningvideo -> library.isscanning(video)
library.isscanningmusic -> library.isscanning(music)
While I realize these changes are targeted at skins. The two above are useful for my new implementation of rob webers xbmc library auto update add on. So will be watching for the commit accordingly. Thanks!
Reply
#18
Just wondering, will fanart.color1/2/3 be removed entirely or will you just not be able to use it as a conditional value anymore.
Reply
#19
@Mntz: Remove entirely was the plan, yes, primarily as only thetvdb.com returns the information anyway.

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#20
jmarshall Wrote:window.isnext(id)

This one is tricky, I admit. It could be done a bunch of ways:

isequal(window.next, id)
window(id).isnext
isequal(gui.nextwindow, id)

etc. The key is that "next" isn't really a property of the window object: the window object doesn't know that it's the next window to be shown. I kinda prefer the last one to be honest because of this.

I dont' really understand what next window is but just wanted to throw it out there, one could go they more objective route and have window.next actually refer to the window, so isequal(gui.nextwindow.id, id) or gui.nextwindow.id.equals(id) ? If window.next actually only points to the id perhaps it would make sense to state that more explicitly in the property?

jmarshall Wrote:@Mntz: Remove entirely was the plan, yes, primarily as only thetvdb.com returns the information anyway.

Cheers,
Jonathan

Wouldn't it be possible to move them to be a completely general property which "might" exist and the skinner can care for it if it really wants to? I have a feeling we will always have some differences between what properties the scrapers provide (movie title, year, plot, cast, poster, fanart are the only enforced afaik) and limiting what can be used seems a bit sad. I guess we need to have some form of nice defaults (or ways in skin to tell if its existing?) item.fanart.color3.exist perhaps? or go the super generic route of item.property(fanart.color3) but I must say I like the properties method rather than the getter method.
If you have problems please read this before posting

Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.

Image

"Well Im gonna download the code and look at it a bit but I'm certainly not a really good C/C++ programer but I'd help as much as I can, I mostly write in C#."
Reply
#21
jmarshall Wrote:@Mntz: Remove entirely was the plan, yes, primarily as only thetvdb.com returns the information anyway.
I do think it's a nice unique skinning feature which has lots of potential but hasn't been used a lot because of the lack of returned colors.
Maybe it's easier that XBMC automatically generates three colors just like the auto generated colors on thetvdb.com instead of relying on defined ones.
I remember doing some skinning and mockup tests with the automated colors and they were at least as good/usable Smile
Reply
#22
that's output of my quickly baked scipt that would convert infolabels http://pastebin.com/q2tZa5NX, currently it just highlight some of the proposed changes - I now get it why You mentioned ListItemNoWrap Wink
Always read the XBMC online-manual, FAQ and search the forums before posting.
Do NOT e-mail Team-XBMC members asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting, make sure you read this first

My previous forum/trac nickname: grajen3
Reply
#23
pieh Wrote:that's output of my quickly baked scipt that would convert infolabels http://pastebin.com/q2tZa5NX, currently it just highlight some of the proposed changes - I now get it why You mentioned ListItemNoWrap Wink

Lol, yeah I got a lot of them. Nice work, obviously I have a lot to learn when it comes to making scripts. Well, no reason for back compatibility if everyone can use a script to make the changes.
Reply
#24
Nice work pieh Smile

@Mntz: basically there's no real reason to remove it other than cleaning the API up. I'll leave these until last in case a better idea comes up - the code overhead really isn't very much anyway.

@topfs2: agreed - id needs to be in the property if that's what we're returning. Basically the windowmanager sets what the next window id is at the start of the window close anim (and then sets previous window id at the start of the following window open anim).

This highlights an issue I hadn't thought about though - by forcing it to return a numeric ID we miss the niceties of being able to label things using the window name. Switching back to window(ID).isnext seems a lot nicer now...

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#25
jmarshall Wrote:This highlights an issue I hadn't thought about though - by forcing it to return a numeric ID we miss the niceties of being able to label things using the window name. Switching back to window(ID).isnext seems a lot nicer now...

While as you said it might be window manager who "knows" it, having it as a property on the window sounds sane to me no matter the niceness stuff. window(ID).isnext is quite clear and is very short in comparison to the others (which is also a factor which might be valid). And as you say, being able to use non numeric identifiers is a real valid thing (almost crucial), and forcing numeric or string might not be good in the slightest. the isequal would need to be quite an advanced function to coop with it.

gui.nextwindow.ID.equals(ID) would be possible to do the more advanced equals I think, if we are able to map it objectively in cpp, which might be hard?. Since if we have a good objective mapping that equals should be possible to override I guess?

The benefit of gui.next.window.equals(id) is it might be possible to have id to be a regexp of sorts, so the niceness stuff is even more. as an example
Code:
gui.nextwindow.ID.equals([audio|video]*)
Would return true on a window beginning with audio or video, this would never work API wise in window(ID).isnext as window(ID) need to return one specific window. I'm not sure if its a sane thing to be able to do but just thought I'd throw it out there for discussions sake.
If you have problems please read this before posting

Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.

Image

"Well Im gonna download the code and look at it a bit but I'm certainly not a really good C/C++ programer but I'd help as much as I can, I mostly write in C#."
Reply
#26
here's initial sketch of script: http://pastebin.com/R4JNWhrD
download and save as info_label.py (add permission if linux) and then run
Code:
python info_label.py path_to_xmls

additional params:
-recursive : go trough dirs in specified path
-byline : each line will ask if change or not (it will produce directory "corrected" with new .xmls)
-autocorrect : will correct each line (it will produce directory "corrected" with new .xmls)
-o=path_to_output : it will print output to seperate file to check it one by one later
this have only few new infolabels entered )just demo) - will update it when new infolabels are settled

It's not finished - but I'm going out of time and thought I can share what I already have,
Cheers!
Always read the XBMC online-manual, FAQ and search the forums before posting.
Do NOT e-mail Team-XBMC members asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting, make sure you read this first

My previous forum/trac nickname: grajen3
Reply
#27
pieh Wrote:......

It's not finished - but I'm going out of time and thought I can share what I already have,
Cheers!

OMG pieh is going into LIMBO where time does not exist Shocked Laugh
Reply
#28
Jezz_X Wrote:OMG pieh is going into LIMBO where time does not exist Shocked Laugh

:-)

I would sure like to see one of you natives trying to speak some Slavic language, I'm sure it would be out of body experience :-)
My skins:

Amber
Quartz

Reply
#29
pieh Wrote:here's initial sketch of script: http://pastebin.com/R4JNWhrD
download and save as info_label.py (add permission if linux) and then run
Code:
python info_label.py path_to_xmls

additional params:
-recursive : go trough dirs in specified path
-byline : each line will ask if change or not (it will produce directory "corrected" with new .xmls)
-autocorrect : will correct each line (it will produce directory "corrected" with new .xmls)
-o=path_to_output : it will print output to seperate file to check it one by one later
this have only few new infolabels entered )just demo) - will update it when new infolabels are settled

It's not finished - but I'm going out of time and thought I can share what I already have,
Cheers!

Script is working as expected here.
Reply

Logout Mark Read Team Forum Stats Members Help
Skin condition cleanup1