Upgrade project environment#754
Merged
Merged
Conversation
Update the gdunit4_tests.yml workflow to use the Godot 4.6.3-stable binary for executing unit tests.
Update the gut_tests.yml workflow to use the Godot 4.6.3-stable binary for executing unit tests.
Update the browser_test.yml workflow to utilize the Godot 4.6.3-stable engine binary and export templates for Web functional testing.
Update the deploy_to_itch.yml workflow to utilize the Godot 4.6.3-stable engine binary and export templates for production builds.
Update the Dockerfile to utilize the Godot 4.6.3-stable engine binary and export templates to ensure the CI/CD container environment is synchronized with the new project standards.
Upgrade the core development environment from Godot 4.5.stable to Godot 4.6.3.stable. This update is expected to address compatibility constraints with newer community add-ons and may resolve existing API filtering issues that currently block framework integration within the Asset Library.
ikostan
added this to the
Milestone 20: Resource-Based Settings & UI Acoustic Integration milestone
Contributor
Reviewer's GuideUpgrades the project environment from Godot 4.5 to 4.6.3, adding checksum-verified engine downloads, introducing a legacy 4.5 Docker image, hardening audio management and encrypted ConfigFile tests, and improving local/CI GUT and GDUnit4 test workflows and documentation. Sequence diagram for AudioManager apply_volume_to_bus safety guardssequenceDiagram
participant AudioManager
participant AudioServer
AudioManager->>AudioManager: apply_volume_to_bus(bus_name, volume, muted)
AudioManager->>AudioManager: is_instance_valid(AudioServer)?
alt AudioServer not valid
AudioManager-->>AudioManager: return
else AudioServer valid
AudioManager->>AudioServer: get_bus_index(bus_name)
AudioServer-->>AudioManager: bus_idx
alt bus_idx != -1
AudioManager->>AudioServer: set_bus_volume_db(bus_idx, linear_to_db(volume))
alt muted
AudioManager->>AudioServer: set_bus_mute(bus_idx, true)
else not muted
AudioManager->>AudioServer: set_bus_mute(bus_idx, false)
end
else bus_idx == -1
AudioManager-->>AudioManager: skip audio bus update
end
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpgrades tooling and assets from Godot 4.5-stable to 4.6.3-stable: CI workflows and Dockerfile now fetch 4.6.3, project files and scenes reflect the newer format, tests add encryption-workaround fixtures and audio test helpers, and the legacy 4.5 Dockerfile is archived under obsolete/. ChangesGodot 4.5 to 4.6.3 Upgrade
Sequence DiagramsequenceDiagram
participant DevEnv as Dockerfile
participant CI as GitHubWorkflows
participant Tests as GUT/GDUnit4 tests
participant Godot as "Godot 4.6.3-stable release"
CI->>Godot: download engine binary & export templates (sha256 verified where present)
DevEnv->>Godot: download engine binary & templates for image
Tests->>DevEnv: run headless engine (uses installed 4.6.3)
Tests->>Godot: rely on 4.6.3 behavior for test fixtures
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was
linked to
issues
Jun 12, 2026
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
.github/workflows/browser_test.yml, thegodot_export_templates_download_urlhas ahhttps://typo that will break the templates download step. - In
.github/workflows/gdunit4_tests.yml, the wget URL was updated to 4.6.3 but theunzipandmvcommands still reference the 4.5 archive and directory names, which will fail at runtime; these should be updated to the 4.6.3 filenames. - Similarly, in
.github/workflows/gut_tests.ymlthe Godot download URL is 4.6.3 but the subsequentunzipandmvcommands still reference 4.5, so the filenames and directory names need to be aligned with the new version.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `.github/workflows/browser_test.yml`, the `godot_export_templates_download_url` has a `hhttps://` typo that will break the templates download step.
- In `.github/workflows/gdunit4_tests.yml`, the wget URL was updated to 4.6.3 but the `unzip` and `mv` commands still reference the 4.5 archive and directory names, which will fail at runtime; these should be updated to the 4.6.3 filenames.
- Similarly, in `.github/workflows/gut_tests.yml` the Godot download URL is 4.6.3 but the subsequent `unzip` and `mv` commands still reference 4.5, so the filenames and directory names need to be aligned with the new version.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python |  |
Jun 17, 2026 3:38a.m. | Review ↗ |
| JavaScript | |
Jun 17, 2026 3:38a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
obsolete/godot_4.5_stable/Dockerfile (1)
73-76: 💤 Low valueRedundant system dependency installation.
Line 75 runs
playwright install-deps, then line 76 runsplaywright install --with-deps chromium. The--with-depsflag already installs system dependencies, making line 75 redundant.For a cleaner build, remove the redundant call:
♻️ Suggested simplification
# Install Playwright Python packages and system deps (as root) RUN pip install playwright pytest-playwright pytest-asyncio \ - && playwright install-deps \ && playwright install --with-deps chromium🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@obsolete/godot_4.5_stable/Dockerfile` around lines 73 - 76, Remove the redundant system dependency installation by deleting the standalone "playwright install-deps" invocation; keep the single command "playwright install --with-deps chromium" (which already installs system deps) so the RUN block only runs pip install ... and then playwright install --with-deps chromium.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/browser_test.yml:
- Around line 50-51: The godot_export_templates_download_url value is malformed
(it starts with "hhttps://") so the export action cannot download templates; fix
the string assigned to godot_export_templates_download_url to the correct URL
("https://github.com/godotengine/godot/releases/download/4.6.3-stable/Godot_v4.6.3-stable_export_templates.tpz")
ensuring it matches the pattern used by godot_executable_download_url and
removes the extra leading 'h' so the firebelley/godot-export step can
successfully fetch export templates.
In @.github/workflows/gdunit4_tests.yml:
- Around line 21-24: The unzip and mv steps still reference
Godot_v4.5-stable_linux.x86_64 while the download uses Godot_v4.6.3-stable;
update the archive and directory names in the install block so they match the
downloaded file: replace occurrences of "Godot_v4.5-stable_linux.x86_64.zip" and
"Godot_v4.5-stable_linux.x86_64" with "Godot_v4.6.3-stable_linux.x86_64.zip" and
"Godot_v4.6.3-stable_linux.x86_64" respectively (the wget/unzip/mv commands in
the workflow install section), and make the same change in the other test
workflow to ensure the godot binary is created.
---
Nitpick comments:
In `@obsolete/godot_4.5_stable/Dockerfile`:
- Around line 73-76: Remove the redundant system dependency installation by
deleting the standalone "playwright install-deps" invocation; keep the single
command "playwright install --with-deps chromium" (which already installs system
deps) so the RUN block only runs pip install ... and then playwright install
--with-deps chromium.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 964d2d3d-d651-4024-a8ab-f8dda99b9416
📒 Files selected for processing (8)
.github/workflows/browser_test.yml.github/workflows/deploy_to_itch.yml.github/workflows/gdunit4_tests.yml.github/workflows/gut_tests.ymlDockerfiledefault_bus_layout.tresobsolete/godot_4.5_stable/Dockerfileproject.godot
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-04-28T02:11:45.806Z
Learnt from: ikostan
Repo: ikostan/SkyLockAssault PR: 588
File: .github/workflows/deploy_to_itch.yml:44-56
Timestamp: 2026-04-28T02:11:45.806Z
Learning: When a CI workflow edits Godot's `project.godot` (INI) to inject custom ProjectSettings values, insert the setting key under the correct section header that matches the `game/` (or other) root in the ProjectSettings path. For example, `ProjectSettings.get_setting("game/security/save_salt", ...)` expects the INI entry under `[game]` with key `security/save_salt` (i.e., `[game]` then `security/save_salt=...`), not under `[application]`. Otherwise the lookup will fall back to the default value at runtime.
Applied to files:
.github/workflows/deploy_to_itch.yml.github/workflows/browser_test.yml.github/workflows/gdunit4_tests.yml.github/workflows/gut_tests.yml
📚 Learning: 2026-05-20T00:01:27.632Z
Learnt from: ikostan
Repo: ikostan/SkyLockAssault PR: 654
File: .github/workflows/browser_test.yml:99-101
Timestamp: 2026-05-20T00:01:27.632Z
Learning: In this repository’s GitHub Actions workflows, treat supply-chain pinning as follows:
- **Do not flag** steps that use **first-party** GitHub-owned actions under `actions/*` (e.g., `actions/checkout`, `actions/cache`) when they use a **major version tag** like `v6` / `v5`.
- **Do flag** **third-party** actions (anything not under `actions/*`, e.g., `firebelley/godot-export`, `codecov/codecov-action`) when they use an unpinned ref such as `vX` or `main` instead of being pinned to a **commit SHA** (i.e., `@<commit-sha>`).
Applied to files:
.github/workflows/deploy_to_itch.yml.github/workflows/browser_test.yml.github/workflows/gdunit4_tests.yml.github/workflows/gut_tests.yml
🔇 Additional comments (11)
obsolete/godot_4.5_stable/Dockerfile (7)
1-10: LGTM!
12-21: LGTM!
23-40: LGTM!
42-55: LGTM!
57-71: LGTM!
78-82: LGTM!
84-92: LGTM!.github/workflows/deploy_to_itch.yml (1)
66-67: LGTM!Dockerfile (1)
20-21: LGTM!Also applies to: 42-55, 92-92
project.godot (1)
11-13: LGTM!Also applies to: 20-20
default_bus_layout.tres (1)
1-1: LGTM!
Owner
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
cleanup_for_testmethod onAudioManageris production-visible on an autoload; consider guarding it behind a debug/editor flag or moving test-only cleanup into a dedicated test helper to avoid accidental use in game code. - Godot version and SHA-256 values are now hard-coded in multiple places (Dockerfile, workflows); centralizing these into build args or workflow environment variables would make future upgrades less error-prone and easier to keep in sync.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `cleanup_for_test` method on `AudioManager` is production-visible on an autoload; consider guarding it behind a debug/editor flag or moving test-only cleanup into a dedicated test helper to avoid accidental use in game code.
- Godot version and SHA-256 values are now hard-coded in multiple places (Dockerfile, workflows); centralizing these into build args or workflow environment variables would make future upgrades less error-prone and easier to keep in sync.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The new cleanup_for_test method on AudioManager is production-visible on an autoload; consider guarding it behind a debug/editor flag or moving test-only cleanup into a dedicated test helper to avoid accidental use in game code.
Owner
Author
Completely agree. Leaving a destructive teardown method exposed on a core global Autoload is an accident waiting to happen. Since GDScript doesn't have a preprocessor for conditional compilation, I've added a hard runtime guard using OS.is_debug_build() along with a developer assert(). This ensures the method is completely inert in production release builds and will fail-fast during development if accidentally invoked in game code. |
Add inputs to GitHub Actions workflows to allow selecting the Godot version and checksum at invocation. .github/workflows/browser_test.yml: add godot_version input and parameterize release download URLs for the export step. .github/workflows/gdunit4_tests.yml and .github/workflows/gut_tests.yml: add godot_version and godot_sha256 inputs, export them to env (GODOT_VERSION/GODOT_SHA256), and update wget, checksum verification, unzip, and mv commands to use the variables. Defaults remain set to 4.6.3-stable and its SHA256. This makes the workflows flexible for testing different Godot releases without editing the workflow files.
Owner
Author
Implementing this will mean your next Godot version upgrade takes 10 seconds and requires changing just one or two lines of code, completely eliminating the risk of mismatched version strings breaking your runtime pipelines. |
Owner
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In
audio_manager.gd::cleanup_for_testtheOS.is_debug_build()guard followed immediately by anassert(OS.is_debug_build(), ...)is redundant because the early return prevents the assert from ever firing in non‑debug builds; consider keeping just one of these checks. - The Dockerfile comment "using trusted cryptographic signature" is misleading since the code only validates a hardcoded SHA-256 checksum, not a signed artifact; consider rewording to accurately describe the verification mechanism.
- The Godot 4.6.3 SHA-256 checksum is duplicated across the Dockerfile and multiple workflows; consider centralizing this value (e.g., via build args, reusable workflow inputs, or a shared env) to avoid drift on future version bumps.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `audio_manager.gd::cleanup_for_test` the `OS.is_debug_build()` guard followed immediately by an `assert(OS.is_debug_build(), ...)` is redundant because the early return prevents the assert from ever firing in non‑debug builds; consider keeping just one of these checks.
- The Dockerfile comment "using trusted cryptographic signature" is misleading since the code only validates a hardcoded SHA-256 checksum, not a signed artifact; consider rewording to accurately describe the verification mechanism.
- The Godot 4.6.3 SHA-256 checksum is duplicated across the Dockerfile and multiple workflows; consider centralizing this value (e.g., via build args, reusable workflow inputs, or a shared env) to avoid drift on future version bumps.
## Individual Comments
### Comment 1
<location path="scripts/managers/audio_manager.gd" line_range="504-516" />
<code_context>
+## Forcefully clear resources to prevent leaks between unit tests.
+## Strictly guarded to prevent accidental execution in production builds.
+func cleanup_for_test() -> void:
+ # 0. Production safety guards
+ if not OS.is_debug_build():
+ return
+ assert(
+ OS.is_debug_build(), "cleanup_for_test() should only be called in test/debug environments."
+ )
</code_context>
<issue_to_address>
**suggestion:** The `OS.is_debug_build()` check is duplicated before and inside the assert, making the assert effectively unreachable in release builds.
Because of the early return, this function is already a no-op in non-debug builds. The subsequent `assert(OS.is_debug_build(), ...)` only runs in debug builds, where the condition is always true, so it never guards against misuse. Consider removing the assert, or replacing it with a check on a different flag (e.g. `ENABLE_TEST_CLEANUP`) if you want an assertion that can actually fail when misused.
```suggestion
## FOR GUT UNIT TEST ONLY
## Forcefully clear resources to prevent leaks between unit tests.
## Strictly guarded to prevent accidental execution in production builds.
func cleanup_for_test() -> void:
# 0. Production safety guards
if not OS.is_debug_build():
return
_sfx_cache.clear()
_missing_sfx_cache.clear()
```
</issue_to_address>
### Comment 2
<location path=".github/workflows/deploy_to_itch.yml" line_range="66-67" />
<code_context>
with:
- godot_executable_download_url: "https://github.com/godotengine/godot-builds/releases/download/4.5-stable/Godot_v4.5-stable_linux.x86_64.zip"
- godot_export_templates_download_url: "https://github.com/godotengine/godot-builds/releases/download/4.5-stable/Godot_v4.5-stable_export_templates.tpz"
+ godot_executable_download_url: "https://github.com/godotengine/godot/releases/download/4.6.3-stable/Godot_v4.6.3-stable_linux.x86_64.zip"
+ godot_export_templates_download_url: "https://github.com/godotengine/godot/releases/download/4.6.3-stable/Godot_v4.6.3-stable_export_templates.tpz"
relative_project_path: "./"
relative_export_path: "./export/web"
</code_context>
<issue_to_address>
**🚨 suggestion (security):** The workflow now downloads Godot artifacts without integrity verification, unlike the main Dockerfile which uses a SHA-256 check.
Please add an integrity check for these downloads as well—e.g., verify a published checksum from a trusted source or hard-code the expected SHA-256 for this Godot version—before using the artifacts in the export step.
Suggested implementation:
```
- name: Download Godot with integrity check
run: |
set -euo pipefail
GODOT_EXECUTABLE_URL="https://github.com/godotengine/godot/releases/download/4.6.3-stable/Godot_v4.6.3-stable_linux.x86_64.zip"
GODOT_EXECUTABLE_SHA256="REPLACE_WITH_REAL_GODOT_EXECUTABLE_SHA256"
GODOT_EXPORT_TEMPLATES_URL="https://github.com/godotengine/godot/releases/download/4.6.3-stable/Godot_v4.6.3-stable_export_templates.tpz"
GODOT_EXPORT_TEMPLATES_SHA256="REPLACE_WITH_REAL_GODOT_EXPORT_TEMPLATES_SHA256"
curl -L "$GODOT_EXECUTABLE_URL" -o /tmp/godot_linux.x86_64.zip
echo "${GODOT_EXECUTABLE_SHA256} /tmp/godot_linux.x86_64.zip" | sha256sum -c -
curl -L "$GODOT_EXPORT_TEMPLATES_URL" -o /tmp/godot_export_templates.tpz
echo "${GODOT_EXPORT_TEMPLATES_SHA256} /tmp/godot_export_templates.tpz" | sha256sum -c -
id: "export"
uses: "firebelley/godot-export@615a6f7afc22d6266bf55c17748fb07447e8bdab"
with:
godot_executable_download_url: "file:///tmp/godot_linux.x86_64.zip"
godot_export_templates_download_url: "file:///tmp/godot_export_templates.tpz"
relative_project_path: "./"
relative_export_path: "./export/web"
archive_output: true
```
1. Replace `REPLACE_WITH_REAL_GODOT_EXECUTABLE_SHA256` and `REPLACE_WITH_REAL_GODOT_EXPORT_TEMPLATES_SHA256` with the actual SHA-256 checksums published by the Godot project for `Godot_v4.6.3-stable_linux.x86_64.zip` and `Godot_v4.6.3-stable_export_templates.tpz`.
2. Confirm that `firebelley/godot-export` accepts `file://` URLs (it typically just downloads the given URL; `file://` should work on GitHub-hosted runners). If not, you may need to fork or adjust the action to accept pre-downloaded paths instead of URLs.
</issue_to_address>
### Comment 3
<location path="files/docs/milestones/20/Part_2_Upgrade_project_environment_to_Godot_4_6_3.md" line_range="92" />
<code_context>
+**DeepsourceReview**: No explicit mentions, comments, or contributions from Deepsource (e.g., @deepsource-io or similar) were visible in the PR conversation, commits, or summaries. If Deepsource was configured for static analysis/linting, its review may have occurred implicitly via CI without a named bot comment.
</code_context>
<issue_to_address>
**suggestion (typo):** Align the capitalization of "DeepSource" consistently.
Use "DeepSource" consistently (e.g., "DeepSource Review") to match the official product name and keep terminology uniform across the docs.
```suggestion
**DeepSource Review**: No explicit mentions, comments, or contributions from DeepSource (e.g., @deepsource-io or similar) were visible in the PR conversation, commits, or summaries. If DeepSource was configured for static analysis/linting, its review may have occurred implicitly via CI without a named bot comment.
```
</issue_to_address>
### Comment 4
<location path="files/docs/milestones/20/Part_2_Upgrade_project_environment_to_Godot_4_6_3.md" line_range="57" />
<code_context>
+| Harden local and CI GUT/GDUnit4 test execution scripts for reliability and clearer diagnostics. | <ul><li>Enhance workspace/run_gut_unit_tests.sh to temporarily disable the gdUnit4 addon while running GUT tests, restoring it via a trap-based cleanup function, and narrow the test directory to res://test/gut/.</li><li>Update gdunit4_tests and gut_tests workflows to download Godot 4.6.3 with checksum verification, clean .godot cache before GUT runs, and enable verbose output for easier debugging of CI failures.</li></ul> | `workspace/run_gut_unit_tests.sh`<br/>`.github/workflows/gdunit4_tests.yml`<br/>`.github/workflows/gut_tests.yml` |
</code_context>
<issue_to_address>
**nitpick (typo):** Use consistent capitalization for GDUnit4 within this description.
In this row, the heading uses "GUT/GDUnit4" while the details use "gdUnit4 addon". Please standardize the framework name’s capitalization (e.g., "GDUnit4 addon"), keeping file names like `gdunit4_tests.yml` unchanged.
```suggestion
| Harden local and CI GUT/GDUnit4 test execution scripts for reliability and clearer diagnostics. | <ul><li>Enhance workspace/run_gut_unit_tests.sh to temporarily disable the GDUnit4 addon while running GUT tests, restoring it via a trap-based cleanup function, and narrow the test directory to res://test/gut/.</li><li>Update gdunit4_tests and gut_tests workflows to download Godot 4.6.3 with checksum verification, clean .godot cache before GUT runs, and enable verbose output for easier debugging of CI failures.</li></ul> | `workspace/run_gut_unit_tests.sh`<br/>`.github/workflows/gdunit4_tests.yml`<br/>`.github/workflows/gut_tests.yml` |
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…_Godot_4_6_3.md Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
…_Godot_4_6_3.md Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Add a workflow step that downloads the Godot 4.6.3-stable executable, export templates, and official SHA-256 checksums, verifies integrity with sha256sum, and starts a local Python HTTP server to host the files. Update the godot-export action to use localhost URLs so the verified binaries are used during the Web export. This ensures binary integrity and avoids direct external downloads at export time.
Owner
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The Godot version and SHA-256 checksum are now hardcoded in multiple places (Dockerfile, gdunit4_tests.yml, gut_tests.yml, deploy_to_itch.yml); consider centralizing them via build-args/ENV or reusable workflow inputs to avoid drift on the next upgrade.
- In the deploy_to_itch workflow, spinning up a local HTTP server just to re-serve the downloaded Godot artifacts adds complexity; you could keep using the GitHub URLs directly and still verify integrity via SHA256SUMS.txt to simplify the job.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Godot version and SHA-256 checksum are now hardcoded in multiple places (Dockerfile, gdunit4_tests.yml, gut_tests.yml, deploy_to_itch.yml); consider centralizing them via build-args/ENV or reusable workflow inputs to avoid drift on the next upgrade.
- In the deploy_to_itch workflow, spinning up a local HTTP server just to re-serve the downloaded Godot artifacts adds complexity; you could keep using the GitHub URLs directly and still verify integrity via SHA256SUMS.txt to simplify the job.
## Individual Comments
### Comment 1
<location path="scripts/managers/audio_manager.gd" line_range="507-516" />
<code_context>
+## FOR GUT UNIT TEST ONLY
+## Forcefully clear resources to prevent leaks between unit tests.
+## Strictly guarded to prevent accidental execution in production builds.
+func cleanup_for_test() -> void:
+ # 0. Production safety guards
+ if not OS.is_debug_build():
+ return
+
+ _sfx_cache.clear()
+ _missing_sfx_cache.clear()
+
+ # 1. Clean the pool deterministically
+ for i in range(_sfx_pool.size() - 1, -1, -1):
+ var player := _sfx_pool[i]
+ if is_instance_valid(player):
+ player.stop()
+ # Reset bus before freeing to avoid dangling references
+ player.bus = "Master"
+ if player.get_parent() == self:
+ remove_child(player)
+ player.free()
+ _sfx_pool.remove_at(i)
+
+ # 2. Re-create the pool
+ for i in range(SFX_POOL_SIZE):
+ var p := AudioStreamPlayer.new()
+ add_child(p)
+ _sfx_pool.append(p)
</code_context>
<issue_to_address>
**issue (testing):** `cleanup_for_test` recreates the SFX pool but may not restore the original configuration of the players.
Because this only does `AudioStreamPlayer.new()` and `add_child(p)`, any custom configuration done in the normal initialization path (bus, volume, autoload, groups, etc.) won’t be reapplied. That means the pool state in tests can diverge from real runtime behavior, and this helper would be unsafe to call outside a tightly controlled test context. Consider extracting a shared pool-initialization routine used by both `_ready()` and `cleanup_for_test` so the recreated pool exactly matches the production configuration.
</issue_to_address>
### Comment 2
<location path="Dockerfile" line_range="43-48" />
<code_context>
- && unzip Godot_v4.5-stable_linux.x86_64.zip \
- && mv Godot_v4.5-stable_linux.x86_64 /usr/local/bin/godot \
+# Download and verify Godot v4.6.3 binary using trusted cryptographic signature
+RUN wget -q https://github.com/godotengine/godot/releases/download/4.6.3-stable/Godot_v4.6.3-stable_linux.x86_64.zip \
+ && echo "d0bc2113065e481c9c2c2b2c37daa4e8be3fe9e27f0ab9ab0b6096e9a37907f3 Godot_v4.6.3-stable_linux.x86_64.zip" | sha256sum --check --status \
+ && unzip Godot_v4.6.3-stable_linux.x86_64.zip \
+ && mv Godot_v4.6.3-stable_linux.x86_64 /usr/local/bin/godot \
&& chmod +x /usr/local/bin/godot \
- && rm Godot_v4.5-stable_linux.x86_64.zip
+ && rm Godot_v4.6.3-stable_linux.x86_64.zip
-# Download and extract export templates, placing them in the user-specific location
</code_context>
<issue_to_address>
**suggestion:** Using a hardcoded SHA-256 hash in the Dockerfile makes updates a bit brittle compared to consuming the official checksum file.
This does improve integrity, but it also tightly couples the Dockerfile to a specific checksum. When updating Godot, it’s easy to miss updating this hash. Consider mirroring the GitHub workflow: download the version’s official `SHA256SUMS` file and verify against it so you only need to change the version string, not an embedded hash.
Suggested implementation:
```
# Download and verify Godot v4.6.3 binary using the official SHA256SUMS file
RUN wget -q https://github.com/godotengine/godot/releases/download/4.6.3-stable/SHA256SUMS.txt \
&& wget -q https://github.com/godotengine/godot/releases/download/4.6.3-stable/Godot_v4.6.3-stable_linux.x86_64.zip \
&& grep " Godot_v4.6.3-stable_linux.x86_64.zip\$" SHA256SUMS.txt | sha256sum --check --status \
&& unzip Godot_v4.6.3-stable_linux.x86_64.zip \
&& mv Godot_v4.6.3-stable_linux.x86_64 /usr/local/bin/godot \
&& chmod +x /usr/local/bin/godot \
&& rm Godot_v4.6.3-stable_linux.x86_64.zip SHA256SUMS.txt
```
```
# Download, verify, and extract export templates using the official SHA256SUMS file
RUN wget -q https://github.com/godotengine/godot/releases/download/4.6.3-stable/SHA256SUMS.txt \
&& wget -q https://github.com/godotengine/godot/releases/download/4.6.3-stable/Godot_v4.6.3-stable_export_templates.tpz \
&& grep " Godot_v4.6.3-stable_export_templates.tpz\$" SHA256SUMS.txt | sha256sum --check --status \
&& mkdir -p "${XDG_DATA_HOME}/godot/export_templates/${GODOT_VERSION}" \
&& unzip Godot_v4.6.3-stable_export_templates.tpz -d /tmp/templates \
&& mv /tmp/templates/templates/* "${XDG_DATA_HOME}/godot/export_templates/${GODOT_VERSION}/" \
&& rm -rf /tmp/templates Godot_v4.6.3-stable_export_templates.tpz SHA256SUMS.txt \
&& chown -R godotuser:godotuser "${XDG_DATA_HOME}"
```
</issue_to_address>
### Comment 3
<location path=".github/workflows/deploy_to_itch.yml" line_range="84-88" />
<code_context>
+ echo "🔒 Verifying binary integrity against official signatures..."
+ sha256sum --check --ignore-missing SHA256SUMS.txt
+
+ echo "✅ Integrity check passed. Starting local HTTP server..."
+ nohup python3 -m http.server 8000 > /dev/null 2>&1 &
+
+ # Allow the background server a brief moment to initialize
+ sleep 2
+
- name: "Export Godot to Web"
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Relying on a local HTTP server for Godot artifacts adds moving parts and a potential race condition.
This approach also hard-codes a specific port and depends on server startup timing. Prefer configuring the export step to use the official HTTPS download URLs directly (with checksum verification handled earlier) or, if supported, local file paths. If you stick with a local HTTP server, replace the fixed `sleep` with a small readiness loop (e.g., curl `http://localhost:8000/` with a timeout) to avoid flakiness.
Suggested implementation:
```
echo "📥 Downloading Godot export templates..."
curl -fL --show-error -o Godot_v4.6.3-stable_export_templates.tpz "$TEMPLATE_URL"
echo "📥 Downloading official SHA-256 checksums..."
curl -fL --show-error -o SHA256SUMS.txt "$CHECKSUM_URL"
echo "🔒 Verifying binary integrity against official signatures..."
sha256sum --check --ignore-missing SHA256SUMS.txt
- name: "Export Godot to Web"
id: "export"
uses: "firebelley/godot-export@615a6f7afc22d6266bf55c17748fb07447e8bdab"
with:
godot_executable_download_url: "${{ env.GODOT_LINUX_DOWNLOAD_URL }}"
godot_export_templates_download_url: "${{ env.GODOT_EXPORT_TEMPLATES_DOWNLOAD_URL }}"
relative_project_path: "./"
relative_export_path: "./export/web"
archive_output: true
```
To fully implement the suggestion and avoid the local HTTP server:
1. Define `GODOT_LINUX_DOWNLOAD_URL` and `GODOT_EXPORT_TEMPLATES_DOWNLOAD_URL` as job- or workflow-level `env` values, pointing to the official HTTPS URLs for:
- The Godot Linux executable zip (`GODOT_LINUX_DOWNLOAD_URL`)
- The Godot export templates tpz (`GODOT_EXPORT_TEMPLATES_DOWNLOAD_URL`)
2. Update the earlier `curl` commands to use these env vars instead of shell-only variables, for example:
- `curl ... -o Godot_v4.6.3-stable_export_templates.tpz "${GODOT_EXPORT_TEMPLATES_DOWNLOAD_URL}"`
- If you also pre-download the executable for verification, use `${GODOT_LINUX_DOWNLOAD_URL}` there as well.
3. Ensure checksum verification (with `sha256sum`) covers all downloaded artifacts you care about (templates and, if desired, the executable), using the official checksum file.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…restore the original configuration of the players. Because this only does AudioStreamPlayer.new() and add_child(p), any custom configuration done in the normal initialization path (bus, volume, autoload, groups, etc.) won’t be reapplied. That means the pool state in tests can diverge from real runtime behavior, and this helper would be unsafe to call outside a tightly controlled test context. Consider extracting a shared pool-initialization routine used by both _ready() and cleanup_for_test so the recreated pool exactly matches the production configuration.
…dates a bit brittle compared to consuming the official checksum file. This does improve integrity, but it also tightly couples the Dockerfile to a specific checksum. When updating Godot, it’s easy to miss updating this hash. Consider mirroring the GitHub workflow: download the version’s official SHA256SUMS file and verify against it so you only need to change the version string, not an embedded hash. Suggested im
Replace a fixed sleep with an active readiness check before exporting the Godot Web build: start the local HTTP server, poll localhost with curl up to a 10s timeout (half-second intervals), and fail fast with a clear error message if the server doesn't become responsive. Adds informative log messages for startup and readiness to improve reliability of the deploy_to_itch workflow.
Delete redundant loop in _ready() that manually created and appended AudioStreamPlayer nodes for the SFX pool. The pool is now initialized solely via _initialize_sfx_pool(), avoiding duplicate child nodes and duplicate entries in the _sfx_pool.
github-project-automation
Bot
moved this from In Progress
to Done
in Sky Lock Assault Project
8 tasks
This was referenced Jun 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
name: Default Pull Request Template
about: Suggesting changes to SkyLockAssault
title: ''
labels: ''
assignees: ''
PR #754: Upgrade project environment to Godot 4.6.3
Author: @ikostan
Status: Merged (as of latest activity)
Linked Epic/Issue: #747
Milestone: Milestone 20: Resource-Based Settings & UI Acoustic Integration
Overview
This comprehensive PR upgrades the SkyLockAssault project from Godot 4.5.stable to Godot 4.6.3.stable. The update modernizes the development, testing, and deployment environment while introducing robustness improvements, security enhancements, and better test isolation. It addresses compatibility constraints with newer add-ons and resolves various engine behavior changes.
Key Changes
Major Upgrade
project.godot, scenes, Docker, CI/CD workflows, and documentation.obsolete/godot_4.5_stable/Dockerfilefor archival and fallback purposes.Security & Reliability Enhancements
AudioManagerwith safer audio bus handling (apply_volume_to_busguards against invalidAudioServeraccess or missing buses) and a newcleanup_for_test()helper for deterministic test cleanup.ConfigFilecrashes by seeding dummy data before encryption in test fixtures.Testing Improvements
workspace/run_gut_unit_tests.shwith temporary gdUnit4 disable/restore logic and better cache handling.audio_settings.tscn,default_bus_layout.tres, etc.).CI/CD Updates
gdunit4_tests.yml,gut_tests.yml,browser_test.yml,deploy_to_itch.yml.Documentation
Impact
This PR exemplifies a well-structured toolchain upgrade with proactive fixes for edge cases in testing and audio systems.
Description
What does this PR do? (e.g., "Fixes player jump physics in level 2" or "Adds
new enemy AI script")
Related Issue
Closes #ISSUE_NUMBER (if applicable)
Changes
system")
Testing
works on Win10 with 60 FPS")
Checklist
Additional Notes
Anything else? (e.g., "Tested on Win10 64-bit; needs Linux validation")
Summary by Sourcery
Upgrade the project to Godot 4.6.3 and update tooling, tests, and CI workflows to be compatible and more robust.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Summary by Sourcery
Upgrade the project environment and tooling to Godot 4.6.3 with stronger CI security and more robust audio/testing behavior.
New Features:
Bug Fixes:
Enhancements:
Summary by CodeRabbit
Chores
Bug Fixes
Tests
Refactor
Bots/AI Contributions to PR #754
This PR primarily upgrades the SkyLockAssault project environment to Godot 4.6.3, updates related CI/CD workflows, Docker setup, tests, documentation, and introduces safety fixes for audio/config handling. AI-assisted tools and bots provided significant value through automated summaries, reviews, code suggestions, and refinements.
AI/Bot-Assisted Contributions (@sourcery-ai, @coderabbitai, etc.)
DeepsourceReview: No explicit mentions, comments, or contributions from Deepsource (e.g., @deepsource-io or similar) were visible in the PR conversation, commits, or summaries. If Deepsource was configured for static analysis/linting, its review may have occurred implicitly via CI without a named bot comment.
These tools enhanced code quality, documentation, and maintainability without replacing core human-driven changes.
Human Contributions (@ikostan)
@ikostan (primary maintainer and author): Drove the entire PR as the main contributor. Key efforts include: