Make shellcheck happy, add linting, ref #105.

.github/workflows/build.yml

@@ -13,6 +13,9 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
+      - name: ShellCheck
+        uses: ludeeus/action-shellcheck@master
+
       - name: Run tests
         run: ./extra/tests.sh
 

.github/workflows/pr.yml

@@ -9,5 +9,8 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
+      - name: ShellCheck
+        uses: ludeeus/action-shellcheck@master
+
       - name: Run tests
         run: ./extra/tests.sh

extra/buildall.sh

@@ -3,37 +3,45 @@
 set -xeu -o pipefail
 
 # exporting for subshell
-export src="$(realpath scripts_src)"
-export dst="$(realpath data/scripts)"
-export scripts_lst="$dst/scripts.lst"
-export compile_exe="$bin_dir/compile.exe"
+src="$(realpath scripts_src)"
+export src
+dst="$(realpath data/scripts)"
+export dst
+scripts_lst="$dst/scripts.lst"
+export scripts_lst
+# shellcheck disable=SC2154  # bin_dir defined in env.sh
+compile_exe="$bin_dir/compile.exe"
+export compile_exe
 
 # wine doesn't return proper error code on missing file
 if [[ ! -f $compile_exe ]]; then
-  echo "compile.exe missing, failed"
-  exit 1
+    echo "compile.exe missing, failed"
+    exit 1
 fi
 
 # compile all
