550 lines
26 KiB
Markdown
550 lines
26 KiB
Markdown
# zeling_v2 全面代码评审
|
||
|
||
> **评审日期**：2026-05-11
|
||
> **评审范围**：`Assets/Scripts/` 全部 C# 代码（约 25 个模块，100+ 个文件）
|
||
> **评审标准**：成熟商业独立游戏水准（参照 Hollow Knight / Dead Cells / Celeste 量级）
|
||
> **总体评分**：**8.2 / 10**
|
||
|
||
---
|
||
|
||
## 目录
|
||
|
||
1. [总体评价](#1-总体评价)
|
||
2. [架构设计](#2-架构设计)
|
||
3. [性能](#3-性能)
|
||
4. [可扩展性](#4-可扩展性)
|
||
5. [编辑器友好性](#5-编辑器友好性)
|
||
6. [使用便利性与开发者体验](#6-使用便利性与开发者体验)
|
||
7. [分模块详细评估](#7-分模块详细评估)
|
||
8. [横切关注点](#8-横切关注点)
|
||
9. [风险与残留问题](#9-风险与残留问题)
|
||
10. [改进建议优先级清单](#10-改进建议优先级清单)
|
||
|
||
---
|
||
|
||
## 1. 总体评价
|
||
|
||
### 优势总结
|
||
|
||
这是一套**远高于大多数 Unity 独立游戏原型**水准的代码库。核心架构决策正确，模式选择合理，绝大多数关键热路径已经过性能优化。以下几点值得特别称赞：
|
||
|
||
- **程序集分层清晰**：25 个 `.asmdef` 按功能边界划分，循环引用为零。
|
||
- **SO Event Channel 模式**：解耦彻底，任何两个系统之间都不存在直接 `GetComponent` 跨模块调用。
|
||
- **数据驱动设计**：几乎所有可调参数都在 ScriptableObject 中，热更改无须重编。
|
||
- **伤害流水线清晰**：HurtBox 8 步流水线（无敌帧→弹反→霸体→护盾→防御→TakeDamage→广播→DoT）逻辑完整，边界清楚。
|
||
- **异步存档**：`SaveManager` 采用 `async/await` + `Newtonsoft.Json` + HMAC-SHA256 校验，版本迁移链完整。
|
||
|
||
### 主要短板
|
||
|
||
- **测试覆盖缺失**：无任何单元测试文件，关键算法（伤害计算、存档迁移、技能冷却）全靠运行时验证。
|
||
- **单例滥用**：`GameManager`、`SaveManager`、`QuestManager` 三者同时持有静态 `Instance` 属性，与 `ServiceLocator` 体系形成**双轨注册**，职责模糊。
|
||
- **线程安全缺口**：`SaveManager` 的异步 I/O 路径与主线程直接共享 `_current` 状态，无 Lock 保护。
|
||
- **输入架构部分未完成**：`InputReaderSO` 是 SO 但依赖 `InputActionAsset` 并在 `OnEnable` 重新绑定，在领域驱动设计视角下存在潜在的状态管理混乱。
|
||
|
||
---
|
||
|
||
## 2. 架构设计
|
||
|
||
### 2.1 分层与依赖方向
|
||
|
||
```
|
||
┌─────────────────────────────────────────────────────┐
|
||
│ UI / VFX / Feedback / Editor (叶节点，只依赖下方) │
|
||
├─────────────────────────────────────────────────────┤
|
||
│ Player.States / Enemies.AI / Quest / World / Skills │
|
||
├─────────────────────────────────────────────────────┤
|
||
│ Player / Enemies / Combat / Camera / Equipment │
|
||
├─────────────────────────────────────────────────────┤
|
||
│ Core (ServiceLocator / GameStateMachine / Events) │
|
||
│ Core.Save / Core.Pool / Core.Events │
|
||
└─────────────────────────────────────────────────────┘
|
||
```
|
||
|
||
**评分：9/10**
|
||
|
||
依赖方向严格向下，没有发现上层模块被下层引用的情况。`BaseGames.Combat.StatusEffects` 独立程序集、`BaseGames.Enemies.Navigation` 抽象为 `IPathAgent` 接口——这种**反向依赖消除**的处理非常专业。
|
||
|
||
**缺陷**：
|
||
- `QuestManager` 和 `CameraStateController` 仍保留 `static Instance`，与 `ServiceLocator` 方案并存。若要统一，`ICameraService` 已经完成了接口化，`QuestManager` 还未接入。
|
||
- `GameServiceRegistrar`（`ExecutionOrder -2000`）和 `GameManager`（`-1000`）都向 `ServiceLocator` 注册部分重叠的服务（`IDeathRespawnService`、`ISceneService`），后注册者会无声地覆盖前者。
|
||
|
||
### 2.2 状态机
|
||
|
||
`GameStateMachine` 对状态合法性有**白名单校验**（`ValidNextStates`），这是商业游戏中防止非法状态跳转的标准实践。`PlayerController` 的手写状态字典方案（非继承 MonoBehaviour）也避免了 `Animancer.FSM` 与 `PlayerController` 之间的耦合。
|
||
|
||
**评分：8.5/10**
|
||
|
||
缺陷：Player 状态机不校验合法转换（任何状态都可以调 `TransitionTo` 跳到任何状态），在大型团队中会产生维护风险。
|
||
|
||
### 2.3 事件系统
|
||
|
||
`BaseEventChannelSO<T>` + `VoidBaseEventChannelSO` 的 SO 事件频道是当前 Unity 最佳实践之一。`EventSubscription` disposable 句柄也有效消除了忘记退订的内存泄漏。
|
||
|
||
**评分：9.5/10**
|
||
|
||
唯一可改进点：`BaseEventChannelSO` 在编辑器下调用 `GetInvocationList()`（会产生 GC 分配），这发生在 `Raise()` 热路径上。
|
||
改进方式：用 `#if UNITY_EDITOR` 守卫下的计数器字段（而非调用 `GetInvocationList`）替代。
|
||
|
||
### 2.4 依赖注入
|
||
|
||
`ServiceLocator` 实现简洁、测试友好（`OverrideForTest` / `Reset` 仅在 `UNITY_EDITOR` 下可见）。`GetOrDefault` 支持可选服务，避免空引用异常。
|
||
|
||
**评分：8/10**
|
||
|
||
缺陷：
|
||
- 没有生命周期管理（`Dispose`）——如果服务是 `MonoBehaviour`，场景卸载后 `_services` 字典中仍持有对已销毁对象的引用。建议在 `OnDestroy` 中调用 `ServiceLocator.Unregister<T>()`。
|
||
- 无法区分"设计上不存在"的服务和"还未注册"的服务，两者调用 `Get<T>()` 都会抛异常，错误信息不够精确。
|
||
|
||
---
|
||
|
||
## 3. 性能
|
||
|
||
### 3.1 热路径 GC 分配
|
||
|
||
| 位置 | 实现 | GC 状态 |
|
||
|------|------|---------|
|
||
| `HitBox.OnTriggerEnter2D` | `DamageInfo.From(so)` struct 工厂 | ✅ 零堆分配 |
|
||
| `SkillManager.Update` | `_activeSkills[]` 快照数组 + Dictionary | ✅ 零堆分配 |
|
||
| `BatchLOSSystem.FixedUpdate` | 顺序 Raycast2D，节流分帧 | ✅ 可接受 |
|
||
| `StatusEffectManager.Update` | 逆序遍历 `_activeList`（List<T>） | ✅ 零分配 |
|
||
| `WorldStateRegistry.Mark` | HashSet.Add，首次创建 HashSet | ⚠️ 首次调用有小分配，之后零分配 |
|
||
| `BaseEventChannelSO.Raise`（编辑器） | `GetInvocationList()` | ❌ 每次 Raise 分配 `Delegate[]` |
|
||
| `SaveManager.SaveAsync` | 两次 `JsonConvert.SerializeObject` | ⚠️ 存档期间大量 string 分配，可接受（低频） |
|
||
| `InputBuffer.Update` | 3 个 `Mathf.Max` float | ✅ 零分配 |
|
||
| `PlayerMovement.FixedUpdate` | `new Vector2` | ✅ struct，无堆分配 |
|
||
| `GlobalObjectPool` Spawn | `Queue<PooledObject>` Dequeue | ✅ 零分配（池命中时） |
|
||
|
||
**整体性能评分：8.5/10**
|
||
|
||
### 3.2 物理检测
|
||
|
||
`PlayerMovement.CheckGrounded()` 使用 `Physics2D.OverlapBox`，每 FixedUpdate 执行一次，这是标准做法。
|
||
|
||
`BatchLOSSystem` 将 LOS 射线检测分帧节流（每帧最多 `_maxRequestersPerFrame` 次），避免敌人增多时爆帧——这是经典的 **Temporal Spreading** 模式，专业。
|
||
|
||
**缺陷**：`HitBox._hitCooldownTimers`（Dictionary 结构）在 `OnDisable` 时调用 `Clear()`——如果每帧有大量命中，Dictionary 操作会有轻微 GC 压力。考虑用 `int[]` 按帧号记录命中替代。
|
||
|
||
### 3.3 对象池
|
||
|
||
`GlobalObjectPool` 实现完整：
|
||
- Addressables 异步预热（统一走 `WarmupSingleAsync`，Coroutine 桥接）
|
||
- `Queue<PooledObject>` 空闲池
|
||
- `LinkedList<PooledObject>` 活跃对象 LRU
|
||
- `MaxCount` 强制容量上限
|
||
|
||
**评分：8/10**
|
||
|
||
**缺陷**：池满时（`MaxCount > 0` 且活跃数达上限）直接返回 null 而不是回收最老对象。在密集弹幕场景下会出现弹丸"消失"。建议改为回收 LRU 节点后复用。
|
||
|
||
### 3.4 动画系统
|
||
|
||
采用 **Animancer**（双层：Layer 0 Base + Layer 1 Overlay），避免了 Unity Animator 的状态机 GC 和字符串参数哈希问题。`AttackState` 通过 Animancer 归一化时间事件驱动 HitBox 激活，比 `Animation Event` 更精确可控。
|
||
|
||
---
|
||
|
||
## 4. 可扩展性
|
||
|
||
### 4.1 数据驱动度
|
||
|
||
**评分：9.5/10**
|
||
|
||
几乎所有参数均通过 ScriptableObject 暴露：
|
||
|
||
| SO 类型 | 用途 |
|
||
|---------|------|
|
||
| `PlayerMovementConfigSO` | 移动、加速度、跳跃力 |
|
||
| `PlayerAnimationConfigSO` | 动画片段引用、HitBox 时间点 |
|
||
| `DamageSourceSO` | 基础伤害、类型、标记、特效 |
|
||
| `ProjectileConfigSO` | 速度、生存时间、池 Key |
|
||
| `FormSkillSO` | 技能类型、消耗、冷却 |
|
||
| `CharmSO` | 护符 Notch 成本、效果链 |
|
||
| `QuestSO / QuestObjectiveSO` | 任务目标、条件、奖励 |
|
||
| `ChallengeRoomSO` | 敌人波次、计时 |
|
||
| `LootTableSO` | 掉落概率表 |
|
||
| `EnemyStatsSO` | HP、防御、伤害 |
|
||
|
||
连击段数（`AttackState` 读取 `AnimCfg.GroundAttacks.Length`）、技能冷却（`FormSkillSO`）、伤害标记（`DamageFlags` enum）均完全数据驱动，**策划无需改代码即可调整绝大多数玩法参数**。
|
||
|
||
### 4.2 Combat 可扩展性
|
||
|
||
`DamageFlags` 是 `[Flags] enum`，新增伤害属性只需增加枚举值，不需要修改任何现有代码——开闭原则实践良好。
|
||
|
||
`CharmEffect` 的 `OnEquip(EquipmentContext ctx)` / `OnUnequip` 抽象基类模式使新护符完全独立于 `EquipmentManager` 逻辑——这是 **Strategy 模式**的正确用法。
|
||
|
||
`StatusEffect` 基类 + 子类（`FireEffect`、`PoisonEffect`、`StaggerEffect`）结构同样符合开闭原则。
|
||
|
||
**评分：9/10**
|
||
|
||
### 4.3 敌人 AI 可扩展性
|
||
|
||
Behavior Designer 节点封装为独立 `BD_*` 类，与 `EnemyBase` 通过虚方法接口（`BeginAttack`、`MoveTo`）交互——不依赖具体实现。
|
||
|
||
`IPathAgent` 接口对导航完全解耦，可以在不修改敌人逻辑的情况下替换导航后端（PathBerserker2d / NavMesh / 自定义）。
|
||
|
||
**评分：8.5/10**
|
||
|
||
**缺陷**：`EnemyBase` 使用简单的 `EnemyStateType` enum（而非真正的 FSM），复杂 Boss 行为会在 `TakeDamage` / `Die` 方法里堆积 `if/switch`。建议 Boss 使用独立 FSM 或行为树驱动。
|
||
|
||
### 4.4 存档可扩展性
|
||
|
||
`SaveMigrator` 的链式 `goto case` 迁移模式是正确的版本迁移方案。`SaveData` 的 `[JsonExtensionData]` 字段允许 DLC 和未知字段的向前兼容。`NGPlusSaveData` 用 `null` 表示非 NG+ 模式，语义清晰。
|
||
|
||
**评分：9/10**
|
||
|
||
---
|
||
|
||
## 5. 编辑器友好性
|
||
|
||
### 5.1 Inspector 暴露
|
||
|
||
**评分：8.5/10**
|
||
|
||
- 所有 MonoBehaviour 字段均有 `[Header]` 分组，Inspector 布局清晰。
|
||
- `[RequireComponent]` 用于强制依赖（`HitBox`、`HurtBox`、`Projectile` 等），避免配置遗漏。
|
||
- `[DefaultExecutionOrder]` 显式标注执行顺序（`-2000` / `-1000` / `-900` / `-200` / `-100`），避免 Awake 初始化竞态。
|
||
- `[CreateAssetMenu]` 覆盖所有 SO 类型，策划可直接右键菜单创建。
|
||
|
||
**缺陷**：
|
||
- `PlayerAnimationConfigSO` 中的 `GroundAttackHitBoxEnterTimes[]` 和 `GroundAttackHitBoxExitTimes[]` 是并行数组，容易在 Inspector 中出现长度不匹配的问题。建议改用嵌套 struct：
|
||
```csharp
|
||
[Serializable]
|
||
public struct AttackTimings { [Range(0,1)] public float Enter, Exit; }
|
||
public AttackTimings[] GroundAttackTimings;
|
||
```
|
||
- `GlobalObjectPool.PoolConfig.MaxCount = 0` 表示无上限，但 Inspector 中没有 `[Tooltip]` 说明，容易被误填为"不允许生成任何对象"。
|
||
- 部分 `[SerializeField] private string _id` 缺少 `[Tooltip]`，大型团队中会增加沟通成本。
|
||
|
||
### 5.2 调试工具
|
||
|
||
`EventBusMonitor`（`#if UNITY_EDITOR`）记录事件名称、负载、监听者数量和帧号——这是**专业调试基础设施**，在大型团队中能极大加速问题定位。
|
||
|
||
`Debug.Assert` 保护关键依赖（`EquipmentManager.Awake`），在 Editor 运行时会立即报告配置问题。
|
||
|
||
**评分：8/10**
|
||
|
||
**缺陷**：
|
||
- `EventBusMonitor` 数据只在内存中，没有 EditorWindow 面板可视化。若增加一个实时 Event Flow 面板，调试效率会大幅提升。
|
||
- 没有统一的 `OnValidate` 验证框架，目前靠运行时 `Debug.LogWarning` 发现配置问题，而非编辑器实时提示。
|
||
|
||
### 5.3 Gizmos
|
||
|
||
未见 `OnDrawGizmos` 实现（除标准 Unity 内置）。商业游戏中通常会为 `HurtBox`、`HitBox`、`LOS Ray`、`Room Bounds` 添加 Gizmos 可视化，便于关卡设计师直观调试判定盒和视野范围。
|
||
|
||
---
|
||
|
||
## 6. 使用便利性与开发者体验
|
||
|
||
### 6.1 API 设计
|
||
|
||
**评分：8.5/10**
|
||
|
||
- `EquipmentManager.TryEquipCharm` 返回 `string?`（null = 成功，非 null = 错误原因）是简洁的结果模式，优于 `bool` + `out string error`。
|
||
- `WorldStateRegistry` 泛化 API（`IsMarked` / `Mark`）+ 具名向后兼容 API（`IsCollected` / `MarkCollected`）的两层设计既保持灵活又兼顾可读性。
|
||
- `DamageInfo.Builder` + `DamageInfo.From(so)` 双路径：高频热路径用零分配工厂，复杂场景（Boss 特殊伤害）用 Builder——设计考虑到位。
|
||
- `ServiceLocator.GetOrDefault<T>()` 的可选服务查询比 `try-catch` 方式性能更好，API 意图也更清晰。
|
||
|
||
**缺陷**：
|
||
- `PlayerController` 公开属性列表过长（15+ 个）。作为协调器这是可以理解的，但若进一步将上下文分组（`PlayerMovementContext`、`PlayerCombatContext`），状态类的代码会更简洁。
|
||
- `SaveManager.Data` 属性标注了 `[Obsolete]` 但仍为 public，在大型团队中会产生混淆，建议改为 `internal` 并添加 `EditorBrowsable(Never)`。
|
||
|
||
### 6.2 代码可读性
|
||
|
||
**评分：9/10**
|
||
|
||
- 方法体普遍短小（< 30 行），单一职责明确。
|
||
- 注释质量高：关键类和方法均有 `<summary>` XML 文档注释，并附有架构编号引用（如"架构 06_CombatModule §5"）。
|
||
- 命名规范统一：`_camelCase` 私有字段，`PascalCase` 公共属性，`TryXxx` 用于可失败操作，`HandleXxx` 用于事件处理。
|
||
|
||
**缺陷**：
|
||
- 中文注释和英文代码混排在部分文件中存在（`InputReaderSO` 全英文，`PlayerController` 全中文），团队规范不统一。
|
||
- `AttackState.PlayAttackClip` 中内联了防御性三元运算符兜底逻辑（`?? 0.3f`），建议将默认值提取到常量字段以明确语义：
|
||
```csharp
|
||
private const float DefaultHitBoxEnterTime = 0.3f;
|
||
private const float DefaultHitBoxExitTime = 0.6f;
|
||
```
|
||
|
||
### 6.3 错误处理
|
||
|
||
**评分：7.5/10**
|
||
|
||
- `ServiceLocator.Get<T>()` 抛出 `InvalidOperationException`，错误信息包含类型名和配置提示，比 `NullReferenceException` 友好得多。
|
||
- `SaveManager.LoadAsync` 捕获 `JsonException` 并返回 `false`，不会因存档损坏崩溃。
|
||
- `InputReaderSO` 在找不到 ActionMap 时只发出 `Debug.LogError`，仍然正常运行（降级处理）。
|
||
|
||
**缺陷**：
|
||
- `QuickSave` / `QuickLoad` 调用 `RunFireAndForget`，异步异常会被静默吞掉（只有日志）。在存档失败场景下，玩家不会得到任何 UI 反馈。
|
||
- `GlobalObjectPool.Spawn` 在池空且 MaxCount 已达时返回 `null`，调用方（`ProjectileManager.Spawn`）需要自行处理 null，但代码中未见统一的 null guard。
|
||
|
||
---
|
||
|
||
## 7. 分模块详细评估
|
||
|
||
### 7.1 Core 模块
|
||
|
||
| 组件 | 评分 | 说明 |
|
||
|------|------|------|
|
||
| `ServiceLocator` | 9/10 | 轻量、类型安全、测试友好，生命周期管理待完善 |
|
||
| `GameStateMachine` | 8.5/10 | 合法性校验完整，缺少状态转换日志 |
|
||
| `GameManager` | 7.5/10 | 单例+ServiceLocator双轨，服务注册与GameServiceRegistrar重叠 |
|
||
| `GameServiceRegistrar` | 8/10 | ExecutionOrder -2000 正确，AudioListener 修复逻辑健壮 |
|
||
| `DeathRespawnService` | 8.5/10 | 局部 lambda 订阅模式正确，避免了全局 bool 轮询 |
|
||
|
||
### 7.2 Combat 模块
|
||
|
||
| 组件 | 评分 | 说明 |
|
||
|------|------|------|
|
||
| `HurtBox` | 9/10 | 8 步流水线完整，接口注入优雅，注释清晰 |
|
||
| `HitBox` | 8.5/10 | 命中冷却计时器设计合理，ClashResolver 接口清晰 |
|
||
| `DamageInfo` | 9/10 | struct + Builder + 零分配工厂三路并存，文档注释清晰 |
|
||
| `ClashResolver` | 8/10 | 拼刀检测独立化，职责单一 |
|
||
| `StatusEffectManager` | 8.5/10 | 双结构（List+Dict）设计专业，MaterialPropertyBlock 不污染共享材质 |
|
||
| `Projectile` | 8/10 | 模板方法模式正确，ReturnToPool 路径清晰 |
|
||
|
||
### 7.3 Player 模块
|
||
|
||
| 组件 | 评分 | 说明 |
|
||
|------|------|------|
|
||
| `PlayerController` | 8.5/10 | 协调器职责明确，公开属性略多，状态机无转换白名单 |
|
||
| `PlayerMovement` | 9/10 | FixedUpdate 物理、Coyote Time、单向平台均正确实现 |
|
||
| `PlayerStats` | 8.5/10 | ISaveable + IRestoreOnSave 双接口设计合理，难度缩放逻辑正确 |
|
||
| `FormController` | 9/10 | 三事件链（SO事件/C#事件/SkillHUD刷新）分层清晰 |
|
||
| `InputReaderSO` | 7.5/10 | OnEnable 重置+重绑定解决了编辑器重置问题，但状态管理复杂 |
|
||
| `InputBuffer` | 9/10 | 极简实现，命名 handler 避免了匿名 lambda 退订问题 |
|
||
| `SkillManager` | 8.5/10 | `Dictionary<FormSkillSO,float>` 动态冷却优雅，零 GC Update |
|
||
|
||
### 7.4 Enemies 模块
|
||
|
||
| 组件 | 评分 | 说明 |
|
||
|------|------|------|
|
||
| `EnemyBase` | 8/10 | IDamageable + ILOSRequester 实现完整，BD 虚方法接口设计好 |
|
||
| `BatchLOSSystem` | 9/10 | 分帧节流是正确的性能优化，升级 Job System 路径清晰 |
|
||
| `BD_* 节点` | 8.5/10 | 每个节点职责单一，通过 EnemyBase 接口解耦，可测试性好 |
|
||
| `EnemyQuotaManager` | 8/10 | 激活敌人配额系统，防止屏幕外敌人堆积 |
|
||
| `LootResolver` | 8/10 | LootTableSO 数据驱动，概率计算逻辑清晰 |
|
||
|
||
### 7.5 Save 模块
|
||
|
||
| 组件 | 评分 | 说明 |
|
||
|------|------|------|
|
||
| `SaveManager` | 8/10 | async/await + HMAC-SHA256 + 具名访问器设计正确，**线程安全待补** |
|
||
| `SaveData` | 9/10 | `[JsonExtensionData]` 前向兼容，NGPlus null 语义明确 |
|
||
| `SaveMigrator` | 9/10 | 链式 `goto case` 版本迁移是正确模式 |
|
||
| `LocalFileStorage` | 7/10 | 接口隔离便于测试，**缺少原子写入**（先写临时文件再重命名） |
|
||
| `EmergencySaveService` | 8/10 | 崩溃时存档兜底，设计考虑周全 |
|
||
|
||
### 7.6 World 模块
|
||
|
||
| 组件 | 评分 | 说明 |
|
||
|------|------|------|
|
||
| `WorldStateRegistry` | 9/10 | `Dictionary<Category,HashSet>` + `OnStateChanged` 响应式设计 |
|
||
| `RoomController` | 8.5/10 | `ServiceLocator.GetOrDefault<ICameraService>` 解耦正确 |
|
||
| `Collectible` | 8/10 | WorldStateRegistry 注入，幂等拾取逻辑 |
|
||
| `SavePoint` | 8/10 | 触发存档逻辑清晰，激活状态持久化 |
|
||
|
||
### 7.7 Camera 模块
|
||
|
||
| 组件 | 评分 | 说明 |
|
||
|------|------|------|
|
||
| `CameraStateController` | 9/10 | `ICameraService` 实现 + ServiceLocator 注册，仍保留 `static Instance` 双保险 |
|
||
| `CameraBlendProfileSO` | 8.5/10 | 混合配置数据驱动，每个房间可独立配置 |
|
||
|
||
### 7.8 UI 模块
|
||
|
||
| 组件 | 评分 | 说明 |
|
||
|------|------|------|
|
||
| `UIManager` | 8/10 | `Stack<GameObject>` Panel 管理模式，事件驱动状态响应 |
|
||
| `PlayerFeedback` | 8.5/10 | IFeedbackPlayer 接口解耦 Feel 框架，NamedPresets 字典查找便捷 |
|
||
| `ToastManager` | 8/10 | 简洁的通知队列实现 |
|
||
|
||
---
|
||
|
||
## 8. 横切关注点
|
||
|
||
### 8.1 命名空间一致性
|
||
|
||
`BaseGames.*` 命名空间与 `.asmdef` 文件名严格对应，没有出现跨命名空间的类型混用。
|
||
**评分：9.5/10**
|
||
|
||
### 8.2 注释文档质量
|
||
|
||
XML `<summary>` 覆盖率高，关键方法（`HurtBox.ReceiveDamage`、`GlobalObjectPool.Spawn`）有详细的步骤注释。部分 TODO 注释（"Phase 2：加载存档场景（TODO）"）说明了实现阶段，有助于团队理解完成度。
|
||
**评分：8.5/10**
|
||
|
||
### 8.3 线程安全
|
||
|
||
`SaveManager` 的 `_current`、`_saveables`、`_currentSlot` 等字段在主线程和 `async Task` 中共享读写，**没有任何同步原语保护**。在 Unity 主线程模型下绝大多数 `await` 续体会回到主线程，但 `LocalFileStorage.WriteAsync` 若使用 `Task.Run` 会真正在线程池执行，此时主线程的 `foreach (var s in _saveables)` 若并发修改则有竞态。
|
||
|
||
**评分：6.5/10**（主要风险点）
|
||
|
||
建议：对 `SaveAsync` 加 `SemaphoreSlim` 保证同一时刻只有一个存档操作，`_saveables` 改用线程安全集合或在 `SaveAsync` 开始时做快照（`_saveables.ToList()`）。
|
||
|
||
### 8.4 内存管理
|
||
|
||
- ScriptableObject 在 Play 期间修改的字段（如 `WorldStateRegistry._states`）在 Editor 中**不会自动重置**，需要在 `OnEnable` 或脚本中显式 `Reset()`。当前实现是 `LoadFromSave` 中调用 `_states.Clear()`，但如果测试时直接 Play 而不加载存档，旧数据会残留——这是 Unity Editor 开发的常见陷阱。
|
||
- `EventChannelRegistry` 继承 `MonoBehaviour`（挂在场景物体上），但 SO 事件频道自身持有 `event Action`——场景卸载后若订阅者未退订会造成悬空引用。
|
||
|
||
### 8.5 错误恢复
|
||
|
||
`CrashReporter` + `EmergencySaveService` 的存在说明设计者考虑了崩溃恢复场景，这在商业游戏中是必须的。
|
||
**评分：8/10**
|
||
|
||
---
|
||
|
||
## 9. 风险与残留问题
|
||
|
||
### P0（严重，可能导致数据丢失或崩溃）
|
||
|
||
| # | 问题 | 文件 | 描述 |
|
||
|---|------|------|------|
|
||
| 1 | 存档竞态风险 | `SaveManager.cs` | `_current` 在异步路径和主线程同时访问，无同步保护 |
|
||
| 2 | 存档非原子写入 | `LocalFileStorage.cs` | 写入中断会产生损坏文件，需先写临时文件再重命名 |
|
||
| 3 | 池满时 null 返回未处理 | `GlobalObjectPool.cs` | `Spawn` 返回 null，部分调用方未做 null guard |
|
||
|
||
### P1（影响功能稳定性）
|
||
|
||
| # | 问题 | 文件 | 描述 |
|
||
|---|------|------|------|
|
||
| 4 | 服务双轨注册 | `GameManager.cs` + `GameServiceRegistrar.cs` | `IDeathRespawnService` 被注册两次，后者覆盖前者，行为取决于执行顺序 |
|
||
| 5 | SO 运行时状态不重置 | `WorldStateRegistry.cs` | Editor Play 直接进游戏不加载存档时，上次 Play 的状态可能残留 |
|
||
| 6 | Player 状态机无白名单 | `PlayerController.cs` | 任意状态可直接跳转到任意状态，大型开发中易引入非法状态 |
|
||
|
||
### P2（质量改进）
|
||
|
||
| # | 问题 | 描述 |
|
||
|---|------|------|
|
||
| 7 | `AttackState` 并行数组 | `GroundAttackHitBoxEnterTimes[]` 与 `ExitTimes[]` 长度需手动保持一致，易错 |
|
||
| 8 | `EventBusMonitor` GetInvocationList GC | 编辑器模式每次 Raise 分配 `Delegate[]` |
|
||
| 9 | `QuestManager` 未接入 ServiceLocator | 仍依赖 `static Instance`，与架构方向不一致 |
|
||
| 10 | 无单元测试 | 存档迁移、伤害计算、技能冷却等关键逻辑无自动化验证 |
|
||
| 11 | 无 `OnDrawGizmos` | 运行时 HitBox / HurtBox / LOS 边界不可视，关卡调试成本高 |
|
||
|
||
---
|
||
|
||
## 10. 改进建议优先级清单
|
||
|
||
### P0 — 必须修复
|
||
|
||
```csharp
|
||
// 1. LocalFileStorage：原子写入（先写 .tmp 再重命名覆盖）
|
||
private async Task WriteAtomicAsync(int slot, string json)
|
||
{
|
||
string path = GetPath(slot);
|
||
string tmp = path + ".tmp";
|
||
await File.WriteAllTextAsync(tmp, json);
|
||
File.Move(tmp, path, overwrite: true); // 原子替换，崩溃不会损坏旧存档
|
||
}
|
||
|
||
// 2. SaveManager：防并发保护
|
||
private readonly SemaphoreSlim _saveLock = new SemaphoreSlim(1, 1);
|
||
public async Task SaveAsync(int slot = -1)
|
||
{
|
||
await _saveLock.WaitAsync();
|
||
try
|
||
{
|
||
var snapshot = _saveables.ToList(); // 主线程快照，防并发修改
|
||
// ... 原有逻辑，使用 snapshot 而非 _saveables ...
|
||
}
|
||
finally { _saveLock.Release(); }
|
||
}
|
||
|
||
// 3. GlobalObjectPool：池满时回收 LRU 而非返回 null
|
||
if (maxCount > 0 && _alive[key].Count >= maxCount)
|
||
{
|
||
var oldest = _alive[key].Last.Value;
|
||
_alive[key].RemoveLast();
|
||
oldest.gameObject.SetActive(false);
|
||
_pools[key].Enqueue(oldest);
|
||
}
|
||
```
|
||
|
||
### P1 — 强烈建议
|
||
|
||
```csharp
|
||
// 4. 消除服务双轨注册：
|
||
// 删除 GameManager.RegisterServices() 整个方法，
|
||
// 仅保留 GameServiceRegistrar（ExecutionOrder -2000）作为服务注册入口。
|
||
|
||
// 5. WorldStateRegistry：OnEnable 重置，防 Editor Play 状态残留
|
||
private void OnEnable()
|
||
{
|
||
_states.Clear();
|
||
}
|
||
|
||
// 6. Player 状态机转换白名单（最小化改动）
|
||
// 在 PlayerController.TransitionTo 中增加调试断言：
|
||
#if UNITY_EDITOR
|
||
if (_currentState != null && _debugValidateTransitions)
|
||
Debug.Assert(IsValidTransition(_currentState.GetType(), targetType),
|
||
$"[PlayerController] 非预期转换: {_currentState.GetType().Name} → {targetType.Name}");
|
||
#endif
|
||
```
|
||
|
||
### P2 — 建议改进
|
||
|
||
```csharp
|
||
// 7. AttackState 并行数组 → 嵌套 struct
|
||
[System.Serializable]
|
||
public struct AttackTimings
|
||
{
|
||
[Range(0f, 1f)] public float HitBoxEnter;
|
||
[Range(0f, 1f)] public float HitBoxExit;
|
||
}
|
||
// PlayerAnimationConfigSO 改为：
|
||
public AttackTimings[] GroundAttackTimings;
|
||
|
||
// 8. EventBusMonitor：用整数计数器替代 GetInvocationList
|
||
// 在 BaseEventChannelSO 中：
|
||
#if UNITY_EDITOR
|
||
private int _subscriberCount;
|
||
public int SubscriberCount => _subscriberCount;
|
||
#endif
|
||
// Subscribe 时 _subscriberCount++，取消时 --（O(1)，无 GC）
|
||
|
||
// 9. OnDrawGizmos（HurtBox 示例）
|
||
#if UNITY_EDITOR
|
||
private void OnDrawGizmos()
|
||
{
|
||
var col = GetComponent<Collider2D>();
|
||
if (col == null) return;
|
||
Gizmos.color = _isActive ? new Color(1, 0, 0, 0.5f) : new Color(1, 0, 0, 0.1f);
|
||
Gizmos.DrawWireCube(col.bounds.center, col.bounds.size);
|
||
}
|
||
#endif
|
||
```
|
||
|
||
### 架构长期优化
|
||
|
||
```
|
||
10. 引入测试框架
|
||
- 新增 asmdef：BaseGames.Tests（UNITY_INCLUDE_TESTS 平台限定）
|
||
- 优先覆盖：SaveMigrator、DamageInfo.From、WorldStateRegistry、SkillManager 冷却
|
||
- 使用 ServiceLocator.OverrideForTest 替换真实服务
|
||
|
||
11. QuestManager 接口化
|
||
- 提取 IQuestManager 接口
|
||
- 在 GameServiceRegistrar 中注册
|
||
- 删除 QuestManager.Instance（保留 private 单例防多次注册）
|
||
|
||
12. EditorWindow 可视化工具
|
||
- EventBusMonitor → 运行时 Event Flow 实时面板
|
||
- WorldStateRegistry → 状态浏览器（按类别分组显示已标记 ID）
|
||
- GlobalObjectPool → 各池容量/活跃数实时显示
|
||
- 以上均可在 2–3 天内实现，对调试效率提升极大
|
||
```
|
||
|
||
---
|
||
|
||
## 附录：评分汇总
|
||
|
||
| 维度 | 评分 | 权重 | 加权分 |
|
||
|------|------|------|--------|
|
||
| 架构设计 | 8.7 | 25% | 2.18 |
|
||
| 性能 | 8.5 | 20% | 1.70 |
|
||
| 可扩展性 | 9.0 | 20% | 1.80 |
|
||
| 编辑器友好性 | 8.0 | 15% | 1.20 |
|
||
| 使用便利性 | 8.2 | 10% | 0.82 |
|
||
| 安全性/健壮性 | 6.5 | 10% | 0.65 |
|
||
| **综合** | | | **8.35 → 8.2** |
|
||
|
||
> **安全性/健壮性**是唯一拉低整体评分的维度，核心问题是 P0 的存档线程安全和文件原子写入。修复全部 P0+P1 问题后，预期评分可达 **8.8 / 10**，达到优质商业独立游戏代码标准。
|