Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Install dxvk and vkd3d on new prefixes if enabled #2873

Merged
merged 8 commits into from
Sep 13, 2023

Conversation

Etaash-mathamsetty
Copy link
Member

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@CommandMC
Copy link
Collaborator

Do we consider DXVK/VKD3D to be part of a "complete prefix"? As in, should we consider a prefix to be unusable if those tools aren't installed while they're enabled in the settings?

  • If we do, I'd say this should happen in verifyWinePrefix instead
  • If we don't, runWineCommand shouldn't do this at all & the caller should instead decide when it wants to take care of these operations

I'm conflicted here, although I'd favor doing it in verifyWinePrefix

@Etaash-mathamsetty
Copy link
Member Author

Etaash-mathamsetty commented Jul 21, 2023

Do we consider DXVK/VKD3D to be part of a "complete prefix"? As in, should we consider a prefix to be unusable if those tools aren't installed while they're enabled in the settings?

* If we do, I'd say this should happen in `verifyWinePrefix` instead

* If we don't, `runWineCommand` shouldn't do this at all & the caller should instead decide when it wants to take care of these operations

I'm conflicted here, although I'd favor doing it in verifyWinePrefix

  1. we can't do it in verifyWinePrefix because we should not need the gameSettings just to verify the wine prefix
  2. The caller cannot do it because that would cause lots of code duplication and the prefix might not exist yet in many places.
  3. This is supposed to be a simple bug fix so that I don't have to keep turning on and off dxvk every time I download a game and try to run it, it's just one click play now :)

@CommandMC
Copy link
Collaborator

verifyWinePrefix already takes GameSettings, it just happens to not look at anything but the Wine version & prefix. If you do want to force DXVK/VKD3D to be installed if they're enabled, installing them in the function preparing the prefix would make more sense to me (rather than the function that "just" runs a given Wine command)

On (3):
DXVK and VKD3D are already auto-installed on launch for all games that are ran with Wine (using one function for all storefronts), see

// If DXVK/VKD3D installation is enabled, install it
if (gameSettings.wineVersion.type === 'wine') {
if (gameSettings.autoInstallDxvk) {
await DXVK.installRemove(gameSettings, 'dxvk', 'backup')
}
if (gameSettings.autoInstallVkd3d) {
await DXVK.installRemove(gameSettings, 'vkd3d', 'backup')
}
}

Testing this locally, it works exactly as designed (it always tries to install those components, even if they're already installed, but that's fine)

@Etaash-mathamsetty
Copy link
Member Author

seems like prepareWineLaunch wasn't run for sideloads

src/backend/main.ts Outdated Show resolved Hide resolved
@flavioislima flavioislima merged commit 1d3f80e into Heroic-Games-Launcher:main Sep 13, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants