stoli / Metropolis

Continuation of Metropolis skin for XBMC
Other
23 stars 22 forks source link

Skin Shortcuts builds menu XML twice after leaving script-skinwidgets.xml window #348

Closed jingai closed 9 years ago

jingai commented 9 years ago

Only when there is a change to the furniture. Not sure why, but it's a tad annoying.

jingai commented 9 years ago

@Ignoble61, if you have a spare moment at any time, would you mind taking a glance at the Skin Shortcuts implementation on the refactor branch here? Just wanting to verify we didn't do anything grossly wrong, since the documentation for it is a bit scattered.

ghost commented 9 years ago

I'll download and play with it when I'm back in front of my own computer next week. Having had a very quick peak at the code, can I suggest two very minor changes:-

Anything you'd particularly like me to look at? (And any areas of the docs that are particularly confusing? I've begun a - very very slow - re-writting-in-a-wiki-style for them, so it's a good time to get any feedback about them in ;))

jingai commented 9 years ago

As for the documentation.. well, it's hard to say exactly, but I just didn't feel like I had a very good overall picture of how it worked from the documentation alone. Probably a more thorough overview at the beginning, with maybe an example step-by-step (barebones, but including main menu and one submenu) implementation, would be good.

Regarding what I'd like you to look at.. nothing in particular, really. Just wanted you to glance at it and make sure nothing was horribly wrong lol.

ghost commented 9 years ago

My bad on the home.xml - that's what I get browsing code on github :blush: Build line there is absolutely fine.

I don't know if you've seen the examples in README.txt - the idea of them is to show the barebones, or are you saying you would like to see more detail? (Either way, they haven't been updated in a very long time, so at the very least need a review to check they're still 100% correct, and I can see from a quick glance there is some now-outdated info in that file)

OK, will have a general play when I get chance :)

jingai commented 9 years ago

I think the out-dated info was honestly most of my problem. I just felt like I didn't have a good grasp on it until I read through the forum thread as well, where I "followed the script's development".

Sorry I'm not being more specific. It was just small feelings of doubt as I went along is all.

Thanks for having a look at Metropolis, btw. Don't feel pressured to spend too much time on it. I know that you, like me, are probably busy with other stuff too :)

ghost commented 9 years ago

Not a problem :) As I say, updated docs are coming - the idea of them is to split every topic (including the bare-bones) out into its own section which will hopefully make things a lot clearer - if you don't mind, when they're close I'll hit you up to take a look through. I'd also love at some point (if I ever find the time and the correct method) to fork Confluence and maintain it as an example skin, but that's a pipe dream for now.

jingai commented 9 years ago

I'll definitely have a look through them when you're done. Just ping me here or on the forums when the time comes.

And a working example that's always up-to-date (fork of Confluence) would be perfect indeed.

MacGyverr commented 9 years ago

Did you intend to not let the user alter the sub-menus? https://github.com/stoli/Metropolis/blob/refactor/720p/script-skinshortcuts.xml#L333

jingai commented 9 years ago

Ugh.. I remember.. it was an artifact from when I was trying to get the dialog to display over top of the others right. Thanks for spotting it!