Skip to content

Commit 6e6e5f5

Browse files
committed
docs: add export verification checklist
1 parent b080538 commit 6e6e5f5

3 files changed

Lines changed: 145 additions & 7 deletions

File tree

docs/code-review.md

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,31 @@
11
# NekoDash Code Review Report
2+
23
**Date:** April 30, 2026
34
**Reviewer:** Gemini CLI
45

56
## Executive Summary
7+
68
The NekoDash codebase demonstrates a solid architectural foundation with clear manager patterns (Autoloads) and adherence to signal-driven decoupling. However, it currently carries significant "hardcoding debt" and several best-practice violations that could lead to maintenance challenges, performance bottlenecks, and production bugs if not addressed before launch.
79

10+
> Status correction on May 9, 2026: the original `PlaytestCapture autoload` finding below is stale. `PlaytestCapture` is no longer registered in the live `project.godot` autoload list. Shipping `Windows Desktop` and `Web` exports were also cleaned up in this session: fresh rebuilds no longer package the excluded test/editor/dev-capture files, and Web capture routing now requires the dedicated `dev_capture` feature via the `Web Dev Capture` preset. Android remains operationally unverified because the current `android/build/nekodash.apk` on disk predates the preset cleanup. See `docs/export-capture-audit.md` for the current verified matrix.
11+
812
---
913

1014
## 1. Hardcoded Values & Paths
15+
1116
The most prevalent issue is the manual management of strings and literal values instead of using Godot's resource-driven data model.
1217

1318
### 1.1 Hardcoded Paths (`res://` and `user://`) - [PARTIALLY FIXED]
19+
1420
There are **over 194 instances** of hardcoded paths inside logic outside of proper constant or export definitions.
21+
1522
- **Scene Navigation:** `src/core/scene_manager.gd` hardcodes paths for all screens and overlays (e.g., `res://scenes/ui/main_menu.tscn`).
1623
- **Audio Routing:** [FIXED] `src/core/music_manager.gd` now uses `GlobalAudioSettings` resource.
1724
- **Tutorial Logic:** [FIXED] `src/gameplay/tutorial_system.gd` now uses `TutorialData` resource via `@export`.
1825
- **World Map:** [FIXED] `src/ui/world_map.gd` now uses `catalogue_override` or default path via `ResourceLoader`.
1926

2027
### 1.2 Magic Numbers & Metadata - [PARTIALLY FIXED]
28+
2129
- **World Metadata:** [FIXED] Moved from `world_map.gd` to `WorldData` resources inside `LevelCatalogue`.
2230
- **Tutorial Triggers:** [FIXED] Moved from `tutorial_system.gd` to `TutorialData.tres`.
2331
- **UI Colors & Sizes:** `src/ui/shell_theme.gd` acts as a central repository for magic colors (e.g., `CREAM`, `PLUM`, `GOLD`) and layout dimensions rather than using a formal `.tres` Theme.
@@ -27,10 +35,12 @@ There are **over 194 instances** of hardcoded paths inside logic outside of prop
2735
## 2. Best Practice Violations
2836

2937
### 2.1 Production Integrity
30-
- **Tool Leakage:** `PlaytestCapture` (tools/playtest_capture.gd) is registered as a global Autoload. This tool logic will run in the production exported build, consuming memory and processing time unnecessarily.
38+
39+
- **Capture Tool Leakage:** This is now partially fixed. Fresh May 9 Windows/Web rebuilds no longer package the excluded capture, test, or editor-only files. The remaining always-shipped capture-adjacent script is `src/ui/web_capture_router.gd`, which stays autoloaded but now only routes snapshots when the build has `dev_capture`. Android still needs a fresh rebuilt APK before the same cleanup can be confirmed there.
3140
- **Debug Leftovers:** There are **over 100 `print()` statements** in the `src/` directory. These should be removed or converted to `push_warning`/`push_error` for proper log handling in production.
3241

3342
### 2.2 Architectural Deviations
43+
3444
- **Bypassing Managers:** `LevelCoordinator.gd` preloads and plays `level_complete.wav` directly, bypassing the `SfxManager` autoload. This breaks centralized volume control, bus routing, and SFX pooling.
3545
- **Node vs. Autoload:** `LevelProgression` is a local node in the gameplay scene. This makes it difficult for UI screens (like the World Map) to query progression state without the gameplay scene being active, violating the intent of **ADR-0001 (Autoload Architecture)**.
3646
- **Manual UI Scaling:** Multiple components manually calculate scaling and positions (e.g., `TutorialSystem.gd` skip button) instead of using Godot's container and anchor system.
@@ -40,11 +50,15 @@ There are **over 194 instances** of hardcoded paths inside logic outside of prop
4050
## 3. Static Analysis & Type Safety
4151

4252
### 3.1 Missing Type Hints
53+
4354
Over **40 variables and function signatures** lack explicit type hints, disabling Godot 4's static analysis benefits.
55+
4456
- **Affected Files:** `src/core/save_manager.gd`, `src/core/scene_manager.gd`, `src/gameplay/tutorial_system.gd`, `src/ui/hud.gd`, `src/ui/level_complete_screen.gd`.
4557

4658
### 3.2 Large Files (>500 lines)
59+
4760
The following files exceed the recommended size for single-responsibility modules:
61+
4862
- `src/ui/shell_theme.gd` (829 lines)
4963
- `src/ui/home_tile_art.gd` (745 lines)
5064
- `src/ui/cat_part_rig.gd` (652 lines)
@@ -56,7 +70,9 @@ The following files exceed the recommended size for single-responsibility module
5670
---
5771

5872
## 4. Incomplete Items (TODOs & HACKs)
73+
5974
There are **35 unresolved debt markers** across the codebase:
75+
6076
- **LevelCoordinator:** Stub handlers for `_on_move_count_changed` and `_on_coverage_updated`.
6177
- **UI Design Tool Addon:** Multiple TODOs regarding font weight fallbacks, bbcode support, and undo/redo efficiency.
6278
- **Godot 4 Migration:** `addons/gut/` and `addons/ui_design_tool/` contain numerous `# TODOGODOT4` markers regarding missing directory listing arguments.
@@ -68,26 +84,29 @@ There are **35 unresolved debt markers** across the codebase:
6884
1. **Resource Refactor:** Create a `WorldData` resource and update `LevelCatalogue` to store an array of worlds instead of a flat list of levels. Move hardcoded titles and audio mappings there. [DONE]
6985
2. **Theme Migration:** Convert the magic numbers in `shell_theme.gd` into a formal `.tres` Godot Theme.
7086
3. **Manager Alignment:** Force all gameplay systems to use `SfxManager` for audio and `MusicManager` for BGM to ensure bus settings are respected.
71-
4. **Build Cleanup:** Remove `PlaytestCapture` from the `project.godot` Autoload list and implement a proper debug/release conditional check.
87+
4. **Build Cleanup:** Maintain the new shipping preset excludes and the dedicated `Web Dev Capture` preset. The remaining gap is Android verification, not Windows/Web policy.
7288
5. **Type Pass:** Conduct a comprehensive pass to add missing `: Type` and `-> Type` hints to all variables and functions.
7389

7490
---
7591

7692
## 6. Fixes & Verification (Post-Review Update)
7793

7894
### 6.1 Issue #1: Hardcoded Path Refactoring
95+
7996
The following major hardcoding issues were resolved to align with Godot's resource-driven architecture:
97+
8098
- **World Map:** Removed `CATALOGUE_PATH` constant. Replaced with `@export var catalogue_override: LevelCatalogue`. The system now dynamically groups `LevelData` into `WorldData` objects, allowing for scalable world metadata.
8199
- **Tutorial System:** Removed `TUTORIAL_DATA_PATH`. Exposed `tutorial_data: TutorialData` as an export, moving trigger levels and UI asset paths to a `.tres` file.
82100
- **Music Manager:** Eliminated hardcoded screen-to-track dictionaries. The manager now interfaces with the `GlobalAudioSettings` resource for all BGM routing.
83101
- **Data Model:** Updated `LevelCatalogue` and `LevelProgression` to support the new `WorldData` resource structure.
84102

85103
### 6.2 Smoke Test & Regression Results
104+
86105
A comprehensive smoke test suite (`tests/test_all_screens_smoke.gd`) was executed using the headless Godot runtime to verify the stability of the changes.
87106

88107
- **Screen Validation:** All screens (`Main Menu`, `World Map`, `Gameplay`, `Skin Select`, `Loading`, `Opening`, `Credits`) and overlays (`Options`, `Pause`, `Level Complete`) successfully instantiated without script errors or path resolution failures.
89108
- **GUT Test Suite:**
90-
- **Total Tests:** 683
91-
- **Passing:** 629
92-
- **Failing:** 19 (Note: All failing tests were pre-existing and related to Level Design obstacle count mismatches in World 1, confirming no new regressions from architectural changes).
93-
- **Performance:** Full suite completed in ~13.3 seconds.
109+
- **Total Tests:** 683
110+
- **Passing:** 629
111+
- **Failing:** 19 (Note: All failing tests were pre-existing and related to Level Design obstacle count mismatches in World 1, confirming no new regressions from architectural changes).
112+
- **Performance:** Full suite completed in ~13.3 seconds.

