search

sonic2kk / steamtinkerlaunch

Linux wrapper tool for use with the Steam client for custom launch options and 3rd party programs
GNU General Public License v3.0
2.04k stars 70 forks source link

Unable to pass windows paths to custom commands #1072

Closed zany130 closed 3 months ago

zany130 commented 3 months ago

System Information

Issue Description

Sometimes you may need to pass a windows path(meaning using \ instead of /) to a custom command (For example to load Persona 3 reloaded with mods from Reloaded II you would use --launch "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe)

however STL seems to store these paths incorrectly (For example CUSTOMCMD_ARGS="--launch "Z:homezany130.localshareSteamsteamappsmmonP3RP3RBinariesWin64P3R.exe"" in my case)

Logs

31337.log

sonic2kk commented 3 months ago

What about a double backslash? I'll check when I'm back on my PC as well.

For example, Z:\\home\\zany130\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe

sonic2kk commented 3 months ago

I don't know about this specific case, but sometimes Wine can handle forward-slashes (ModOrganizer 2 I think has some / paths). Might be worth trying that and seeing how it goes too.

sonic2kk commented 3 months ago

Double backslash appears to work for me, but has this problem:

  1. Set custom command as --launch "Z:\\your\\custom\\path"
  2. Save (not "Save & Play")
  3. When menu re-opens, path will be --launch Z:\your\custom\path a. Note the striped quotes. I'm not sure how we handle paths that have spaces in this way. For example if your path was Z:\\run\\media\\gaben\\Games Drive\\steamapps\\common\\Half-Life 3\\InYourDreams.exe, would it interpret this path properly? I guess that depends on how we pass the arguments. If we try to split it up into an array, this is probably bad news as it would break everything up with a space. b. This quote stripping is a general problem with how Yad handles inputs, it happens even with a simple Yad project. You can't even escape them. There's no solution I'm aware of to get around this for Yad. c. For GameScope, some flags can take paths (such as the cursor image). There is a crazy workaround in place for flags that take paths. It basically tries to infer when a space is part of a path and when it denotes the beginning of a new flag. This might work for this specific use-case, but I'm not sure if it's worthwhile investigating implementing generally. I have no idea what kinds of arguments custom commands may take, and it may have unintended side-effects if no paths are provided. We may end up incorrectly breaking up custom commands, or we may not. I have no idea.
  4. If you save again, the backslashes will be stripped, because they are not "re-escaped".

Essentially when you save you go from Z:\\your\\custom\\path" -> Z:\your\custom\path -> Z:yourcustompath. This means we have two issues we may have to deal with:

  1. Replace all occurances of single \ in CUSTOMCMD_ARGS with \\. I wonder if we can use parameter expansion to do this when we pass the variable to Yad... We already do a bit of parameter expansion but maybe we could do some whacky chaining in a $().
  2. Deal with paths which have spaces, if these don't work. For your example, if the following path does not work, we'll need to think of a solution: Z:\run\media\zany130\Spaced Path Name\Oh No This Is Probably Broken\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe - From looking at the code I'm pretty sure custom commands are solely split up based on spaces, so each space is a different argument (meaning it would only pass Z:\run\media\zany130\Spaced as the path to the custom command...) a. Escaping the spaces probably doesn't work because of how mapfile breaks at the space, but it might be worth trying. In a quick local test in a Bash prompt it didn't work though.

In the short-term though for a workaround, double-escaping each time you go to the Game Menu should work. Alternatively, forward-slashes may also work, it just depends on each program I think. It might be worth testing Proton 9.0 Beta, Proton Experimental, GE-Proton9, or anything else based on Wine 9 just to see.

In the longer-term, we have some considerations about how better to handle custom command paths, such as those with spaces.

zany130 commented 3 months ago

What about a double backslash? I'll check when I'm back on my PC as well.

For example, Z:\\home\\zany130\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe

yup that writes out the path correctly like you said above. though I having trouble getting it to atchually run , but don't know if that's just with reloded II....

sonic2kk commented 3 months ago

Ah, there's a missing backslash in my example after \.local. It should be Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe

If this still doesn't work, you can try pasting it manually into the game config file, and loading without touching the Game Menu, so STL will read it directly from the config file and there should be no risk of Yad mangling it. Hopefully if I can figure out some parameter expansion/sed to replace single backslashes, Yad will hopefully not mangle the path anymore, so this should be temporary and just for testing. I'm sure there's some sed -E regex for it if all else fails, but as they say, once you start using regex to solve a problem, you now have two problems. :sweat_smile:

If this still doesn't work, you can try quadruple-escapes! Same again, just paste this into your game config and launch without touching the Game Menu: Z:\\\\home\\\\zany130\\\\.local\\\\share\\\\Steam\\\\steamapps\\\\common\\\\P3R\\\\P3R\\\\Binaries\\\\Win64\\\\P3R.exe.

If both of these still don't work, you can once again try these in your config file, once again avoiding the Game Menu (Yad will always mangle these because it hates quotes for some reason):

If none of these work, it will be very sad times. Perhaps if they all fail you can try manually from terminal with Wine and see if passing the same command fails, that would help narrow down if it's an STL problem or not. I am foolishly optimistic that one of these will yield a result though :wink:

sonic2kk commented 3 months ago

Ah interesting, it seems like using printf to feed the option to Yad, correctly preserves the backslashes.

Because this is a bug, I created a new branch to test the fix. For some reason, using printf prevents the backslash mangling. No idea why...

You can check out the branch here and see if it fixes your backslash woes over at customcmd-args-backslash-fix. Before testing, remove /dev/shm/steamtinkerlaunch. When fiddling with any UI fixes it's always good to remove that, and I forgot about that until now when I was banging my head against the wall :sweat_smile:

I don't know if Reloaded II will work, but this should at least fix the issue of the paths going from Z:\this\is\text -> Z:\thisistext. Or at least in my testing it has :-)

This won't fix paths with spaces, though, that one's another beast...

zany130 commented 3 months ago

Oh cool Ill give that a try when I get the chance rn I'm trying to troubleshoot why nothing loads when I try to run Reloaded II with P3R exe as a arg.

for testing purposes I was able to get this to work as a separate steam shortcut by adding Reloaded II as a custom steam game then in the launch options adding 

STEAM_COMPAT_DATA_PATH=/home/zany130/.steam/steam/steamapps/compatdata/2161700/ %command% --launch "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"

this launches P3R with mods but I can't get this to work on the P3R steam entry with STL for some reason...

BTW: I tried using forward slashes in the above steam launch option and it didn't work it need to be backslashes apparently

looking at my log I see its running 

'/home/zany130/mods/Persona 3 Reload/Release/Reloaded-II.exe' '--launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe'

2161700.log

sonic2kk commented 3 months ago

If you use quotes for the STL option, in the game config, does that work?

If not, if you change your non-STL command to these, does it work?

  1. STEAM_COMPAT_DATA_PATH=/home/zany130/.steam/steam/steamapps/compatdata/2161700/ %command% "--launch" "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"
  2. STEAM_COMPAT_DATA_PATH=/home/zany130/.steam/steam/steamapps/compatdata/2161700/ %command% "--launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"
  3. STEAM_COMPAT_DATA_PATH=/home/zany130/.steam/steam/steamapps/compatdata/2161700/ %command% --launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe

This will help work out the following:

  1. STL is hopefully passing the command line this, with each space breaking things up to separate "arguments" so to speak
  2. STL may be passing the entire argument string as one single string, which it probably shouldn't be as this may break things.
  3. STL is stripping the quotes from the custom command, because Yad things, so I am wondering if Reloaded II works here without quotes. That'll narrow down if the lack of quotes is a potential issue

My guess is that 1 and 3 will work though.

sonic2kk commented 3 months ago

Did a bit of hardcoded logging in extProtonRun.

With this I can see each element in the RUNPROGARGS array. I'm not using Reloaded II, just P3R with some nonsense argument for testing :-) I can see STL is splitting the arguments as I expected in the first example above: Fri 22 Mar 03:32:09 GMT 2024 INFO - extProtonRun - RUNPROGARGS values are:'--launch' 'Z:\not\real\path'- So here, the first element is"--launch"and the second element is"Z:\not\real\path"`. Which is, usually, the correct way to pass paths to a launch command array (putting them as one string or with an empty string between can cause crashes).

I'm not sure why Reloaded II would be having troubles picking up the path. I may have to get it set up over the weekend for troubleshooting if we can't figure it out!

zany130 commented 3 months ago

hmm, intresting when adding those commands to my R2 steam shortcut (that's what I'm going to call the mod loader now lol) the following happens

  1. gets stuck launching the command (which is the same behavior I am getting in STL in fact looking at my log it doesn't even look complete ie didn't finish "launching the game")
  2. Immediately crashes so I guess R2 doesn't like that
  3. Immeditly crashes ( my guess is R2 is specifically looking for 
    --launch "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"

    quotes and everything because that's how it knows what mod profile to use (because that mod profile has "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe" set as its "game") when I pass 

    --launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe

    or 

"--launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"

.... I'm tired I don't want to see another f*ck \ ` or any other weird character 😅

sonic2kk commented 3 months ago

So if it's looking specifically for a mod profile with quotes in the name, does editing the ~/.config/steamtinkerlaunch/gamecfgs/title/P3R.conf file (I think that's the path to the config file) to have CUSTOMCMD_ARGS as the following work?

CUSTOMCMD_ARGS="--launch \"Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe\""

Then try launching the game without touching the game menu (because Yad will mangle it), that is, just let the wait requester time out when you launch P3R. That should hopefully then show in your log that the launch args are --launch "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe" (without the backslashes, because Bash should escape it properly when reading the config).

I'm tired I don't want to see another f*ck \ ` or any other weird character 😅

Mood, I will probably be heading to bed shortly and will pick this up tomorrow (probably gonna explore R2 myself because I am quite interested!))

Parsing with Yad is fun :woozy_face:

zany130 commented 3 months ago

launching the game without touching the game menu (because Yad will mangle it)

Good idea don't want to add yad shanagiens into the mix lol

Mood, I will probably be heading to bed shortly and will pick this up tomorrow (probably gonna explore R2 myself because I am quite interested!))

same lol. this prob isn't really even a stl bug per se (well other than yad stuff) so no worries if you want to prioritize something else.

I prob be trying this a bit on my end to just cuz I'm ocd like that (prob would just be easier to use the r2 shortcut and then add stl as a compat tool for that or something)

anyway if you do want to check r2 out I recommend using this guide https://gamebanana.com/tuts/17166 its written for the deck but can be adapted to desktop Linux fairly easily

sonic2kk commented 3 months ago

The escaped quotes (\") in the config didn't seem to log properly on my end. I'm not using R2 but just if you're curious andwant to try some more:

  1. CUSTOMCMD_ARGS="--launch \"Z:\\double\\escaped\\path\""
  2. CUSTOMCMD_ARGS="--launch "Z:\this\is\an\unescaped\abomination""

anyway if you do want to check r2 out I recommend using this guide

This should be handy. Thanks!

Let me know if you find anything out in the meantime. I'll keep this open as it is interesting, I wonder what is going wrong with the path being sent to R2.

sonic2kk commented 3 months ago

Random thought before falling asleep:

What if you set the following as your launch options for Persona 3 Reload (not R2), still using STL as the compatibility tool: --launch "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe" - This is part of the working launch option for R2.

After doing this, go to the STL main menu and then check your log to see if it logs anything interesting for incoming commands from Steam. Likewise it might be interesting to check the Game Menu as this may be read in and displayed somewhere (although the Game Menu may mangle quotes).

The purpose of this is: STL I believe has a facility to read launch commands coming in from Steam. So, this might give us some insight into what Steam "sees".

Then again, this might be a sleep-deprived nonsense idea. Only one way to find out :-) I would experiment but my PC is off now.

zany130 commented 3 months ago

What if you set the following as your launch options for Persona 3 Reload (not R2), still using STL as the compatibility tool: --launch "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe" - This is part of the working launch option for R2.

EDIT: to answer your question it appears like this

Fri Mar 22 08:38:44 PM EDT 2024 INFO - main - Checking command line: incoming arguments 'waitforexitandrun /home/zany130/.local/share/Steam/steamapps/common/P3R/P3R/Binaries/Win64/P3R.exe --launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe'

so no " even though I did have them around the path part in the steam launch command

hmm that gave me an idea to run my R2 shortcut in steamtinkerlaunch and see what happens... well it crashes but the log is somewhat interesting

steamtinkerlaunch.log

in that it seems STL is receiving the arguments mangled as you can see here

'waitforexitandrun /home/zany130/mods/Persona 3 Reload/Release/Reloaded-II.exe --launch Z:homezany130.localshareSteamsteamappscommonP3RP3RBinariesWin64P3R.exe'
sonic2kk commented 3 months ago

Huh now that is interesting, because coincidentally, I just tried as well. I tried with regular P3R (don't have R2 installed) but the principle should be the same, even though the launch options of course won't do anything here, it's just to see what they look like.

For P3R I set the launch command as exactly this: %command% --launch "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe" (even though this path ofc doesn't exist for me heh)

And in SteamTinkerLaunch, I can see:

So, I tried a Non-Steam Game just to see if the results were different, and they were not.

I did a Ctrl+F for --launch to see if the paths get mangled elsewhere for me and the backslashes disappear, just in case for some reason for me they were passed correctly but for messed up later on. But all occurances of the incoming command line options from Steam were correct.

If the launch option is given without quotes, such as --launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe, then the backslashes will be removed. Maybe this is the problem. The log then looks like this: --launch Z:homezany130.localshareSteamsteamappscommonP3RP3RBinariesWin64P3R.exe

As an aside: There is a part of the SteamTinkerLaunch Game Menu UI that I thought was supposed to display the incoming launch options from Steam, that is, the ones we're passing here. I thought this was the "Game command arguments (Steam Launch Option) (SLOARGS)" item, but this is hardcoded to "none". I wanted to use this to try and see how the options were being passed, but the log worked fine.

I am going to do some tests to see what Steam sees when it launches a game with launch options, to get some insight into any escaping it might do.

sonic2kk commented 3 months ago

Steam doesn't do any wrapping, its launch command is just this:

home/emma/.local/share/Steam/ubuntu12_32/reaper SteamLaunch AppId=2161700 -- /home/emma/.local/share/Steam/ubuntu12_32/steam-launch-wrapper -- '/home/emma/.local/share/Steam/compatibilitytools.d/SteamTinkerLaunch'/steamtinkerlaunch waitforexitandrun '/run/media/emma/Steiner/Games/steamapps/common/P3R/P3R/Binaries/Win64/P3R.exe' --launch "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"

Which is what, ideally, SteamTinkerLaunch would also pass. Let me see what it'll pass if I give some escaped quotes in the config file and launch without going to the game menu... If the launch command can use quotes and if that can resolve the problem, then probably we need a way to internally wrap paths from the Game Menu.

Or, as another simpler solutiion, an option to pass a game's launch options to custom commands. Then we don't have to worry about Yad destroying the string.

sonic2kk commented 3 months ago

Oh interesting also, on the Main Menu, the launch option path not displayed properly.

image

This could probably be fixed with printf instead of echo...

sonic2kk commented 3 months ago

Hmm, I just discovered printf has a %q option. If we use this when parsing the incoming game commandline options, it will display backslashes properly. But I don't know if this causes other issues when actually using them.

For now, maybe using this solely for display purposes is fine. It still won't display quotes for some reason, though. It will escape it internally, though: --launch\ Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\ Spaced\\Binaries\\Win64\\P3R.exe

sonic2kk commented 3 months ago

I'm going to look into this %q option some more. I will have to refactor a little how custom command arguments are passed but it shouldn't be too bad. Then, if things look promising from the log, I'll push a branch where the arguments array is built using printf "%q" and you can see if the custom command receives the launch option correctly.

The path will probably have to be entered with a double-escape on the Game Menu, pending a branch that fixes this. I'll give example data if I get anywhere with this investigation path, no worries :-)

zany130 commented 3 months ago

nice!!

I been trying to see exactly how steam sees the launch options for r2 shortcut. I got the following by adding PROTON_LOG=1 to the beginning of the launch commands so

PROTON_LOG=1 STEAM_COMPAT_DATA_PATH=/home/zany130/.steam/steam/steamapps/compatdata/2161700/ %command% --launch "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"

gets written out on the proton log as

Command: ['/home/zany130/mods/Persona 3 Reload/Release/Reloaded-II.exe', '--launch', 'Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe']

so that's what its supposed to look like when it gets launched correctly at least according to the proton log...

running steam in a terminal and then running the R2 shortcut also gives this

/bin/sh\0-c\0PROTON_LOG=1 STEAM_COMPAT_DATA_PATH=/home/zany130/.steam/steam/steamapps/compatdata/2161700/  /home/zany130/.local/share/Steam/ubuntu12_32/reaper SteamLaunch AppId=3829416151 -- /home/zany130/.local/share/Steam/ubuntu12_32/steam-launch-wrapper -- '/home/zany130/.local/share/Steam/steamapps/common/SteamLinuxRuntime_sniper'/_v2-entry-point --verb=waitforexitandrun -- '/home/zany130/.local/share/Steam/compatibilitytools.d/GE-Proton9-2'/proton waitforexitandrun  "/home/zany130/mods/Persona 3 Reload/Release/Reloaded-II.exe"  --launch "Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"\0
chdir "/home/zany130/mods/Persona 3 Reload/Release"

EDIT: interesting without the " around Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe

steam apparently passes 

Z:homezany130.localshareSteamsteamappscommonP3RP3RBinariesWin64P3R.exe

which is why I need the " however double escaping like this 

PROTON_LOG=1 STEAM_COMPAT_DATA_PATH=/home/zany130/.steam/steam/steamapps/compatdata/2161700/ %command% --launch Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe

actually works :eyes:

sonic2kk commented 3 months ago

So the double escaping is required! Very interesting! That probably won't work with spaces though.

Could you do the following?

  1. Set CUSTOMCMD_ARGS in your P3R config to CUSTOMCMD_ARGS="--launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"
  2. Run P3R with your R2 custom command, from this branch without going to the game menu: https://github.com/sonic2kk/steamtinkerlaunch/tree/extProtonRun-printf-q

With that branch, the custom command args passed looks like this: '--launch\ Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe which seems promising based on your above!

interesting without the " around [path] steam apparently passes [path without backslashes]

Yup, this is what I suspected was causing for me, the path had backslashes in my STL log but for you was removing backslashes. This confirms it then :-)

zany130 commented 3 months ago

I GOT IT WORKING quadrple escaping was the answer :eyes:

in the menu

this wrote out 

CUSTOMCMD_ARGS="--launch Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe"

to the config which I could of sworn I tried but it works!!!

sonic2kk commented 3 months ago

That is awesome news!

Can you try the branch here and see if it also works? Change your CUSTOMCMD_ARGS to only use single backslashes before testing: https://github.com/sonic2kk/steamtinkerlaunch/tree/extProtonRun-printf-q

The config file will only store single backslashes, but when generating the custom command argument array, backslashes should be double-escaped. This should do the same as what you're doing manually.

Also, the caveat to what you're doing is that going to the Game Menu and saving will result in the double-backslashes being removed, meaning they'd have to be re-entered each time. If the branch above works, then this won't be an issue :-)

Finally, if all of this works, then the final bit we need is this branch: https://github.com/sonic2kk/steamtinkerlaunch/tree/customcmd-args-backslash-fix (be sure to remove /dev/shm/steamtinkerlaunch after installing it).

This branch fixes the issue of the path becoming Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe -> Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe -> Z:homezany130.localshareSteamsteamappscommonP3RP3RBinariesWin64P3R.exe.

So with both of those branches together, assuming they work, we'll end up with this behaviour:

Which should allow you to get an out-of-box experience with R2 profile commands by just passing --launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe into the textbox on the Game Menu. Then when you save, the path will be preserved. And when you finally launch, it should be passed correctly to R2 and give you the same behaviour that you're doing now :-)

sonic2kk commented 3 months ago

I just realised that's kinda a lot and you probably want to get stuck into actually playing, but I hope it makes sense :sweat_smile: Both in terms of what the branches do and why I want them tested. The ideal is that both of these branches will allow what you're doing manually to "just work"

zany130 commented 3 months ago

Finally, if all of this works, then the final bit we need is this branch: customcmd-args-backslash-fix (be sure to remove /dev/shm/steamtinkerlaunch after installing it).

just tested this part seems to work fine (intrestingly it still mangles " but we already established we don't need (and prob don't want )" wrapping around the path here)

I just realised that's kinda a lot and you probably want to get stuck into actually playing, but I hope it makes sense 😅 Both in terms of what the branches do and why I want them tested. The ideal is that both of these branches will allow what you're doing manually to "just work"

no worries I want to see this through the whole way lol. I already know I'm prob not playing to much today (ah well so is the life of an eternal tinkerer :sweat_smile: )

sonic2kk commented 3 months ago

just tested this part seems to work fine

Hell yeah! So it can be merged then.

intrestingly it still mangles " but we already established we don't need

Yup, plus it looks like Proton doesn't take this anyway (https://github.com/sonic2kk/steamtinkerlaunch/issues/1072#issuecomment-2016276706) from the Proton log arguments you provided.

So assuming the branch that should contain the launch fix works, I think we are good! That branch should double-escape the single-backslashes in CUSTOMCMD_ARGS, so the first branch will correctly save the paths, and the second branch will then fix launching by double escaping the fixed strings from the first branch.

I already know I'm prob not playing to much today

If it's any consolation, your troubleshooting is super appreciated (and putting up with my rambly explanations :sweat_smile:)

sonic2kk commented 3 months ago

PRs are up for each branch:

If both branches work for you, I will merge both of these PRs tonight.

zany130 commented 3 months ago

If it's any consolation, your troubleshooting is super appreciated (and putting up with my rambly explanations 😅)

haha no worries it was your rambling that made me think "maybe I should look at how to command is supposed to look once its passed" lol

1074 works for me so that is all good I belive

1073 however only loads the mod manager without the game so looks like its not passing something correctly to R2 (but still better than before where it would just hang and do nothing)

my guess from comparing my log from the successful run and this one is its the extra \ after --launch

Fri Mar 22 10:27:29 PM EDT 2024 INFO - extProtonRun - Starting '/home/zany130/mods/Persona 3 Reload/Release/Reloaded-II.exe' with arguments '--launch\ Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe' regularly

EDIT for reference this log had CUSTOMCMD_ARGS set to "--launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"

not-working-STL-LOG

and this one had CUSTOMCMD_ARGS set to "--launch Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe"

steamtinkerlaunch.log

sonic2kk commented 3 months ago

Hmm! Good catch. Yeah, I could see this being a problem.

Maybe using %q is not the right path then. We may have to resort to something a little more "hacky" where we only use "%q" for paths... I'l investigate

sonic2kk commented 3 months ago

There's a workaround with sed (sed 's/\\\\/\\/g'), but in testing, I found that paths with spaces are totallly broken here, so I'm looking into a separate fix.

sonic2kk commented 3 months ago

Okay, dear god, I came up with a bit of an abomination to handle spaces, and in the end we can stick with printf "%q".

Basically, extProtonRun takes in a string representing the arguments to pass to custom commands as PROGARGS. For us here, this is CUSTOMCMD_ARGS.

We have to break this into an array of strings to pass it to an executable. This is equivalent to the Proton log's Commands block, where it has a Python list for each command given. For our example, this would mean we have a Bash array like this: ( "--launch" "Z:\\path\\to\\exe" ).

The problem is, we use mapfile to do this, and it splits on spaces. This is fine for everything except paths with spaces. mapfile sadly cannot handle escaped spaces. So if a path has spaces, you end up with an array like this: ( "--launch" "Z;\\path\\to" "a_spaced" "path\\location\\game.exe" ) - Every element in the array is split on a space. This is important to separate things like flags from paths, but becomes a problem when the paths themselves have spaces.

Steam doesn't have a problem with this, because it can handle quotes. Over here in STL, PROGARGS will never have quotes.

Even if you escape the paths yourself, mapfile will give you something like this: ( "--launch" "Z;\\path\\to\\" "a_spaced\\" "path\\location\\game.exe" )

So our problem is that paths with spaces are treated as separate arguments, meaning they can't be read.

I think I have come up with a solution for this though, with a couple of caveats.

If we assume PROGARGS has spaced but that they are escaped, we can always assume that when mapfile splits the string, a split path will end with a backslash. However, this will forbid paths from ending in a backslash! I think this is acceptable though.

So, we can use a for loop to check the current and track the previous elements to see if the previous element has a backslash. If the previous element ended in a backslash, we can update the current array element to be ${previous_element} ${current_element}, basically modifying the array to contain the joined strings.

This has the problem of the existing array that is being modified containing null strings, but we can workaround this by generating a new array based on this modified one, with null strings filtered out. We can do this in mapfile by printfing the array and using grep -v "^$", where "^$" pattern matches all empty elements, and -v inverts this to print everything that is not blank.

The reason we can't have paths ending in a backslash, is that anything after the path would be assumed to be part of the path, since the previous element ended in a backlash. For example, --launch Z:\\home\\gaben\\ RELEASE_HALF_LIFE_THREE=1 would result in the code seeing RELEASE_HALF_LIFE_THREE as part of the path. But leaving off the ending \\ would result in RELEASE_HALF_LIFE_THREE=1 correctly being added as a separate element to the arguments array.

The final caveat, and one that I would like to fix, is that this does not work for paths using forward slashes, so traditional unix paths will still be split.

Anyway, here's the code snippet I've been testing with. It's a bit of a Pepsi-fuelled sleep-deprived nightmare, so bear with me:

# Play around with this one, I've commented some other test strings
PROGARGS="--launch Z:\home\zany130 spaced\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"  # Expected length: 2
#PROGARGS="--launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe"  # Expected length: 2
# PROGARGS="--launch Z:\\home\\zany130\\ is\\ spaced\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe MYVAR=1"  # Expected length: 3

# Create initial args array
mapfile -d " " -t -O "${#RUNPROGARGS[@]}" RUNPROGARGS < <( printf "%q" "$PROGARGS" )

FINAL_RUNPROGARGS=()
for i in "${!RUNPROGARGS[@]}"; do
    # Remove trailing backslash, i.e. turn `--launch\` into `--launch`
    RUNPROGARGS[i]="${RUNPROGARGS[i]%\\}"

    # If the last seen element in the array ended with a backslash, assume
    # this is an incomplete path and join them
    #
    # This is not perfect as valid paths that just end with backslashes will not work,
    # but we can document this on the wiki
    #
    # i.e. "Z:\this\is\a\path\ MY_VAR=2" will not work, but "Z:\this\is\a\path MY_VAR=2" will work
    if [[ $LASTRUNPROGARG = *"\\" ]]; then
        # Remove 'i-1' (previous element), because 'i' (current element) will contain 'i-1'
        unset RUNPROGARGS[i-1]
        RUNPROGARGS[i]="${LASTRUNPROGARG} ${RUNPROGARGS[i]}"        
    fi
    LASTRUNPROGARG="${RUNPROGARGS[i]}"
done

# Generate new array with null strings removed.
mapfile -t -O "${#FINAL_RUNPROGARGS[@]}" FINAL_RUNPROGARGS < <( printf "%s\n" "${RUNPROGARGS[@]}" | grep -v "^$" )

# Print verification
echo "${FINAL_RUNPROGARGS[0]}"
echo "${FINAL_RUNPROGARGS[1]}"
# Print length to verify it is correct, i.e. a flag and a path with spaces should only have 2 elements
# since the path with spaces will only count as one element
echo "${#FINAL_RUNPROGARGS[@]}"

I'm going to push this up to the branch but I will probably only minimally test this against STL tonight and will give it another looking over tomorrow. Wow, that was a journey lol.

sonic2kk commented 3 months ago

Sorry, I totally missed your note that #1074 seems to work. It works for me too, so I'll merge it as-is. That means I can rebase #1073 on it which will make testing a little easier.

If issues come up we'll deal with them then :-)

On that note, #1073 has the changes pushed. In my tests, the launch command looks good: --launch Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe - It has the trailing \ in --launch\ stripped. Paths with spaces should work but if you can't test this don't worry about it too much.

With the newly pushed changes, hopefully #1073 now works for you. I've verified that the argument array should be correct, looking like this: ("--launch" "Z:\\home\\zany130\\.local\\share\\Steam\\steamapps\\common\\P3R\\P3R\\Binaries\\Win64\\P3R.exe") - Based on our understanding from all the discussion in this issue, that should match what Steam passes, and thus should work.

If that above use-case works for you, then awesome! I will focus after that on taking a look at fixing spaced paths for paths with forward slashes. I may not get anywhere and if not, and the rest of the PR works, I will just merge it as-is. There is already a TODO to note that paths with forward slashes don't work yet.

sonic2kk commented 3 months ago

Okay, #1073 has been rebased. It is now ready for testing whenever you're able (could just git pull extProtonRun-printf-q to pull the latest changes in as you probably already have the branch :-))

No major hurry on testing or anything, just letting you know the changes are ready now.

zany130 commented 3 months ago

Yup it boots with just setting CUSTOMCMD_ARGS to --launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe though on further testing it seems the path still sometimes gets mangled in the GUI.

it seems if I save CUSTOMCMD_ARGS="--launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe" directly into the ini and then load the game menu it is rendered correctly

If I hit save when it comes back, it still renders correctly (before a save would overwrite the good path with a mangled one).

However,if I make CUSTOMCMD_ARGS= (blank) and then save it. When I later add --launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe it gets mangled again

So it seems like the "fix" only prevents an already correctly saved path with \ from getting mangled.

Which is still an improvement :sweat_smile:

sonic2kk commented 3 months ago

Well this is still good news! The path saving issue is probably more related to #1074.

Did you try with CUSTOMCMD_ARGS="" (with quotes)? The config shouldn't ever have CUSTOMCMD_ARGS= (without quotes). I'll give it a try on my end, too.

sonic2kk commented 3 months ago

Hmm! Okay, I can reproduce the problem. Double-escaping on initial save will work, but when saving a path with single backslashes, it gets saved to the config like this: --launch Z:homezany130 estpath - My path was Z:\home\zany130\testpath. Note that the spacing in the mangled path is actually a tab character, which lines up with \t.

It seems something is wrong with saving elements which have backslashes still, but only the first time.

sonic2kk commented 3 months ago

So I have a suspicion that, wherever we're saving the updated elements into the config file, we're using echo instead of printf. Example:

CUSTOMCMD_ARGS="--launch Z:\home\zany130\testpath"
echo "${CUSTOMCMD_ARGS}"  # Prints Z:\home\zany130 estpath
printf "%s\n" "${CUSTOMCMD_ARGS}"  # Prints Z:\home\zany130\testpath

I'll have to figure out where the saving magic happens, but I think this is a safe assumption to make that this is the problem.

Changing to using printf if we're not already is probably safe to do, because no text options in the menus should be formatted in echo, i.e. we would never want strings with \t on the Game Menu to be escaped as a tab character. We want them saved as string literals.

EDIT: The part we're looking for is updateConfigEntry. This uses echo instead of printf for new entries, and should be a straightforward fix. The less-straightforward fix comes from updating existing entries (and re-activating commented-out entries) which uses sed. I'll need to experiment to see how sed treats escape sequences...

sonic2kk commented 3 months ago

We may be able to edit the sed command to be like this:

# Old command, expands escape sequences
sed -i "/^${CFGCAT}=/c$CFGCAT=\"$CFGVALUE\"" "$CFGFILE"

# Updated command, surrounds the variables in double quotes
# but keeps the sed syntax in single quotes.
sed -i '/^'"${CFGCAT}"'=/c\'"${CFGCAT}=\"$CFGVALUE\"" "$CFGFILE"

This is changing a VERY core part of STL, though, and will need tested thoroughly. I also haven't ran this past ShellCheck yet,

As this will need a lot of testing, as a workaround in the interim, double-escaping new strings before saving should resolve the problem. You shouldn't need to double-escape after that. After investigation, it sems the echo in updateConfigEntry is actually safe, so my guess is that this is why subsequent saves are fine, but newly-added paths get mangled.

sonic2kk commented 3 months ago

Actually, I'm dumb, that doesn't work. The testing example I was using in my Bash shell spelled CFGCAT wrong when giving it to sed.

There may be an alternative solution though, if we just escape CFGVALUE when we pass it to updateConfigEntry.

CFGVALUE="$( echo "$2" | sed 's/\\/\\\\/g' )"

And then just use our existing sed command.

This probably isn't fool-proof per-se, since this is all escaping, but should prevent problems if the user only enters valid paths. I.e. paths with single or double backslashes should work fine. If a user deliberately tries to break things, it'll probably be possible to make it fall apart. But the only thing they have to gain there is their own config file getting messed up :sweat_smile:

sonic2kk commented 3 months ago

There is a WIP PR up at #1076, it's still WIP though (it may be very broken so I don't recommend testing yet). I'll ping when there's more work done on it.

Also, based on https://github.com/sonic2kk/steamtinkerlaunch/issues/1072#issuecomment-2016674914, do you think #1073 can be merged? If so, I'll get that in and rebase #1076 against it so it gets the spaced launch option fix.

EDIT: Yeah, #1076 does not work. It saves checkbox values as "TRUE" instead of as 0/1 values. Not sure why. EDIT2: Oh, duh, because we map TRUE/FALSE to 1/0 later on. Just gotta move where we set ESCAPED_CFGVALUE I guess.

sonic2kk commented 3 months ago

Okay, #1076 should be in a working state now. It should allow you to go from a blank CUSTOMCMD_ARGS, to having --launch Z:\this\is\sparta and being able to save it appropriately to the game config.

However, before testing, please either:

  1. Back up your entire ~/.config/steamtinkerlaunch, if it is not unreasonably huge, or
  2. If it is too huge, at minimum back up: a. global.conf. b. default_template.conf. c. your entire gamecfgs folder.

I mangled mine by accident during testing :sweat_smile:

With #1073 and #1076, if they both work, we should be out of the woods with this issue. #1076 in particular if it works should be good at more widely fixing issues saving values with backslashes. So this has been a very helpful issue, the scope of what it fixes extends beyond just custom commands!

zany130 commented 3 months ago

Yeah, I think #1073 can be merged. I have been using it since yesterday, and I have been able to boot P3R with my mods by first setting CUSTOMCMD_ARGS="--launch Z:\home\zany130\.local\share\Steam\steamapps\common\P3R\P3R\Binaries\Win64\P3R.exe" in my game config file. Afterward, I can save it with the GUI, and everything will be fine.

can you merge #1073 and then rebase #1076 on that? I'll then test #1076 if I can set the CUSTOMCMD_ARGS from an empty state to the backslashed path without restoring to editing the config or escaping the slashes.

Back up your entire ~/.config/steamtinkerlaunch

no worries there I always backup my .config folder don't know how many times I broke something and had to restore lol

sonic2kk commented 3 months ago

Thanks! #1073 has been merged, #1076 has been rebased :-)

zany130 commented 3 months ago

Testing #1076 now everything seems like its working as it should. I decided to delete (after backing up ofc) the P3R config file and see if I could set it all up from scratch... everything worked. I was able to set the custom command to the mod loader and then pass the arguments to launch P3R, and everything worked. I didn't have to escape or do anything like that.

so seems like its working perfectly and I even haven't been able to find any regressions in writing the config file out so that's also good

sonic2kk commented 3 months ago

Awesome, thank you so much for testing that branch and all the others :-)

I will do a bit more testing and if there are no issues I will merge it. If I encounter any problems from then on running on master we can revert the merge and re-visit this issue.

Once #1076 is merged I will close this issue. I'll link the PR to this issue now so it can be done automatically on merge.

sonic2kk commented 3 months ago

This issue should be fixed now that #1073, #1074, and #1076 have been merged.

Thanks for all your patience testing this and helping to get it resolved! Hope it helps :-)