59 replies to this topic

[DimitarCC]

Hmmm but keeping instantiation of the screen will cause showing up of pvrstate screen. Or you mean something else?

[littlesat]

Modify the pvrstate screen with your code was my suggestion then it is 100% compatible with existing code. Actually not add it in InfoBar.py.
I’m sure this might break plugins when the instantiate pvrstate screen class will be removed (as it should actually with your code)
Adding it to the existing class will also result in the plugins having the graphs...

Edited by littlesat, 4 May 2021 - 18:58.

[MCelliotG]

How about OpenATV's approach?

https://github.com/o...ens/PVRState.py

[littlesat]

That part is not complete.... but the pvrstate.py gives a kind of logic and I suppose there is more code elsewhere... but code is last changed in 2013... by someone from openvix.

Edited by littlesat, 4 May 2021 - 21:36.

[DimitarCC]

That part is not complete.... but the pvrstate.py gives a kind of logic and I suppose there is more code elsewhere... but code is last changed in 2013... by someone from openvix.

 

I have changed completely the way status icons are loaded. Look the pull request. I tried to keep the old PvrState screen as optional so to be possible to use old behavior in plugins.

I hope it will works. Check it out please

[littlesat]

The merge request still removes the pvrstate screen from the movie infobar as far I can see.

[DimitarCC]

Well when we use icons in movie infobar we dont need pvrstate screen. It can be added back if show_external_dialog is set to true. I can make that to be true by default. But in my opinion in movieplayer screen pvrstate is no longer needed. Am i wrong?

Otherwise InfoBarPVRState is inited on line 192

Edited by DimitarCC, 4 May 2021 - 22:17.

[littlesat]

Is is indeed not needed but plugins may expect it (especially plugins that play eg iptv).. You did not remove the orriginal code yet and when changed (with your good idea) it should be removed and due to this these plugins might still work.
My two cents a choice need to be made...
1. Remove the pvrstate screen, and then also remove the py file, with risk breaking 3rd parry closed source plugins... (where the sources are available from can be solved). And accept the code in InfkBarGenerics
2. Adapt the pvrstate.py code.

My guts are for option 1.... which also means remove that pvrsfate.py and take the risk for that closed source stuff... (someone can still add the py manually when they like and in a 3rd party package the py can be added)

Edited by littlesat, 5 May 2021 - 04:59.

[DimitarCC]

I understand your point. I am also for choice 1. It is better to get rid of that screen in my opinion. And if someone needed it to be provided by third party package.

I can remove the screen or change the current code a bit more to instantiate the dialog by default and just not show it for movieplayer screen.

However you guys have to make that choice in which way to go. Still i think removing pvstate screen completely is better choice

[littlesat]

When you are willing to offer a new merge request where the old code is removed.... thousand thanks! Note that it also simplifies the code and add more features... maybe also consider to add the graphs to our standard skin

Edited by littlesat, 5 May 2021 - 07:37.

[DimitarCC]

Sure. I will do it. Well sorry for the dumb question but where is the repo for the standard skins? 

[Dimitrij]

https://github.com/l...esat/skin-PLiHD

[DimitarCC]

When you are willing to offer a new merge request where the old code is removed.... thousand thanks! Note that it also simplifies the code and add more features... maybe also consider to add the graphs to our standard skin

 

The changes are done. PVRState.py is completely gone. All refs and calls are removed.

Now i am going to modify the skins in the  repo pointed by @Dimitrij

[DimitarCC]

@littlesat I created a pull request for skin modifications here https://github.com/l...-PLiHD/pull/881.

However i just added the graphic files (SVG) in correct folders but the MoviePlayer infobar skin have to be modified to put the correct widgets at correct places.

[littlesat]

I’m a bit confused as I cannot check it now on a real box.... can I merge and what do I need to merge?

[DimitarCC]

If you are talking about the skins you can safely merge. Generally it will not cause any issues. But to be able to test in MoviePlayer skin definition should be added the new custom widgets. Otherwise there will be no pvr states shown.

The widgets are:

<widget name="stateicon" position="160,915" size="90,90" alphatest="blend" zPosition="10"/>
<widget name="statespeedtext" foregroundColor="white" halign="left" position="260,960" size="90,45" font="Bold; 40" transparent="1"/> 
<widget name="statetext"  foregroundColor="white" halign="left" position="260,960" size="90,45" font="Bold; 40" transparent="1"/> 

And ofcource you have to get InfoBar.py and InfoBarGenerics.py from my other pull request.

Edited by DimitarCC, 5 May 2021 - 18:28.

[Taapat]

I don't think it's thoughtful to remove the PVRState screen.

The fact that you are repairing skin-PLiHD does not mean that all other skins will be corrected.

 

Remove the PVRState screen and ask to completely change all skins just because you want to display images on your skin without backward compatibility in my opinion is not a good option.

I don't see reasons why you can't introduce images in the PVRState screen or in MoviePlayer without breaking any other skin.

Edited by Taapat, 5 May 2021 - 18:27.

[DimitarCC]

Well my first idea was to keep Pvrstate screen and just to extend a bit movie player skin. However there were proposal from @littlesat to remove it completly which i did. In my opinion @littlesat is right that pvrstate screen is unneded. Besides the place of states graphics or text is in infobar not in some separate screen. At least that is my opinion
If skins are not changed it will work with no problem Just will not display any pvr states icons nor text.

[Taapat]

1. If you want to add icons in enigma2 then you have to add them in data/skin_default/icons not in skin-PLiHD skins: https://github.com/O...n_default/icons

Then everyone will be able to use them.

 

2. I think not good practice is introduced in the python even more hard coded skin images.

I think it is better to introduce in python only widgets but leave the images in the skin so that the skiner can choose what to use for the widget.

Edited by Taapat, 5 May 2021 - 19:13.

[DimitarCC]

I understand your point.
1. Put the images in data/skin_default is good idea but then it should be made possibility that images to be overrided from the skin. Otherwise there is no point to do it since standard images may not fit to the current skin

2. The idea of not have hardcoded images in python is good however achieving what you need might be tricky with only widgets.

I am sure many improuvments can be done to my proposal... But in some cases it might be better to be introduced something new even if something have to be adjusted later. Otherwise there is limited possibility things to become better