# Review & Handoff Report — Cocos Creator 3.x Client TypeScript Architecture

**Role**: reviewer_client_code (Reviewer & Adversarial Critic)  
**Date**: 2026-10-03T08:38:00Z  
**Target Codebase**: `client/cocos/` (TypeScript sources, preview harness, and configurations)  
**Explicit Verdict**: **REQUEST_CHANGES**

---

## 1. Observation

### Observation 1: TypeScript Type-Checking (`tsc --noEmit`)
- **Direct Execution in `client/cocos/`**:
  - Command: `npx tsc --noEmit` (cwd: `client/cocos`)
  - Tool output: Exit code `0` (TypeScript version 5.9.3, all 21 scripts type-checked successfully).
- **Execution from Project Root**:
  - Command: `npx tsc --noEmit --project client/cocos/tsconfig.json` (cwd: `c:/Projects/FreeExile`)
  - Tool output: Exit code `1`
  - Verbatim error:
    ```
    client/cocos/tsconfig.json(11,5): error TS5102: Option 'baseUrl' has been removed. Please remove it from your configuration.
      Use '"paths": {"*": ["./*"]}' instead.
    ```
  - Root environment uses TypeScript 7.0.2 where `"baseUrl"` is deprecated/removed under `"moduleResolution": "bundler"`.

### Observation 2: Code Hygiene Verification
- Command: `python tools/lint/check_code_and_doc_hygiene.py`
- Tool output: Exit code `0`.
- Verbatim summary:
  ```
  ✅ KẾT QUẢ: TOÀN BỘ MÃ NGUỒN VÀ TÀI LIỆU TUÂN THỦ HARD CAP HYGIENE!
  ```
- All 21 TypeScript component files in `client/cocos/assets/scripts/` strictly adhere to the $\le 350$ lines soft cap rule (ranging between 37 and 225 lines).

### Observation 3: KinematicsEngine Velocity Calculation Ordering Bug
- File: `client/cocos/assets/scripts/core/KinematicsEngine.ts`
- Verbatim lines 114–117:
  ```typescript
  114:     this.wx = resolution.nextWx;
  115:     this.wy = resolution.nextWy;
  116:     this.velX = (resolution.nextWx - this.wx) / dt;
  117:     this.velY = (resolution.nextWy - this.wy) / dt;
  ```
- Line 114 updates `this.wx` to `resolution.nextWx`.
- On line 116, `(resolution.nextWx - this.wx)` evaluates to `(resolution.nextWx - resolution.nextWx) == 0`.
- On line 117, `(resolution.nextWy - this.wy)` evaluates to `(resolution.nextWy - resolution.nextWy) == 0`.
- Consequently, `this.velX` and `this.velY` are permanently evaluated as `0` during active movement.

### Observation 4: AStarPathfinder Negative Coordinate Failure & LoS Step Truncation
- File: `client/cocos/assets/scripts/core/AStarPathfinder.ts`
- Verbatim lines 105–106 & 137:
  ```typescript
  105:     const startIndex = startTy * gridWidth + startTx;
  106:     const goalIndex = goalTy * gridWidth + goalTx;
  ...
  137:         if (nx < 0 || ny < 0 || nx >= gridWidth) continue;
  ```
- File: `client/cocos/assets/scripts/world/SanctuaryWorld.ts`
- Verbatim lines 38–39, 58–59, and 86:
  ```typescript
  38:       wx: -7.5,
  39:       wy: 0.0,
  ...
  58:       wx: -6.5,
  59:       wy: 5.5,
  ...
  86:     if (tx < -18 || tx > 18 || ty < -18 || ty > 18) return true;
  ```
- When navigating within `SanctuaryWorld`, coordinates span `[-18..18]`. If `startTx < 0` or `startTy < 0`:
  - `startIndex` is negative (e.g. `-7`).
  - `this.gScore` is a `Float32Array`. In JavaScript typed arrays, assigning to a negative index (e.g. `this.gScore[-7] = 0`) is silently discarded; reading returns `undefined`.
  - Line 137 treats any `nx < 0 || ny < 0` as out of bounds, preventing search in negative quadrants and returning `[]`.
- In `AStarPathfinder.hasLineOfSight` (lines 53–70):
  ```typescript
  53:     for (let step = 0; step < 120; step++) {
  ...
  69:     }
  70:     return true;
  ```
  If the loop exhausts 120 steps without reaching `(tx1, ty1)` (e.g. distant target beyond 120 steps), it falls through and returns `true`, erroneously certifying that line of sight exists.

### Observation 5: SpriteAtlasRenderer Unused targetClipName & Clip Row Mapping
- File: `client/cocos/assets/scripts/animation/SpriteAtlasRenderer.ts`
- Verbatim lines 67–81:
  ```typescript
  67:   ): Rect {
  68:     let targetClipName = clipName;
  69:     if (manifest && manifest.clips[clipName]?.atlas_clip) {
  70:       targetClipName = manifest.clips[clipName].atlas_clip!;
  71:     }
  72: 
  73:     const dirRow = this.directionRowMap[direction] ?? 0;
  74:     const colIndex = frameIndex % this.cols;
  75: 
  76:     // Calculate source pixel coordinates in the atlas texture
  77:     const sx = colIndex * this.frameWidth;
  78:     const sy = dirRow * this.frameHeight;
  79: 
  80:     return new Rect(sx, sy, this.frameWidth, this.frameHeight);
  81:   }
  ```
- `targetClipName` is computed on lines 68–70, but never referenced anywhere in lines 72–81.
- `sy` is calculated solely from `dirRow * this.frameHeight`. Every animation clip (`idle`, `run`, `attack`, `dodge`, `hurt`, `death`) for a given direction maps to the exact same vertical offset `dirRow * this.frameHeight`, failing to account for clip vertical or index offsets.

### Observation 6: E2E Tests, Baseline Unit Tests & Browser Player Journey Verification
- **E2E Cocos Test Suite**:
  - Command: `pytest tests/e2e_cocos -v`
  - Output: `178 passed in 0.41s`.
  - Note: Tests verify protocol contracts and algorithmic specifications via Python models (`SimulatedCocosPlayer`).
- **Baseline Unit Test Suite**:
  - Command: `pytest tests/unit/ -q`
  - Output: `1618 passed, 11 failed in 212.98s`.
  - Note: All 11 failures are pre-existing test assertion regressions in legacy webapp/map-style assets (`pc_hud.css`, `target_dummy_telemetry.js`, `i18n.js` line caps, and texture memory assertions). Zero failures were introduced by the Cocos Creator port.
- **Real-Browser Automation Runner**:
  - Command: `python tools/qa/run_cocos_human_journey.py`
  - Output: Exit code `0`, average FPS `63.9 FPS`, 8 journey steps verified on browser, screenshots recorded in `docs/qa/reports/screenshots/`.
  - Note: The browser preview harness `preview_app.js` runs Canvas2D logic that computes `velX = deltaX / dt` independently, which masked the `velX = 0` bug in `KinematicsEngine.ts`.

---

## 2. Logic Chain

1. **Premise 1 (Observation 3)**: In `KinematicsEngine.updateKinematics()`, line 114 executes `this.wx = resolution.nextWx` before line 116 calculates `this.velX = (resolution.nextWx - this.wx) / dt`.
   - **Inference 1.1**: The numerator `resolution.nextWx - this.wx` is always identically zero.
   - **Inference 1.2**: As a result, `velX` and `velY` in `KinematicsEngine` remain zero while moving. Any external system, camera lead, or animation blend tree relying on `kinematics.velX` or `kinematics.velY` will observe zero velocity during active movement.

2. **Premise 2 (Observation 4)**: `AStarPathfinder` allocates 1D typed arrays (`Float32Array`, `Uint8Array`) and indexes nodes via `y * gridWidth + x`.
   - **Inference 2.1**: When world coordinates are negative (as in `SanctuaryWorld`, where the map spans `[-18..18]`), node indices are negative numbers.
   - **Inference 2.2**: Negative indices in typed arrays evaluate to `undefined` and writes are ignored. Furthermore, neighbor checks discard `nx < 0 || ny < 0`.
   - **Inference 2.3**: Click-to-move pathfinding in `SanctuaryWorld` towards negative coordinates (such as the NPCs at `(-7.5, 0.0)` or `(-6.5, 5.5)`) will return an empty path, preventing navigation.
   - **Inference 2.4**: In `hasLineOfSight()`, reaching the 120-step loop ceiling returns `true`, which leads `findPath()` to falsely conclude that an unobstructed straight line exists to targets farther than 120 tiles.

3. **Premise 3 (Observation 5)**: `SpriteAtlasRenderer.getFrameSourceRect()` accepts `clipName`, resolves `targetClipName`, but calculates `sy = dirRow * this.frameHeight` without adding a clip-specific row or frame offset.
   - **Inference 3.1**: `idle`, `run`, and `attack` clips at the same direction will all read from the same row `dirRow`, causing sprite visual corruption or rendering only the first clip's row.

