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

Feature/Steam Workshop support #4186

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

FliesWithWind
Copy link
Contributor

Description

For a while, I had a horrible but working script for downloading and updating Arma 3 mods from Steam Workshop. I figured it was about time I try adding it to LinuxGSM. The script was based on https://github.com/arkmanager/ark-server-tools

Changes in this PR are working, but are far from being pretty or complete in my opinion. I'm no bash magician, so I wanted to show it as it is, hoping for an initial review and suggestions.

I've added it as a separate module, to avoid conflicts with the current mods module.
For now, it downloads mods for Arma 3 taken from the server config file.

workshopmods="450814997;2131302796"

Once we reach a working and pretty solution, the next steps would be to add support for other games, like Arc and Starboud, and add Steam Collections support.

Fixes #[3514] #[1894] #[1623] #[960]

Type of change

  • Bug fix (a change which fixes an issue).
  • New feature (a change which adds functionality).
  • New Server (new server added).
  • Refactor (restructures existing code).
  • Comment update (typo, spelling, explanation, examples, etc).

Checklist

PR will not be merged until all steps are complete.

  • This pull request links to an issue.
  • This pull request uses the develop branch as its base.
  • This pull request subject follows the Conventional Commits standard.
  • This code follows the style guidelines of this project.
  • I have performed a self-review of my code.
  • I have checked that this code is commented where required.
  • I have provided a detailed enough description of this PR.
  • I have checked if documentation needs updating.

Documentation

Documentation will definitely need an update, once this is ready for merge. :)

if [[ "${serverresp}" =~ \"hcontent_file\":[[:space:]]*([^,]*) ]]; then
remupd="${BASH_REMATCH[1]}"
fi
echo "${remupd}" | tr -d '"'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of loading up tr(1) and using a subshell (via pipe) for this, it's far more efficient to use BASH's parameter expansion's pattern substitution, as it's already in the language and it's a very small amount of data.

echo "${remupd//\"/}"

Two initial '/' indicate a greedy match, while one is non-greedy.

This feature is available from at least BASH 3.0, including the option to omit the final '/' if no replacement string is provided.

# therefore, we have to separate the end of the filename to only lowercase it rather than the whole line
# Gather parent dir, filename lowercase filename, and set lowercase destination name
latestparentdir=$(dirname "${src}")
latestfilelc=$(basename "${src}" | tr '[:upper:]' '[:lower:]')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use BASH's parameter expansion to change the first or all letters in the variable to lower- or uppercase, without the need for calling other programs, which is more efficient, depending on the amount of data handled.

For example:

SomeVar='test string'
printf '%s\n' "${SomeVar^^}"
SomeVar='test string'
printf '%s\n' "${SomeVar,,}"

Two '^' or ',' indicate a greedy conversion to upper- or lowercase, respectively, while one of these characters changes just the first character, akin to Perl's uc() vs. uc_first(), if you're familiar with that.

This feature is available in >= BASH 4.0.

if [ ! -d "${workshopmodsdldir}" ]; then
echo -en "creating LinuxGSM Steam Workshop data directory ${workshopmodsdldir}..."
mkdir -p "${workshopmodsdldir}"
exitcode=$?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (L257)) is redundant. Sometimes it's useful to specifically catch $?, but this is mainly when you're testing for something other than just 0 or non-0, in my experience.

In this case, it's easier and more practical to directly act upon the exit status, rather than separately store the status, check the value, then act based on the value, especially if you're using [ (inefficient). The if statement in shell programming essentially does this for us.

For example:

if mkdir -p "${workshopmodsdldir}"; then
    ...
else
    ...
fi

While this isolated case isn't a huge deal, a lot of redundant code can be a problem, both to readability and performance.

This is core BASH functionality.

echo -en "creating LinuxGSM Steam Workshop data directory ${workshopmodsdldir}..."
mkdir -p "${workshopmodsdldir}"
exitcode=$?
if [ "${exitcode}" != 0 ]; then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference, -eq is the numeric comparison operator for equal to in BASH, whether in [ or [[. The = or == is a string comparison operator for equal to. Admittedly, the difference isn't often too important, but it's worth knowing and worth using the correct syntax to improve readability and avoid anything unexpected.

See CONDITIONAL EXPRESSIONS in bash(1), and note that the ARITHMETIC EVALUATION section is different, for expanding or evaluating arithmetic in, for example, ((...)), $((...)), ${NAME[...]}, etc.

# Counts how many mods were installed.
fn_workshop_count_installed() {
if [ -f "${workshopmodsdir}" ]; then
installedworkshopmodscount=$(ls -l "${workshopmodsdir}" | grep -c ^d)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's almost never a good idea to include ls(1) in a script, because it's typically redundant, inefficient, and unreliable. The shell already supports glob filename pattern matching, and counting results is simple.

For example

Count=0
for File in /path/to/files/*; {
    (( Count++ ))
}

printf '%d\n' $Count

Or, more concisely and more efficiently, albeit with far less flexibility:

Files=(/path/to/files/*)
printf '%d\n' ${#Files[@]}

This last one is an awesome feature that often gets overlooked.

These features are available in at least >= BASH 3.0.

I'm not sure why you're looking for '^d', so I apologise if I've missed a nuanced and obscure detail. Granted, BASH can effortlessly do what grep(1) is doing here as well.

For example:

Count=0
for File in *; {
    if [[ $File == *^d* ]]; then
        (( Count++ ))
    fi
}

printf '%d\n' $Count

Provided you're looking for '^d' being in filenames. By using these features, that's another 2 programs and a subshell you'd take out of the equation.

# Builds list of installed Steam Workshop mods.
fn_workshop_installed_list() {
fn_workshop_count_installed
for folder in ${workshopmodsdir}/*; do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unprotected variable, although there are a few of these. I'm not one of those people who insists on everything being quote-protected, because not everything needs to be, such as integers. However, since it's a directory, one which is presumably changeable by the user or a developer, it's always best to quote-protect it. Unprotected variables like paths can lead to destructive results.

local modid="$1"
local serverresp="$(curl -s -d "itemcount=1&publishedfileids[0]=${modid}" "http://api.steampowered.com/ISteamRemoteStorage/GetPublishedFileDetails/v1")"
local title=
if [[ "${serverresp}" =~ \"title\":[[:space:]]*([^,]*) ]]; then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that REGEX in BASH, while an awesome feature, is slower than glob pattern matching. You might be thinking that glob pattern matching is limited, so you need REGEX. I used to think the same. However, [[ uses extended globbing out of the box.

For example:

if [[ "${serverresp}" == *\"title\":*([[:space:]])(*([^,]))* ]]; then

Please test first, in-case I missed something. It's almost 5am. This took much longer than I thought it would.

Refer to extglob in bash(1) for more information. It's a useful feature which usually needs to be manually enabled with shopt -s extglob, with the exception of [[. It's not particularly advanced, but it does cater to most of the common use-cases.

Extended globbing is available in at least >= BASH 3.0, but it being included in [[ is in >= BASH 4.1.

@FliesWithWind
Copy link
Contributor Author

Thanks for the awesome review! I'll apply your suggestions once I have more time. :)

@Dr-Shadow
Copy link

@FliesWithWind Do you plan to resume your work on this soon ? I would like to add a specific contribution for DayZ Mods which are currently supported only if we rename files with uppercase characters to lowercase.

@terminalforlife
Copy link

terminalforlife commented Jul 5, 2023

@Dr-Shadow

This project is alien to me and I might've misinterpreted your goal, so please take this with a grain of salt.

You can bulk-rename files to lowercase with:

for File in *; do
    if [[ -f $File ]]; then
        mv "$File" "${File,,}"
    fi
done

The ,, feature of parameter expansion was introduced in BASH 4.0.

That would immediately rename all files in the current working directory to their lowercase counterpart. This is a non-recursive approach, by the way.

However, this should be done very carefully, perhaps with --no-clobber, because mv(1) will by default blast away any existing files, should they exist. Another issue with bulk-renaming files in this manner is that you may have conflicts, which you'd have to handle afterwards, such as 'FilE' and 'FILE' being completely different, but both being the same when renamed to all lowercase.

If you need recursion, you can use the globstar shell option. See bash(1), heading SHELL BUILTIN COMMANDS, under 'shopt', where you should find a list of valid shell options which the shopt builtin can handle.

@FliesWithWind
Copy link
Contributor Author

@Dr-Shadow at the moment I have my hands full, but I will try to find some time this month to apply suggestions from @terminalforlife. You are more than welcome to contribute. Arma 3 mod files and folders also need to be in lowercase, so it will likely be the same for DayZ.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants