ScummVM / lr-scummvm Launcher and Interoperability Improvements

Lolonois

@mitu Nvm. There is no SLA (service level agreement) attached to open source. :) But thanks for being polite.

From a Linux only viewpoint I don't care much about the filename of the *.svm file. Only / seems to be problematic when using the description= entry from the scummvm.ini as filename (which also the OP proposed script uses and it filters / already out.).

What I dislike in the current implementation is that the *.svm files are generated everytime, after ScummVM has been run. This might overwrite changes in the *.svm file the user did (e.g. instead of using lba-fr instead of lba-gb (the default/generated by the script) for "Little Big Adventure")

The scraping with a used defined *.svm filename should also be successful as long as the majority of the game title is used in the filename, but I have not tested it with the builtin scraper of ES.

I agree that the initial "bulk" add should happen from the ScummVM GUI, although it may add some demos/teasers which are not really a game.
The on-the-fly-add would be handy for later occasional adding of a game.
The errormessage I added is the last option, usually something else has been wrong beforehand. If all gamefiles are present autodetect works well.

I did not invent the "contract" that the *.svm and folder must share the same name. It has been done by other contributors for a good reason: To be able to run games with lr-scummvm without having to consult the native ScummVM scummvm.ini file for identifying the path= of a game.
In other words: If this contract is not granted a user must install and configure ScummVM before being able to use retroarch ScummVM. In turn, when running lr-scummvm must do an indirection via the native scummvm.ini to find the gamefolder (which is an annoyance from a decoupling perspective/voiding the orthogonality of each emulator).
However, a user may decide to also run "bulk" add in lr-scummvm to avoid this step, but then the annoyance is transfered to the user (as he or she has to rerun Add Game also for lr-scummvm while she or he did it already in the native ScummVM). This is also contrary to all other systems sharing the same romfiles and is not needed there when changing the emulator.

PS: Maybe it is advisable to move this to "Ideas and Development", regardless if/how it will be implemented, to avoid getting covered by a pile of support questions.

Lolonois

FTR here [1] are some other observations to consider on the Mass add... via GUI (first paragraph). So it seems advisable to review/clean the list of detected games after such run.

[1] github.com/libretro/scummvm/issues/179#issuecomment-1013292476

mitu

NOTE: I've split the conversation from https://retropie.org.uk/forum/topic/29349/ to a new topic.

What I dislike in the current implementation is that the *.svm files are generated everytime, after ScummVM has been run. This might overwrite changes in the *.svm file the user did (e.g. instead of using lba-fr instead of lba-gb (the default/generated by the script) for "Little Big Adventure")

Yes, I agree w.r.t. auto-generation running each time. Maybe checking for added games in scummvm.ini (file timestamp?) and cross-checking with existing .svm file so any additions will not overwrite user edits (checking for game id maybe).

The scraping with a used defined *.svm filename should also be successful as long as the majority of the game title is used in the filename, but I have not tested it with the builtin scraper of ES.

I've briefly tested with a few manually created .svm files and it worked with ScreenScraper. TGDB (the other scraping source) doesn't have a ScummVM platform, so it doesn't apply to it.

I agree that the initial "bulk" add should happen from the ScummVM GUI, although it may add some demos/teasers which are not really a game.
The on-the-fly-add would be handy for later occasional adding of a game.
The errormessage I added is the last option, usually something else has been wrong beforehand. If all gamefiles are present autodetect works well.

I did not invent the "contract" that the *.svm and folder must share the same name. It has been done by other contributors for a good reason: To be able to run games with lr-scummvm without having to consult the native ScummVM scummvm.ini file for identifying the path= of a game.
[..]

I haven't looked in deep at how lr-scummvm works, but the docs on the libretro site describe roughly the same steps as the standalone - start the core, then scan for games [1]. I can only guess that this modifies the core's scummvm.ini.

Alternatively, I see it uses the a .scummvm file, which is created in the game's folder. My idea was to generate this file similar to how .svm files are generated, just to appease both emulators. I guess the launcher can check both scummvm.ini files and - if something new is added - try to generate a .svm/.scummvm file depending on which emulator is installed. They can be basically identical: GameName as the filename and gameid as content.

[1] https://docs.libretro.com/library/scummvm/#adding-a-game-to-the-scummvm-launcher.

Lolonois

Re: auto-generation run every time

Using time on a system with an undisciplined clock by default was too shaky for me. I check if hashes over all sections before and after the launch of ScummVM differ (see my branch in +Start ScummVM.sh).

Re: Scraping

Thanks for testing. Good to know.

Re: lr-scummvm

