Conversation
Adds a new GUT test suite (test_full_encryption_workflow.gd) that exercises the complete Settings encryption workflow: fresh install encryption/save/load, plaintext fallback and migration, mixed-systems concurrent writes, restart behavior after migration, and corrupted file handling. Tests isolate filesystem state at user://test_encryption_workflow.cfg and use per-test setup/teardown to backup/restore ProjectSettings salt, clear Globals' cached key, and swap Globals.settings to avoid polluting global state. Also adds the corresponding test .uid file.
Enhance encryption-related logging across Globals and Settings: add detailed, emoji-prefixed error/debug messages for save/load/encryption key operations, surface decryption failures, plaintext migration warnings, and confirm cached key generation. Ensure use of the centralized ensure_encryption_key helper when saving, and add logging in ensure_encryption_key itself. Add GUT tests (test_encryption_logging.gd + uid) to exercise the new logging branches (key generation, missing config, successful encrypt/decrypt, plaintext load, and save failures). These changes make encryption issues easier to diagnose during development and CI runs.
Add a project setting placeholder for 'game/security/save_salt' and harden the encryption key generation: if the salt is empty, _get_encryption_key now logs a warning and aborts to plaintext (returns empty) to avoid silent weak-key fallbacks. Adjust the production guard to crash only on the dev_fallback_salt while still allowing automated/browser-driven tests. Add GUT tests (test_encryption_failsafe.gd + .uid) that verify the salt property is registered, that missing salt triggers the plaintext fallback, and that a valid salt results in an encrypted save; tests also isolate ProjectSettings and clear the cached encryption key during setup/teardown.
Create workspace package (workspace/__init__.py) and move various CI/test/lint scripts into the workspace/ directory. Renamed/moved: run_browser_tests.sh, run_gdlint.sh, run_gdunit4_unit_tests.sh, run_gut_unit_tests.sh, run_markdown_lint.sh, run_pipeline.sh, run_unit_tests.sh, and run_yaml_lint.sh to consolidate tooling and improve project organization.
Call Globals.set_test_encryption_key() in several GUT tests and add explicit loading flags to prevent auto-saves during test setup. Modified tests: test_combined_multi_manager_loads.gd, test_globals_resource.gd, test_preserve_other_sections.gd, and test_settings_observer.gd. This ensures the empty project salt is overridden with a valid test key so Globals._save_settings uses encryption (avoiding plaintext fallback) and prevents test actions/signals from triggering unintended saves to production files.
Add *.pyc to .gitignore and adjust tests to tolerate known encryption fallback messages. audio_flow_test now ignores logs containing 'encryption aborted' when checking for unexpected warnings. difficulty_flow_test accepts either an encrypted save log or a plaintext fallback message and improves related assertion messages to avoid false failures on WebGL/plaintext fallbacks.
Add a new how-to document explaining how to locate and read the virtual file system created by Godot web exports (Emscripten IDBFS / IndexedDB) and include a JavaScript snippet to extract file bytes in Chrome DevTools. Also update README to link to the new guide so users can inspect saved files and verify contents before and after encryption.
Add two Milestone 17 docs describing UI audio and config encryption work. Part 1 documents a dedicated, rate-limited slider SFX and centralized AudioManager.play_sfx with stream caching and an AudioStreamPlayer pool; it also details VolumeSlider hardening (interaction guards, programmatic update API, delta/rate gating, invalid-bus protection) and extensive GUT/GdUnit4 test updates to prevent feedback loops and flaky CI. Part 2 documents centralized encryption key management in Globals (deterministic per-device key with test/web overrides), safe encrypted ConfigFile load/save with legacy plaintext migration, CI injection of a PRODUCTION_SALT, and broad test/tooling adjustments to validate migration and preserve unrelated config sections. Both files summarize added tests, CI/deploy changes, and migration/backward-compatibility safeguards.
…SkyLockAssault into fix-encryption-pipeline
Add two Milestone 17 documentation files: Part 1 describes audio/UI changes including a dedicated slider SFX, AudioManager pooling/caching, VolumeSlider hardening and test-suite updates; Part 2 documents centralized encryption key management for config files, plaintext→encrypted migration, CI salt injection and test adaptations. Remove the older duplicate milestone doc files.
Apply markdown style and linting fixes across documentation and the lint runner. Changes include: normalize list markers and spacing, remove trailing/extra blank lines, reflow wrapped lines for readability, convert an inline image to a reference-style link, and standardize table formatting in milestone docs. Also tweak README milestone wording for clarity. Minor shell script formatting added to workspace/run_markdown_lint.sh to improve readability. These edits are purely stylistic to satisfy markdownlint and improve docs consistency.
Cosmetic cleanup across docs and scripts: standardize list markers (replace leading '+' list markers with '-'), fix blank-line/whitespace inconsistencies, and tidy minor punctuation/spacing in README and several docs. Also add a small readability newline in workspace/run_markdown_lint.sh. No functional changes.
…ers_only_during_manual_slider_adjustments.md
…ers_only_during_manual_slider_adjustments.md
Add a new script .github/scripts/inject_salt.sh that injects security/save_salt into Godot project files, properly escaping backslashes and quotes and handling presence/absence of the [game] section. Update .github/workflows/lint_test_deploy.yml to run a CI job that exercises the shared script, and make browser functional tests depend on it. Update tests/ci/test_salt_injection.py to call the shared bash script instead of using an inline AWK blob, simplify the test flow, and adjust expected output for the escaped salt. This centralizes salt injection logic and makes CI/tests use the same implementation.
Test Plan 5: UI Mute Signal Propagation File: res://test/unit/test_ui_mute_logic.gd Logic Instantiate a test instance of the audio settings menu. Simulate toggling the Menu/UI mute control. Allow signal handlers to execute. Assertions The corresponding AudioServer bus becomes muted. The associated volume slider becomes non-editable while muted. The slider becomes editable again when unmuted.
Test Plan 6: Missing Configuration Defaults File: res://test/unit/test_ui_audio_persistence.gd Logic Create an empty or incomplete settings file. Load audio settings. Verify default Menu/UI audio values are applied. Assertions No errors occur during load. Default volume is correctly assigned. Default mute state is correctly assigned.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Tighten and clarify the menu/UI mute test by adding an assertion that the SFX menu bus exists on AudioServer and verifying the bus is unmuted before simulating user toggles. Comments and step numbering were refined for clarity; the test now checks backend preconditions (bus presence and unmuted state) prior to emitting mute/unmute signals and asserting slider editability.
…/ikostan/SkyLockAssault into test-plan-for-ui-bus-persistence
Expand and harden UI/Menu audio persistence tests: add named constants for tolerance and defaults, replace magic numbers, and include mute/AudioServer persistence cases. Dynamically create and track the Menu audio bus for headless environments and ensure it is removed during teardown to avoid side effects. Replace repeated numeric tolerances and default values with constants and clarify test comments; also tighten encrypted test fixture error message.
Track and remove an AudioServer bus created by the test to restore global state after each run. Introduces a _bus_created_by_test flag set in before_each() when the Menu/UI bus is added, and after_each() now removes that bus if it was created by the test. Also includes small test cleanup and formatting tweaks (removed redundant comments/whitespace) to keep the test focused and ensure AudioServer state is restored between tests.
Point 2: Path Mismatch (test/gut/ vs res://test/unit/) 🟩 Excellent Catch. Looking closely at your Godot editor file tree from the screenshots, your test files live under test/gut/, but our written PR documentation listed them as res://test/unit/. Aligning this prevent anyone reading the PR from getting lost.
Test plan for UI bus persistence
ikostan
added this to the
Milestone 19: UI Audio Infrastructure & Centralized SFX Systems milestone
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
1 similar comment
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
ikostan
added
github actions
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @ikostan, your pull request is larger than the review limit of 150000 diff characters
DeepSource Code ReviewWe reviewed changes in See full review on DeepSource ↗ Code Review Summary
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 |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene  |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python |  |
May 23, 2026 3:25a.m. | Review ↗ |
| JavaScript |  |
May 23, 2026 3:25a.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.
github-project-automation
Bot
moved this from In Progress
to Done
in Sky Lock Assault Project
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
2 participants
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: ''
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")