Video View For All Themes

Zigurana

@backstander I think that maintaining separate theme repos for older versions of ES is a maintenance burden that brings us nothing but hassle.
Why would we need to keep supporting old ES users? And more importantly, for how long? If we want to keep the momentum of ES development going, as small as it is, we should look forward, not backwards.
I agree that backwards compatibility is nice to have, but the current state of ES simply does not allow for much development. Future redesigns might change that, ie. make ES theming a bit more forgiving to the presence of certain tags or not, but we'll have to make breaking changes along the way to get there.

Let's put it differently, why would someone not want to upgrade their ES? If there are relevant issues, those should be adressed.

jdrassa

@Nismo Thanks. I realized right away my mistake with the system logos. I was so focused on testing the video that I hadn't noticed that they were missing. I have added these changes to the pull request.

Nismo

@jdrassa Thanks, I really need to know how to use github but sincerely it's not one of my prioritys.

@robertybob if you need help to add video support to your themes I can help you.

jdrassa

@Nismo it doesn't take long to learn the basics of using github. If you don't want to use the cli, they have a Github Desktop app that is pretty simple to use. If you want help figuring it out, just let me know.

herb_fargus

@Zigurana what I meant by culling is to remove any themes that aren't complete/don't have video view.

I agree the burden of maintenance is too much to maintain two versions of each theme. If it's possible to have backwards compatibility with old themes along with having video mode enabled that would be ideal but I haven't tested enough to know if/how it breaks stuff.

I agree in that the main goal should be looking forward. It may also be possible at some point that grid view is incorporated as well which we will have to go through this process all over again ;)

jdrassa

@herb_fargus looking through the code it looks like the theme will fail to parse when it hits the video element.

One way that we could try to address this would be to have the themes include 2 versions of their core xml file, one that supports video and one that does not. Using carbon as an example. Its root directory would contain carbon.xml which would be the current version and carbon-video.xml which would be the version that supports video.

Then there are a couple of options on how to enable video support. The user could be required to know to go in and swap the files to enable video support. Alternatively, the theme installation script could be updated to do it either automatically, prompt the user if they want to do it, or conditionally based on what version of emulationstation is installed.

herb_fargus

@jdrassa perhaps, it may also be possible to amend the install script to pull from a static commit prior to video mode updates. Might be good to get @BuZz input on what he thinks would be the best approach for it

I know themes are his favourite thing to talk about ;)

backstander

@Zigurana
I'm okay with that!! Personally I'd rather be like Apple and just force everyone to update ;-)

I was just worried about post on the forum from people with old ES doing an update of just Carbon or Pixel and now they got the dreaded "White Screen of Death"!

Nismo

@jdrassa @herb_fargus Having two version of core.xml of a theme it's not possible, just because all themes i've ported to support video need to modify individual theme.xml for each system, even in carbon theme, so you need not only the main.xml, you need as well a theme.xml with video support for each system inside the main theme.

vbs

Just for me to understand it: So latest ES is not compatible with non-video-themes and old ES is not compatible with video-enabled-themes?
Does it mean if I update now my ES to latest version (without updating themes) then it will break?

jdrassa

@Nismo From a backwards compatibility perspective there are 2 things in the theme xml that we have to be concerned about.

The first is the video element itself

<video name="md_video">...</video>

and the second is view elements that specify video in their name attribute

<view name="video">...</view>

I will need to test to verify, but looking through the code it looks like only the video element should be an issue.

You are correct though that this approach would not work for themes that need system specific placement of the video.

Zigurana

@herb_fargus said in Video View For All Themes:

It may also be possible at some point that grid view is incorporated as well which we will have to go through this process all over again ;)

My thoughts exactly.

Nismo

@jdrassa said in Video View For All Themes:

You are correct though that this approach would not work for themes that need system specific placement of the video.

Just like my theme... and you only speaking about video element but not marquee.

I didn't test new theme on old ES anyway...

@vbs Latest ES it's compatible with old themes and new video themes, but old ES isn't compatible with new video themes.

jdrassa

@Nismo Your theme is being built explicitly with the video feature in mind so I don't know that making it work with the non-video ES is necessary.

I was just trying to come up with a way to ease the transition for existing themes. I am not sure how many of them are using system specific image positioning.

As for the marquee, it is an image element and its presence in the theme XML shouldn't be an issue since images are already supported, although it won't be rendered, but I don't think that is an issue for users who haven't updated ES.

Nismo

@jdrassa that's true, we need to test if can be done without issues...

jdrassa

I did some testing tonight with my EmulationStation build in windows and it worked exactly how I anticipated above. The video element broke the parsing of the theme, but the marquee image and including "video" in the name attribute of a view element had no effect and the theme worked as expected.

One thing we could do to try to prevent this if/when grid view is incorporated would be to tweak the theme parsing code so that it is a little more flexible and having it log and ignore unexpected elements vs. throwing an error and failing to load the theme.

herb_fargus

@jdrassa agreed. I just tested and got the same results:

lvl0: 	from theme "/home/pi/.emulationstation/themes/pixel video/snes/theme.xml"
  (from included file "/home/pi/.emulationstation/themes/pixel video/snes/../pixel.xml")
    Unknown element of type "video"!
lvl0: 	from theme "/home/pi/.emulationstation/themes/pixel video/retropie/theme.xml"
  (from included file "/home/pi/.emulationstation/themes/pixel video/retropie/../pixel.xml")
    Unknown element of type "video"!

Perhaps some of the stuff @fieldofcows is working on with metadata may help with this. Still doesn't really fix backward compatibility but then again we are looking toward the future and there is only so much you can do.

mattrixk

@Zigurana said in Video View For All Themes:

Let's put it differently, why would someone not want to upgrade their ES? If there are relevant issues, those should be adressed.

I'm personally not planning on updating ES because I have it working how I like it, and I'm too lazy to go through the hassle of fixing any potential breaks caused by updating. I also have no real interest in video previews, but people have asked about it for MetaPixel, so I'm going to have to look into it anyway.

fieldofcows

I had a quick look at this last night. What I was looking for was a way to write the theme whereby older versions of ES would just ignore the unsupported new functionality.

Going forward it makes sense to build this into ES - we can either just warn about and ignore unknown tags rather than aborting or we could implement some sort of versioning. However this doesn't help for anyone that does not what to update their ES.

I did have a quick try to see if I could introduce some version checks that older ES would ignore with some success. If you put something like this into your theme:

<version minimum="5">
  <view name="video>
    <video name="md_video">
    ...
  </view>
</version>

All of the above is ignored by older versions of ES. I could then implement and submit a change that checks the version tag when parsing the theme and conditionally includes the XML within the version elements if the version is greater or equal. I don't think it would be too hard to do this and would allow us to make breaking changes to themes whilst maintaining support for older ES. It would mean a bit more work for themers though because they would potentially need to test on multiple versions of ES.

Nismo

@fieldofcows I agree, this doesn't help for anyone that does not want to update their ES, so instead implementing this and make things difficult for theme developers, it's more easy for everybody to update ES, or we can try the two main.xml thing...