Updated EmulationStation for Windows

LiveFreeDead

@f-caruso I tested with a game I know crashes (3DS (Citra) - Harvest Moon 3D (Crashes just after you set a birth date)), with Hide When Running (is enabled) it comes back with Gamepad still working, but when disabled controller is still broken :( - I'll just leave hide when running enabled at least until I finish sorting all these roms out anyway.

ThePlagueIsBack

@f-caruso Your build is just excellent but I miss the shutdown functionality, I'm setting up a Windows machine and everything is working as expected but the shutdown. On the build from @jdrassa the shutdown and reboot options do not contain the necessary Windows code to work and on your build it doesn't appear, I guess you hided it. Is there anyway you could add it back via config file with the corresponding missing shutdown command for Windows? If not then if you could help me out setup AppVeyor I can try to do it myself, the code is already a pull request on @jdrassa's GitHub, but I can't seem to setup AppVeyor to build correctly and produce the artifacts, plus can't get the code to compile locally on my Windows machine.
Thank you

f.caruso

@ThePlagueIsBack Hi, Yes it was intentionnally hidden ( with a preprocessor directive for Windows). I assumed that on Windows most users Simply quit ES - and exiting was too long going in submenus - but don't shut down or restart Windows that way. Most users seems happy with that until now. I personnally don't understand the interest unless ES is completely replacing the shell… Furthermore, there's a button on every PC which is sending shutting down signal… (not the case on a pi )

However, I can always make it an option in my version. This is pretty easy to do.

Also, concerning code : there is a win32 api for that : ExitWindowsEx which is better that using shutdown command.
I'll see what I can do when I'll be back ( I'm currently on holidays )

jdrassa

@ThePlagueIsBack the reason that RetroPie changed the way shutdown/reboot work (and in turn broke them on Windows) is that with the previous implementation, the shutdown/reboot command will execute before ES has a chance to properly shutdown. This will potentially cause a loss of data as gamelists will not be saved.

f.caruso

@jdrassa @ThePlagueIsBack Yes, but the implementation should be different : QuitEs pushes SQL_QUIT : perfect It will break the main loop, but touch/shutdown commands should not be executed at that moment. QuitEs should only store the "Reason" (as a string or better : as an enum) and the Reason should be handled in the main, after deleteSystems where we can run the commands without risks of loosing datas.
Batocera's team (they are now using a 'modified' version of Retropie's ES ) has made a perfect implementation of it.
If you want I'll submit a PR with that when I'll be back.

ThePlagueIsBack

@f-caruso This build is for a friend that I'm helping setting up a Windows 7 arcade machine and we happen to replace the shell with ES so the shutdown was in fact much needed else he ES would not save the favorites/recent-played upon triggering the hard shutdown button. For now I'm using the command line shutdown with a 5 seconds delay which should be enough for ES to save the lists.

@jdrassa Thanks for the info, now it makes sense to me why they removed it from the retropie repo and yes it does prevent ES from saving the lists on Windows. Also, thanks a lot for providing me your AppVeyor yaml config, wouldn't be able to set it up on my own.

@f-caruso Man, that's a really nice implementation to get the shutdown gracefully working, it would be awesome if you could come up with that PR.

For now I've come up with a Frankenstein build forked from @jdrassa + feats from @f-caruso that includes the hacky shutdown as a temporary solution for anyone that would be interested here, I also removed "RESTART ES" from the quit menu since it wasn't working on Windows: https://github.com/Cereal-Killa/EmulationStation/releases

jdrassa

@f-caruso When I was looking over the current implementation, I was thinking about a similar implementation. If you want to submit a PR feel free, if not I have added it to my TODO list.

f.caruso

@jdrassa Ok I will. furthermore I have to start with something… ;)

dukeblooders

@f-caruso Hi,

There's a small issue with the image preview at launch.

