Merge branch upstream/master while introducing compile errors and reverting parts of Android changes, video BPP options are gone

.github/ISSUE_TEMPLATE/bug.md

@@ -1,17 +0,0 @@
----
-name: Bugs
-about: Found a bug in OpenTTD?
-title: "Bug Report"
----
-
-## Version of OpenTTD
-<!-- Fill in below what version of OpenTTD you are using, including your OS. -->
-
-## Expected result
-<!-- Describe in a few words what you expected to happen. -->
-
-## Actual result
-<!-- Describe in a few words what actually happens. -->
-
-## Steps to reproduce
-<!-- As detailed as possible, please tell us how we can reproduce this. Feel free to attach a savegame (zip it first) to make it more clear. -->

.github/ISSUE_TEMPLATE/bug.yaml

@@ -0,0 +1,41 @@
+name: Bug Report
+description: Found a bug in OpenTTD?
+title: "[Bug]: "
+body:
+  - type: markdown
+    attributes:
+      value: |
+        Thanks for taking the time to fill out this bug report!
+  - type: input
+    id: version
+    attributes:
+      label: Version of OpenTTD
+      description: Fill in below what version of OpenTTD you are using, including your OS.
+      placeholder: ex. 1.11.2, Windows 10
+    validations:
+      required: true
+  - type: textarea
+    id: expected
+    attributes:
+      label: Expected result
+      description: Describe in a few words what you expected to happen.
+    validations:
+      required: true
+  - type: textarea
+    id: actual
+    attributes:
+      label: Actual result
+      description: Describe in a few words what actually happens.
+    validations:
+      required: true
+  - type: textarea
+    id: reproduce
+    attributes:
+      label: Steps to reproduce
+      description: As detailed as possible, please tell us how we can reproduce this. Feel free to attach a savegame (zip it first) to make it more clear.
+      placeholder: |
+        1. Loaded the attached savegame.
+        2. Click on the button left of that other icon.
+        3. The window doesn't open.
+    validations:
+      required: true

.github/ISSUE_TEMPLATE/crash.md

@@ -1,12 +0,0 @@
----
-name: Crash
-about: Did OpenTTD crash?
-title: "Crash Report"
----
-<!-- Please zip the crash.log, crash.dmp and crash.sav and attach it to this crash report. -->
-
-## Version of OpenTTD
-<!-- Fill in below what version of OpenTTD you are using, including your OS. -->
-
-## Steps to reproduce
-<!-- Please spend a few words if you can reproduce this problem. -->

.github/ISSUE_TEMPLATE/crash.yaml

@@ -0,0 +1,37 @@
+name: Crash
+description: Did OpenTTD crash?
+title: "[Crash]: "
+body:
+  - type: markdown
+    attributes:
+      value: |
+        Thanks for taking the time to fill out this crash report!
+  - type: input
+    id: version
+    attributes:
+      label: Version of OpenTTD
+      description: Fill in below what version of OpenTTD you are using, including your OS.
+      placeholder: ex. 1.11.2, Windows 10
+    validations:
+      required: true
+  - type: textarea
+    id: reproduce
+    attributes:
+      label: Steps to reproduce
+      description: Please spend a few words if you can reproduce this problem.
+      placeholder: |
+        1. Bought a new train.
+        2. The game crashed.
+    validations:
+      required: true
+  - type: textarea
+    id: crashlogs
+    attributes:
+      label: Upload crash files
+      description: With the `crash.log`, `crash.dmp`, and `crash.sav` we can analyze the crash in detail; this way you allow us to easier triage and fix the problem.
+      placeholder: |
+        1. Zip the `crash.log`, `crash.dmp` and `crash.sav`.
+        2. Click on this field.
+        3. Drag and drop the zip file in here.
+    validations:
+      required: true

.github/PULL_REQUEST_TEMPLATE.md

@@ -39,6 +39,7 @@ Describe here
 
 Some things are not automated, and forgotten often. This list is a reminder for the reviewers.
 * The bug fix is important enough to be backported? (label: 'backport requested')
