Steam Deck dependency enhancement

[AtomHare]

This draft implements the base discussed in #859 (comment).

Here what's implemented in this draft and some hurdles I noticed that we will need to fix before marking it ready.

• 1. STL starts up on Steam Deck.
• 2. If the Automatic Update checkbox is off, don't do any checks and skip.
• 3. Otherwise, we check if SteamTinkerLaunch has updated by comparing the value in lastvers with that of the incoming version (we can get the incoming version from PROGVERS) - managed by checkSteamDeckSTLUpdated
• 4. If the version stored in lastvers is the same as the incoming version, do nothing (this will prevent the constant echo spam on each launch stating that STL needs to be updated)
• 5. If the version in lastvers does not match the incoming version string, we should try to update our dependencies, as the newer STL version may specify a newer version of a given dependency
• 6. Once everything is complete, update the version string in lastvers so that we know we already checked for dependency updates for this version. - See in issues, I think we could force the overwrite each time the libs are updated

Issues:

• If coming from an already existing install, we will create the lastvers file ⇒ leading to not update the libs on the first stl update after this PR, maybe we need to create the lastvers after the installation of the libs each time (overwrite the lastvers or creating it for the first time, either way it's a simple bash pipe), and make checkSteamDeckSTLUpdated return 0 if the file doesn't exist either.
• Is this PR also related with the notification spam of STL at the launch of a game ? (Only in desktop mode)
• Already downloaded archives and installed libs may cause issues. Maybe we should clean up the downloaded archives after their successful extraction ? The handling probably needs to be updated too, I'm not very familiar with the codebase.

Note: Also while editing I saw that sometimes there are 4 spaces while in the script it's mainly tabs. Should we make some reformat to fix this later on ?
Note 2: This is currently untested

[sonic2kk]

Woah! Thank you for making this PR!

If the Automatic Update checkbox is off, don't do any checks and skip. - not sure where it belongs

We can put this on the Global Menu, but indeed, it is tricky to know the best spot for this... Maybe under "Misc" settings?

https://github.com/sonic2kk/steamtinkerlaunch/blob/master/steamtinkerlaunch#L5664

If the version stored in lastvers is the same as the incoming version, do nothing (this will prevent the constant echo spam on each launch stating that STL needs to be updated) - Not sure if checkSteamDeckSTLUpdated already does it.

From a quick glance at the logic here (https://github.com/sonic2kk/steamtinkerlaunch/pull/1111/files#diff-77c6d4bb1bf7c1988ff5a068c856b6c24e113663f2e61cb3a2c98723c0fafc04R25928) it seems like it would already handle it.

The if check is if [ -f "$(command -v "$DEPCMD")" ] && [ "$CHECKCMD" = "OK" ] && [ "$( checkSteamDeckSTLUpdated )" -eq 0 ]; then - And if all of these are true, then we assume we don't need to install any dependencies.

• `-f "$(command -v "$DEPCMD")"`` - If the dependency file exists.
• "$CHECKCMD" = "OK" - If the dependency can actually be ran.
• "$( checkSteamDeckSTLUpdated )" -eq 0 - If the SteamTinkerLaunch version has not changed since the last run.

Then skip the dependency update logic. That should prevent STL constantly trying to update itself.

If coming from an already existing install, we will create the lastvers file ⇒ leading to not update the libs on the first stl update after this PR, maybe we need to create the lastvers after the installation of the libs each time (overwrite the lastvers or creating it for the first time, either way it's a simple bash pipe), and make checkSteamDeckSTLUpdated return 0 if the file doesn't exist either.

I think the easiest solution here is to run clearDeckDeps if there is no lastvers; assume all dependencies are bad and need wiped, and pull the fresh ones. Then everytime a user updates they'll get fresh dependencies.

If I'm missing something here let me know :-)

Is this PR also related with the notification spam of STL at the launch of a game ? (Only in desktop mode)

Yes. The reason it's only in Desktop Mode is because notifiers don't work in a GameScope session.

Already downloaded archives and installed libs may cause issues. Maybe we should clean up the downloaded archives after their successful extraction ? The handling probably needs to be updated too, I'm not very familiar with the codebase.

Yeah, there's going to be a conflict here.

SteamTinkerLaunch explicitly does NOT clean up archives because I want the ability to use it entirely offline eventually. So the idea was to allow users to manually place the archives and have them be extracted.

But with automatically updating the dependencies, I'm not sure how to serve both needs. The complexity around solving this is very off-putting.

Also while editing I saw that sometimes there are 4 spaces while in the script it's mainly tabs. Should we make some reformat to fix this later on ?

Some consistency around this would be good, but making sure it doesn't interrupt places where spaces are used deliberately is important. Unless we go with using spaces instead of tabs rather than the other way around.

I guess it's something I can look into at another time :-)

[AtomHare]

This draft is still UNTESTED.

What's new :

• Clear deps on the first run if the lastvers file doesn't exist, fix any weird issues in the case of an existing install
• Added the autoupdater settings - if you have any naming recommendation feel free to tell me I will correct it
• Set the PROGVERS to differentiate it from the master branch

Does everything look good to you ? Before jumping on the ship in order to test it, I wanted to know if "theoretically" all the logic is in place.

[sonic2kk]

I left a couple of comments, but overall this makes sense to me. This is very well implemented from reading and following the code. If might see just how difficult it would be to set up a SteamOS VM to test this, because I'd like to give this the proper testing and review it deserves.

I appreciate being able to follow the commits for the new changes too. The only one I left a comment about really was the clearDeckDeps addition. I think it's fine but I wanted to call out a small concern I had with it, because that might be good to double-check in testing. While writing I kinda went back and forth if it was worth noting but in the end I thought, better to say it than not :-)

Overall, freaking excellent job 👍 I think once we resolve the open comments this is good to start testing. I don't really remember what the problems were with a SteamOS VM that others reported but I guess there's only one way to find out the problem and that's to tinker around with it for myself 😄

[AtomHare]

Thank you a lot for all your explanations on the different part of the code. I will start testing on an SteamOS VM very soon. I found this guide which I will use as a base to try the installation !

[sonic2kk]

Resolved all the comments, thanks for your explanations.

Also thanks for the guide. I wonder if it works without VirtualBox. It doesn't seem like any of the steps here are specific to it (the Guest Additions can probably be replaced with Spice). Most of it seems like general Arch installation steps with a little extra flavour to set the default boot session and such :-)

[lolcatplays]

Thank you a lot for all your explanations on the different part of the code. I will start testing on an SteamOS VM very soon. I found this guide which I will use as a base to try the installation !

I have tested this on my steam deck but so far seems to not be working. I added the yad URL as listed in the master commit as it downloaded a 0B file but I'm still getting the update spam. On the "Main" (bleeding-edge branch) of SteamOS.

steamtinkerlaunch vortex winetricks
Preparing to install SteamTinkerLaunch on Steam Deck
Downloading latest SteamTinkerLaunch

Installing dependency 'innoextract'
Downloading dependency 'innoextract-1.9-8-x86_64.pkg.tar.zst'...
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  214k  100  214k    0     0   564k      0 --:--:-- --:--:-- --:--:--  563k
Successfully downloaded innoextract-1.9-8-x86_64.pkg.tar.zst!
Extracting dependency 'innoextract-1.9-8-x86_64.pkg.tar.zst'...
Successfully extracted 'innoextract-1.9-8-x86_64.pkg.tar.zst' to '/home/deck/stl/deps'
Dependency 'cabextract' already installed, nothing to do.
/home/deck/stl/prefix/steamtinkerlaunch: line 26219: [: : integer expression expected
Dependency 'yad' already installed, nothing to do.

Finished updating SteamTinkerLaunch ('v14.0.20240601-1 (steamdeck-dependency-handling)')!

[AtomHare]

Thank you a lot for your testing. I rebased to have the Yad url fix in this PR.
it looks like it failed the checkSteamDeckSTLUpdated check there https://github.com/sonic2kk/steamtinkerlaunch/blob/f699a630d47fd96bdf299dec47d011ba65b6b6bf/steamtinkerlaunch#L26217.

Not sure what went wrong, I will test it on my steam deck tonight too and see how I can prevent notification spam.

[sonic2kk]

Since we're checking if it's equal to one, perhaps we can simply do this:

function updateSteamDeckLastVers {
	# This function updates the 'lastvers' file after a dependency update if there was a version change
	if checkSteamDeckSTLUpdated ; then
		echo "$PROGVERS" > "$STLSTEAMDECKLASTVERS"
	fi
}

This is how we use, for example, checkSGDbApi and, more recently, haveAnySteamShortcuts. We'd need to rework how updateSteamDeckLastVers is checked in any other places we use it. Also, we'd need to check the return for checkSteamDeckSTLUpdated as well. Returning 0 is a success status, so if STL updated, we'd want checkSteamDeckSTLUpdated to return 0.

The return didn't work because using a subshell with $( func_name ) will only capture the echo/printf/etc, I guess. Which I'm a bit surprised about to be honest.

I'll do a scan of the PR later to see if we need to fix this up in other places. This is my bad for not realising, I'm too used to convenience in other languages 😅 Both in not catching it in this PR and not capturing it when I prototyped this logic before.

(I didn't run the workflow again because afaict the last force-push was just a rebase, so I'll run the workflow again when this is marked ready for review.)

[sonic2kk]

Should've caught this before, very sorry I only realised these now 😓

Give these a try locally and see if they pan out. If so push them up and the update checks should be fine :-)

[sonic2kk]

Suggested change

	return 0
	return 1

[sonic2kk]

We need to return 0 because 0 is a success code.

Suggested change

	return 1
	return 0

[sonic2kk]

With the changes to how checkSteamDeckSTLUpdated works, it'll return 0 (positive, meaning an update occurred), so we can use it like a true boolean value. Note how it can be used without square brackets, because it's not a test condition (Bash is weird).

Suggested change

	if [[ -f "$(command -v "$DEPCMD")" && "$CHECKCMD" = "OK" && ( "$( checkSteamDeckSTLUpdated )" -eq 0 || "$STEAMDECK_AUTOUP" -eq 0 ) ]]; then
	if [[ -f "$(command -v "$DEPCMD")" && "$CHECKCMD" = "OK" && ( checkSteamDeckSTLUpdated || "$STEAMDECK_AUTOUP" -eq 0 ) ]]; then

ShellCheck also shouldn't complain about this, with the code below at least it was happy.

---

I verified this sort of logic works with a little test script, hopefully it helps illustrate the logic a little better :-)

#!/usr/bin/bash

PROGVERS="1.2.3"

function hasUpdated {
    oldvers="1..3"
    
    if [ "${oldvers}" = "${PROGVERS}" ]; then
        # no update
        return 1
    else
        return 0
    fi
}

boolvar=1  # Set this to 0 to get "no update"
if ( hasUpdated && [ "${boolvar}" -eq 1 ] ); then
    echo "Has updated"
else
    echo "no update"
fi

[sonic2kk]

Was about to fall asleep and it just dawned on me that this should probably be a ! checkSteamDeckSTLUpdated check, since before we were checking -eq 0.

I think there is some weirdness around how Bash treats the ! negation, so please test if you can to make sure the negation doesn't affect the whole statement and only that checkSteamDeckSTLUpdated check.

I'll try do some testing in a test script tomorrow to see how Bash handles this. I don't know if there's any examples in the codebase of this kind of check in a compound if check.

[AtomHare]

I just tried this solution and came across an issue, it doesn't look like the return value of a function is supported inside a test [[. See https://stackoverflow.com/a/42971383
I'm not sure how we should tweak it, one idea could be to "preload" the function response before the answer output

[sonic2kk]

I just tried this solution and came across an issue, it doesn't look like the return value of a function is supported inside a test

That's a bit strange, is the code snippet I provided invalid then? It seemed to work in my testing...

Plus, we use if funcThatReturnsZero ; then syntax around the codebase already.

[AtomHare]

It's not invalid, in the current if it's more like if [[ hasUpdated && ( "${boolvar}" -eq 1 ) ]] and I don't know how to properly wrap the or to override the update if they're disabled

[sonic2kk]

Would this work?

if [[ -f "$(command -v "$DEPCMD")" && "$CHECKCMD" = "OK" ]] && ( checkSteamDeckSTLUpdated || [[ "$STEAMDECK_AUTOUP" -eq 0 ]] ); then
    # ...
fi

I'm playing around with this in a test script. The logic is a little complex here, but I think this should work, since ( checkSteamDeckSTLUpdated ) should capture the return, and we wrap the check on $STEAMDECK_AUTOUP in a separate [[ test.

---

In my tests with the above:

• If DEPCMD fails, we will need to update.
• If CHECKCMD fails even if DEPCMD passes, we need to update.
• If checkSteamDeckSTLUpdated returns 1 (meaning no update was found), we will only need to update if
  • DEPCMD fails
  • CHECKCMD fails
  • Both DEPCMD and CHECKCMD fails
  • Or if STEAMDECK_AUTOUP is 1 (meaning we want to always check for updates).
• If STEAMDECK_AUTOUP is 1, we will always need to update regardless of DEPCMD, CHECKCMD, and checkSteamDeckSTLUpdated

I think that logic is correct, but I might be missing something obvious 😅

[sonic2kk]

Same idea here with how checkSteamDeckSTLUpdated is used:

Suggested change

	if [ "$( checkSteamDeckSTLUpdated )" -eq 1 ]; then
	if checkSteamDeckSTLUpdated; then

[AtomHare]

I just tried your suggestions, we're close to making it work but it looks like we need to make some tweaking !

And I noticed other things:
log spam still occurs: "preparing to install SteamTinkerLaunch", "Downloading latest SteamTinkerLaunch", "Add steamtinkerlaunch as compatibility tool", "Finished updating SteamTinkerLaunch". Maybe if the lastvers file is detected, we could remove all of those notifications ?

Recap of the remaining issues :

• Tweak the if formula
• Prevent the log spam -> in a separate PR
• Avoid redownloading the libs on each update if they don't change, (it looks like it redownloads the libs, even though they were downloaded before?)

[sonic2kk]

Maybe if the lastvers file is detected, we could remove all of those notifications ?

Yes, we should update notifier logic I think, but I think all the remaining notifier-related "problems" can be done in a separate PR.

• Notifier for installing SteamTinkerLaunch, including finishing installation
  • We should keep this, because when a user double-clicks the script to install STL we will want to show this visual feedback. If it is not running only during installation, we should fix that, but it can go in a separate PR, nothing to worry about here :-)
• Notifier for updating SteamTinkerLaunch, including finishing updating
  • We can remove the notifiers around preparing to install SteamTinkerLaunch, as the auto-update logic (which doesn't really work) will be removed shortly. Users can just manually update STL, and when they do, this PR will update the dependencies automatically!
• Notifier for adding SteamTinkerLaunch as a compatibility tool
  • This should be reworked to only add SteamTinkerLaunch as a compatibility tool if it isn't already there. We should move the notifier logic into this function, wrapped in Steam Deck-specific checks: https://github.com/sonic2kk/steamtinkerlaunch/blob/bddc3b36369632ef6bcaeba36eddf05c552dae65/steamtinkerlaunch#L21800 -- So we notify if the symlink is created/updated only, and skipped when the symlink already exists.

So we can remove two notifiers related to automatic updates once that logic is stripped out. If the notifier logic for installing and finished installing is incorrect, that should be fixed to only show on initial installation (i.e. when the script is executed to install STL initially, or when installed from ProtonUp-Qt which just runs ./steamtinkerlaunch as well). Finally, the notifier for adding SteamTinkerLaunch as a compatibility tool should be updated to be inside of the CompatTool function and should only run when the compat tool symlink is added/updated. And all of these changes can go in separate PRs, so no need to stress about them here. You're already working on enough in this PR. If you'd like though you can contribute to improving the notifier in a future PR!

Users that are bothered by the notification spam are probably not users who STL is aimed at, they tend to be the non-developers that bought Steam Decks in my experience. And STL on Steam Deck is mainly an enthusiast/developer curiosity, not something to be used seriously in its current form, hence the wiki's emphasis that SteamTinkerLaunch on SteamOS is in early stages, yet you are one of the few devs who actually stood up to contribute, there aren't many contributors on the desktop side and virtually none for SteamOS, so it's nice to see someone that cares enough to actually improve the situation. So to that end feel free to pick up the notifier work in a future PR, but there is absolutely no obligation on you to do so in this PR (it is probably cleaner in a separate PR anyway).

---

I will take a look at the if check and your comment above when I have some time, I am curious as to why it isn't working properly myself :-)

EDIT: I left a comment about it, we can discuss the if formula further there. I probably shouldn't have even mentioned it here.

Avoid redownloading the libs on each update if they don't change, (it looks like it redownloads the libs, even though they were downloaded before?)

Hmm, something is going wrong in installDependencyVersionFromURL then? I'm not sure why from looking at the code...

It might be worth checking the steamtinkerlaunch.log file to see if there's any indication of where things are going sideways.

[AtomHare]

Thank you for your comments, sadly I won't be able to look and tinker with the code until Monday but will as soon as I'm available!

I will also make a separate PR for the steam deck notifications.

And I may look into a debug var that enables the steam deck behavior to make it easier to test. I don't need it to run the libs but mainly to test the codes/logic! (I'll probably add a dev check to prevent people wanting the steam deck behavior in desktop mode)

[sonic2kk]

No worries, you can work on this whenever you have time. All your work is appreciated thus far as well! :-)

[sonic2kk]

I went ahead and linked this PR to #859, as this is the last thing discussed in that issue related to Steam Deck improvements.

You've expressed interest in doing more work, and you are absolutely welcome to contribute any improvements in the future. But as far as that issue goes, this PR would implement all that was left specific to that issue. So once this is merged, that issue would be done, even if there is more work you or anyone else may want to do!

Of course that doesn't mean there is any rush on this either. I've just been doing a little "issue cleanup" and this was part of it. You can work on this whenever you have time and motivation. If we hit any major blockers in a worst-case and we can't merge this (which I don't foresee happening, imo this is already most of the way there, once again fantastic work on your part), linking the issues gives a good paper-trail on what went on during development. Development transparency is very important to me with this project and giving everyone insight into what decisions were made, where discussion took place, etc is something I strive for (even if I don't always live up to that in the way I want to).

So yeah, please don't feel any additional pressure here. Just some repo management 😄