Plus, can you translate the collection texts into french ? (Various, This collection contains…)
Ce serait super cool :) (à moins qu'il y ait un moyen facile de le faire ?)

f.caruso

@dukeblooders I don't have the problem, you must provide more information : at least, screen resolution & image dimensions.
Concerning the translation, you can edit emulationstation2.po located in resources/locales/fr. But I may have forgotten some entries ( Il faut que ce soit prévu dans le code, sinon ca reste en anglais... dans ce cas, indique moi les libellés précis manquants )

dukeblooders

@f-caruso I use two config, one for test, but I have the same issue on both

Config 1 : 16/9 screen (1920x1080), 4/3 image (800x600)
Config 2 : 16/9 screen (3840x2160), 4/3 image (1280x960)

And I use video with snapshot image :
< video name="md_video" />
< origin>0.5 0< /origin>
< pos>0.555 0.32< /pos>
< maxSize>0.385 0.55< /maxSize>
< delay>2.00< /delay>
< showSnapshotNoVideo>true< /showSnapshotNoVideo>
< showSnapshotDelay>true< /showSnapshotDelay>
< /video >

For translations, I looked into emulationstation2.po but I didn't find those entries :

Various : when developer/genre is different in the same collection
This collection contains X games… : description of the collection

f.caruso

@LiveFreeDead I tried something about your pad issue in my last version try it and tell me if it is better for you ( with further testings : I reproduced it too with jrassa's version )

@dukeblooders Fixed in the last version

@ThePlagueIsBack Fixed the shutdown in my version + added a option to determine if you want the full quit menu or only the simple quit function.

@jdrassa I just did a PR with the fixed shutdown just like I described. I'm new to github, so I don't understand everything yet : I had to create a second account/fork of Retropie to do this.
If you want me to propose other PR, I'm fine with it. The thing is : I was about to start the menu theming when I saw you started a branch called "menu-theming" in your fork containing the code. As you already started it... I don't really know now what interests you ( and what are you gonna accept )...

jdrassa

@f-caruso I saw your PR. You shouldn't have needed to create a new github account. You should have been able to create PR on the RetroPie repository even though you originally forked my repository. If you would prefer to clean it up and resubmit from you main account, I can step you through it.

I looked at the implementation from Recalbox previously and didn't really like the approach. The various generic GUI components used in the menus are tightly coupled to the menus and the menu theming. I am not really sure if there is a better approach so I am going to experiment to see if I can come up with something better.

If you haven't already seen it, you should check out https://github.com/RetroPie/EmulationStation/pull/584. One of the other developers has been refactoring the rendering code.

LiveFreeDead

@f-caruso Perfect Bud, got that bug licked.... EWWWW - anyway I can now crash RetroArch and ES doesn't suffer (from controllers being disabled). Thanks again.

dukeblooders

@f-caruso Awesome ! Thanks.

f.caruso

@jdrassa I didn't manage to create a branch in my fork from your (or retropie's) master in my main Fork. I can only create a branch from my own master, but if I try to PR with it, it pushes my entire changes. The only way I found would be to delete my fork, recreate it and import back my changes into a branch keeping my master in sync with yours... I didn't find another way. So I prefered to create a new account to keep my history and create a new fork, added my main account as contributor, in order to work for you. Unless you know a better way, I'm totally fine with it : I have my Fork with my own version and I can continue my experiments into it, and this another fork in which I can do things for you. It's fine.

I suppose you are taking about the menu theming in the second part : What I did is largely inspired from Recalbox implementation, the main idea was to make their menu theming 100% compatible. I didn't try to change the way they coupled things or refactor. I agree, gui components are now coupled with menu theming, but they already are all coupled to ThemeData anyway, so I'm fine with it. The true thing I don't like at all & I wanted to change in the future is the MenuIconElement structure -> The list of icons is hardcoded in this structure and in ThemeData's declarations, it makes it complex to declare another icon in a menu. MenuIconElement should be a dynamically loaded map, and icons should be handled by "tags" in menu items.

Yes, I've seen the tomaz82's refactoring about the renderer.
I don't know why you talk to me about it. Do you want my opinion or a review ?

@LiveFreeDead @dukeblooders My pleasure...

jdrassa

@f-caruso said in Updated EmulationStation for Windows:

Unless you know a better way, I'm totally fine with it : I have my Fork with my own version and I can continue my experiments into it, and this another fork in which I can do things for you. It's fine.

I 'll post some instructions later in case you would like to combine them.

What I did is largely inspired from Recalbox implementation, the main idea was to make their menu theming 100% compatible.

Personally, I'm not overly concerned about maintaining compatibility. Would rather have the best implementation.

I agree, gui components are now coupled with menu theming, but they already are all coupled to ThemeData anyway, so I'm fine with it.

I would consider the gui components to be loosely coupled to ThemeData in that they are just passed the ThemeData by the view, where as the components used in the menu are directly requesting the ThemeData. Ideally, the menus would work similarly where only the root menu object was accessing the ThemeData and passing it to the child components.

Yes, I've seen the tomaz82's refactoring about the renderer.
I don't know why you talk to me about it. Do you want my opinion or a review ?

Just giving you a heads up since it is a big change that effects a lot of the code.

jdrassa

@f-caruso

Add branch fixed-shutdown from new repository to your original repository:

# add your new repository as an alternate remote repository
git remote add alternate git@github.com:fabricecaruso72/Retropie-EmulationStation.git
# fetch branches from alternate repository
git fetch alternate
# checkout branch from your alternate repository
git checkout -b fixed-shutdown alternate/fixed-shutdown
# push branch to your primary repository
git push -u origin fixed-shutdown

Similarly, if you want to start a new branch for other work based on the master RetroPie repository:

# add RetroPie remote repository as 'upstream'
git remote add upstream git@github.com:RetroPie/EmulationStation.git
# fetch branches from upstream repository
git fetch upstream
# create new branch based on master from upstream repository
git checkout -b new-branch upstream/master
# push branch to your primary repository
git push -u origin new-branch

f.caruso

@jdrassa Thx for your instructions, I'll give it a try.

I'm not overly concerned about maintaining compatibility

This is a real debate ! Users don't care about implementation, but care about functionnality & compatibility. I'm sure some users would like to use recalbox's themes !!! I did !! Some of them are truly awesome but don't work with Retropie ES, so it was totally impossible for a recalbox user to find a Windows version loading his recalbox themes ( except my version now ). This is one of the reasons why I started working on ES.

Developpers sometimes don't care about compatibility but implementation -> This often leads to lot of frustration for the users.

Both are important... Well architectured code is more maintable, and more efficient.
But user's satisfaction is always more important.

Concerning tomaz82's refactorings : it's a very good idea to isolate GL code. It's a good first step.
Still : it remains as a set of apis in a namespace. As you say you are looking for the best implementation you will agree with me : Doing things this way is not ideal for making future DX/Vulkan/GL3 renderers, or it will be a set of #define or switches at build time, and won't be configurable by users for runtime.
The best implementation would have been to create an "interface", implement it in each renderer, and inject it... Next step I imagine...

jdrassa

@f-caruso said in Updated EmulationStation for Windows:

@jdrassa Thx for your instructions, I'll give it a try.

I'm not overly concerned about maintaining compatibility

This is a real debate ! Users don't care about implementation, but care about functionnality & compatibility. I'm sure some users would like to use recalbox's themes !!! I did !! Some of them are truly awesome but don't work with Retropie ES, so it was totally impossible for a recalbox user to find a Windows version loading his recalbox themes ( except my version now ). This is one of the reasons why I started working on ES.

Developpers sometimes don't care about compatibility but implementation -> This often leads to lot of frustration for the users.

Both are important... Well architectured code is more maintable, and more efficient.
But user's satisfaction is always more important.

I didn't intend for this to come off so callous. Let me provide a bit of background. In the few years now that I have been involved with RetroPie, there have many theme improvements (some minor, some major) that have originated in RetroPie. In some cases these have been added to recalbox as is (i.e. carousel themeing, z-index), some have seen similar but incompatible features added (i.e. variable support, video) and some have not been adopted at all (i.e. default themes, datetime enhancements).

From my perspective, if recalbox is not maintaining compatibility with RetroPie themes, I don't feel a strong need to maintain compatability with recalbox themes. Especially if that comes at the expense of potentially providing a better solution.

I would also argue that while a more maintainable, efficient code base may potentially come at a cost (temporary lower user satification), in the long run it will result in overall higher user satisfaction as it will better enable the development of new features.