docs/export-capture-audit.md

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# Export Tooling Audit
2+
3+
**Date:** May 9, 2026
4+
5+
This audit now covers both capture-related tooling and the broader non-game files that were leaking into shipping exports. It reflects the current preset configuration in `export_presets.cfg`, fresh Windows and Web rebuilds performed on May 9, 2026, browser-based verification of the new Web capture split, and the still-stale Android APK currently checked into `android/build/nekodash.apk`.
6+
7+
## Pre-Ship Checklist
8+
9+
- Rebuild every shipping target that is part of the release: `Windows Desktop`, `Web`, and `Android` when Android is shipping.
10+
- Review each fresh exporter log for `savepack: end` and verify there are no `Storing File:` entries for `tests/**`, `addons/gut/**`, `addons/ui_design_tool/**`, `addons/gui_auto_layout/**`, or the excluded dev-only `tools/*.gd` helpers.
11+
- Serve the shipping Web export and confirm `capture_ui=1&screen=options` stays inert instead of routing away from the normal main-menu flow.
12+
- Serve the `Web Dev Capture` export and confirm the same URL opens the requested capture screen, which verifies the `dev_capture` split still works.
13+
- Run the full GUT suite.
14+
- Run `tools/ui_snapshot_capture.gd`; if headless Windows skips screenshots because of the dummy renderer, run `tools/web_ui_snapshot_capture.ps1` against a served `Web Dev Capture` export and inspect the generated options, pause, and level-complete images.
15+
- For Android, confirm the APK timestamp changed before trusting any APK string probe or packaging result.
16+
17+
## Current Preset Configuration
18+
19+
- `Android`, `Windows Desktop`, and `Web` still use `export_filter="all_resources"`.
20+
- All three shipping presets now have expanded `exclude_filter` values that remove:
21+
- `tests/**`
22+
- `addons/gut/**`
23+
- `addons/ui_design_tool/**`
24+
- `addons/gui_auto_layout/**`
25+
- dev-only capture and snapshot scripts under `tools/`
26+
- editor-only helpers
27+
- A dedicated `Web Dev Capture` preset now exists with `custom_features="dev_capture"` and export output at `export/web-dev/index.html`.
28+
29+
## Runtime Rules After The Split
30+
31+
- `tools/playtest_capture.gd` still exists in the repo, but it is not autoloaded in `project.godot`.
32+
- `src/ui/web_capture_router.gd` remains autoloaded and therefore remains present in exports that include project autoloads.
33+
- `WebCaptureRouter` now always applies the Web viewport fix on HTML5 builds, but it only routes capture flows when all of the following are true:
34+
- the build has the custom feature `dev_capture`
35+
- the URL includes `capture_ui=1`
36+
- the requested screen is one of the known capture routes
37+
- Result: the shipping Web preset ignores `capture_ui=1`, while the `Web Dev Capture` preset still supports browser-driven snapshot routing.
38+
39+
## Runtime-Required Exceptions
40+
41+
These files are under tooling-style paths, but they are intentionally still shipped because gameplay or the platform shell depends on them:
42+
43+
- `src/ui/web_capture_router.gd`
44+
- `tools/level_solver.gd`
45+
- `tools/web_custom_shell.html`
46+
- `addons/godot_ui_animations/UIAnimationHandler.tscn`
47+
48+
`tools/level_solver.gd` is loaded by gameplay code, `tools/web_custom_shell.html` is the configured Web shell, and `addons/godot_ui_animations` is required by the `UIAnimation` autoload.
49+
50+
## Pre-Fix Leakage
51+
52+
Before the preset cleanup in this session, direct payload probes showed all three platforms shipping some combination of:
53+
54+
- `tests/**`
55+
- `addons/gut/**`
56+
- `addons/ui_design_tool/**`
57+
- `addons/gui_auto_layout/**`
58+
- `tools/playtest_capture.gd`
59+
- `tools/playtest_m2_runner.gd`
60+
- `tools/playtest_runner.gd`
61+
- `tools/shell_polish_capture.gd`
62+
- `tools/shell_polish_playtest_capture.gd`
63+
- `tools/shell_smoke_capture.gd`
64+
- `tools/ui_snapshot_capture.gd`
65+
66+
The root cause was not autoload registration. It was `all_resources` plus narrow excludes.
67+
68+
## Post-Cleanup Packaging Status
69+
70+
| Platform | Artifact status | Dev-only capture/test/editor tooling | Runtime-required exceptions | Notes |
71+
| --------------- | ---------------------------------------------- | ------------------------------------ | ---------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
72+
| Windows Desktop | Fresh rebuild completed on May 9 | Not present in `savepack` output | Present | Export finished with pre-existing project warnings, but no `Storing File:` matches remained for `tests/**`, `addons/gut/**`, `addons/ui_design_tool/**`, `addons/gui_auto_layout/**`, or the excluded `tools/*.gd` helpers |
73+
| Web | Fresh rebuild completed on May 9 | Not present in `savepack` output | Present | Same packaging result as Windows Desktop |
74+
| Web Dev Capture | Fresh rebuild completed on May 9 | Same exclusions as shipping Web | Present | Adds only the `dev_capture` custom feature, not the excluded dev-only scripts |
75+
| Android | No fresh post-fix APK produced in this session | Post-fix status unverified | Stale APK still contains pre-fix leakage | `android/build/nekodash.apk` still has an April 3, 2026 timestamp and still exposes old leaked paths, so it cannot be treated as a post-fix result |
76+
77+
## Capture-Related Files Still Ending Up In Shipped Builds
78+
79+
For the fresh May 9 shipping rebuilds that were actually produced in this session:
80+
81+
- `src/ui/web_capture_router.gd` still ships in Windows and Web because it is an autoloaded project script.
82+
83+
For the currently checked-in Android APK on disk:
84+
85+
- `assets/tests/**`
86+
- `assets/tools/playtest_capture.gd`
87+
- `assets/tools/playtest_runner.gd`
88+
- other pre-fix tooling strings still appear in the stale APK payload
89+
90+
That Android APK is not a valid post-cleanup verification artifact.
91+
92+
## Browser Verification Of The New Web Split
93+
94+
Two direct browser probes were run against local servers:
95+
96+
- Shipping Web export with `capture_ui=1&screen=options&delay_ms=5000`
97+
- Result: remained on the main menu
98+
- Evidence screenshot: `screenshots/export_audit/web_shipping_options_probe.png`
99+
- `Web Dev Capture` export with the same query string
100+
- Result: opened the options overlay as expected
101+
- Evidence screenshot: `screenshots/export_audit/web_dev_options_probe.png`
102+
103+
This confirms that the runtime capture behavior is now behind the dedicated `dev_capture` feature rather than being available to the normal Web export.
104+
105+
## Validation Notes
106+
107+
- Full GUT suite after the change: 691 / 691 passing.
108+
- `tools/ui_snapshot_capture.gd` still hits the known headless dummy-renderer limitation on Windows and reports null-image skips.
109+
- The documented fallback `tools/web_ui_snapshot_capture.ps1` was run against the served `Web Dev Capture` export.
110+
- Fresh desktop screenshots were inspected for:
111+
- `options_desktop_web.png`
112+
- `pause_desktop_web.png`
113+
- `level_complete_desktop_web.png`
114+
115+
## Evidence Methodology
116+
117+
- For fresh Windows and Web exports, the exporter `savepack` log is the authoritative source for whether a file was packaged.
118+
- Raw `rg -a` probes against `.pck` files can still surface excluded paths from metadata such as `project.binary` or script caches, so they are useful for spot checks but not authoritative on their own.
119+
- For Android, the stale artifact timestamp is the decisive reason the current APK probe cannot be used as post-fix evidence.

docs/gdd/game-concept.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ _Status: Approved_
77

88
## Elevator Pitch
99

10-
> A kawaii mobile puzzle game where you swipe a cute cat across a top-down grid, gliding until it hits a wall, with the goal of covering every tile in the fewest moves possible. It's like Pokémon's ice cave puzzles, AND ALSO your entire score is always visible as a challenge to beat.
10+
> A kawaii mobile puzzle game where you swipe a cute cat across a top-down grid, gliding until it hits a wall, with the goal of covering every tile in the fewest moves possible. It's like [Pokémon's ice cave puzzles](https://bulbapedia.bulbagarden.net/wiki/Ice_Path), AND ALSO your entire score is always visible as a challenge to beat.
1111
1212
---
1313

0 commit comments

Comments
 (0)