Overriding plugin loadSkin is not longer possible

[technic]

Hi, Some bigger plugins do not store screen xml data in the python, but use separate xml file instead. Then it is loaded with help of loadSkin(path) function. For long time it was possible to redefine plugin skin with the xml coming from global skin. The trick was that plugin load before skin does and loadSkin does not override screen entry if it already exists https://github.com/OpenPLi/enigma2/blob/release-4/skin.py#L683 But this is gone in this commit https://github.com/OpenPLi/enigma2/commit/dc3a7911f3de586c1069b606bccb43becddc4d17#diff-b2a9ece706415994fb9593fe11dfdc8aL881 Is there another option now?

[nautilus7]

The code you linked to isn't exactly the same though? I don't see any change.

[technic]

The code you linked to isn't exactly the same though? I don't see any change.

The second link is the patch that removes this code.

[nautilus7]

Sorry, I just woke up and didn't notice the color difference. My bad.

[technic]

Yep, this is not really documented anywhere. However, there was this comment

Now a utility for plugins to add skin data to the screens.

And the feature to override plugin skin is also clearly available, so I think this should be fixed.

[IanSav]

Can someone please provide specific code examples of this issue so I can investigate if my recent changes are involved.

[technic]

So what I mean is the following. You have a plugin, and in plugin.py you have

loadSkin("/path/to/my_plugin_skin.xml")

class MyScreen(Screen):
  def __init__(...):
    ...

Where in the my_plugin_skin.xml you define <screen name="MyScreen"> Then one can not override this skin in the "/usr/share/enigma2/CurrentSystemSkin/skin.xml".

[IanSav]

Why are you using loadSkin? What are you trying to do?

Playing with the dom is not advisable.

[IanSav]

If you simply want to load a screen from disk rather than have it in your code just load the skin into a "skin" variable and make the skin embedded.

Even better make the screen dynamic so that it can self adjust to the current skin and resolution in use.

[RobvanderDoes]

This is about existing plugins, not about creating one.

[Huevos]

These are the ones I can find...

Why are these plugins doing this?

enigma2-plugins/advancedmovieselection/src/Source/LocaleInit.py
enigma2-plugins/advancedmovieselection/src/__init__.py
enigma2-plugins/bmediacenter/src/plugin.py
enigma2-plugins/merlinepgcenter/src/MerlinEPGCenter.py
enigma2-plugins/merlinepgcenter/src/SkinFinder.py
enigma2-plugins/widgets/src/plugin.py
enigma2-plugins/youtubeplayer/src/SkinLoader.py

[technic]

@IanSav having xml code in python also looks bad. Enigma is not html so forget about "dynamic".

Anyway, please provide a correct way of doing this in the documentation and try to be back compatible. This functionality was in the code before, and it has comment "Now a utility for plugins to add skin data to the screens", that tells you exactly that this is valid usage.

[IanSav]

@technic there is absolutely no way I am forgetting about dynamic skins. Huevos and I have only just finished implementing them in our code.

Do your skins adapt to the current skin look and feel and the current resolution? If not would you consider updating your skins? If you want to move forward then I would be happy to give you sample code.

Forcing plugin screens into the dom is not the correct thing to do. It breaks the skin access order.

[Huevos]

I don't get it. The skin is in an xml file. So in your python code use the skin class variable, e.g...

class MyScreen(Screen):
    skin = open("my_skin.xml", "r").read()
    def __init__(...):

That will be loaded at boot time and won't be poking around in the dom. Or do similar with a dynamic skin.

[technic]

What do you mean by adapt? Having two resolutions? Ok, I provide two copies of skin when I am not lazy. But are you also talking about style? Like loading custom font and list selectors? I don't get it. I can load one xml file, but I have many screens, so I need to have my own dom and select screen accordingly. Is it the suggested solution?

Forcing plugin screens into the dom is not the correct thing to do. It breaks the skin access order.

Then why that comment is still there?

[IanSav]

Can I please have a link to the plugin that is having problems. I want to see in more detail what you are trying to achieve.

