From 150e98c2852fb53c7bfad5be73bb4b0aedcbb8e7 Mon Sep 17 00:00:00 2001 From: Paul Joannon <437025+paulloz@users.noreply.github.com> Date: Wed, 4 May 2022 04:45:23 +0200 Subject: [PATCH] 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 :warning: 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 --- .../collision-layers-and-masks-check.yml | 51 ++++++++ .../collision-layers-and-masks-report.yml | 62 ++++++++++ .../tools/check_collision_layers_and_masks.gd | 117 ++++++++++++++++++ 3 files changed, 230 insertions(+) create mode 100644 .github/workflows/collision-layers-and-masks-check.yml create mode 100644 .github/workflows/collision-layers-and-masks-report.yml create mode 100644 scripts/tools/check_collision_layers_and_masks.gd diff --git a/.github/workflows/collision-layers-and-masks-check.yml b/.github/workflows/collision-layers-and-masks-check.yml new file mode 100644 index 000000000..acd78e047 --- /dev/null +++ b/.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 diff --git a/.github/workflows/collision-layers-and-masks-report.yml b/.github/workflows/collision-layers-and-masks-report.yml new file mode 100644 index 000000000..8e4194033 --- /dev/null +++ b/.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, + }); + + diff --git a/scripts/tools/check_collision_layers_and_masks.gd b/scripts/tools/check_collision_layers_and_masks.gd new file mode 100644 index 000000000..c3a864be8 --- /dev/null +++ b/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=\"(?[^\"]+)\" 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])