BigNoid / script.skinshortcuts

GNU General Public License v2.0
23 stars 69 forks source link

submenuOther template error with labelid matching #220

Closed jurialmunkey closed 7 years ago

jurialmunkey commented 7 years ago

On krypton, when using templates and building widgets for submenus there is an error with the conditions generated for matching submenu items. submenuOther will generate a condition based upon the LabelID. However, when the LabelID is a localized string it doesn't match because the condition matches to the number not the localized string.

e.g. a condition will be generated for the submenu widget: [String.IsEqual(Container(300).ListItem.Property(submenuVisibility),music) + String.IsEqual(Container(302).ListItem.Property(labelID),744)]

However, the LabelID wont match the 744 because the property gets translated to the localized string, but the 744 part does not. This can be fixed fairly easily I imagine by wrapping number in a localize tag: e.g. String.IsEqual(Container(302).ListItem.Property(labelID),$LOCALIZE[744])

ghost commented 7 years ago

Can you please provide a working example of a template/defaults where this doesn't work?

jurialmunkey commented 7 years ago

Yes, in my new skin https://github.com/jurialmunkey/skin.fuse.neue

Templates: https://github.com/jurialmunkey/skin.fuse.neue/blob/master/shortcuts/template.xml

For instance, if I create a submenu widget for a default submenu item I get the following generated in the template section of script-skinshortcuts-includes.xml http://pastebin.com/raw/vBwQMCfM The StringCompare doesn't match when it is a string id number (369 in this case)

However, if I change the name of the submenu label to a custom label it works, generating this: http://pastebin.com/raw/mvLsSYTq The StringCompare matches because it is a non-localized string (editname in this case)

ghost commented 7 years ago

Apologies, but I still can't reproduce this with your new skin + it's defaults.

    <include name="skinshortcuts-template-home-widget-Master user">
        <control id="301" type="fixedlist">
            <visible>Control.HasFocus(300) | String.IsEmpty(Container(302).ListItem.Property(widgetType))</visible>
            <visible>String.IsEqual(Container(300).ListItem.Property(submenuVisibility),num-3) | String.IsEqual(Container(300).ListItem.Property(submenuVisibility),num-20342) | String.IsEqual(Container(300).ListItem.Property(submenuVisibility),tvshows) | String.IsEqual(Container(300).ListItem.Property(submenuVisibility),num-2) | String.IsEqual(Container(300).ListItem.Property(submenuVisibility),num-24001) | String.IsEqual(Container(300).ListItem.Property(submenuVisibility),settings)</visible>
            <autoscroll time="10000">!Control.HasFocus(301)</autoscroll>
            <include content="View_Posters_Layout">
                <param name="id" value="301" />
            </include>
            <content target="">$VAR[Defs_Widget_Content]</content>
        </control>
    </include>

Do I need to follow any particular steps to reproduce this issue? (Add anything specific to a menu, for example?)

jurialmunkey commented 7 years ago

It only happens for user created submenu widgets. You need to create a widget for a submenu item and that submenu item has to be a default submenu item (e.g. sets or recently added episodes). The widgets for main menu items work fine.

What happens is the visibility condition for that submenu widget template that is generated uses a stringcompare to the labelid of that submenu item (these are not my skin included default items, but rather the default submenu items provided by skinshortcuts). However, for some default submenu items the labelid is simply an id number rather than an actual string. Because the stringcompare condition doesn't wrap the string id number in a $LOCALIZE[] tag it doesn't match (because its matching to a number rather than the localized string it references). I know this is the issue because when I copy the skinshortcuts-includes to a separate file and modify the condition so the string id is in a localize tag it will then display.

ghost commented 7 years ago

Thanks for the clarification, I've now managed to reproduce. I'll fix it as soon as I can - Thursday with luck, early next week without.