-
Notifications
You must be signed in to change notification settings - Fork 74
Fix many problems with run.sh #74
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
base: master
Are you sure you want to change the base?
Conversation
I can resubmit these as individual PRs if requested but I haven't tested that they would all merge cleanly if done that way. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can see this can be merged as is. I'll wait for someone that can verify it working on Linux first though.
Is this a regression compared to the current version? |
I believe the check that produces this error message is probably in the wrong location, but this would not be a regression. It is a continuation of previously bad behavior. It shouldn't be fixed to work but should have a better error messages and be checked in better locations. The script overwrites I'll work on another patch to address this behavior. |
0060443
to
991945d
Compare
New patches fixed the issue. Log
Edit: looks like EtG requires the libdoorstop bundled with BepInEx on Thunderstore, but with that version of libdoorstop the script causes a coredump |
To run this via Steam it appears that I need to set This does introduced a regression and is incompatible with the shared BepInEx package on Thunderstore because we can't set one executable name that works across all games. (EG: Valheim, Lethal League Blaze, etc) I understand if this is required before we launch, but can we add a check for the first argument to see if it's something along the lines of Immediately backing out when no executable name is defined in the script makes this entire part redundant:
Unless it's fine to have any string in the executable name? Feels wrong though. |
That section is checking if $1 is a file that exists and has the executable bit set in the filesystem permissions as per https://pubs.opengroup.org/onlinepubs/009604399/utilities/test.html. It is not checking that $1 has a value
Right now I rewrote it to use the executable_name set as the filename (ex game.x86_64) and either run that in the current directory, or use executable_name to determine which argument is the absolute path to the game executable as passed by Steam. It needs to know exactly which argument that is for the SteamLaunch section. So yes this is a regression if there is a package out there that supports multiple games.
Neither of these use a shared package. It looks like Lethal Company, Content Warning, REPO, Sailwind, Ultimate Chicken Horse and some others use a shared package that doesn't include the linux doorstop_libs or a launch script. But of those I've checked so far only Ultimate Chicken Horse has a native linux version on Steam. So I find it unlikely the regression would actually have any impact on linux native games that used to work. |
That's fair, although if the package is shared with any further communities then we'd need it to work, otherwise it becomes a generic BepInEx-BepInExPack package which only works for UCH on Linux. That then means subsequent games with native Linux support then end up requiring their own BepInEx package. We can also end up with situations where a game later gets native Linux support, so a shared package then ends up requiring native Linux support. |
As an option, the script could try finding a file or an argument with pattern But this can be unreliable if a game has something like Editor.x86_64 in it's directory, but that's better than entirely relying on |
I was thinking about this too. If I only used check for The launch script has always required executable_name to be set if launching outside of Steam. |
It's better to check for |
My thought here is not to try to find the executable on disk but only to find which argument passed by Steam is the executable. Let Steam figure out which executable to pick and so we don't need to handle edge cases like a game only having x86 binary running on x86_64 system. Or a possible future where Valve introduces a compatibility layer for running x86(_64) on aarch64. The launch script previously needed either executable_name to be set or $1 to be a path to a binary file with the executable permission set. I'm very hesitant to introduce an attempt to locate the binary file on disk as it seems very outside the scope of the original intentions and needs of the launch script. Sure it would be nice to do everything but if a user doing something more complicated than using Steam and a mod manager, they can probably read some instructions in the rare cases they might need to do some manual setup. |
8d46b84
to
245ad5e
Compare
I force pushed a new stack of commits so its cleaner to read. The SteamLaunch check now supports executable_name being empty and in that situation checks for *.x86_64 or *.x86. Updated overview of changes. Don't output to stdout when not about to exit because of an error. |
Did not touch It's the same thing that was happening at the beginning, it wants executable in the working directory instead of near the script file |
You need to pass a path not just a filename, for example |
Nope
Also tried it with EtG before, didn't work. But it did even without ./ when it was fixed the first time (probably because I have PATH=. in my shell config) |
if [ -x "$1" ] ; then
executable_name="$1"
shift
fi
if [ -z "${executable_name}" ] || [ ! -x "${executable_name}" ]; then
echo "Please set executable_name to a valid name in a text editor or as the first command line parameter"
exit 1
fi This checks if @Damglador Either your executable's don't have the executable bit or your attempts to run the script without changing into the directory first are causing the problems. |
From good news, I can confirm that it's working with the latest BepInEx release in Valheim in Steam. And also I think it breaks out of Steam runtime. Steam wrappers are passed where For context, arguments with
in Steam, from the script perspective look like this:
|
On the first line you check if
|
No, that should not be possible with the logic. I believe you are interpreting something incorrectly or seeing it the 2nd time the script is invoked later in the Steam Runtime and believing it has skipped the runtime. Set
This is extremely confusing as I don't know what doorstop.sh is or why you are passing an extra appid.
Absolutely incorrect. You are suggesting looking for the executable relative to the location of the script instead of the working directory. If you make this change you break storing mods in a separate directory with mod managers.
Again, Steam passed the executable as an absolute path. This is in your own log above as arg 12. Launching from r2modman invokes Steam which sets PWD to the game directory. All of your problems are because you are trying to execute the script with the wrong PWD. |
I see, I was indeed wrong on the runtime, it does show
Dirty example. Here's a simple script that just shows what is passed in
Added it to launch arguments for Valheim in Steam as
I guess these wrappers are not important?
Yeah, it is. Sorry for that. But there is one regression that caused my confusion. If I set |
The wrappers do two major things, the first is setup a baseline environment using the same shared libraries used by all users of Steam for Linux no matter what distro they are on. In the before times if a game dev compiled their game against the latest non LTS version of Ubuntu that had a new glibc, the game might not launch at all on other distros that had not yet upgraded to use that new version of glibc. Now game devs compile against the shared runtime and we all have it on our systems. The second is to inject the libraries for the Steam Overlay and some other Steam specific features. This is actually what causes more trouble for us because we need to use the same method to get doorstop loaded but need to avoid stepping on each others toes. I'm using a SteamDeck so I very much don't want to risk breaking the menus or Steam Input.
Ah ha. So I actually introduced that new behavior in one patch by moving the checks and then reverted it by not including it in this patchset. If you go back to mainline version it should have the same behavior as my current patchset. So this is not a regression in the PR. I guess maybe the check could be moved lower until after the path has been resolved. I need to think about this. |
The latest releases of BepInExPack_Valheim are using UnityDoorstop 4.4.0 with a start_game_bepinex.sh script that includes the current changes from this PR. A small handful of users on Linux have told me that they successfully used script to launch Valheim with mods. I have heard of zero problems so far related to the run script. |
I can also confirm that the latest BepInExPack_Valheim release has made things work without the user having to do any additional modification. |
Don't output to stdout when not about to exit because of an error.
Make BASEDIR usable earlier such as in the config section.
Update the SteamLaunch check with a check that actually works currently and will hopefully be resilient to future changes in Steam.
Preserve spaces in passed arguments so it will not break on games installed to a path with spaces.