This is an example of what I mean by a dynamic skin: https://github.com/OpenViX/enigma2/blob/master/lib/python/Screens/MultiBootSelector.py

In this case the template can be read from disk though alignment with the scaleData can become unstable.

[technic]

This does not solve the problem with pixmaps in skin, and it easy to make a mistake in the string substituition. Since there are only two reasonable resolutions I would better do it with preprocessing, rather that in runtime.

There are already several examples of such plugins above. What I want to achieve is defining skins in the xml file instead of python source, and optionally having several plugin skins between which user can choose. In this case I just load corresponding xml file.

Upd: or something like this in my code https://github.com/technic/iptvdream4x/blob/master/src/manager.py#L47

[IanSav]

Where is a sample "iptvdream.xml" file? I didn't see it in the repository.

[IanSav]

The "SkinManager" class seems to be doing insane things like looking at all the directories in "/usr/share/enigma2/" and calling them all skins and putting them in a ConfigSelection list together with a default skin of "IPtvDream" . What are you trying to achieve because this makes no sense to me.

Is this plugin creating a skin called "IPtvDream"? Is it checking that it exists?

Also, what is this meant to be doing "skins_dir = resolveFilename(SCOPE_SKIN, '.')"?

Why are you selecting a "iptvdream.xml" file from any "skin" as generated above without any consideration of the currently selected UI skin?

I must be too tired as none of this makes any sense to me.

[Huevos]

Why are you doing this?

SKIN_FILE = 'iptvdream.xml'

Should be...

skin = open("iptvdream.xml", "r").read()

[technic]

Sorry it might be confusing. The iptvdream.xml is generated from skin.xml where several skins are defined. User can create another folders and put there customized iptvdream.xml. Ok, I can just make skin = .... If you think this solution is better, then loadSkin should not be recommended for use in plugins. Although old plugin used it.

[IanSav]

The important term in your post is "old". Enigma2 is constantly changing and coders should keep an eye on the changes and adapt with them.

Some points / answers from my previous post are:

• The correct way to get the path of a SCOPE is to use "resolveFilename(SCOPE)".
• The scope to find the current skin is "SCOPE_CURRENT_SKIN".
• Unless your different resolution skins are defined differently you should consider using the screen scaling system I suggested earlier. It is one piece of rather simple code that scales the screen to what ever the current skin resolution may be. There is no messing around with duplicated screen definitions.

[technic]

Ok. Thanks, for your answers. Then the official answer is "The loadSkin method is no longer suitable to use in plugins". So, please remove this line https://github.com/OpenPLi/enigma2/blob/develop/skin.py#L873. Or have I understood it wrong?

[IanSav]

Before we leave this topic I would like to know exactly what was wrong with the loadSkin() method. Some code still uses it so until they also update I would like to understand what you found broken about it.

[technic]

@IanSav see my first message. It works, but xml skin of the plugin screen can not be changed from general skin.

[IanSav]

I need more information to investigate the issue.

Do you have log files of the problem? Were there any error messages? How can I replicate the problem?

[technic]

To reproduce: install this plugin, or any other plugin from the list above. Try to override skin of some plugin's screen by adding this screen in the system skin. This doesn't work. Compare to release-4 branch. Look carefully at the change I've pointed out, see "Screen already defined" line.

[IanSav]

I tried installing MerlinEPGCentre on my box and it doesn't run at all. Rather than me trying to debug someone else's pugin are you able to provide me with logs from boot time up until after the problem occurs?

[technic]

There is nothing related to this problem in the logs. It is easy to reproduce with any plugin, even the most simple. Instead of defining skin=... put it to xml and use loadSkin. With this simple modification you will see the issue.

[Huevos]

Why would you want to do that? There is a correct order to load a screen. No screen should be added if it already exists in the active skin.

[technic]

The order is incorrect, because it is opposite. What is not clear? I am not insisting on any particular enigma2 behavior. Just tell me what is the latest, remove misleading comments in code, and we mark other plugins incompatible.

[IanSav]

I looked at the loadSkin() code in skin.py and couldn't specifically fault it. Without seeing a specific example of what is being done and how the results are broken I find this an almost impossible issue to trace. So far all the information and code you have provided don't paint a complete picture of the issue.

