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.

@ivyl ivyl force-pushed the experimental_7.0 branch from b066633 to c9d56d1 Compare March 22, 2023 21:24
@alice945 alice945 force-pushed the patch-1 branch 2 times, most recently from eeb91d4 to c32e921 Compare July 29, 2023 00:48
@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