2015-01-04, 16:17
(2015-01-04, 14:53)null_pointer Wrote: Do we have a wiki page or other documentation on how this new feature works?
this functionality has not been included in Kodi yet.
this functionality has not been included in Kodi yet.
2015-01-04, 21:57
oh, based on the posts here and the comments in the Pull request I thought it was in.
Any reason for not making it into Helix?
Any reason for not making it into Helix?
2015-01-04, 21:59
(2015-01-04, 21:57)null_pointer Wrote: oh, based on the posts here and the comments in the Pull request I thought it was in.
Any reason for not making it into Helix?
it wasn't ready
2015-01-04, 22:44
Has the conversation continued elsewhere abut adding this feature? Its just that this thread and the git pull request have had no activity for months, is there another thread somewhere?
2015-01-05, 01:33
(2015-01-04, 22:44)null_pointer Wrote: Has the conversation continued elsewhere abut adding this feature? Its just that this thread and the git pull request have had no activity for months, is there another thread somewhere?looks like development has stalled.
rahotep has not visited this forum in months:
Last Visit: 2014-08-30 12:09
2015-01-05, 02:43
It would be disappointing if this feature got dropped, so do we have any idea as to why it was declared as not ready? From the git pull comments it looked like it was on track.
2015-01-05, 04:11
I think development was (and still is) on track, in fact the feature is practically complete, although in two separate Git branches that we might need to integrate. To me it seems the reason for stalling is simply because after having a rather long discussion on Git Pull Reqeust, we still haven't reached the consensus about what the final syntax for the feature should look like. More precisely, we were not sure whether to allow automatic propagation of passed parameters from one include level to another or not. Yes, it leads to a possibly shorter syntax in some (or most) cases, but it could also have nasty side-effects and can be horribly abused. There was also a suggestion to disable it by default and leave it up to skinner to turn it on at skin-level if desired. If anyone's interested, I could give a recap of the current state of the feature here.
Also, in my last post there, I proposed another feature I could develop myself (already did a prototype on my machine), to go hand in hand with the original one, but sadly got no feedback on that one either. This one would make skins easier to debug by letting skinner dump preprocessed skin XMLs from memory back to disk during application run, at the point when the dialog/window in question is opened from the app. Dumped XMLs would have all includes, constants, defaults, etc. resolved. Don't know if others would find this useful.
So, I'm basically all for reviving the discussion on github or here and finalizing the feature
Also, in my last post there, I proposed another feature I could develop myself (already did a prototype on my machine), to go hand in hand with the original one, but sadly got no feedback on that one either. This one would make skins easier to debug by letting skinner dump preprocessed skin XMLs from memory back to disk during application run, at the point when the dialog/window in question is opened from the app. Dumped XMLs would have all includes, constants, defaults, etc. resolved. Don't know if others would find this useful.
So, I'm basically all for reviving the discussion on github or here and finalizing the feature
2015-01-05, 04:33
Thanks for the info rohotep
My gut feel is that params should not auto propagate at all, you should need to set any params that you want to pass on, yes it might be a pain to have to set them for each include but it is safer and easier to understand what is going on that way. So my vote is no auto propagation at all. This might allow unblocking of the feature and allow it to be added also since it looks like the only blocking issue.
Dumping of the processed skin XML data would be awesome, this would make it much easier to track down skin include, var etc issues.
Continuing the discussion here might be the best place as I only found the other discussion by accident when I was interested in the code changes and then saw the full discussion on the git pull request.
My gut feel is that params should not auto propagate at all, you should need to set any params that you want to pass on, yes it might be a pain to have to set them for each include but it is safer and easier to understand what is going on that way. So my vote is no auto propagation at all. This might allow unblocking of the feature and allow it to be added also since it looks like the only blocking issue.
Dumping of the processed skin XML data would be awesome, this would make it much easier to track down skin include, var etc issues.
Continuing the discussion here might be the best place as I only found the other discussion by accident when I was interested in the code changes and then saw the full discussion on the git pull request.
2015-01-05, 10:32
I'm not a skinner so I have no "end-user" view on the whole discussion but magic automatic propagation sounds like trouble to me in general. From the discussion on github it sounds like it would be possible to add a global skin-specific flag to enable/disable it but personally I'd disable it by default. Automated magic can be nice as long as everything is rather simple but as soon as it gets complicated anything that is done automatically and hidden from the developer is usually ausing more problems than it helps.
2015-01-05, 12:40
(2015-01-05, 04:11)rahotep Wrote: More precisely, we were not sure whether to allow automatic propagation of passed parameters from one include level to another or not. Yes, it leads to a possibly shorter syntax in some (or most) cases, but it could also have nasty side-effects and can be horribly abused.
I would stick to a initial idea and leave propagation out completely. IMAO, there are not many cases (if any) when one would need this and it can easily lead to code horror. In addition, such a code could be very difficult to handle by newcomers, modders or someone trying to keep old skin up-to-date when original skin author is not available.
2015-01-08, 00:28
so the question now is, how do we get the changes accepted and included?
2015-01-08, 01:13
The initial feature with explicit forwarding only (p:posx="$PARAM[posx]") is ready to be included.
Like you guys, I'm also for going with the initial design first, and see how that goes. Then, after getting some real-world usage of the feature, if explicit parameter forwarding turns out to be a common pattern in writing modern skins and too cumbersome to write in current form, we could introduce an improved shorter syntax for that without breaking anything. What do you think?
Here's one thing I didn't like about the current proposal for the parameter forwarding shortcut forwardparams="true" (or forwardparams="all") which forwards all passed parameters from enclosing include to the nested include. Since our include definitions don't contain formal parameter lists, nested include call <include forwardparams="true">MyControl</include> wouldn't actually know what exactly it is forwarding and could potentially forward "too much" to deeper levels and override the defaults set there. This looks like a bad design to me, just being able to touch any deep internal stuff from the outside, intentionally or not. This feature is not included in the current Git Pull Request.
Like you guys, I'm also for going with the initial design first, and see how that goes. Then, after getting some real-world usage of the feature, if explicit parameter forwarding turns out to be a common pattern in writing modern skins and too cumbersome to write in current form, we could introduce an improved shorter syntax for that without breaking anything. What do you think?
Here's one thing I didn't like about the current proposal for the parameter forwarding shortcut forwardparams="true" (or forwardparams="all") which forwards all passed parameters from enclosing include to the nested include. Since our include definitions don't contain formal parameter lists, nested include call <include forwardparams="true">MyControl</include> wouldn't actually know what exactly it is forwarding and could potentially forward "too much" to deeper levels and override the defaults set there. This looks like a bad design to me, just being able to touch any deep internal stuff from the outside, intentionally or not. This feature is not included in the current Git Pull Request.
2015-01-08, 04:19
yep agree, explicit forwarding to start with and then when there are more skin devs aware of this feature then perhaps add forwarding as an update in the near future.
2015-04-03, 22:40
OMG ! It's seems that it is what I was looking for. Gone crazy with setting tricky conditional positioning. Is this to set dynamic positions ? If yes, have you further infos on how it works ? Just developped a dynamic layout with user customization ... But this should make me rewrite and save hundreds lines of code !
