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.13k stars 71 forks source link

ShellCheck: Fix warnings since ShellCheck v0.10.0 #1064

Closed sonic2kk closed 6 months ago

sonic2kk commented 6 months ago

Prerequisite to #1056.

Overview

Fixes ShellCheck warnings not caught in v0.8.0 that are caught now that we can use v0.10.0.

ShellCheck v0.9.0 added a new data flow analysis engine, but this sucks up ram on certain scripts (not necessarily tied to length) including SteamTinkerLaunch, so we were stuck on v0.8.0, which is a couple years out of date.

Recently, ShellCheck v0.10.0 came out, and added an option to disable the new data analysis engine introduced in v0.9.0 with a directive. SteamTinkerLaunch now has this directive (#1061), so we can now use ShellCheck v0.10.0! Woohoo!

ShellCheck v0.10.0 flags up quite a lot of warnings that v0.8.0 didn't. They are all existing warnings but it's better at catching them. This PR will aim to address as many as possible, and from a quick glance, I think all of them can be addressed.

Warning Types

The three main warning types we're running into now are:

I will address each of these in their own commit.


Opening now but only SC2086 has been dealt with so far. I confirmed that adding the quotes was safe.

SC2004 should be straightforward enough too, but I am concerned about solving 2028 without breaking functionality. It will probably be sorted out last.

sonic2kk commented 6 months ago

Fixed the SC2004 warnings with fae36c5. I was a bit concerned about these, but I tested in the "debug" commandline option with gameScopeArgs and it worked fine so I think it's okay.

sonic2kk commented 6 months ago

Fixed the SC2028 warnings. However, there is a change in behaviour for the ModOrganizer 2 INI logic. See below the difference in what echo gives and what printf gives.

echo "download_directory=${GLOBZMOIN}\\\\downloads"  # download_directory=Z:\\home\\username\\.config\\steamtinkerlaunch\\mo2\\compatdata\\pfx\\drive_c\\users\\steamuser\\AppData\\Local\\ModOrganizer\\\\downloads

printf "download_directory=%s\\\\downloads\n" "${GLOBZMOIN}"  # download_directory=Z:\\home\\username\\.config\\steamtinkerlaunch\\mo2\\compatdata\\pfx\\drive_c\\users\\steamuser\\AppData\\Local\\ModOrganizer\\\downloads\n   

I'm pretty sure the behaviour printf is giving is correct, it looks strange to me to have the two sets of \\. So I don't really have any concerns.

For the ReShade changes to use printf, there is no change in behaviour.


That's all the warnings fixed, ShellCheck is happy after this. I will do some more manual review and testing of this and merge once I have confidence everything is fine.

sonic2kk commented 6 months ago

Some of the backslash counts look a little odd, but nothing that was changed in this PR, so I will leave them as-is for now. One example is:

printf "mod_directory=%s\\\mods\n" "${GLOBZMOIN}"

This has three backslashes instead of four for some reason. But, since it's been like that for a while, it's probably fine. If using printf breaks this we can try and fix it by adding another backslash.

sonic2kk commented 6 months ago

This is ready to merge!!

frostworx commented 6 months ago

the weird count of backslashes should be correct - at least it was when I wrote it. I don't remember any details, but I had to write it to make "the windows part" of MO2 happy. so it was/is some confusing combination of linux backslash escaping in quotes and additional one or two backslahses for the windows path.

sonic2kk commented 6 months ago

Working with backslashes when doing things with Wine is really weird and confusing, at least for me it is :smile: So if this becomes a problem, I'll add extra backslashes to compensate for printf (potentially) removing them.

MO2 may also be smart enough to just account for this itself.