Scraping multi disc roms

mediamogul

That would be most helpful, thanks!

pjft

So, what bisect does is narrow down the change in the code (i.e. the git commit) that introduced the misbehavior, by means of a binary search - basically at each commit in the code history it asks you whether it's good or bad, and then depending on the answer it will select a new version for you to test that's halfway between the current one you just reviewed and the previous known one you reviewed.

It needs to start with two tags from you - a version that you know that it has the problem (let's say, a "bad" one - for this purpose it's the current version but we can probably go further back) and one that doesn't have it - a "good" one.

This post has the instructions I shared for someone else to bisect the code. They may be helpful here.

Still, here they are, adapted to this case. Some assumptions:

Your post was from July 20th 2017, so this was the last commit hash you would possibly have been able to been running at that date:
b28fa2c

The challenge will be to find a previous version where it was working. For the sake of the argument, try this one:
4cc0ced

Which was the last commit before I added metadata to folders, in April 2017.

I'd recommend checking out these two versions first and compiling, to make sure they are indeed bad - and good - as you'd need those two starting points to bisect. I believe that, after you clone the repository to your local drive, you can run:

git checkout <hash>

So, the first step would be to clone it to your Pi somewhere, with the recursive flag.

git clone --recursive --depth 1 https://github.com/RetroPie/EmulationStation

Then run

git checkout stable
git checkout <bad hash>
git bisect start
git bisect bad
git bisect good <good hash>

and then, in each iteration, run

cmake .
make
./emulationstation

and see if it works or not.

If it works, exit ES and run

git bisect good

if not,

git bisect bad

It will then checkout a new version, at which point you'd re-compile and test, using the commands mentioned above, and classify it as good or bad.

You'd do this until it'd tell you exactly the commit that broke that.

It may take a bit of time (depending on the number of steps it requires and how long it takes to re-compile the changes each time), but if you have the time and availability that will help really narrow down what might be the matter. If you don't, it's not a bother - it's understandable.

Sorry for the trouble, and thanks for taking the time to help improve this.

Let me know if this helps.

mediamogul

@pjft

Man that write up was above and beyond. I'm backing up all my metadata just i case and I'll give it a go afterwards. I'm amazed at how useful this process can be. I was reading that it can be used to isolate any changes that are unaccountable, even the odd performance boost. Very keen.

Sorry for the trouble, and thanks for taking the time to help improve this.

You can't make an omelette without breaking some eggs and the improvements you've made are well worth troubleshooting a few unintended side effects. I certainly don't mind taking the time, and if nothing conclusive comes from it, at least we can say it was explored. Thanks again for the instructions and your time in general.

mediamogul

@pjft

I've cloned the repository using:

git clone --recursive --depth 1 https://github.com/RetroPie/EmulationStation

changed directories to ./EmulationStation/ and ran

git checkout 4cc0ced

However, I'm getting:

error: pathspec '4cc0ced' did not match any file(s) known to git.

The same thing goes for:

git checkout b28fa2c

mediamogul

@pjft

I don't know what the potential downsides may be, but removing --depth 1 when cloning seems to have done the trick. 4cc0ced is compiling now.

mediamogul

@pjft

Well, you were right. 4cc0ced didn't crash and b28fa2c did. However, if I'm reading it right, the result of the bisect seems like it can't possibly be an issue, as the commit appears to be nothing more that a version bump. The final word was:

b28fa2ce9f6a4f9b0d8ad3af7bf09fc008c80d59 is the first bad commit
commit b28fa2ce9f6a4f9b0d8ad3af7bf09fc008c80d59
Author: Jools Wills <redacted@redacted.org.uk>
Date:   Tue Jul 11 16:20:03 2017 +0100

    bump version to 2.4.1

:040000 040000 d7404be1e95b25ec121eece939d165655147a167 f0652c9eef80ad41f5b5ff4e40a16f3e0c78f088 M	es-app

pjft

@mediamogul thanks for going through this.

Hm, that is indeed weird.

To confirm, you did test b28fa2c for crashes? When you ran it, did it show "2.4.1" as the version in the menu?
Would you have the log for the entire bisect session?

I suppose, to confirm, you could always checkout b28fa2c and confirm that it crashes, and 227d5b8 (the previous commit) and confirm whether it doesn't crash.

I do agree that that commit is extremely likely not to be the culprit here, so we might have taken a wrong turn in the bisect process. Odds are that either:

b28fa2c doesn't crash but it's being marked by the bisect process as "bad" because it is the last commit he's working with.
The instructions I've provided might have misled bisect (sorry if that's the case!).
We might have mistakenly flagged one revision as "good" or "bad" by mistake in the process. I've had my fair share of those.

To confirm, what this suggests is that in your tests throughout all the bisect process, it never crashed. Is that what happened?