What I have noticed that lr-scummvm only keeps scummvm.ini entries if they are done via the UI. Manual adds or edits are silently removed. The UI puts the games with dashes into its ini, e.g. [dw2-gb] for Discworld II.
This effect makes my python helper script not usable for the lr-scummvm ini file. :(
I only changed the launcher (romdir-launcher.sh) to create a symlink with extension .scummvm of the .svm in the parent folder. If the .svm does not contain any game id a dummy (.autodetect) file is given to lr-scummvm to trigger autodetection.
(On a side note: I was not able to persist any game specific changes in the libretro scummvm.ini, e.g., setting different volumes for music, sfx and speech - guess I will have to table that to the RA guys...)

However, I had a look at the RA documentation [1, from above] and changed the +Start ScummVM.sh accordingly, which moved the implementation after running Mass Add... / Add Game in a more concise direction:

Re: Adding games via UI and creation/update of *.svm files

First, I could remove all lr-scummvm ini logic from the scummvm_helper.py script.
Second, as retroarch follows the "no dashes" policy for game identifiers I changed the logic in the scummvm_helper.py to act the same way. This means any .svm file should only contain the game id (as in the compability chart of https://www.scummvm.org/compatibility/) and as it is currently documented in the RetroPie Wiki.

The Add Game prompts the user which game to add when variants of one game are found (variants will end up having dashes in their identifier). Retroarch documents that one should remove the dashes and retain only the game short name, but as documentation is rarly consulted IMHO my Python script does the same.
When running Mass Add... this is even more handy, as there is no interaction with the user. Eventually, after applying the Python script, the scummvm.ini has exactly one section for every game which has no dashes and thus can be seamlessly used also for lr-scummvm. But more importantly: When a game id from the .svm is not exactly matched no game specific settings are saved (e.g., volume settings). Additionally it is an annoyance for the user to make the match exact between game id and scummvm.ini section by editing either file and even error prone. IMHO identifying the game id including variant is nearly impossible if the user just deploys the gamefiles in a folder. And -as I wrote earlier- getting the folder name right during game file deployment to match the description= value of the scummvm.ini is also impossible (Chicken and egg problem).
TL;DR: No game id with dashes either in .svm file or in scummvm.ini`s [section].

ctrueden

Thanks @lolonois @mitu and everyone for discussing this issue further!

I wrote a comment on my PR which for convenience I will reproduce here:

Reading over the forum topic, this is a bigger can of worms than I realized. I don't think I'm going to have enough time any time soon to think more carefully about this problem—especially the dichotomy between the two ScummVM flavors.

Off the top of my head, my instinct for solving that issue would be first figure out how to discern which of the two flavors of ScummVM is in use within that installation, and then have the behavior of the shell script contingent on that information. Of course, one must still make decisions about how aggressively to generate .svm files. Best is probably to add some flags to the top of the shell script letting people control this, and then in the documentation on the website explain how to configure these things, so that everyone can decide for themselves how they want it to work. Finally, for default behavior, I think it should come as close as possible to "just working" out of the box as possible, which to me means trying to stay in sync with the ScummVM UI, i.e. autogenerating .svm files always to match what the UI has configured. To accommodate manually created or tuned .svm files, there could be a comment in the file indicating it was autogenerated, which users can remove if they want that .svm to become "sticky" i.e. not get deleted by the auto-syncing logic.

I'll leave this PR open as is, in case anyone else has time to work further on this.

Thanks again to all for everything you do to make RetroPie awesome. ☺️

Lolonois

@ctrueden Glad you followed-up on the "can of worms" sorting. IMO there is nothing wrong to pause a suggested change. However, you are right the ScummVM launcher is an odd landmark and contradicts with the documentation atm.

Luckily we dug up most (if not all) the details. Surprsingly, it turned out there is more to consider as I assumed as I went down the rabbithole. Anyway, the information is in thread and can be picked up at some time. I made my suggestions in a branch [1], all what's needed is some consent with/between the maintainers on which logic to integrate it into the mainline.

Maybe @hhromic and @pjft want to add here, as they were discussing the lr-scummvm [2] module initailly.

[1] https://github.com/Gemba/RetroPie-Setup/tree/fb_scummvm_lrscummvm_scriptmodules (see scriptmodules scummvm.sh and lr-scummvm.sh)
[2] https://retropie.org.uk/forum/topic/19693/lr-scummvm-request-for-comments-and-testing

Lolonois

Did some updates as it turned out that a more recent ScummVM core (2.5+) in lr-scummvm does use the scummvm.ini.

My scummvm_helper.py does apply also again for the lr-scummvm, and:
When a Add Game or Mass Add... is done in lr-scummvm UI it follows the same logic as the native ScummVM:
- Make unique game id entries in scummvm.ini by chopping of any variant suffix.
- It creates *.svm files, if missing. The *.scummvm extension should only be used by the rom-launcher.sh internally as it might produce doubles in the gamelist if these would be enabled in es_systems.cfg.

Update Jan 2024
Tested ok with RetroPie 4.8.6 (ad3ca8c) and ScummVM 2.8 / lr-scummvm 2.8

For those who want to testdrive, ssh to your Pi, then:

git clone --depth 1 --branch fb_scummvm_lrscummvm_scriptmodules https://github.com/Gemba/RetroPie-Setup/ rp_setup_scummvm
cd rp_setup_scummvm
sudo ./retropie_packages.sh scummvm install_bin 
sudo ./retropie_packages.sh scummvm configure 
sudo ./retropie_packages.sh lr-scummvm install_bin 
sudo ./retropie_packages.sh lr-scummvm configure

To return to the original scriptmodules do run the retropie_package.sh commands in /home/pi/RetroPie-Setup folder. No need to run those with install_bin again.

windg

Thanks for the trying to improve the function of scummvm. I test it but it doesn't work very well for me or i did something wrong.

I have the game in the scummvm folder with name : Broken Sword - The Shadow of the Templars . Inside i put the empty file Broken Sword - The Shadow of the Templars.svm. When i choose to launch the game an error from scummvm appear Error running game:Game data didn't found, here is runcommand.log:

Parameters: 
Executing: bash /home/pi/RetroPie/roms/scummvm/+Start\ ScummVM.sh "Broken Sword - The Shadow of the Templars"
/home/pi/RetroPie/roms/scummvm/+Start ScummVM.sh: line 55: /home/pi/RetroPie/roms/scummvm/scummvm_helper.py: Permission denied
Using game controller: Logitech F710 Gamepad (XInput)
Virtual keyboard pack 'vkeybd_default' loaded successfully
WARNING: No path was provided. Assuming the data files are in the current directory!
WARNING: Broken Sword: The Shadow of the Templars failed to instantiate engine: Game data not found (target 'sword1', path '/home/pi/RetroPie/roms/scummvm')!
User picked target 'sword1' (engine ID 'sword1', game ID 'sword1')...
   Looking for a plugin supporting this target... Broken Sword: The Shadow of the Templars
Using game controller: Logitech F710 Gamepad (XInput)
WARNING: initWithU32String: Fribidi not available, will use input strings as fallback.!

Thanks

Lolonois

@windg said in ScummVM / lr-scummvm Launcher and Interoperability Improvements:

/home/pi/RetroPie/roms/scummvm/+Start ScummVM.sh: line 55: /home/pi/RetroPie/roms/scummvm/scummvm_helper.py: Permission denied

This one is on me, I will fix it in the branch. In the meantime you can do:

sudo chmod a+x /home/pi/RetroPie/roms/scummvm/scummvm_helper.py

The other one is that the .svm should be alongside/sibling to the folder thus:

Broken Sword - The Shadow of the Templars/
Broken Sword - The Shadow of the Templars.svm

and not:

Broken Sword - The Shadow of the Templars/Broken Sword - The Shadow of the Templars.svm

Thanks for having it tested on your setup in either case.

mitu

@Lolonois said in ScummVM / lr-scummvm Launcher and Interoperability Improvements:

sudo chmod a+x /home/pi/RetroPie/roms/scummvm/scummvm_helper.py

You don't need sudo for that, but the reason it might not work is an USB mountpoint for roms, which gets mounted by default with noexec. It's safer to use python3 /path/to/script.py to work around this.

windg

Permissions issue fixed and i put .svm
file into the scummvm folder but the scummvm shows the same error: about no game data found. Game is working if i choose to start it from the gui.

I looked in the scummvm Gui and the game is successfully added and svm file have the correct short name.

I saw mitu comment and i have to say that i am running roms from a usb stick.

Update: I reinstall again the cores, i move the games in the sd-card and they are running, Hurray!!

Lolonois

Thanks both for testing and highlighting the culprit. The Windows/USB mount use case constantly escapes me, @mitu .

Anyhow, I just pushed changes to allow also USB/Windows formatted romfolders. Tested ok on my side. Happy for feedback from your side @windg when you can spare some time.

mitu

@Lolonois said in ScummVM / lr-scummvm Launcher and Interoperability Improvements:

The Windows/USB mount use case constantly escapes me, @mitu .

That's easy to fix - buy an USB stick and store your ROMs on it.

windg

@Lolonois I tested it but it's failing with the same error. Also the permission issue is still here.

Parameters: 
Executing: bash /home/pi/RetroPie/roms/scummvm/+Start\ ScummVM.sh "Return to Zork (CD DOS)"
/home/pi/RetroPie/roms/scummvm/+Start ScummVM.sh: line 52: python3 "/home/pi/RetroPie/roms/scummvm/scummvm_helper.py": No such file or directory
Using game controller: Logitech F710 Gamepad (XInput)
Virtual keyboard pack 'vkeybd_default' loaded successfully
WARNING: No path was provided. Assuming the data files are in the current directory!
WARNING: MADE failed to instantiate engine: Game data not found (target 'rtz', path '/home/pi/RetroPie/roms/scummvm')!
User picked target 'rtz' (engine ID 'made', game ID 'rtz')...
   Looking for a plugin supporting this target... MADE
Using game controller: Logitech F710 Gamepad (XInput)
WARNING: initWithU32String: Fribidi not available, will use input strings as fallback.!

Something I noticed, the helper.py file is on the usb stick, into the rom folder, alongside with the +scummvm sh that is normally here.

Lolonois

@windg The location of the helper script is intended, as it can also be used without the launcher. That has not changed with my update.

Just to cross this out: Did you run git pull in the folder with my branch before running configure again?

windg

@Lolonois I didn't did that. THe steps i follow was :

uninstall both scummvm cores
Delete the folder
Reinstall again with the above commands
Run the permission fix command

Thanks!