Closed BobCratchett closed 3 years ago
Also one of those absolutely-inconsequential-but actually-means-a-lot things. Make sure you add your name to the provider-name in the addon.xml
@BobCratchett Sorry about late reply I never got an email so just happened to notice the new issue now...
1) Doc had no link at all and I couldn't find where it was supposed to link to. This is what was there before I deleted it: [Default Icon]() - [Read More]()
2) Due to the removal of the old way to set art and the continuing development of Kodi where things can sometimes change drastically I was gonna make it Leia or even just Matrix and above. Seems like a never ending battle adding backward compatibility so was just gonna keep the upgrade action function. Unless that doesn't actually do what I think it does... 😕
Your comment makes me think it is the part to focus on when users have issues updating shortcuts and not have the code to actually function on previous kodi versions. Hopefully that makes sense.
I can do minor point bump, wasn't thinking of that cause it was just a branch that I was playing around with and wanted to make sure it wasn't a stupid idea before actually committing to it.
I kinda like the idea of the script updating shortcuts on its own for new Kodi versions so skinners don't have to add any of that stuff themselves. If you think that makes sense and as long someone can continue to make work 😏
I don't know about adding my name. Always feels weird cause you, marcel, and others put so much time and effort into the script then I come in and plop my name down. Just doesn't feel right somehow, probably being silly I know but it feels like I am taking credit for something when I don't think I deserve it...
OK, fair enough on (1) - That was already broken. I think it was meant to link to https://kodi.wiki/view/Default_Icons
(2) Yes, upgrade actions does what you think it does. However, it only does it for versions its programmed for. So, if it's programmed for, lets say, upgrading Kodi version 13 to 15, that's fine. Here's the problem - if it doesn't know about Kodi 13, and the user is going from 14 to 15, then the upgrades won't work.
Honestly, I've been away long enough that, frankly, I know this is has already been an issue with updated Kodi versions. More pressing, the current way the function works is to look at the original function, and if necessary write an updated function to the includes whilst keeping the original function as-is. If only specific versions are supported, then the actual .xml file needs the updated function writing to it, rather than merely working with whatever-the-updated-function is. (if that makes sense!)
Yes, you should absolutely add your name. Though that is dependant on a very difficult conversation - who is going to maintain this script going forward? I think my attempts to even come back and code sparingly - which I'm really struggling with, though I'll keep going - mean I'm not going to be leading the development of this script any time soon. Marcelvedlt has moved onto pastures new. The honest truth is this script needs someone who can keep plugging away, though not necessarily quickly - and who will add their name to the file - if it's going to survive much longer.
Now that I know where the link is supposed to go I will add it back to the docs. Thanks for pointing the way. I thought it was a link to one of the md files and I just couldn't find it.
I plan on taking care of the script, if I can, and I am hoping to learn python well enough to do so. When I have trouble I will ask for help or just keep trying things until it works 😉
Regarding the upgrade function. Just want to make sure I understand what is actually happening.
It does update the script-skinshortcuts-includes.xml file but the old action is still saved in the addon_data/script.skinshortcuts
xml files?
So the next time an upgrade happens the same thing occurs cause those original actions defined there are the same? I hope I got that right...
Currently there are 4 versions of the script in the Kodi repo already. Gotham v0.4.5 Helix 0.5.4 Isengard v1.0.13 Jarvis v1.0.17
Wouldn't the if int( KODIVERSION ) <= 15:
cover Kodi 13 though? I am assuming here but the less than or equal to 15 should cover 13 as well. I didn't check the skin engine changes to make sure the necessary path updates are the same ones that are defined in upgradeAction though.
Thanks 😃
https://github.com/mikesilvo164/script.skinshortcuts/releases/tag/2.0.0.alpha2 or higher is now Kodi 19+/Python 3 only
Two comments regarding the removing backward compatibility branch...
(1) Docs - at least one case where the docs probably had incorrect markup, but the update removes the link entirely rather than corrects it - for example resources/docs/advanced/Overriding icons.md - line 49.
(2) Is the script going to be limited to a minimum Kodi version? For various reasons, the idea has always been to support as older a version of Kodi as possible. Hence why we have $ISEMPTY, rather than the (now) String.IsEmpty (for example, default.py line 144.). It's not a bad idea to do so, but, if so, you need to ensure that the upgrades work from any version of Kodi previous to the minimum version you're specifying with this version / the previous version (it was probably a really bad decision early on, but I decided rather than try to maintain a number of versions of the script for a number of Kodi versions - particularly when the team was promoting 'rero' which they seem to have dropped - it was going to be easier to maintain one version of the script that supported a number of versions of Kodi...)
You also are almost certainly going to want to do a minor pint bump (x.Y.x), rather than a revision point bump (x.x.Y) - it might seem inconsequential, but it really does make sense when you're making breaking changes.,
I can't see any particular issues, but you need to pay particular attention to dataFunctions.py/upgradeAction(), especially if you're only going to support Kodi version x and above in a particular version of the script (and, if you wanted a real challenge to get your python skills in shape ... this would be so much better with the necessary upgrades being defined in the overrides.xml, rather than in python itself...)