Thanks, and sorry for having you spend time on this.

mediamogul

@pjft

It seems the problem might have been in how the crash was being tested each time. I took for granted that scraping a folder would always reproduce the thrown exception. However, it appears that if the folder has already been scraped successfully, as it was by first testing 4cc0ced, each time after will also be successful, no matter what build you're using. Learning that, I started over from the beginning by first testing the two commits that were thought to be good and bad. After confirming the status of both, I'm now running the full bisect process again, making sure to wipe the metadata from my test folder each time. Hopefully, this will yield a more trustworthy result.

pjft

@mediamogul Oh, sorry about the trouble.

If you set "Save data on exit" to false, maybe you don't need to delete the MD each time?

Thank you.

mediamogul

@pjft said in Scraping multi disc roms:

Oh, sorry about the trouble.

Not your fault. All part of the cutting down the trees to see the forest.

set "Save data on exit"

Good call. You know, I haven't looked at that setting in so long, I forgot about it.

pjft

@mediamogul Well, I can't promise it'll work as intended for scraping, but it's worth testing once and see what to expect. I don't know if scraping has a separate saving process.

mediamogul

@pjft

Bingo, bango, bongo! The preexisting metadata must have been the issue on the original bisect. This time the results have the first "bad" commit as being:

d0cdbf215958e01ffea585a1c6fd75c03b306c46 is the first bad commit
commit d0cdbf215958e01ffea585a1c6fd75c03b306c46
Author: pjft <redacted@gmail.com>
Date:   Mon Jun 12 17:38:59 2017 +0100

    Adding support for Virtual Systems
    - Refactoring System Environment data
    - Added Virtual System Manager class
    - Added "all", "favorites" and "last played" systems
    - Added GuiInfoPopup class for notifications
    - Added Favorites to metadata, as well as a shortcut to toggle favorites
    - Added warning if enabling systems but themes don't support it
    - Added "filter by favorites" per system
    - Adjusted "Go to Random Game" behavior to account for the fact that we now have an "All Games" system
    - Added "sort by system name" for the collections

:040000 040000 d5855f78ce5752cd936b89b1f979dd27eadb682e daa56330c7bbbf0e4a6fde6940db6f8f1e6dc91c M	es-app
:040000 040000 ce4e1e2612ef9020a97c41d2def9edd07e208d2d 5815f4bd6a1e41ec15440579955bd826393109ed M	es-core

pjft

@mediamogul thanks! So your intuition about being related to the favorites was correct. Well done.

I'll try to look into it then - my main concern is actually getting a consistent way to replicate the issue in order to then try to address it.

I'll see what I can do with it.

Would you be able to share exactly what steps you take and at what exact stage it will crash?

Thank you and congrats on your first bisect:)

HurricaneFan

@pjft My scenario was I had a folder inside the SegaCD folder and 2 bin and 2 cue files in that folder. When I try to scrape the folder, it finds the data and image for the game. But when you hit the save icon. Hard crash to command line with no way of using your keyboard any more.

This is using the built in EmulationStation scraper.

pjft

@hurricanefan ok. When exactly hit save for the specific game inside the folder, it crashes.

Is your error similar to the one above, the one for map()?

HurricaneFan

@pjft Yeah, the scrape finds the data, its the save button press that starts the save process and crashes to command line with that map() error.

mediamogul

@pjft

To make it as easy as possible, I believe the issue can be replicated when scraping any folder at all that doesn't already have a <folder></folder> entry attached to it in it's respective gamelist.xml file. Once 'Save' is selected after scraping, the issue occurs.

Thank you and congrats on your first bisect:)

Thank you for the detailed write up. It really saved me a lot of time.

pjft

@mediamogul so, I played a bit with the code as I don't have current access to the Pi, but this might do the trick.

Would you be so kind as to test whether this prevents the crash?

git clone --recursive https://github.com/pjft/EmulationStation ES-pjft
cd ES-pjft
git checkout scraper-crash
cmake .
make
./emulationstation

I believe I was able to replicate it and test the most common scenarios.

If it doesn't crash, could I ask you for a few tests, both on scraping folders and games?

Thanks.

EDIT: Actually, the change has been merged to the ES-dev branch, if it makes it easier for you to run.

HurricaneFan

@pjft Your build works for me. I tested scraping several individual games and then scraped a folder by itself. All of the test cases were good to me.

By the way thegamesdb scrape source is super slow today. But thats outside of all of our control.

Do you need a pi to test code on? I have an extra pi that isn't being used.

mediamogul

@hurricanefan said in Scraping multi disc roms:

Your build works for me.

Were you using the dev branch or the method above? I had already compiled based on above earlier and it crashed when scraping a folder. I 'm in and out today, but I'll compile the dev branch next time I'm back and see if it makes any difference.