minigalaxy icon indicating copy to clipboard operation
minigalaxy copied to clipboard

Some Wine games have issues caused by moving game files after install

Open LeXofLeviafan opened this issue 3 years ago • 12 comments

Currently, when installing a Wine game, Minigalaxy appears to be doing the following:

  1. Create a folder named ~/.minigalaxy/extract/$ID
  2. Create a wineprefix inside
  3. Run installation process via Wine
  4. Move the contents of said folder into designated installation path (e.g. ~/Games/GOG/$NAME)

I'm not sure how necessary it is to do the install in a different folder than the files actually end up, but some games (mostly old ones, e.g. Return to Krondor, Betrayal in Antara, Rage of Mages 1/2, Escape from Monkey Island, Die By the Sword) refuse to start up due to not finding the files where they're supposed to be (e.g. instead of Z:\home\$USER\Games\GOG\Return to Krondor\GameData\RtkGame.def it's looking for Z:\home\$USER\.minigalaxy\extract\$ID\GameData\RtkGame.def).

I can think of a way to avoid this issue: add virtual drive into the prefix (e.g. D:) pointing at the extraction folder before the 3rd step (Wine installer), then change it to point at the designated installation path at the end of installation process; this way, as far as the game is concerned, its files aren't moved at all. (Though I'm not sure if making the game installation folder a virtual drive is actually valid, as the usual practice seemed to install the game into a folder at the top level of the drive.)

LeXofLeviafan avatar Jul 24 '22 17:07 LeXofLeviafan

Thanks for the report. This makes sense to me. It also causes some other issues like #313. We want to move away from actually running the Windows installers, though. We're a pretty good way in too. If you install innoextract, this should no longer be a problem. Although Innoextract has some issues as well.

sharkwouter avatar Sep 05 '22 15:09 sharkwouter

If you install innoextract, this should no longer be a problem.

Unfortunately it only seems to work with games that don't need to have their install path in system registry; when I've installed Soulbringer with innoextract, trying to run it only resulted in an error message saying the game is not installed (…and the only way to install it normally as of now is via hacky means such as disabling innoextract usage manually in the application code).

LeXofLeviafan avatar Sep 11 '22 17:09 LeXofLeviafan

Okay, that's bad. There has to be a clean solution to this. Changing the way installations work would be a good start, but the innoextract method should not result in a game that doesn't work either.

sharkwouter avatar Sep 11 '22 19:09 sharkwouter

At the very least it should be made possible for the user to choose installation method (e.g. by offering a global setting to [dis]allow using innoextract when it's set).

…Speaking of which, allowing to choose how the games are run would be good as well (at least by giving start.sh higher priority so that Wine games can be wrapped in a custom-made script or something), but that's probably a separate matter :-)

LeXofLeviafan avatar Sep 11 '22 23:09 LeXofLeviafan

Here a little note for myself or anyone else who feels like implementing a fix for this, there is quite promising documentation on a way to just download the files for the Windows release of a game rather than an installer: https://github.com/Lariaa/GameLauncherResearch/wiki/GoG-:-Installing-games

This of course needs quite some testing and would take a while to implement, but it could improve the current situation.

sharkwouter avatar Sep 25 '22 20:09 sharkwouter

I kinda suspect that the results would be hardly different from those of using innoextract (in regards to the context of this discussion at least), as it doesn't appear to address the original issue in the slightest (that being the lack of the actual install path in prefix files/registry for games that require it).

So far I can't think of a way to install the affected games correctly other than by retaining consistent (in-prefix) paths between installation and play time, as I suggested earlier.

LeXofLeviafan avatar Sep 26 '22 18:09 LeXofLeviafan

Okay, that is fair. At the moment we have 2 ways of installing Windows games, though, and they both have a lot of bugs. If you have innoextract installed, registry entries will not be added at all. If you do not, they get broken by being moved. I would like to unify them into one system and make that one work well instead of fixing both now and then spending a lot of time on the new system too afterwards and losing those benefits.

sharkwouter avatar Sep 26 '22 19:09 sharkwouter

In some cases (Escape from Monkey Island, Die By the Sword) the game has a *.script file in its installed files, which appears to have some information related to registry keys. It's not consistent, however (as the other games I listed don't seem to include this file), and there's no reason to believe it contains all the registry information either; so the only way to actually have these values set is by running the GOG installer, unless you manage to extract, parse and simulate the entire installation process description out of it (if there is such a thing in the first place).

As for how much effort it would take to fix the original issue (in the Wine installer process), it's mostly a matter of creating a single symlink (prefix/dosdevices/d:) before running the installer, and changing said symlink afterwards.

…Here's a version of install function that works with all games I've listed in the issue description:

def extract_by_wine(game, installer, temp_dir):
    err_msg = ""
    # Set the prefix for Windows games
    prefix_dir = os.path.join(game.install_dir, "prefix")
    drive = os.path.join(prefix_dir, "dosdevices", "d:")
    if not os.path.exists(prefix_dir):
        os.makedirs(prefix_dir, mode=0o755)
        # Creating the prefix before modifying dosdevices
        _exe_cmd(["env", "WINEPREFIX={}".format(prefix_dir), "wine", "start", "/B", "cmd", "/C", "exit"])
    if os.path.exists(drive):
        os.unlink(drive)
    os.symlink(temp_dir, drive)
    _dir = os.path.join(temp_dir, os.path.basename(game.install_dir))  # can't install to drive root
    # It's possible to set install dir as argument before installation
    command = ["env", "WINEPREFIX={}".format(prefix_dir), "wine", installer, "/dir={}".format(_dir), "/VERYSILENT"]
    stdout, stderr, exitcode = _exe_cmd(command)
    if exitcode not in [0]:
        err_msg = _("Wine extraction failed.")
    else:
        # Preparing files for the move
        for it in os.listdir(_dir):
            os.rename(os.path.join(_dir, it), os.path.join(temp_dir, it))
        os.rmdir(_dir)
        os.unlink(drive)
        os.symlink("../../..", drive)
    return err_msg

LeXofLeviafan avatar Sep 26 '22 23:09 LeXofLeviafan

Oh wow, that's easier than I thought it would be. Would you be willing to make a pull request?

sharkwouter avatar Sep 27 '22 07:09 sharkwouter

The implementation above has a few issues to be ironed out… in particular, if the game directory contains a file/folder matching by name the game folder itself, the moving part would fail (and trying to work around it directly won't work as the issue would be sorta recursive). I can think of a few ways to fix it but I'm not sure which one to pick as each has a problem to deal with:

  • Adding an extra folder with unlikely-to-be-used name inside _dir path, such as os.path.join(temp_dir, " ", …); this would likely eliminate the chance of name collision, but it might fail depending on filesystem (or fail outright – I've already tried doing it with a few names, and some of them were outright rejected by Wine while others prevented detection of D: by Wine somehow).
  • Renaming _dir to os.path.join(temp_dir, " ") (or something to the effect) right before starting to move files out of it; this avoids issues with Wine but still might fail depending on filesystem (or due to some other limitation).
  • Placing _dir outside of temp_dir; this would remove the possibility of a name collision, but I'm not sure where it could be reasonably placed (temp_dir + "_"?).
  • Same as above, but the moving process would be changed to deleting temp_dir and renaming _dir to temp_dir; this would make the process more straightforward, but it has the same problem as the previous option.

…Also there's probably a need for at least one try/catch block to be added here, since without it any error leads to the installation process getting stuck (as far as UI is concerned, anyway). Or possibly it should be put in the place where installation process is invoked so that any such issue would be handled similarly to Wine errors when running the game (i.e. by catching the error and showing it in a message)?

LeXofLeviafan avatar Sep 27 '22 09:09 LeXofLeviafan

That's a shame, we might just want to separate out the steps for a Windows install in it's own function then. That would make it much easier to make it work well. Then you can just do the installation and be done with it.

sharkwouter avatar Sep 27 '22 09:09 sharkwouter

…You mean like this?

def move_and_overwrite(game, temp_dir):
    # Copy the game files into the correct directory
    error_message = ""
    source_dir = (os.path.join(temp_dir, "data", "noarch") if game.platform == 'linux'    else
                  temp_dir                                 if shutil.which('innoextract') else
                  os.path.join(temp_dir, os.path.basename(game.install_dir)))
    target_dir = game.install_dir
    _mv(source_dir, target_dir)

    # Remove the temporary directory
    shutil.rmtree(temp_dir, ignore_errors=True)
    if game.platform == 'windows' and "unins000.exe" not in os.listdir(game.install_dir):
        open(os.path.join(game.install_dir, "unins000.exe"), 'w').close()
    return error_message

(Though I'd prefer turning innoextract check into an argument, both here and in the unpack function.)

LeXofLeviafan avatar Sep 27 '22 10:09 LeXofLeviafan