Skyscraper now officially part of RetroPie, please test

mitu

@muldjord Almost all of the scriptmodules don't modify the system's folders and I'd like to keep it in line with this principle. What we can do is add an option in the Advanced section to add the skyscraper installation folder to the user's $PATH. The install path is documented in the wiki page and I've added - similar to the scraper's doc - the command line path if the user wants to use it from outside the RetroPie script.
I've looked over the reddit topic and I've seen the topic, but it looks like the user didn't understand how to install and run Skyscraper - neither the normal way from your github repo, nor through the RetroPie-Setup script.

muldjord

@mitu Yes, it's purely for convenience. Typing the entire path each time is a bit of a hassle and strays from the Skyscraper github documentation. So any solution that makes it possible to just type "Skyscraper" is fine by me. I'm mostly worried about this on a support level. But I agree that I might be jumping the gun on this one. We'll see if it becomes a frequently asked question.

muldjord

EDIT5: Yes, it's definitely the binary release that is broken. It doesn't copy any of the required files and results in a non-working version of Skyscraper.

EDIT6: The source release is missing the following important files (the examples are copied and used if none exists. Especially the priorities.xml.example is really important):

dbs/priorities.xml.example
dbs/README.md
import/definitions.dat.example1
import/definitions.dat.example2
import/README.md

@mitu The Retropie-Setup Skyscraper binary installation seems to be broken (EDIT4: Currently compiling source release to see if it has the same problems). Are you in control of that? I just installed a clean RetroPie 4.4 to test the Skyscraper installation. I installed it from binary (no idea how that works) and it runs it just fine, but the artwork.xml is not copied to the .skyscraper folder, so I'm guessing other files don't either. EDIT3: Yep, just checked the folder and none of the required files are there. It's missing the following files:

mameMap.csv
README.md
hints.txt
config.ini.example
ARTWORK.md
artwork.xml
artwork.xml.example1
artwork.xml.example2
artwork.xml.example3
artwork.xml.example4
tgdb_developers.json
tgdb_publishers.json
dbs/priorities.xml.example
dbs/README.md
import/definitions.dat.example1
import/definitions.dat.example2
import/README.md

All of these files are required for Skyscraper to run properly. Some of the example files are copied over if none exists and so on, so they are quite important.

I also noticed that it by default inserts paths for dbFolder, importFolder, inputFolder and artworkXml in the default config.ini it creates. Could you remove those? They don't do anything right now as there is the symlink to the .skyscraper folder already and the inputFolder is the default one used by Skyscraper so it doesn't need to be set.

EDIT: Noticed that my config.ini has wheels="false" even though it is set to true in the ui.

EDIT2: You can also remove maxFails="30" as it is also the default and most users don't need to set this so there's no need to expose it.

mitu

@muldjord Hm, install from binary - that's something new. I'm not in control of that, the binaries are built by @BuZz and published in the RetroPie server. I see the problem also when doing a binary install - the config files are not created. Let me check the binary package, but I think it only contains the (no pun intended) binary.
About the 1st - edit

EDIT: Noticed that my config.ini has wheels="false" even though it is set to true in the ui.

The GUI uses the commandline to set these values, doesn't alter the .ini. Almost all artwork is enabled and disabled via CLI (except videos). I guess the wheels slipped through and I've set it to false by default.

EDIT6: The source release is missing the following files: [...]

I can add those to the source release. I'll do a PR for it.

muldjord

One more thing: In the .skyscraper/import folder it currently creates a "boxart" and "snaps" folder. These should not be created as they are leftover from back when I called them that. They aren't used by Skyscraper anymore.

Sorry to lay all this on you, this is the first time I've had the opportunity to check it out on a fresh install. Just got an extra SD card so I could test it. :)

mitu

@muldjord said in Skyscraper now officially part of RetroPie, please test:

Sorry to lay all this on you, this is the first time I've had the opportunity to check it out on a fresh install. Just got an extra SD card so I could test it. :)

No problem. Just do a source install and check other files that may be missing. Btw, you can do this on your Ubuntu install, its not restricted to the PI.

@buzz - I think you created a binary package for the skyscraper module, but I haven't accounted for that in the scriptmodule - so configuration expects some files in the build folder to be present so it fails.
Can you please remove the binary package until I figure this out ? This way the source install would work fine.

BuZz

@mitu hmm ok. The install stage should put everything that's needed into the install folder. Anything else outside of that needs to be in the configure stage. Maybe it's best to put this in experimental for now as I don't build binaries for those. Probably should have put it in exp initially in hindsight as its a new module etc.

mitu

@BuZz said in Skyscraper now officially part of RetroPie, please test:

Probably should have put it in exp initially in hindsight as its a new module etc.

Yes, I didn't know about binaries built automatically != exp or the nobin flag. I should have picked one of them. Thanks. I'll incorporate some feedback from @muldjord and do a PR then probably add another change later to accomodate the binary install.

BuZz

@mitu the nobin flag was only just re-introduced again so it wasn't there when you built the module. I don't build binaries automatically for experimental packages though. I have moved the module to experimental for now.

To make the module work from a binary install, everything needed will need to be in /opt/retropie/supplementary/skyscraper.

the configure script can copy data from this location (eg default configs), later. Then it will work from a binary install.

mitu

@BuZz said in Skyscraper now officially part of RetroPie, please test:

To make the module work from a binary install, everything needed will need to be in /opt/retropie/supplementary/skyscraper.

Submitted https://github.com/RetroPie/RetroPie-Setup/pull/2520 to fix this. Now all config files are pulled from $md_inst and the install has all the necessary files included.

@muldjord I've incorporated almost all your suggestions - what's left is the config.ini settings for the input, roms and dbs (I removed the artwork.xml). We can probably remove them in a subsequent modifications.
One thing I added as default is --unpack, I think it should be a default since most ROMs are zipped and it should make problems like I renamed by ROM to XYZ.zip be less prevalent.
I also fixed the Couldn't calculate sha1 hash sum of rom file '', please check permissions and try again, now exiting..." error, reported in

https://retropie.org.uk/forum/topic/19739/ and I think also in
https://old.reddit.com/r/RetroPie/comments/9qqi23/help_skyscraper_not_working/

I've encountered this error when testing, but I assumed it was because some of my systems had bogus zip files and Skyscraper couldn't unpack them (seemed to work fine with other scraping sources that don't use checksums). But it turned out it's a shell params expansion thing and I've found a simple workaround for it.

muldjord

@mitu said in Skyscraper now officially part of RetroPie, please test:

One thing I added as default is --unpack, I think it should be a default since most ROMs are zipped and it should make problems like I renamed by ROM to XYZ.zip be less prevalent.

Nonono, please remove that again. --unpack is implemented in a way that could give users problems with memory (and a performance hit) since it doesn't feed the checksum calculation data in segments. It works just fine without it as most zips are already in the screenscraper db, and all other sources are name based so they don't use it anyway.

I general anything that isn't default shouldn't be added by default. I usually add options to Skyscraper in a way that things that should be default, I just enable them, and then I add the opposite bool option to allow users to disable it. So, for instance, for --unpack, if it should be enabled by default, I would add '--nounpack'.

mitu

@muldjord OK, no problem.

movisman

@muldjord @mitu

Hi,

Just tried using this scraper for the first time when chopping down my ROMs to a smaller number, and so far, so good! I have only done a few systems, but it works exceptionally well - the RetroPie-Setup integration is also a massive plus for convenience.

I have one question/suggestion though, I noticed that when doing re-scrapes on a platform, if any roms had been deleted since the last scrape, the gamelist.xml would be updated to reflect that (eg. old game removed), but the media will always stay there.

I assume this is intentional so it is safer for the end user, but is there a switch or a way to have it so the old orphaned media is purged when a scrape takes place?

I thought the --cleandb switch might do this (which isn't available in the GUI) but when I tried to run it via CLI, it didn't seem to remove the media.

Do either of you have any suggestions to help me keep the media tidy/in sync with the roms and gamelist.xml?

Cheers!

mitu

@movisman

I have one question/suggestion though, I noticed that when doing re-scrapes on a platform, if any roms had been deleted since the last scrape, the gamelist.xml would be updated to reflect that (eg. old game removed), but the media will always stay there.

That's the behavior of every scraper, it cannot erase what was downloaded through another program.

I assume this is intentional so it is safer for the end user, but is there a switch or a way to have it so the old orphaned media is purged when a scrape takes place?

Not from the scraper, since they're out the actual gamelist and they could be - potentially - everywhere. Furthermore, it could cause problems for users that are just trying out Skyscraper and might have custom artwork/screenshots - we don't want that to get erased just from running Skyscraper.

I thought the --cleandb switch might do this (which isn't available in the GUI) but when I tried to run it via CLI, it didn't seem to remove the media.

It's normal, since --cleandb cleans the Local DB and not the output of the scraping (i.e. arwork referenced in gamelist).

However, since Skyscraper caches the media, you can remove (or just move out of the way) the old media(s) folder (used by the old gamelists) and re-scrape again, it will re-generate a clean gamelist and media folder without downloading anything :).

movisman

@mitu

Hi there,

Ah, just to clarify, there was no media or gamelist present before starting to use Skyscraper. I deleted all of the files for that system before scraping. So in my test case, I removed all of the media and the gamelist.xml from Atari 2600 and 7800 before scraping.

With this in mind, should it be cleaning up media files if a ROM is removed and a rescrape is performed (bearing in mind the first scrape is done by Skyscraper)?

Thanks!

brimby

@mitu I'm seeing it in "experimental packages" not optional. Was this changed?

muldjord

@brimby Yes, it was moved to Experimental as we found some problems with it. They have been fixed now though, so it might be moved back to Optional again soon.

muldjord

@movisman If you mean the cached data, then yes, this will be possible in a future version of Skyscraper. It's not possible currently though. But it's on my roadmap. Skyscraper won't ever remove any media from the media folder. As @mitu mentioned it is not possible to know what files the user wants to keep and which to delete. So that will never be an option.

IanDaemon

@muldjord said in Skyscraper now officially part of RetroPie, please test:

@brimby Yes, it was moved to Experimental as we found some problems with it. They have been fixed now though, so it might be moved back to Optional again soon.

I can't find it in either Optional nor Experimental. Was this removed temporarily?

mitu

@IanDaemon What version of RetroPie do you have installed ? Did you scroll down in the experimental section to see all packages ?