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

[UX/Fix] Respect experimental UMU support disabled #4191

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

arielj
Copy link
Collaborator

@arielj arielj commented Dec 21, 2024

A few days ago I made this PR #4175 making UMU the default when running games with Proton.

There's a problem with that PR: if a game was running with Proton and UMU disabled, updating heroic will make the game run with UMU incorrectly (affecting its prefix irreversibly, maybe breaking it if the game is incompatible with UMU).

To avoid that, we have to also check if UMU was not used before for games that are already installed.

So: for new games, the disableUMU setting will be false by default or true if changed by the user, so it will ignore the old experimental feature value. For games installed before this change, the disableUMU setting will be undefined so we check the experimental feature value.

How to test:

  • revert some commit to have the experimental UMU and disable it

  • install a game and run it once using proton without the experimental feature

  • update to this commit

  • run the game again

  • it should NOT use UMU (even though the Disable UMU check is disabled in the advanced tab)

  • revert some commit to have the experimental UMU and enable it

  • install a game and run it once using proton with the experimental feature

  • update to this commit

  • run the game again

  • it should use UMU


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)

@arielj arielj added the pr:ready-for-review Feature-complete, ready for the grind! :P label Dec 21, 2024
@arielj arielj requested review from Etaash-mathamsetty, a team, flavioislima, CommandMC, Nocccer and imLinguin and removed request for a team December 21, 2024 02:57
@CommandMC
Copy link
Collaborator

Are there actually cases where games break if umu is applied after the prefix is already created? I understand not using umu on installed games would be the safer bet, but do we really expect the average user to go out and manually enable umu for all of their existing games?

In my mind, we'll eventually remove the "Disable umu" checkbox completely, and make it the only valid option of running Proton (as it's the only wrapper running it like Steam does). So we'll have to bite the "Upgrading old prefixes" bullet at some point

@arielj
Copy link
Collaborator Author

arielj commented Jan 2, 2025

users won't have to manually enable UMU in all cases, it will be enabled by default for all new games and if the users where already using the experimental feature

but at least for now we need the Disable UMU checkbox because some games don't work with UMU at all (there are some cases in discord, Ghost Master is the last one I remember)

I don't know exactly what happens if you enable UMU for a proton prefix that was not created with UMU before, but I know some games don't work with UMU so we don't want to enable UMU for those

Maybe it doesn't break the prefix and a user can then go and Disable UMU if needed (they'll have to know that's what broke their game), but I can't confirm the prefix doesn't change in a way that keeps the game broken after disabling UMU, but knowing some games don't work with UMU I'd rather not risk it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants