Godofgrunts collision checks (#485)

* Added checking script for collision layers/mask that have 1 set. Also added disabled workflow.

* Activate check-collision-mask-layers action

* Move collision mask check script into scripts/tools/

* Fix script

- Do NOT use blindly OS.execute ⚠️ are you kidding me?
- Actually parse the files correctly
- Do not print several times the same warnings

* Split workflow into two parts

Co-authored-by: GodofGrunts <dergottdergrunten@gmail.com>

.github/workflows/collision-layers-and-masks-check.yml

@@ -0,0 +1,51 @@
+name: Check collision layers and masks
+
+on:
+  pull_request:
+    paths-ignore: # do not build for game-irrelevant changes
+      - 'LICENSE'
+      - 'ACKNOWLEDGEMENTS'
+      - '**.md'
+jobs:
+  check_collision_layers_and_masks:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout latest code
+        uses: actions/checkout@v2
+
+      - name: Setup Godot
+        uses: vitorgus/setup-godot@v1
+        with:
+          godot-version: 3.4.4
+          download-templates: false
+
+      - name: Run checks on collision_layer and collision_mask
+        run: |
+          mkdir layers-and-masks
+          godot --script "scripts/tools/check_collision_layers_and_masks.gd" --quit | grep -i "collision" >> layers-and-masks/all
+          WARNINGS=$(wc -l layers-and-masks/all | awk '{print $1}')
+          echo "WARNINGS=$WARNINGS" >> $GITHUB_ENV
+          echo 
+
+      - name: Retrieve PR files
+        if: ${{ env.WARNINGS != 0 }}
+        id: changed-files
+        uses: tj-actions/changed-files@v18.7
+
+      - name: Determine if PR files have raised warnings
+        if: ${{ env.WARNINGS != 0 }}
+        run: |
+          for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
+            grep -F "$file" layers-and-masks/all >> layers-and-masks/report || true
+          done
+
+      - name: Save PR metadata
+        env:
+          PR_NUMBER: ${{ github.event.number }}
+        run: |
+          echo $PR_NUMBER > ./layers-and-masks/pr_number
+
+      - uses: actions/upload-artifact@v3
+        with:
+          name: layers-and-masks
+          path: layers-and-masks

.github/workflows/collision-layers-and-masks-report.yml

@@ -0,0 +1,62 @@
+name: Report collision layers and masks warnings
+
+on:
+  workflow_run:
+    workflows: ["Check collision layers and masks"]
+    types:
+      - completed
+
+jobs:
+  report:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Download report artifact
+        uses: actions/github-script@v3.1.0
+        with:
+          script: |
+            var artifacts = await github.actions.listWorkflowRunArtifacts({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               run_id: ${{ github.event.workflow_run.id }},
+            });
+            var matchArtifact = artifacts.data.artifacts.filter((artifact) => {
+              return artifact.name == "layers-and-masks"
+            })[0];
+            var download = await github.actions.downloadArtifact({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               artifact_id: matchArtifact.id,
+               archive_format: 'zip',
+            });
+            var fs = require('fs');
+            fs.writeFileSync('${{ github.workspace }}/layers-and-masks.zip', Buffer.from(download.data));
+
+      - name: Unzip artifact
+        run: unzip -d layers-and-masks layers-and-masks.zip && rm layers-and-masks.zip
+
+      - name: Store PR and report info in ENV
+        run: |
+          PR_NUMBER=$(cat layers-and-masks/pr_number)
+          echo "PR_NUMBER=$PR_NUMBER" >> $GITHUB_ENV
+          REPORT_EXISTS=$(test -s layers-and-masks/report; echo $?)
+          echo "REPORT_EXISTS=$REPORT_EXISTS" >> $GITHUB_ENV
+
+      - name: Comment on the PR
+        if: ${{ env.REPORT_EXISTS == 0 }}
+        uses: actions/github-script@v3
+        with:
+          script: |
+            var fs = require('fs');
+            var wikiUrl = '${{ github.server_url }}/${{ github.repository }}/wiki/Coding-tips-and-best-practices#collision-layers-and-masks=';
+            var report = fs.readFileSync('layers-and-masks/report', 'utf-8');
+            var body = '👀 There seem to be misconfigured collision properties in this PR. '+
+                       'Check out the [wiki](' + wikiUrl + ') for more information.\n\n' +
+                       '| File | Node | Type |  |\n|-|-|-|-|\n' + report;
+            await github.issues.createComment({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              issue_number: process.env.PR_NUMBER,
+              body: body,
+            });
+
+

scripts/tools/check_collision_layers_and_masks.gd

@@ -0,0 +1,117 @@
+# This file is solely used for a GitHub Action and can be ignored for game developers.
+# If you want to run this locally, navigate to the root of the project and run
+# godot --script scripts/tools/collision_mask_check.gd --quit
+# This file will recursively parse every scene (.tscn) in the current directory structure,
+# grab every node native to that scene (meaning it will skip nodes that are instanced), 
+# and check to see if its properties contain the collision_layer and/or collision_mask properties
+# If it does, it will check to see if those properties contain an odd number in the .tscn file.
+# If it is an odd number or the setting does not exist (1 is the default setting and doesn't show up),
+# it will print out information informing the user of the problems found.
+# The GitHub action will manipulate this data to only show the files the submitter has changed
+# in their pull request.
+
+extends SceneTree
+
+var type_cache: Dictionary = {}
+
+func _init() -> void:
+	var files: Array = list_files_in_directory()
+	parse_files(files)
+	quit()
+
+# Recursively search the current directory and return a file list array
+func list_files_in_directory() -> Array:
+	var dir_queue: Array = ["res://"]
+	var dir: Directory
+	var files: Array = []
+	var file: String
+
+	while file or not dir_queue.empty():
+		if file:
+			if dir.current_is_dir():
+				dir_queue.append("%s/%s" % [dir.get_current_dir(), file])
+			elif file.ends_with(".tscn"):
+				files.append("%s/%s" % [dir.get_current_dir(), file.get_file()])
+		else:
+			if dir:
+				dir.list_dir_end()
+			if dir_queue.empty():
+				break
+			dir = Directory.new()
+			dir.open(dir_queue.pop_front())
+			dir.list_dir_begin(true, true)
+		file = dir.get_next()
+	return files
+
+# Read each file in the files array, parse information needed
+func parse_files(files: Array) -> void:
+	var header_regex: RegEx = RegEx.new()
+	header_regex.compile("^\\[node name=\"(?<name>[^\"]+)\" type=\"(?<type>[^\"]+)\"")
+	for file in files:
+		var content: String = read_file(file)
+		for section in extract_node_sections(content):
+			var result: RegExMatch = header_regex.search(section.substr(0, section.find("\n")))
+			if not result:
+				continue
+			search_node(result.get_string("name"), result.get_string("type"), file, section)
+
+# Actually read the file
+func read_file(file: String) -> String:
+	var full_text: String
+	var f: File = File.new()
+	var err: int = f.open(file, File.READ)
+	if err != OK:
+		print("Error reading file %s" % str(f))
+	else:
+		full_text = f.get_as_text()
+	return full_text
+
+# Split the file content into node sections
+func extract_node_sections(text: String) -> Array:
+	var sections: Array = []
+	var from: int = -1
+	var idx: int = text.find("[node name=")
+	while idx >= 0:
+		if from >= 0:
+			sections.append(text.substr(from, idx - from - 1))
+		from = idx
+		idx = text.find("[node name=", idx + 1)
+	if from >= 0:
+		sections.append(text.substr(from, len(text) - from))
+	return sections
+
+# Spin up each node_type and search its property list for collision_mask/layer.
+# If found pass the information along to find_bits
+func search_node(node_name, node_type, file, section) -> void:
+	if node_type in type_cache:
+		for domain in type_cache[node_type]:
+			find_bits(node_name, node_type, file, domain, section)
+		return
+
+	type_cache[node_type] = []
+
+	var script: GDScript = GDScript.new()
+	script.set_source_code("func eval():\n\treturn %s.new()" % node_type)
+	script.reload()
+
+	var evaluator: Object = Reference.new()
+	evaluator.set_script(script)
+
+	for item in evaluator.eval().get_property_list():
+		for domain in ['collision_layer', 'collision_mask']:
+			if item['name'] == domain:
+				type_cache[node_type].append(domain)
+				find_bits(node_name, node_type, file, domain, section)
+
+# Search the section for the collision_domain and find its bit value
+# If nothing is found, assume the bit value is 1 which is the default
+# Find the modulo of the bit_num. If it isn't 0 we know the bit_num is odd.
+# If bit_num is odd, print out information
+func find_bits(node_name, node_type, file, collision_domain, section) -> void:
+	var bit_num : int = 1
+	for line in section.split("\n"):
+		if line.begins_with(collision_domain):
+			bit_num = line.split(" = ", false, 2)[1].to_int()
+			break
+	if bit_num % 2 != 0:
+		print("|%s|%s|%s|%s is set to %d|" % [file, node_name, node_type, collision_domain, bit_num])