I am surprised that the log files didn't shed any light on the issue.

[IanSav]

Some plugin authors are not following Enigma2 development and don't seem to want to keep their plugins up to date. I a restricted from removing some code during my refactors as some plugins are closed source and can't be fixed.

[technic]

Ok. Do you understand the issue? Do you understand that it is not a crash, but wrong priority between system skin and loadSkin call in plugin.py?

[IanSav]

I suspect that issues you may be experiencing could be due to the screen names you are using. Are the screen names already defined in the current skin or the emergemncy skin?

[technic]

Screen name is same in all cases.

[IanSav]

No, I don't understand the issue. Without the logs it is almost impossible for me to see what has been going on.

The new skin.py has extra logging to help users and developers understand how the skins are being processed.

PLEASE give me some logs to review.

[IanSav]

If the logs you give me don't have the information I need then I can give you modified versions with extra logging to demonstrate the issue and its underlying cause.

[technic]

Yep, I've already did extra logging, and I told you everything you need to reproduce and the explained the cause. I believe it is clear enough.

[technic]

B.t.w., there is no single debug line in the loadSkin(...) method.

[IanSav]

It is obviously not clear enough or I wouldn't ask you. The code in loadSkin() doesn't need debugging statements. What we need to see is how the skins and screens are being used.

I have never had so much resistance trying to help someone diagnose an issue.

Why don't you want to give me the log files to analyse?

[technic]

I am currently using open-atv. I can provide you a log file for open-atv.

[IanSav]

OpenATV skin.py has NO relationship to the skin code running on OpenPLi!!! I refactored skin.py for OpenPLi and OpenViX. I am not getting cooperation from OpenATV to make the changes there.

If OpenATV want to have more of my refactored code then they need to respond to my attempts at contact. While I am submitting simple updates to OpenATV there are many issues where OpenATV has deviated from both OpenPLi and OpenViX. In those cases my code either needs significant reworking or OpenATV need to move back to more conventional Enigma2 code conventions. My attempts at contact to address these issues are not being returned.

An OpenATV log may help whoever is maintaining the OpenATV skin.py but it is worthless to me. Have you actually tested your code on OpenPLi or OpenViX? If you can reproduce the issue on OpenPLi or OpenViX then please report back. If this is only a problem on OpenATV then you need to pursue the issue there and the issue on OpenPLi should be closed.

[technic]

Yes I can reproduce it on openpli as well.

[IanSav]

Well please provide Full OpenPLi logs of the issue. Please state what screens are not working as expected.

[IanSav]

If the code is not working on OpenATV as well then this must be a VERY old issue! It probably has nothing to do with the recent refactor. The OpenATV version of skin.py is using the very old skin code.

I am also starting to wonder what you are expecting loadSkin() to do? What do you think it is meant to be doing for you?

[technic]

It should allow to have skin in the separate XML file instead of putting it to skin variable.

[betacentauri]

I read the discussion and then realized that my plugin is also using loadSkin(). I saw it somewhere else and thought it’s nice to separate code from skin. (By the way I’m following e2 development, but that plugin developers shouldn’t use it, wasn’t clear for me.) Maybe you can try my plugin TelekomSport. It’s available in the PLi feeds or here https://github.com/E2OpenPlugins/e2openplugin-TelekomSport/blob/master/plugin/plugin.py

As far as I understood technic you need to add screen for this plugin to your used skin (it’s not included in the default PLi skin yet). Then this screen is not used. The in the plugin defines skin is used. Right technic?

[technic]

@betacentauri I think you understood my issue correctly.

[IanSav]

That is the point I was trying to clarify. The loadSkin() code mustn't try to redefine existing screens. It it is the old way to add a completely new screen. If that is how it is being used then it should work fine.

@betacentauri is your plugin working correctly or not? (Will your plugin work for a DVB-T user in Australia?)

[betacentauri]

As no skin has added a screen for my plugin yet, I didn’t recognize this problem. When I have time I try to reproduce.