+* This PR touches english.txt or translations? Check the [guidelines](https://github.com/OpenTTD/OpenTTD/blob/master/docs/eints.md)
 * This PR affects the save game format? (label 'savegame upgrade')
 * This PR affects the GS/AI API? (label 'needs review: Script API')
     * ai_changelog.hpp, gs_changelog.hpp need updating.

.github/changelog.sh

@@ -4,9 +4,9 @@ tag=$(git name-rev --name-only --tags --no-undefined HEAD 2>/dev/null | sed 's@\
 
 # If we are a tag, show the part of the changelog till (but excluding) the last stable
 if [ -n "$tag" ]; then
-    grep='^[0-9]\+\.[0-9]\+\.[0-9]\+[^-]'
+    grep='^[0-9]\+\.[0-9]\+[^-]'
     next=$(cat changelog.txt | grep '^[0-9]' | awk 'BEGIN { show="false" } // { if (show=="true") print $0; if ($1=="'$tag'") show="true"} ' | grep "$grep" | head -n1 | sed 's/ .*//')
-    cat changelog.txt | awk 'BEGIN { show="false" } /^[0-9]+.[0-9]+.[0-9]+/ { if ($1=="'$next'") show="false"; if ($1=="'$tag'") show="true";} // { if (show=="true") print $0 }'
+    cat changelog.txt | awk 'BEGIN { show="false" } /^[0-9]+.[0-9]+/ { if ($1=="'$next'") show="false"; if ($1=="'$tag'") show="true";} // { if (show=="true") print $0 }'
     exit 0
 fi
 

.github/unused-strings.py

@@ -0,0 +1,221 @@
+"""
+Script to scan the OpenTTD source-tree for STR_ entries that are defined but
+no longer used.
+
+This is not completely trivial, as OpenTTD references a lot of strings in
+relation to another string. The most obvious example of this is a list. OpenTTD
+only references the first entry in the list, and does "+ <var>" to get to the
+correct string.
+
+There are other ways OpenTTD does use relative values. This script tries to
+account for all of them, to give the best approximation we have for "this
+string is unused".
+"""
+
+import glob
+import os
+import re
+import subprocess
+import sys
+
+from enum import Enum
+
+LENGTH_NAME_LOOKUP = {
+    "VEHICLE_TYPES": 4,
+}
+
+
+class SkipType(Enum):
+    NONE = 1
+    LENGTH = 2
+    EXTERNAL = 3
+    ZERO_IS_SPECIAL = 4
+    EXPECT_NEWLINE = 5
+
+
+def read_language_file(filename, strings_found, errors):
+    strings_defined = []
+
+    skip = SkipType.NONE
+    length = 0
+    common_prefix = ""
+    last_tiny_string = ""
+
+    with open(filename) as fp:
+        for line in fp.readlines():
+            if not line.strip():
+                if skip == SkipType.EXPECT_NEWLINE:
+                    skip = SkipType.NONE
+                continue
+
+            line = line.strip()
+
+            if skip == SkipType.EXPECT_NEWLINE:
+                # The only thing allowed after a list, is this next marker, or a newline.
+                if line == "###next-name-looks-similar":
+                    # "###next-name-looks-similar"
+                    # Indicates the common prefix of the last list has a very
+                    # similar name to the next entry, but isn't part of the
+                    # list. So do not emit a warning about them looking very
+                    # similar.
+
+                    if length != 0:
+                        errors.append(f"ERROR: list around {name} is shorted than indicated by ###length")
+
+                    common_prefix = ""
+                else:
+                    errors.append(f"ERROR: expected a newline after a list, but didn't find any around {name}. Did you add an entry to the list without increasing the length?")
+
+                skip = SkipType.NONE
+
+            if line[0] == "#":
+                if line.startswith("###length "):
+                    # "###length <count>"
+                    # Indicates the next few entries are part of a list. Only
+                    # the first entry is possibly referenced, and the rest are
+                    # indirectly.
+
+                    if length != 0:
+                        errors.append(f"ERROR: list around {name} is shorted than indicated by ###length")
+
+                    length = line.split(" ")[1].strip()
+
+                    if length.isnumeric():
+                        length = int(length)
+                    else:
+                        length = LENGTH_NAME_LOOKUP[length]
+
+                    skip = SkipType.LENGTH
+                elif line.startswith("###external "):
+                    # "###external <count>"
+                    # Indicates the next few entries are used outside the
+                    # source and will not be referenced.
+
+                    if length != 0:
+                        errors.append(f"ERROR: list around {name} is shorted than indicated by ###length")
+
+                    length = line.split(" ")[1].strip()
+                    length = int(length)
+
+                    skip = SkipType.EXTERNAL
+                elif line.startswith("###setting-zero-is-special"):
+                    # "###setting-zero-is-special"
+                    # Indicates the next entry is part of the "zero is special"
+                    # flag of settings. These entries are not referenced
+                    # directly in the code.
+
+                    if length != 0:
+                        errors.append(f"ERROR: list around {name} is shorted than indicated by ###length")
+
+                    skip = SkipType.ZERO_IS_SPECIAL
+
+                continue
+
+            name = line.split(":")[0].strip()
+            strings_defined.append(name)
+
+            # If a string ends on _TINY or _SMALL, it can be the {TINY} variant.
+            # Check for this by some fuzzy matching.
+            if name.endswith(("_SMALL", "_TINY")):
+                last_tiny_string = name
+            elif last_tiny_string:
+                matching_name = "_".join(last_tiny_string.split("_")[:-1])
+                if name == matching_name:
+                    strings_found.add(last_tiny_string)
+            else:
+                last_tiny_string = ""
+
+            if skip == SkipType.EXTERNAL:
+                strings_found.add(name)
+                skip = SkipType.LENGTH
+
+            if skip == SkipType.LENGTH:
+                skip = SkipType.NONE
+                length -= 1
+                common_prefix = name
+            elif skip == SkipType.ZERO_IS_SPECIAL:
+                strings_found.add(name)
+            elif length > 0:
+                strings_found.add(name)
+                length -= 1
+
+                # Find the common prefix of these strings
+                for i in range(len(common_prefix)):
+                    if common_prefix[0 : i + 1] != name[0 : i + 1]:
+                        common_prefix = common_prefix[0:i]
+                        break
+
+                if length == 0:
+                    skip = SkipType.EXPECT_NEWLINE
+
+                    if len(common_prefix) < 6:
+                        errors.append(f"ERROR: common prefix of block including {name} was reduced to {common_prefix}. This means the names in the list are not consistent.")
+            elif common_prefix:
+                if name.startswith(common_prefix):
+                    errors.append(f"ERROR: {name} looks a lot like block above with prefix {common_prefix}. This mostly means that the list length was too short. Use '###next-name-looks-similar' if it is not.")
+                common_prefix = ""
+
+    return strings_defined
+
+
+def scan_source_files(path, strings_found):
+    for new_path in glob.glob(f"{path}/*"):
+        if os.path.isdir(new_path):
+            scan_source_files(new_path, strings_found)
+            continue
+
+        if not new_path.endswith((".c", ".h", ".cpp", ".hpp", ".ini")):
+            continue
+
+        # Most files we can just open, but some use magic, that requires the
+        # G++ preprocessor before we can make sense out of it.
+        if new_path == "src/table/cargo_const.h":
+            p = subprocess.run(["g++", "-E", new_path], stdout=subprocess.PIPE)
+            output = p.stdout.decode()
+        else:
+            with open(new_path) as fp:
+                output = fp.read()
+
+        # Find all the string references.
+        matches = re.findall(r"[^A-Z_](STR_[A-Z0-9_]*)", output)
+        strings_found.update(matches)
+
+
+def main():
+    strings_found = set()
+    errors = []
+
+    scan_source_files("src", strings_found)
+    strings_defined = read_language_file("src/lang/english.txt", strings_found, errors)
+
+    # STR_LAST_STRINGID is special, and not really a string.
+    strings_found.remove("STR_LAST_STRINGID")
+    # These are mentioned in comments, not really a string.
+    strings_found.remove("STR_XXX")
+    strings_found.remove("STR_NEWS")
+    strings_found.remove("STR_CONTENT_TYPE_")
+
+    # This string is added for completion, but never used.
+    strings_defined.remove("STR_JUST_DATE_SHORT")
+
+    strings_defined = sorted(strings_defined)
+    strings_found = sorted(list(strings_found))
+
+    for string in strings_found:
+        if string not in strings_defined:
+            errors.append(f"ERROR: {string} found but never defined.")
+
+    for string in strings_defined:
+        if string not in strings_found:
+            errors.append(f"ERROR: {string} is (possibly) no longer needed.")
+
+    if errors:
+        for error in errors:
+            print(error)
+        sys.exit(1)
+    else:
+        print("OK")
+
+
+if __name__ == "__main__":
+    main()

.github/workflows/ci-build.yml

@@ -108,7 +108,7 @@ jobs:
 
     runs-on: macos-latest
     env:
-      MACOSX_DEPLOYMENT_TARGET: 10.9
+      MACOSX_DEPLOYMENT_TARGET: 10.14
 
     steps:
     - name: Checkout

.github/workflows/commit-checker.yml

@@ -15,37 +15,7 @@ jobs:
         fetch-depth: 4
 
     - name: Get pull-request commits
-      run: |
-        set -x
-        # actions/checkout did a merge checkout of the pull-request. As such, the first
-        # commit is the merge commit. This means that on HEAD^ is the base branch, and
-        # on HEAD^2 are the commits from the pull-request. We now check if those trees
-        # have a common parent. If not, we fetch a few more commits till we do. In result,
-        # the log between HEAD^ and HEAD^2 will be the commits in the pull-request.
-        DEPTH=4
-        while [ -z "$(git merge-base HEAD^ HEAD^2)" ]; do
-            # Prevent infinite recursion
-            if [ ${DEPTH} -gt 256 ]; then
-                echo "No common parent between '${GITHUB_HEAD_REF}' and '${GITHUB_BASE_REF}'." >&2
-                exit 1
-            fi
-
-            git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --deepen=${DEPTH} origin HEAD
-            DEPTH=$(( ${DEPTH} * 4 ))
-        done
-
-        # Just to show which commits we are going to evaluate.
-        git log --oneline HEAD^..HEAD^2
-
-    - name: Checkout commit-checker
-      uses: actions/checkout@v2
-      with:
-        repository: OpenTTD/OpenTTD-git-hooks
-        path: git-hooks
-        ref: master
+      uses: OpenTTD/actions/checkout-pull-request@v2
 
     - name: Check commits
-      run: |
-        set -x
-        HOOKS_DIR=./git-hooks/hooks GIT_DIR=.git ./git-hooks/hooks/check-commits.sh HEAD^..HEAD^2
-        echo "Commit checks passed"
+      uses: OpenTTD/OpenTTD-git-hooks@main

.github/workflows/release.yml

@@ -373,14 +373,19 @@ jobs:
         include:
         - container_image: "ubuntu:18.04"
           bundle_name: "bionic"
+          compiler: "g++-8"
         - container_image: "ubuntu:20.04"
           bundle_name: "focal"
+          compiler: "g++"
         - container_image: "ubuntu:20.10"
           bundle_name: "groovy"
+          compiler: "g++"
         - container_image: "debian:buster"
           bundle_name: "buster"
+          compiler: "g++"
         - container_image: "debian:bullseye"
           bundle_name: "bullseye"
+          compiler: "g++"
 
     runs-on: ubuntu-20.04
     container:
@@ -406,7 +411,7 @@ jobs:
         apt-get install -y --no-install-recommends \
           cmake \
           debhelper \
-          g++ \
+          ${{ matrix.compiler }} \
           git \
           make \
           openssl \
@@ -432,7 +437,7 @@ jobs:
         cd build
 
         echo "::group::CMake"
-        cmake ${GITHUB_WORKSPACE} \
+        CXX=${{ matrix.compiler }} cmake ${GITHUB_WORKSPACE} \
           -DCMAKE_BUILD_TYPE=RelWithDebInfo \
           -DCMAKE_INSTALL_PREFIX=/usr \
           # EOF
@@ -470,7 +475,7 @@ jobs:
 
     runs-on: macos-10.15
     env:
-      MACOSX_DEPLOYMENT_TARGET: 10.9
+      MACOSX_DEPLOYMENT_TARGET: 10.14
 
     steps:
     - name: Download source
@@ -746,6 +751,21 @@ jobs:
       with:
         arch: ${{ matrix.host }}
 
+    - name: Import code signing certificate
+      shell: powershell
+      # If this is run on a fork, there may not be a certificate set up - continue in this case
+      continue-on-error: true
+      run: |
+        $tempFile = [System.IO.Path]::GetTempFileName()
+        $bytes = [System.Convert]::FromBase64String($env:WINDOWS_CERTIFICATE_P12)
+        [IO.File]::WriteAllBytes($tempFile, $bytes)
+        $pwd = ConvertTo-SecureString $env:WINDOWS_CERTIFICATE_PASSWORD -AsPlainText -Force
+        Import-PfxCertificate -FilePath $tempFile -CertStoreLocation Cert:\CurrentUser\My -Password $pwd
+        Remove-Item $tempFile
+      env:
+        WINDOWS_CERTIFICATE_P12: ${{ secrets.WINDOWS_CERTIFICATE_P12 }}
+        WINDOWS_CERTIFICATE_PASSWORD: ${{ secrets.WINDOWS_CERTIFICATE_PASSWORD }}
+
     - name: Build (with installer)
       if: needs.source.outputs.is_tag == 'true'
       shell: bash
@@ -761,12 +781,15 @@ jobs:
           -DOPTION_USE_NSIS=ON \
           -DHOST_BINARY_DIR=${GITHUB_WORKSPACE}/build-host \
           -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+          -DWINDOWS_CERTIFICATE_COMMON_NAME="${WINDOWS_CERTIFICATE_COMMON_NAME}" \
           # EOF
         echo "::endgroup::"
 
         echo "::group::Build"
         cmake --build .
         echo "::endgroup::"
+      env:
+        WINDOWS_CERTIFICATE_COMMON_NAME: ${{ secrets.WINDOWS_CERTIFICATE_COMMON_NAME }}
 
     - name: Build (without installer)
       if: needs.source.outputs.is_tag != 'true'
@@ -782,12 +805,15 @@ jobs:
           -DCMAKE_TOOLCHAIN_FILE="c:\vcpkg\scripts\buildsystems\vcpkg.cmake" \
           -DHOST_BINARY_DIR=${GITHUB_WORKSPACE}/build-host \
           -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+          -DWINDOWS_CERTIFICATE_COMMON_NAME="${WINDOWS_CERTIFICATE_COMMON_NAME}" \
           # EOF
         echo "::endgroup::"
 
         echo "::group::Build"
         cmake --build .
         echo "::endgroup::"
+      env:
+        WINDOWS_CERTIFICATE_COMMON_NAME: ${{ secrets.WINDOWS_CERTIFICATE_COMMON_NAME }}
 
     - name: Create bundles
       shell: bash
@@ -809,6 +835,17 @@ jobs:
         rm -f bundles/*.sha256
         echo "::endgroup::"
 
+    - name: Sign installer
+      if: needs.source.outputs.is_tag == 'true'
+      shell: bash
+      # If this is run on a fork, there may not be a certificate set up - continue in this case
+      continue-on-error: true
+      run: |
+        cd ${GITHUB_WORKSPACE}/build/bundles
+        ../../os/windows/sign.bat *.exe "${WINDOWS_CERTIFICATE_COMMON_NAME}"
+      env:
+        WINDOWS_CERTIFICATE_COMMON_NAME: ${{ secrets.WINDOWS_CERTIFICATE_COMMON_NAME }}
+
     - name: Store bundles
       uses: actions/upload-artifact@v2
       with:

.github/workflows/unused-strings.yml

@@ -0,0 +1,18 @@
+name: Unused strings
+
+on:
+  pull_request:
+
+jobs:
+  unused-strings:
+    name: Unused strings
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v2
+
+    - name: Check for unused strings
+      run: |
+        set -ex
+        python3 .github/unused-strings.py