sharkwouter / minigalaxy

A simple GOG client for Linux
https://sharkwouter.github.io/minigalaxy/
GNU General Public License v3.0
1.11k stars 72 forks source link

installer.py:create_applications_file can't handle launch commands with env #623

Open GB609 opened 2 days ago

GB609 commented 2 days ago

Note: A part of this issue is theoretical and can't be reproduced in the current version 1.3.1 because at the point in time when shortcuts are created, none of the current command creation logic returns 'env ...' (yet). I've stumbled over then when i tried to fix the wine install and launch commands in my fork.

The method installer.py:create_applications_file has an error in how the value for Exec is calculated:

exe_cmd = get_exec_line(game)  # calls launcher.py:get_execute_command(game)
"game_bin_path": os.path.join('"{}"'.format(game.install_dir.replace('"', '\\"')), exe_cmd)

It is simply prepending the game's install directory in front of the command to execute, assuming that the command will always start with a relative path to the game binary. Wine commands, however, start with env WINEPREFIX=..., at least in the installer. The exe command for wine from launcher.py, however, does not prepend 'env', instead it manipulates the environment of minigalaxy itself (i don't think that this is a good idea anyway)

def get_windows_exe_cmd(game, files):
    exe_cmd = [""]
    prefix = os.path.join(game.install_dir, "prefix")
    os.environ["WINEPREFIX"] = prefix
    #...

I can only guess that this was done that way because using 'env' broke the .desktop file creation during install. But that results in a different behavior of wine games when launched within minigalaxy vs. launched via the desktop shortcut created by minigalaxy: WINEPREFIX won't be set to the game.install_dir/prefix when the shortcut is used. Thus, it will either be unset or point to whatever a user might have pre-defined via startup profile in the environment. However, most likely it won't be the prefix configured for the respective game.

A possible solution would be to let all binary path/command creation methods return canonical paths to the binaries and then use the entire command for game_bin_path. I'd also drop the manual quoting/masking done in get_exec_line(), and instead just use shlex, effectively resulting in:

def create_applications_file(game):
    #...
    exe_cmd = get_execute_command(game)
    #...
    "game_bin_path": shlex.join(exe_cmd)
    #...

Afterwards, the bug/behavior in get_windows_exe_cmd can be fixed and the returned command could use env.

There is one additional issue with shortcuts: They are not updated when the user changes game properties, but most of the options there (gamemode, mangohud, env, arguments) actually change the final exec command.

I have 3 ideas how to fix this:

  1. The simple way: Re-create the shortcut when properties are changed and it exists. The advantage is that the shortcuts created this way are theoretically independent from minigalaxy and thus minigalaxy can be skipped over in the process chain when using the shortcut (maybe relevant for performance, considering that all game io needs to be piped through minigalaxy otherwise). It could even be uninstalled and the games installed up to this point would still continue to work with their last configuration.
  2. Provide a command line interface to start a game specified by id or installation directory and change new shortcuts to just call this interface of minigalaxy with the right arguments
  3. Do a bit of both. Provide a command line interface, but let it just stdout the full launch command instead of starting it directly. The shortcuts could then do something like sh -c $(minigalaxy --get-launch-cmd <gameid>) and will always get the current configuration. No need to update the shortcuts after their first creation.
GB609 commented 1 day ago

624 will change shortcut creation to also handle precommands like env or mangohud correctly.

The issue that shortcuts are not updated when game properties are changed is not touched this PR.