Skyscraper now officially part of RetroPie, please test

Clyde

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

It has the option to do masks and frame overlays and can rotate the resources to fit the different sections of the cabinet.

So it can put scanlines in the videos? That would be great, since ES and the OMX Player can't do that on the fly like Attract Mode can, for example.

muldjord

@Clyde OMG IT'S CLYDE FROM PAC-MAN! I'm a big fan :D

No, it can't add effects to videos unfortunately, but you could add a scanline effect to the graphics using a frame effect with transparency applied to a layer.

mitu

@muldjord

So it can put scanlines in the videos?

Does it work for videos ?

muldjord

@mitu Sorry, no, I was just editing my post while you were posting. :D No, it can't process videos.

Clyde

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

@Clyde OMG IT'S CLYDE FROM PAC-MAN! I'm a big fan :D

:)

No, it can't add effects to videos unfortunately, but you could add a scanline effect to the graphics using a frame effect with transparency applied to a layer.

So, it can put scanlines over the videos? That's what I actually meant, since Attract Mode doesn't modify the videos themselves either.

Just to be sure I understand you correctly: I would create a scanline image with transparent lines between the scanlines, and use that as a "frame" although its no actual frame?

muldjord

@Clyde Yes, you can do that for images, but not for videos unfortunately.

Clyde

@muldjord Damn, just when I saw a silver lining on the horizon.

Thanks for the clarification, though.

muldjord

@mitu Two users just asked on reddit why they got "File not found" when trying to run Skyscraper from command line after installing it with the setup script. Could we add a sudo ln -s /opt/retropie/supplementary/skyscraper/Skyscraper /usr/local/bin/Skyscraper line to the installation script somewhere so users can run it easily from command line?

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.