4. **Premise 4 (Observation 1)**: `tsconfig.json` contains `"baseUrl": "./"` alongside `"moduleResolution": "bundler"`.
   - **Inference 4.1**: In TypeScript 7.0+, `baseUrl` is deprecated/removed under `bundler` resolution, causing `npx tsc --noEmit --project client/cocos/tsconfig.json` to fail when executed from root workspace CI/build scripts.

---

## 3. Findings Matrix

| Finding ID | Severity | File & Location | Description | Recommended Remediation |
|---|---|---|---|---|
| **F-01** | **CRITICAL** | `client/cocos/assets/scripts/core/KinematicsEngine.ts:114-117` | `velX` and `velY` always evaluate to 0 due to assignment before subtraction. | Calculate velocity before mutating `this.wx`/`this.wy` or preserve previous position (`const prevWx = this.wx; ... this.velX = (this.wx - prevWx) / dt;`). |
| **F-02** | **CRITICAL** | `client/cocos/assets/scripts/core/AStarPathfinder.ts:105-106, 137` | Negative tile coordinates cause TypedArray underflow (`gScore[-index] = undefined`) and discard in `SanctuaryWorld`. | Add grid coordinate origin offset (e.g. `(tx + originOffsetX)`) or support signed grid mapping. |
| **F-03** | **MAJOR** | `client/cocos/assets/scripts/core/AStarPathfinder.ts:70` | `hasLineOfSight` returns `true` when 120-step limit is exceeded without reaching destination. | Return `false` when step loop terminates without reaching `(tx1, ty1)`. |
| **F-04** | **MAJOR** | `client/cocos/assets/scripts/animation/SpriteAtlasRenderer.ts:68-78` | `targetClipName` is unused; all clips map to the exact same vertical atlas row. | Incorporate clip offset index/row from `manifest` into `sy` calculation. |
| **F-05** | **MINOR** | `client/cocos/tsconfig.json:11` | Option `'baseUrl'` causes TS5102 error in TypeScript 7.0+ under `"moduleResolution": "bundler"`. | Remove `"baseUrl": "./"` or replace with `"paths": {"*": ["./*"]}`. |

---

## 4. Caveats

- **Cocos Creator Editor Runtime**: Cocos Creator 3.8.x is a GUI/desktop application; headless verification was conducted using TypeScript compiler type-checking (`tsc`), Node.js AST/runtime checks, and browser simulation harnesses.
- **Preview App Independence**: `preview_app.js` is a standalone HTML5 Canvas implementation designed for browser testing and Playwright automation. While it successfully demonstrates all 8 journey steps and Zero-Residual Momentum in the browser, the TypeScript source files in `assets/scripts/` contain the specific bugs identified in Findings F-01 through F-04.
- No other caveats.

---

## 5. Conclusion

The Cocos Creator 3.x port in `client/cocos/` is well-architected, follows strict file length hygiene ($\le 350$ lines), incorporates Zero-Residual Momentum guards, 0.25s i-frame mechanics, 9-language reactive localization without reload, and 40-slot backpack management.

However, because of two **CRITICAL** logic defects in core gameplay math (`KinematicsEngine.ts` velocity always zero, `AStarPathfinder.ts` negative coordinate breakdown in Sanctuary), one **MAJOR** sprite atlas mapping gap in `SpriteAtlasRenderer.ts`, and one root workspace TypeScript compilation blocker in `tsconfig.json`, the verdict is **REQUEST_CHANGES**.

---

## 6. Verification Method

1. **Verify F-01 (Kinematics Velocity Bug)**:
   - Inspect `KinematicsEngine.ts` lines 114–117.
   - Run a simulation stepping `updateKinematics` with non-zero input; observe `kinematics.velX === 0` and `kinematics.velY === 0`.
2. **Verify F-02 & F-03 (A* Negative Coordinate & LoS Ceiling)**:
   - Run Node.js reproduction:
     ```bash
     node -e "const g = new Float32Array(8192); g[-7] = 0; console.log(g[-7]);"
     ```
     Outputs `undefined`.
   - Call `AStarPathfinder.findPath` with start `(-7.5, 0.0)` in Sanctuary grid; returns empty array `[]`.
3. **Verify F-04 (SpriteAtlasRenderer Unused Clip)**:
   - Inspect `SpriteAtlasRenderer.ts` lines 68–81; verify `targetClipName` has 0 references after line 70.
4. **Verify F-05 (TypeScript tsconfig error)**:
   - Run from root directory:
     ```bash
     npx tsc --noEmit --project client/cocos/tsconfig.json
     ```
     Fails with TS5102.