-for d in $(ls $src); do
-  if [[ -d "$src/$d" && "$d" != "template" ]]; then # if it's a dir and not a template
-    cd "$src/$d"
-    files=""
-    for f in $(ls | grep -i "\.ssl$"); do # build file list
-      int="$(echo $f | sed 's|\.ssl$|.int|')"
-      if grep -qi "^$int " "$scripts_lst" || [[ "$d" == "global" ]]; then # if file is in scripts.lst or a global script
-        script_name="$(echo "$f" | sed 's|\.ssl$|.int|')"
-        gcc -E -x c -P -Werror -Wfatal-errors -o "${f}.tmp" "$f" # preprocess
-        set +e
-        wine "$compile_exe" -n -l -q -O2 "$f.tmp" -o "$dst/$script_name" # compile
-        if [[ "$?" != "0" ]]; then # 1 retry on wine connection reset
-          set -e
-          sleep 1
-          wine "$compile_exe" -n -l -q -O2 "$f.tmp" -o "$dst/$script_name" # compile
-        fi
-        set -e
-      fi
-    done
-    cd ..
-  fi
+for d in "$src"/*; do
+    if [[ -d "$src/$d" && "$d" != "template" ]]; then # if it's a dir and not a template
+        cd "$src/$d"
+        # shellcheck disable=SC2010  # We shouldn't have non-alphanumeric names here.
+        for f in $(# build file list
+            ls | grep -i "\.ssl$"
+        ); do
+            # shellcheck disable=SC2001  # Simple replacement doesn't work, need regex.
+            int="$(echo "$f" | sed 's|\.ssl$|.int|')"
+            if grep -qi "^$int " "$scripts_lst" || [[ "$d" == "global" ]]; then # if file is in scripts.lst or a global script
+                gcc -E -x c -P -Werror -Wfatal-errors -o "${f}.tmp" "$f"        # preprocess
+                set +e
+                wine "$compile_exe" -n -l -q -O2 "$f.tmp" -o "$dst/$int" # compile
+                # shellcheck disable=SC2181  # Long commands are unwieldy in if conditions.
+                if [[ "$?" != "0" ]]; then # 1 retry on wine connection reset
+                    set -e
+                    sleep 1
+                    wine "$compile_exe" -n -l -q -O2 "$f.tmp" -o "$dst/$int" # compile
+                fi
+                set -e
+            fi
+        done
+        cd ..
+    fi
 done

extra/env.sh

@@ -2,14 +2,17 @@
 
 set -xeu -o pipefail
 
-export extra_dir="$(realpath extra)"
+extra_dir="$(realpath extra)"
+export extra_dir
 export bin_dir="$extra_dir/bin"
-export release_dir="$(realpath release)"
+release_dir="$(realpath release)"
+export release_dir
 export mods_dir="$release_dir/mods"
 export mpack_version=${mpack_version:-4.4}
 export mpack_7z="mpack.7z"
 export sfall_version=${sfall_version:-4.4.0.1}
 export WINEARCH=win32
 export WINEDEBUG=-all
 export mod_name=upu
-export trans_dir="$(realpath translations)"
+trans_dir="$(realpath translations)"
+export trans_dir

extra/full.sh

@@ -2,8 +2,9 @@
 
 set -xeu -o pipefail
 
-rm -f *.7z *.zip *.exe *.list
+rm -f ./*.7z ./*.zip ./*.exe ./*.list
 
+# shellcheck source=/dev/null  # doesn't matter, no vars used in this script
 source ./extra/env.sh
 ./extra/prepare.sh
 ./extra/buildall.sh

extra/package.sh

@@ -3,24 +3,28 @@
 set -xeu -o pipefail
 
 export comp_dir="components"
+# shellcheck disable=SC2154  #  bin_dir from env.sh
 export dat2="wine $bin_dir/dat2.exe"
 export dat2a="wine $bin_dir/dat2.exe a"
-export trans_dir="$(realpath translations)"
-export file_list="$(realpath dat2.list)"
+trans_dir="$(realpath translations)"
+export trans_dir
+file_list="$(realpath dat2.list)"
+export file_list
 
 short_sha="$(git rev-parse --short HEAD)"
 # defaults, local build or github non-tagged
 export version="git$short_sha"
 export vversion="$version" # in package names
 export uversion="$version" # in game
 
-if [[ ! -z "${GITHUB_REF-}" ]]; then # github build
-  if echo "$GITHUB_REF" | grep "refs/tags"; then # tagged
-    version="$(echo $GITHUB_REF | sed 's|refs\/tags\/v||')"
-    export version
-    export vversion="v$version"
-    export uversion="u$version" # in game
-  fi
+if [[ -n "${GITHUB_REF-}" ]]; then                 # github build
+    if echo "$GITHUB_REF" | grep "refs/tags"; then # tagged
+        # shellcheck disable=SC2001  #  sed is more readable
+        version="$(echo "$GITHUB_REF" | sed 's|refs\/tags\/v||')"
+        export version
+        export vversion="v$version"
+        export uversion="u$version" # in game
+    fi
 fi
 
 # cleanup for local build
@@ -29,22 +33,25 @@ git clean -fd release
 git clean -fdX release
 
 # wine pollutes the log with "wine: Read access denied for device" if z is linked to /
-z="$(readlink -f ~/.wine/dosdevices/z\:)"
+z="$(readlink -f ~/.wine/dosdevices/z:)"
 if [[ "$z" == "/" ]]; then
-  rm -f ~/.wine/dosdevices/z\:
-  ln -s /home ~/.wine/dosdevices/z\:
+    rm -f ~/.wine/dosdevices/z:
+    ln -s /home ~/.wine/dosdevices/z:
 fi
 
 # translations packaged first, to get extra text out of the way
+# shellcheck disable=SC2154  # extra_dir from env.sh
 "$extra_dir"/package/translations.sh
 
 # data
+# shellcheck disable=SC2154  # from env.sh
 dat="$mod_name.dat"
+# shellcheck disable=SC2154  # from env.sh
 mkdir -p "$mods_dir"
 
 cd data
 # I don't know how to pack recursively
-find . -type f | sed -e 's|^\.\/||' -e 's|\/|\\|g' | sort > "$file_list" # replace slashes with backslashes
+find . -type f | sed -e 's|^\.\/||' -e 's|\/|\\|g' | sort >"$file_list" # replace slashes with backslashes
 $dat2a "$mods_dir/$dat" @"$file_list"
 cd ..
 
@@ -59,8 +66,9 @@ cd ..
 
 # manual package: linux/mac os
 pushd .
+# shellcheck disable=SC2154  # from env.sh
 cd "$release_dir"
-zip -r "${mod_name}_${vversion}.zip" * # our package
+zip -r "${mod_name}_${vversion}.zip" -- * # our package
 popd
 mv "$release_dir/${mod_name}_${vversion}.zip" .
 

extra/package/ammo.sh

@@ -2,9 +2,11 @@
 
 set -xeu -o pipefail
 
+# shellcheck disable=SC2154  # from env.sh
 scripts_dir="$release_dir/data/scripts"
 
 mkdir -p "$scripts_dir"
+# shellcheck disable=SC2154  # from env.sh
 7zr e "$mpack_7z" example_mods/AmmoMod/gl_ammomod.int
 mv gl_ammomod.int "$release_dir/"
 

extra/package/animation_fixes.sh

@@ -8,8 +8,8 @@ export optional_dir="optional"
 mkdir -p "$optional_dir"
 cd "$optional_dir"
 for f in goris_fast_derobing_low_fps.dat walk_speed_fix_low_fps.dat; do
-  url0=$(curl "$api_url")
-  echo "$url0"
-  url="$(echo "$url0" | grep browser_download_url | grep $f | awk -F '"' '{print $4}')"
-  wget -nv "$url"
+    url0=$(curl "$api_url")
+    echo "$url0"
+    url="$(echo "$url0" | grep browser_download_url | grep $f | awk -F '"' '{print $4}')"
+    wget -nv "$url"
 done

extra/package/cassidy_head.sh

@@ -1,11 +1,13 @@
 #!/bin/bash
 
+# This is just a precaution against adding heads.lst accidentally.
+
 set -xeu -o pipefail
 
 if [[ -f data/art/heads/heads.lst ]]; then
-  echo "data/art/heads/heads.lst file was found"
-  echo "This will break Cassidy head mod, which ships its own heads.lst"
-  echo "Because upu_* is loaded after cassidy_*"
-  echo "Resolve this before publishing new releases"
-  exit 1
+    echo "data/art/heads/heads.lst file was found"
+    echo "This will break Cassidy head mod, which ships its own heads.lst"
+    echo "Because upu_* is loaded after cassidy_*"
+    echo "Resolve this before publishing new releases"
+    exit 1
 fi

extra/package/inno.sh

@@ -11,19 +11,24 @@ mkdir -p Output
 chmod 0777 Output
 
 # delete unnecessary files
+# shellcheck disable=SC2154  # from env.sh
 rm -f "$release_dir"/{upu-install.sh,upu-install.command}
+# shellcheck disable=SC2154  # from env.sh
 sed -i "s|define uversion .*|define uversion \"${uversion}\"|" "$install_iss"
+# shellcheck disable=SC2154  # from env.sh
 sed -i "s|define vversion .*|define vversion \"${vversion}\"|" "$install_iss"
 
 rm -rf release translations
-cp -r $release_dir ./
+cp -r "$release_dir" ./
 mkdir translations
-mv $trans_dir/*.dat translations/
+# shellcheck disable=SC2154  # from env.sh
+mv "$trans_dir"/*.dat translations/
 
 # alternative animations, not included into manual install
+# shellcheck disable=SC2154  # from env.sh
 "$extra_dir"/package/animation_fixes.sh
 
-docker run --rm -i -v $PWD:/work amake/innosetup "$install_iss"
+docker run --rm -i -v "$PWD":/work amake/innosetup "$install_iss"
 rm -rf release translations
 popd
 

extra/package/sfall.sh

@@ -2,9 +2,11 @@
 
 set -xeu -o pipefail
 
+# shellcheck disable=SC2154  # from env.sh
 release_ini="$release_dir/ddraw.ini"
+# shellcheck disable=SC2154  # from env.sh
 custom_ini="$extra_dir/package/ddraw.ini"
-
+# shellcheck disable=SC2154  # from env.sh
 sfall_url="https://sourceforge.net/projects/sfall/files/sfall/sfall_$sfall_version.7z/download"
 
 cd "$release_dir"
@@ -25,15 +27,16 @@ done
 rm -f sfall.7z
 
 # uncomment ini settings to preserve options' placement in ddraw.ini
-entries="$(cat $custom_ini | grep '=' | awk -F '=' '{print $1}')"
+entries="$(grep '=' "$custom_ini" | awk -F '=' '{print $1}')"
 for e in $entries; do
   sed -i "s|^;$e=|$e=|" "$release_ini"
 done
 
 # then merge custom settings
-crudini --merge "$release_ini" < "$custom_ini"
+crudini --merge "$release_ini" <"$custom_ini"
 # set version string
 sed -i "s|^;VersionString=|VersionString=|" "$release_ini"
+# shellcheck disable=SC2154  # from env.sh
 crudini --set "$release_ini" "Misc" "VersionString" "FALLOUT II 1.02.31${uversion}"
 # crudini adds spaces arouns the values, need to remove them
 sed -i "s|^\([[:alnum:]]\+\) = |\1=|" "$release_ini"

extra/package/translations.sh

@@ -10,9 +10,10 @@ rm -rf "$lang_dir"/{po,readme.md,translation.patch}
 rm -f "$lang_dir/english/cuts/intro.sve"
 
 # update patchinf
-for lang in $(ls $lang_dir); do
+for lang in "$lang_dir"/*; do
   patchinf="$lang_dir/$lang/dialog/patchinf.msg"
   # display current version
+  # shellcheck disable=SC2154  # from package.sh
   sed -i "s|1\.02\.31u[0-9]*\.|1.02.31${uversion}.|" "$patchinf"
   # if the string is untranslated, still show correct language
   lang_name="${lang^}"
@@ -23,6 +24,7 @@ done
 cp -r "$lang_dir"/english/cuts{,_female}
 
 # move texts to translation package dir
+# shellcheck disable=SC2010  # shouldn't have non-alphanumeric filenames here
 for d in $(ls $lang_dir | grep -v english); do
   rm -rf "$trans_dir/$d/text"
   mkdir -p "$trans_dir/$d/text"
@@ -31,19 +33,23 @@ done
 
 # package into dats
 cd "$trans_dir"
-rm -f *.dat
-for d in $(ls); do
+rm -f ./*.dat
+for d in *; do
+  # shellcheck disable=SC2154  # from env.sh
   dat="${mod_name}_$d.dat"
   cd "$d"
-  find . -type f | sed -e 's|^\.\/||' -e 's|\/|\\|g' | sort > "$file_list"
+  # shellcheck disable=SC2154  # from package.sh
+  find . -type f | sed -e 's|^\.\/||' -e 's|\/|\\|g' | sort >"$file_list"
+  # shellcheck disable=SC2154  # from package.sh
   $dat2a "$dat" @"$file_list"
-  mv $dat ..
+  mv "$dat" ..
   cd ..
 done
 cd ..
 
 pushd .
 cd "$trans_dir"
-zip -r "${mod_name}_${vversion}_translations.zip" *.dat # all translations, just text and graphics
+# shellcheck disable=SC2154  # from package.sh
+zip -r "${mod_name}_${vversion}_translations.zip" -- *.dat # all translations, just text and graphics
 popd
 mv "$trans_dir/${mod_name}_${vversion}_translations.zip" .

extra/prepare.sh

@@ -2,15 +2,18 @@
 
 set -xeu -o pipefail
 
+# shellcheck disable=SC2154  # from env.sh
 mpack_file="modderspack_$mpack_version.7z"
 mpack_url="https://sourceforge.net/projects/sfall/files/Modders%20pack/$mpack_file/download"
 compile_exe="compile.exe"
 mpack_compile="ScriptEditor/resources/$compile_exe"
 sfall_headers_dir="scripts_src/sfall"
 
 # compile.exe
+# shellcheck disable=SC2154  # from env.sh
 wget -nv "$mpack_url" -O "$mpack_7z"
 7zr e "$mpack_7z" "$mpack_compile"
+# shellcheck disable=SC2154  # from env.sh
 mv -f "$compile_exe" "$bin_dir/"
 
 # sfall headers