Please Test: Adding support for "All", "Favorites" and "Last Played" systems

pjft

@cyperghost Just the file path.

As I stated, ES has the system settings in es_systems.cfg for the respective folders where the source roms are. No new folders need to be created. No files need to be moved around.

I don't plan on creating yet another file type that's redundant.

cyperghost

@pjft Man it's so easy... you are so genious ;)
Shame on me..... The solution is so brilliant!

You just compare the file path related to the rom ;)

So a Gameboy file is stored to ~/RetroPie/roms/gb/
You save the ROM Tetris (UE) 1.1.gb into a annother file Tetris (UE) 1.1.gb for ex. as gamelist???
In the Tetris (UE) 1.1.gbis just the filepath+romname stored ~/RetroPie/roms/gb/Tetris (UE) 1.1.gb

We "launch" that file and now you compare the filestring after the last / with the es_system.cfg credentials ... Voila you know that must be a GBfile

Ahhhhhhhhhhhhhhhh... Easy, effictive, brilliant
Congrats @pjft

Hope I got that now ;)

pjft

That is indeed the gist of it, but since it all happens inside ES, it's all much simpler - no need for string parsing and all that.

The changes I've made in this PR make the system execution data (i.e. knowing it's a GB rom) be tied to the actual game entry in ES, rather than to the "system" entry. As such, regardless of how many collections a game it belongs to, it always knows what to run it in.

For the case you're describing, it will run it as if you had clicked the Tetris entry in the GB list - which is what currently happens, regardless of whether you're clicking it in "All Games", "Favorites" or "Last Played".

We'll store

/home/pi/RetroPie/roms/arcade/ffight.zip
/home/pi/RetroPie/roms/gb/gargoylesquest.zip
/home/pi/RetroPie/roms/nes/ducktales.zip
/home/pi/RetroPie/roms/fba/sf2.zip

for a Capcom system, for instance.

At least that's how I envision it. Whether that will work - and whether I have the ability to pull it off - remains to be seen. But I'm confident - the hardest part seems to have been done.

cyperghost

@pjft Whatever ... But I agree that those system hacks may be abolished in future or are just useable in a rare rare cases.
The FAV description got one advantage :) You can use it for system calls like the retropie menu :)
Thanks for the long and deep solid explanation

cyperghost

@pjft
About coding style ... nice description of crap

It's not because it isn't good code, but because I don't think it's the right approach here, as well as that porting code is in my opinion a lot more effort than writing it from scratch. :)

There are the same routines used for 4 or 5 times in that code. That would be better outsourced by an easy include or function call to strip down that source code.
The variables are not well definated and missused for other values (not good coding style)
I oversized the include by making file access compatible to VB an INCLUDE of only file.bi would be better and reduce footprint size.
The comments ... are there but ... rare - better than nothing ;)
Who needs taps and spaces? It looks like a mess

The only good style (imho) is the use of while-loops with definated ends and case selections! And no goto :) A good goto is allowed!

But back to topic. I'm looking forward that is branch is the next breakthrough in RetroPie. It's really a pitty that it tooks several years to include a milestone like this. Thank you very much :)

btw. Hope that the PR of IO will be accepted

mattrixk

@cyperghost said in Please Test: Adding support for "All", "Favorites" and "Last Played" systems:

Hope that the PR of IO will be accepted

Your wish has been granted.

Seriously though, I'm looking forward to this all being finalised. I've been trying to follow meleu's and TMNTturtlguy's virtual systems stuff, but I've been getting confused. I've been getting confused reading this thread too, but maybe that's just because it's too early in the morning for my brain to work properly. I'm just going to wait until it becomes part of core so I don't have to resort to hacks.

meleu

@mattrixk said in Please Test: Adding support for "All", "Favorites" and "Last Played" systems:

I'm just going to wait until it becomes part of core so I don't have to resort to hacks.

It's a wise decision. The @pjft work on this stuff is very well done and much more user-friendly than those hacks.

By the way, I composed a couplet in @pjft honor:

Paulo's code is great and can't be beat
If I coded like him that would be neat

lilbud

@meleu

0_1498776563147_1aac44dc-a742-403c-ac63-e8a728df3705-image.png

jdrassa

@pjft said in Please Test: Adding support for "All", "Favorites" and "Last Played" systems:

They will be based on unused existing folders in the user's theme, as ES has a 1:1 relationship to "system-level entries" and "theme folders". And I don't want users to shoot themselves in the foot by creating collections that will not be themed and look ugly as hell. This will address most of the use cases people have been playing with, when creating a "megaman" theme folder or something, and then packing it with megaman games, or "fighting", "sports" or whatever.

Creating custom collections (without an existing theme folder) is out of scope, as there are too many sub-optimally defined user flows, and the end user experience will always be less than desirable.

I really need to get back around to finishing up the work I was doing on theme variables. Part of that also included the concept of a default theme that would be applied if there was not a theme for the selected "system".

pjft

@jdrassa I think that would be fantastic. I toyed with the idea of giving the option of reusing the first entry in the theme, but without the items that are usually very system specific (name, logo) but it felt quite hacky and not like something that would really produce good results.

It was going to be a "compatibility mode". Geez. The workarounds we consider.

pjft

@meleu @lilbud Thanks for the couplet and for the image!

If some people have a full-fledged robot @meleu printed out, so can I have this :)

pjft

Hi all.

This weekend was spent cleaning up the current PR thanks to @zigurana's very timely and relevant comments on it!

As such, I'd appreciate it if anyone would have the availability and patience to recompile the latest build and test it/use it the same way as before.

Functionality-wise it should be equivalent, just more compliant with proper coding guidelines :)

If nobody runs into any trouble, that's one step closer to getting this approved - and one step closer to me starting to work on the custom collections piece.

Thanks in advance for your time as usual, and hope you have a great week!

cyperghost

Can anyone provide the binary please?

@pjft Thank you for your effort in making good things better. But do you see a chance to implent @hex 's power saving mod? Can a pull request to your mod be made to implent this?

Hex

@cyperghost We are all contributing to Retropie repo. If you want you can patch it your self. Let me know if you are interested to have both features combined.

Also why dont you compile your own binaries?

cyperghost

@Hex Please tell me how? ;)

pjft

@cyperghost said in Please Test: Adding support for "All", "Favorites" and "Last Played" systems:

But do you see a chance to implent @hex 's power saving mod? Are can a pull request to your mod be made?

What exactly are you looking for? To be able to test this code with the power saving changes? Or to get the power saving changes merged into the main branch, bundled with my changes?

If it's the latter, that's out of scope, and would not in any way help with getting any such change approved. It's good practice to submit well defined and scoped out changes to the project, so that it's also easier to test and revert should anything go wrong. These two features are sufficiently different in scope that it doesn't make sense to bundle them in a single PR.

@Hex has done all the development on the power saving features, when he feels the code is ready to be merged, he will create one such PR to the main branch. I am certain there is a reason for those changes to not be merged yet. He has contributed with a lot of code to the main branch so far, so I imagine it'll be done in due time.

If it's the former, while I certainly lack the time to devote to merge that work into this branch, if anyone wants to fork my code and merge it with @Hex's changes, I certainly won't oppose, but I fail to see the benefit. My changes should be added to the main EmulationStation in the coming weeks, hopefully, so anything done with this current branch will be short-lived.

Any such tests will be able to be done based off the main branch in a few weeks.

Hope this helps.

EDIT: Compiling binaries, see instructions in first post. Alternatively, on your Pi, after exiting ES:

git clone --recursive https://github.com/pjft/EmulationStation.git ES-pjft
cd ES-pjft
git checkout RetroPie-Virtual-Systems-PR
cmake .
make

and

./emulationstation

to run it.

cyperghost

@pjft Yes exactly a combination of the power savings @hex and your latest efforts in "favorite" support. It isn't necessary for a quick release but imho the energy saving feature should be installed by default so that user in the case of building portables can profit from longer battery live (I focus on Pi0 builds). The CPU load from different branches is listed here - maybe you missed it :D

pjft

@cyperghost Thanks for clarifying.

As I said, then, that's completely out of scope for these changes. That's something that will make it into the main branch when @Hex feels it's ready. It'd be a waste of anyone else's time to re-invent the wheel as @Hex is actively developing it, and if he's already looking into that, he has a lot more insight, knowledge and skills to make it happen the right way than others would, as few have ever devoted a moment to think about that problem.

Those developments are in good hands.

EDIT: It's not that I missed those posts, it's just that I personally don't have a pressing need to look into that problem, given that others are already doing so. Don't take this wrong - there are just so many things that we all want to look into, and only a finite amount of time in our weeks to devote to it.

Also, from the results shown in that thread, I don't see any particular downgrade in performance between 2.1.x and 2.3.x which at least gives me reassurance that nothing has been significantly broken or made worse in recent developments. If there's no regression, then it's fair game if someone wants to improve it - but it'll take as much importance or priority as any other new development.

All in all, I'm very thankful that @Hex is looking into that. Suggesting others to pick up his work and merge it so it is made "faster" is somewhat disrespectful of the time and effort he's putting into it.

The best thing that you and the community can do to help speed any of that work up - or any other RetroPie-related development, as a matter of fact - is help testing things. @Hex, @zigurana, @jdrassa and others would be very much thankful for that, as everybody wants to make sure that their work will not cause any problems.

I'll also include here @fluffypillow and @phulshof who are doing a tremendous job on their own respective frontends that will hopefully one day be fully available for RetroPie users, as well as everyone who is producing tools, scripts, overlays, etc. I'm missing a lot more people here as well, namely the project maintainers, but I don't want to turn this into a spam-fest of mentions.

Hope this sets the right expectation. Sorry for the long post-script, and thanks for the encouragement and support.

cyperghost

@pjft Is it okay for you if I upload the generated binary to github? Or am I breaking laws or a codex by doing this?

Thank you again for providing us with your mod - You made a good job and I appreciate this.

Hex

@cyperghost @pjft

Ok so those of you who dont know how awesome git is when it does what you want, well here is how you can merge two or more features in different repositories.

Follow these instructions :
Download my script https://github.com/hex007/es-dev/blob/master/es-tests.sh

create and paste / append to es-repos.txt file

https://github.com/pjft/EmulationStation RetroPie-Virtual-Systems-PR

Then create another file es-patches.txt, and paste in it

https://github.com/hex007/EmulationStation feature-powersaver d2aa4861704e4d93659246c13ecc982a6fd18314 Hex's powersaver patch

Run my script and where it asks if you want to apply patches go for yes and select Hex's powersaver patch

And continue with the build.

OR For those comfortable with git only:

git clone --recurse-submodules https://github.com/pjft/EmulationStation -b RetroPie-Virtual-Systems-PR pjft-vs

mkdir pjft-vs/build && cd pjft-vs/build

git  fetch https://github.com/hex007/EmulationStation feature-powersaver
git cherry-pick d2aa4861704e4d93659246c13ecc982a6fd18314