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 instead of CreateProcess to launch wine processes in the steam helper. #3239

Closed
wants to merge 1 commit into from

Conversation

Guy1524
Copy link
Contributor

@Guy1524 Guy1524 commented Nov 20, 2019

This more closely matches what windows steam does, and is needed due to games such as Star Wars Jedi: Fallen Order starting via a protocol handler, which CreateProcess doesn't know what to do with.

Note: I didn't test this with an official proton build, but rather proton-tkg, which uses a rewritten build system, so I'm not sure if using the C++ STL will work here.

Signed-off-by: Derek Lesho <dlesho@codeweavers.com>
@kisak-valve kisak-valve added the cw label Nov 20, 2019
@aeikum
Copy link
Collaborator

aeikum commented Nov 21, 2019

Thanks, will take a look soon.

Some games that are sensitive to how we start the process are Ultimate Doom (2280) and Doom II (2230). Those use BAT files with special characters. And ZACH-LIKE (1098840) which is actually a winconsole game (see d73b927). I think this one fails for other reasons, but at least opens a console now or something.

Anyway, worth checking that those are all still working as they do now.

@Guy1524
Copy link
Contributor Author

Guy1524 commented Nov 21, 2019

I just tested those three games. Doom II and Ultimate Doom both work, but I wasn't able to get ZACH-LIKE working on either Proton 4.11-8 or my newer build. It seems to fail due to some unrelated dotnet error.

@GloriousEggroll
Copy link
Contributor

GloriousEggroll commented Dec 1, 2019

I went to test this using the default build environment (vagrant) and was hitting:

In file included from .././obj-tools32/include/wine/windows/objbase.h:258:0,
                 from .././obj-tools32/include/wine/windows/ole2.h:25,
                 from .././obj-tools32/include/wine/windows/wtypes.h:13,
                 from .././obj-tools32/include/wine/windows/winscard.h:22,
                 from .././obj-tools32/include/wine/windows/windows.h:70,
                 from .././syn-steam/steam/steam.cpp:35:
.././obj-tools32/include/wine/windows/objidl.h:6100:15: warning: 'union _userSTGMEDIUM::<anonymous struct>::__WIDL_objidl_generated_name_0000000C' invalid; an anonymous struct can only have non-static data members [-fpermissive]
         union __WIDL_objidl_generated_name_0000000C {
               ^
.././syn-steam/steam/steam.cpp: In function 'void* run_process()':
.././syn-steam/steam/steam.cpp:187:23: error: 'nullptr' was not declared in this scope
             cmdline = nullptr;
                       ^
winegcc: g++ failed
make: *** [Makefile:90: steam.o] Error 2

nullptr was introduced in c++11
https://en.cppreference.com/w/cpp/language/nullptr

according to:
https://stackoverflow.com/questions/10033373/c-error-nullptr-was-not-declared-in-this-scope-in-eclipse-ide

This is resolved by adding -std=gnu++0x to the build flags. Although -std=c++11 also works, -std=gnu++0x covers a wider range of extensions per the stackoverflow article:

@@ -800,7 +799,7 @@ steam_configure: $(STEAMEXE_CONFIGURE_FILES)
 
 steam: SHELL = $(CONTAINER_SHELL32)
 steam: $(STEAMEXE_CONFIGURE_FILES) | $(WINE_BUILDTOOLS32) $(filter $(MAKECMDGOALS),wine64 wine32 wine)
-    +env PATH="$(abspath $(TOOLS_DIR32))/bin:$(PATH)" LDFLAGS="-m32 -fpermissive" CXXFLAGS="-m32 -fpermissive -Wno-attributes $(COMMON_FLAGS) -g" CFLAGS="-m32 -fpermissive $(COMMON_FLAGS) -g" \
+    +env PATH="$(abspath $(TOOLS_DIR32))/bin:$(PATH)" LDFLAGS="-m32 -fpermissive" CXXFLAGS="-m32 -std=gnu++0x -fpermissive -Wno-attributes $(COMMON_FLAGS) -g" CFLAGS="-m32 -fpermissive $(COMMON_FLAGS) -g" \
         $(MAKE) -C $(STEAMEXE_OBJ)
     [ x"$(STRIP)" = x ] || $(STRIP) $(STEAMEXE_OBJ)/steam.exe.so
     mkdir -pv $(DST_DIR)/lib/wine/

Can also confirm Ultimate Doom and Doom II Launched just fine.

@aeikum
Copy link
Collaborator

aeikum commented Dec 2, 2019

Thanks @GloriousEggroll.

@Guy1524 this looks OK, but can you split it up into two commits? One to switch to using std::wstring and another to change to using ShellExecute. (Or just keep using the C API; either is fine).

@Guy1524
Copy link
Contributor Author

Guy1524 commented Jan 4, 2020

Sorry I didn't see your earlier message. @GloriousEggroll has reported an additional with this PR to me, and I'll split up the commits before investigating.

@GloriousEggroll
Copy link
Contributor

GloriousEggroll commented Jan 4, 2020

Further testing on this currently reveals some games do get launched incorrectly and fail (clustertruck), it also prevents status bars from showing up when preparing to launch a game for the first time.

Clustertruck issue:
fnf

@Guy1524
Copy link
Contributor Author

Guy1524 commented Feb 12, 2020

See #3518

@Guy1524 Guy1524 closed this Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants