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

Use ShellExecute when not launching .exes #3518

Closed

Conversation

Joshua-Ashton
Copy link
Contributor

This behaviour more closely matches what Steam on Windows does when
launching things that aren't executables.

I tested this with some wacky edge-cases on my own appid (launching image files, random blah://frog links etc.) both on Windows and via. Proton this appears to mirror the behaviour fairly well.

This behaviour more closely matches what Steam on Windows does when
launching things that aren't executables.
@GloriousEggroll
Copy link
Contributor

GloriousEggroll commented Feb 12, 2020

Just some feedback on this - Doom II and Ultimate Doom both launch, clustertruck launches. First launch progress bars work. zach-like does not launch still.

note: zach-like fails even on normal proton. it opens a small black console box then closes.

@Joshua-Ashton
Copy link
Contributor Author

This patch is not meant to address the Zach-Like issue, that is entirely separate.

I will have a look into fixing that tomorrow 😄 🐸

@GloriousEggroll
Copy link
Contributor

zach-like was brought up in the previous bug, that's the only reason I brought it up. seems it has a different issue.

Tk-Glitch added a commit to Tk-Glitch/PKGBUILDS that referenced this pull request Feb 24, 2020
…hes from Josh (ValveSoftware/Proton#3518) and Derek (ValveSoftware/Proton#3568) by default.

Those are necessary to get SW Jedi Fallen Order to run through Steam+Origin
@aeikum
Copy link
Collaborator

aeikum commented Feb 27, 2020

@Joshua-Ashton Thanks. Instead of trying to parse the command, can we use the result from SHGetFileInfo to determine whether to use ShellExecute or CreateProcess? E.g. if non-zero (i.e. a PE file or COM file), use CreateProcess; else, use ShellExecute. I tested this out with Origin and Jedi Fallen Order and it seems to work fine.

For non-game processes, is it important that Steam.exe stick around? Can we just exit immediately in the ShellExecute case instead of introducing another call to __wine_make_process_system?

I can make all these changes, just want to run them by you. If they sound good I'll share my changes with you for review.

@aeikum
Copy link
Collaborator

aeikum commented Feb 27, 2020

@Joshua-Ashton Here's something you could review: aeikum@d43dd89

@Joshua-Ashton
Copy link
Contributor Author

Joshua-Ashton commented Feb 27, 2020

So I did some extra debugging of this issue:

When we call CreateProcessW, we should always pass in the flag CREATE_UNICODE_ENVIRONMENT.

SHGetFileInfoW is never called, so I don't think this is a good determiner of things. I think we should stick as close to what native Steam does as possible.

steamclient.dll does end up testing if the application name isn't NULL [do we need to add this check?] and ends in .exe, like I do in my patch:

if ( firstArg && !V_StringFindSuffix(firstArg, ".exe") )

If it ends in .exe then we go on the CreateProcessW path like this:

   DWORD dwCreationFlags = SomeFunctionArgThatIsZeroWhenCalled | CREATE_UNICODE_ENVIRONMENT;
   
   // Something about starting with SUSPEND for game overlay crap.
   
    if ( CreateProcessW(
           lpApplicationName,
           lpCommandLine,
           NULL,
           NULL,
           FALSE,
           dwCreationFlags,
           lpEnvironment,
           lpCurrentDirectory,
           &StartupInfo,
           &ProcessInformation) )
       ...

and if it doesn't then it attempts to create steam_appid.txt (and if that fails, it will Assert) and then calls through to some helper which ends up calling either ShellExecuteExW via tier0_s.dll [This seems to have changed from doing ShellExecuteA with the open verb via vgui2.dll since I last looked at it... Unless I frogged something up the first time.] I will look at that.

For non-game processes, is it important that Steam.exe stick around?

I'd imagine it would be given some titles on Steam end up calling their own proprietary launchers like myfroglauncher://rungame//BigFrogsLatestGreatestReturn2, which may have some shitty DRM checks with linking that could check if Steam.exe is alive.
I don't think any that depend on that exist right now, but I am just thinking ahead 🐸

@aeikum
Copy link
Collaborator

aeikum commented Feb 27, 2020

Sure, we can add that flag in a separate commit.

We're currently using SHGetFileInfoW to determine if the application is a console application, and pass in the CREATE_NEW_CONSOLE flag so the application will run in a console. That's what the ZACH-LIKE program uses. Since we're already using this function it seemed straightforward to use it for both purposes. (Also I really dislike string parsing; it's error prone and we already do a lot.)

I'd imagine it would be given some titles on Steam end up calling their own proprietary launchers l

Steam launches the installscript stuff without the SteamGameId variable set; I call these "non-game processes". When it gets around to the game's launch command, it does have that variable set, so this is the "game process." Whether it's actually the Game.exe or not isn't important. My point was more that we don't want a bunch of Steam.exe processes hanging around. We should have just one, which is the one we start alongside the "game process".

@Joshua-Ashton
Copy link
Contributor Author

Joshua-Ashton commented Feb 27, 2020

There is no logic in Steam to pass in CREATE_NEW_CONSOLE as a flag so the problem is elsewhere:

When I start ZACH-LIKE on Windows I get CREATE_UNICODE_ENVIRONMENT | CREATE_SUSPENDED. (latter for the weird game overlay injection stuff that we don't need.)

The real reason ZACH-LIKE fails is that the Steam helper exe is built for the Console subsystem (this defaults to -mconsole), not Windows. (-mwindows.)
[ie. it needs to be a GUI app for the process to spawn its own console automagically without that flag]

@Joshua-Ashton
Copy link
Contributor Author

My point was more that we don't want a bunch of Steam.exe processes hanging around.

My reasoning was more for titles that don't use the Steam API or interface with Steam at all expecting Steam.exe to be present after the ShellExecute (otherwise we are already dead, unless I am mistaken in that case?)

@aeikum aeikum added this to the Next Release milestone Mar 4, 2020
@aeikum
Copy link
Collaborator

aeikum commented Mar 11, 2020

This was merged, somewhat modified, as 68f982b.

@aeikum aeikum closed this Mar 11, 2020
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.

None yet

3 participants