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

proton: Fix a couple simple crashes #6633

Open
wants to merge 1 commit into
base: proton_8.0
Choose a base branch
from

Conversation

alice945
Copy link

Fixes a crash when no verb is provided due to accessing a nonexistent index in sys.argv.
Allows the STEAM_COMPAT_CLIENT_INSTALL_PATH environment variable to not be set.

@alice945 alice945 changed the base branch from experimental_7.0 to proton_8.0 July 29, 2023 00:57
@ivyl
Copy link
Collaborator

ivyl commented Dec 13, 2023

I assume this is an attempt to make Proton usable outside of Steam? I think it's a good thing overall. IMO we should reorganize code a bit:

  1. Don't fail if environment variables / verb are not provided.
  2. Print a help / refer to README.
  3. The current verbs should still encode assumptions about the environment and fail in an obvious way.
  4. Introduce new verbs that allow you to run outside of the Steam Client.

@@ -792,7 +792,7 @@ class CompatData:
os.symlink("/", self.prefix_dir + "/dosdevices/z:")

# collect configuration info
steamdir = os.environ["STEAM_COMPAT_CLIENT_INSTALL_PATH"]
steamdir = os.environ.get("STEAM_COMPAT_CLIENT_INSTALL_PATH", "")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We depend on Steam setting up this variable and using [] will make this fail in a clear way if this ever regresses in Steam.

By doing .get() the way you do here we would silence this but also copying of the steam DLLs would silently fail.

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.

2 participants