Take and Scrape Your Own Screenshots

meleu

@herb_fargus
When I was playing with RetroArch code I found that there is an option named auto_screenshot_filename.

As far as I understood, if this flag is set to "true" in retroarch.cfg, the screenshot filename is always Game Name.png, therefore it overwrites the old screenshot. It would be a really really nice feature for the trick we want to do here. Because we could turn on/off the trick based on auto_screenshot_filename flag.

The only problem is: I didn't get it working!

I made some tests here and it seems to be ignored by RetroArch. What a pity...

Sorry for this good followed by a bad news...

herb_fargus

@meleu that auto screen name thing would be ideal.

If the filename retains the date and time that will require a bit more logic to clean for the filenames when scraping. Its a step in the right direction though

meleu

Oh! But I have good news now:
My PR was merged! Let's update RetroArch from source!! :D

Now we have a problem here... our trick here will be incompatible soon. And if we adapt to fit the changes in the screenshot names, it won't be useful for outdated versions of RetroArch...

Oh god! Good followed by a bad news again... :-/

[EDIT: well, maybe if we adapt the RegEx to get the timestamp instead of the "RetroArch" string it can work as we want. I'll take a look at it later...]

herb_fargus

@meleu simple enough fix, just have both versions or add a disclaimer for people to update from source - or we could figure out that extra setting ;)

meleu

@herb_fargus said in Take and Scrape Your Own Screenshots:

@meleu that auto screen name thing would be ideal.

damn! You hit me! I'll check what's wrong with the auto_screenshot_filename option and try to make it work!

meleu

@herb_fargus
Man! What a shame!

facepalm

Actually the auto_screenshot_filename flag does his job very well!!

The trailing timestamp is what the "auto" means! If you want a screenshot with the "ROM Name.png" only, you just have to set auto_screenshot_filename to false (the default is true).

Our script here can take advantage of this feature!

herb_fargus

@meleu so you mean it worked this whole time? :P ha ha that's funny. Ok well I'll give it a test once I get some time to compile retroarch

meleu

@herb_fargus said in Take and Scrape Your Own Screenshots:

@meleu so you mean it worked this whole time? :P ha ha that's funny. Ok well I'll give it a test once I get some time to compile retroarch

yeah man! it works even before my PR was merged. There is no need to compile retroarch to get those changes I made, because the auto_screenshot_filename = "false" is what we want! My changes only takes effect when auto_screenshot_filename = "true", and the default (if this option aren't in retroarch.cfg) is true.

We can adapt the "take and scrape your own screenshots" trick to work only when the auto_screenshot_filename = "false". I'm at work now, but I check it as soon as get in home...

herb_fargus

@meleu heh talk about an over engineered solution well regardless a bug was still fixed as a result so good job :) I'll see if I can update it. Maybe there's a way I can have it automatically generate folders and/or configs this time around- perhaps maybe come up with a module for it or something

herb_fargus

@meleu

Alright here's a prelimnary more or less untested psuedo code for runcommand-onend.sh:

# Variables
system="$1"
imgdir="$HOME/RetroPie/roms/$system/images" 

# check if images folder exists for each system, if not create it

if [[ ! -d "$imgdir" ]]; then
    mkdir $imgdir
fi

####### Possible configs to automate potentially with inifuncts?

#### overall retroarch.cfg in /opt/retropie/configs/all:

# Hotkey config to take screenshot with controller
input_screenshot_btn = "12"

# This makes sure the screenshot filename is named ROM NAME.ext
auto_screenshot_filename = "false"

#### system based retroarch.cfg in /opt/retropie/configs/$system/

# Add screenshot per system directory
screenshot_directory = "$HOME/RetroPie/roms/$system/images/"

Really if you take out the automation, the runcommand-onend.sh is really no longer necessary, the user just has to manually edit 3 things:

create an images folder for each system they are scraping with
specify the screenshot directory in each system based retroarch.cfg e.g. screenshot_directory = "$HOME/RetroPie/roms/$system/images/"
add the auto_screenshot_filename = "false" to the main retroarch.cfg

Just thinking out loud but it should be simple enough to use inifuncts with this recent change:

https://github.com/RetroPie/RetroPie-Setup/commit/7d806d52e9fc55fd557cd4a7f931df6e07509bf3

(a little more discussion on it here: https://retropie.org.uk/forum/topic/3285/i-think-we-finally-have-a-solution-for-rgui-fans/11)

I'm just unsure if we have to initialise the inifuncts script on its own in the runcommand-onend since its technically already initialised here:

https://github.com/RetroPie/RetroPie-Setup/blob/406728c368d038270651159430ded40a92df2649/scriptmodules/supplementary/runcommand/runcommand.sh#L57

but then again variables like the $romdir $rootdir etc are initialised in the retropie_packages.sh which are obviously used in the main runcommand.sh but aren't initialised in the runcommand-onend.sh

anyways what I'm saying is the variables are all already in the code, we may just have to call inifuncs.sh and retropie_packages.sh, or duplicate code to initialise the variables manually. idk. just ideas.

herb_fargus

@meleu

Ok I think I've got it. I've tried to simplify it as best as I could. Could probably be refactored a bit anyhow. so this one is a little different in that everything is automated so many of the steps on the first post are unnecessary and it uses runcommand-onstart rather than runcommand-onend (primarily because I want it to create the image folder first before screenshots are taken so they have somewhere to go)

so create a file with the following contents called rucommand-onstart.sh and place it in /opt/retropie/configs/all

#!/usr/bin/env bash

system="$1"
imgdir="$HOME/RetroPie/roms/$system/images"
configdir="/opt/retropie/configs" 

mainretroarch="$configdir/all/retroarch.cfg"
systemretroarch="$configdir/$system/retroarch.cfg"

source "/opt/retropie/lib/inifuncs.sh"

# If there is no auto screenshot setting in the main retroarch.cfg add it
function add_overall_screenshot() {
    if ! grep -q "auto_screenshot_filename" $mainretroarch; then
        iniConfig " = " '"' "$mainretroarch"
        iniSet "auto_screenshot_filename" "false"
fi
}

# check if an images folder exists in the systems rom folder, if not create it
function make_screenshot_dir() {
    if [[ ! -d "$imgdir" ]]; then
        mkdir $imgdir
fi
}

# If there is no system based screenshot directory defined then define it in the system based retroarch.cfg
function add_system_screenshot() {
    if ! grep -q "screenshot_directory" $systemretroarch; then
        iniConfig " = " '"' "$systemretroarch"
        iniSet "screenshot_directory" "$imgdir"
        retroarchIncludeToEnd
fi
}

add_overall_screenshot
make_screenshot_dir
add_system_screenshot

It may also be possible to automate sselphs scraper, but I dont think its a good fit with the onstart or onend command. perhaps its something better left as a manual step.

meleu

@herb_fargus
I'll come back to this topic soon (I'm very sleepy after a 12 hours turn). But first I would like to share some tips/thoughts with you:

the scraper command you use in the first post can be faster if you use the options -skip_check=true and -use_gdb=false.
I don't like to use a subdir for images in the roms directory. You can use the -image_path="path/to/screenshots" to make the scraper get the images from another place.
I really would like to keep my previous gamelist.xml and only change the entry for the game that we recently took the screenshot. I think the way to achieve it is the -append=true option. But we need some XML parsing trick to exclude the game entry from gamelist.xml before call the scraper in order to make it consider the game as a new entry.

I think we are on the right way...

herb_fargus

@meleu those are good tips to keep in mind. I suppose that we could set the screenshot directory to just one screenshot directory (perhaps even just leave the default) and then use sselphs scraper to point to that, eliminates another config step but I like the idea of having all the images and gamelists in each rom folder because you only have to scrape once and then you can just backup/copy the roms folder complete with images etc with no need to scrape again.

also if you use the -skip_check=true and -use_gdb=false would that not eliminate the metadata for the detailed view so you only end up having an image?

I guess we'd have to get feedback to see what people prefer

herb_fargus

@hiulit I've created a wiki page for it with the latest working code. See first post for link

hiulit

@herb_fargus nice! Thanks!

meleu

@herb_fargus
[EDIT: I've edited the wiki including those two options for a faster scraper execution. I intentionally didn't edit the runcommand-onstart.sh to let it as you prefer.]

Some points on the wiki runcommand-onstart.sh:

You can set the iniConfig just once and then pass the file to the iniSet in the 3rd argument.
the function make_screenshot_dir can be replaced by mkdir -p "$imgdir" (the -p option means "no error if existing, make parent directories as needed").
There is no need to explicitly call retroarchIncludeToEnd, since it is automatically executed when editing any file with a name that ends with "retroarch.cfg" (inifuncs feature).

The runcommand-onstart.sh can be simplified to this:

#!/usr/bin/env bash

system="$1"
imgdir="$HOME/RetroPie/roms/$system/images"
configdir="/opt/retropie/configs" 

mainretroarch="$configdir/all/retroarch.cfg"
systemretroarch="$configdir/$system/retroarch.cfg"

source "/opt/retropie/lib/inifuncs.sh"

iniConfig " = " '"'

# If there is no auto screenshot setting in the main retroarch.cfg add it
if ! grep -q "auto_screenshot_filename" "$mainretroarch"; then
    iniSet "auto_screenshot_filename" "false" "$mainretroarch"
fi

mkdir -p "$imgdir"

# If there is no system based screenshot directory defined then define it in the system based retroarch.cfg
if ! grep -q "screenshot_directory" $systemretroarch; then
    iniSet "screenshot_directory" "$imgdir" "$systemretroarch"
fi

herb_fargus

@meleu you still didn't address:

also if you use the -skip_check=true and -use_gdb=false would that not eliminate the metadata for the detailed view so you only end up having an image?

meleu

@herb_fargus I was so focused on the images thing and didn't give a proper attention to this important detail. Sorry.

Re-edited the wiki excluding these options.

herb_fargus

@meleu I personally don't care about metadata but I know lots of others do.

Once gridview is incorporated I think those extra flags will be more useful (though I presume that it may be possible to theme metadata in with that at some point) might be a cool design to have a box on the side that changes with each game. I'll have to play with that in the future.

Also have you tested your code changes ;)

herb_fargus

@meleu I tested your code changes and they work :) thanks for the refactor. I've updated the wiki page accordingly.